[kie-issues#1606] Modify columns in the flyway migration scripts which uses sql reserved words #2139
Conversation
Signed-off-by: Deepak Joseph <[email protected]>
Signed-off-by: Deepak Joseph <[email protected]>
Signed-off-by: Deepak Joseph <[email protected]>
|
PR job Reproducerbuild-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-kogito-apps -u #2139 --skipParallelCheckout NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution Please look here: https://ci-builds.apache.org/job/KIE/job/kogito/job/main/job/pullrequest_jobs/job/kogito-apps-pr/job/PR-2139/2/display/redirect Test results:
Those are the test failures: org.kie.kogito.persistence.postgresql.reporting.BasicTypeMappingIT.testBasicTypeMappingDeleteJDBC exception executing SQL [SELECT ROW_NUMBER() OVER (ORDER BY name, key) as id, name, key, field1MappedField, field2MappedField FROM BasicTypeExtract] [ERROR: relation "basictypeextract" does not existPosition: 107] [n/a] org.kie.kogito.persistence.postgresql.reporting.BasicTypeMappingIT.testBasicTypeMappingJDBC exception executing SQL [SELECT ROW_NUMBER() OVER (ORDER BY name, key) as id, name, key, field1MappedField, field2MappedField FROM BasicTypeExtract] [ERROR: relation "basictypeextract" does not existPosition: 107] [n/a] org.kie.kogito.persistence.postgresql.reporting.BasicTypeMappingIT.testBasicTypeMappingUpdateJDBC exception executing SQL [SELECT ROW_NUMBER() OVER (ORDER BY name, key) as id, name, key, field1MappedField, field2MappedField FROM BasicTypeExtract] [ERROR: relation "basictypeextract" does not existPosition: 107] [n/a] org.kie.kogito.persistence.postgresql.reporting.ComplexHierarchicalTypeMappingIT.testComplexHierarchicalTypeMappingJDBC exception executing SQL [SELECT ROW_NUMBER() OVER (ORDER BY name, key) as id, name, key, root, nestedBasicMappedField, nestedComplexCollectionMappedField1, nestedComplexCollectionMappedSubField1 FROM ComplexHierarchicalTypeExtract] [ERROR: relation "complexhierarchicaltypeextract" does not existPosition: 176] [n/a] org.kie.kogito.persistence.postgresql.reporting.ComplexHierarchicalTypeMappingIT.testComplexHierarchicalTypeMappingDeleteJDBC exception executing SQL [SELECT ROW_NUMBER() OVER (ORDER BY name, key) as id, name, key, root, nestedBasicMappedField, nestedComplexCollectionMappedField1, nestedComplexCollectionMappedSubField1 FROM ComplexHierarchicalTypeExtract] [ERROR: relation "complexhierarchicaltypeextract" does not existPosition: 176] [n/a] org.kie.kogito.persistence.postgresql.reporting.ComplexHierarchicalTypeMappingIT.testComplexHierarchicalTypeMappingUpdateJDBC exception executing SQL [SELECT ROW_NUMBER() OVER (ORDER BY name, key) as id, name, key, root, nestedBasicMappedField, nestedComplexCollectionMappedField1, nestedComplexCollectionMappedSubField1 FROM ComplexHierarchicalTypeExtract] [ERROR: relation "complexhierarchicaltypeextract" does not existPosition: 176] [n/a] org.kie.kogito.persistence.postgresql.reporting.HierarchicalTypeMappingIT.testHierarchicalTypeMappingDeleteJDBC exception executing SQL [SELECT ROW_NUMBER() OVER (ORDER BY name, key) as id, name, key, root, nestedBasicMappedField, nestedBasicCollectionMappedField FROM HierarchicalTypeExtract] [ERROR: relation "hierarchicaltypeextract" does not existPosition: 133] [n/a] org.kie.kogito.persistence.postgresql.reporting.HierarchicalTypeMappingIT.testHierarchicalTypeMappingUpdateJDBC exception executing SQL [SELECT ROW_NUMBER() OVER (ORDER BY name, key) as id, name, key, root, nestedBasicMappedField, nestedBasicCollectionMappedField FROM HierarchicalTypeExtract] [ERROR: relation "hierarchicaltypeextract" does not existPosition: 133] [n/a] org.kie.kogito.persistence.postgresql.reporting.HierarchicalTypeMappingIT.testHierarchicalTypeMappingJDBC exception executing SQL [SELECT ROW_NUMBER() OVER (ORDER BY name, key) as id, name, key, root, nestedBasicMappedField, nestedBasicCollectionMappedField FROM HierarchicalTypeExtract] [ERROR: relation "hierarchicaltypeextract" does not existPosition: 133] [n/a] org.kie.kogito.persistence.postgresql.reporting.api.PostgresMappingsApiV1IT.dynamicMappingDefinitionCreationexpected: <200> but was: <500>org.kie.kogito.persistence.postgresql.reporting.database.sqlbuilders.PostgresApplyMappingSqlBuilderIT.testApplyMappingToExistingDataJDBC exception executing SQL [CREATE TABLE DynamicTypeExtract (key null, name varchar, field1MappedField text, field2MappedField numeric ); ] [ERROR: syntax error at or near "null" Position: 42] [n/a] |
Signed-off-by: Deepak Joseph <[email protected]>
|
@pefernan I guess we can merge it, WDYT? |
|
hi, before merging, have you explore the possibility of using the property |
|
I would like to highlight, that we will soon have users running |
There was a problem hiding this comment.
I fully support @gmunozfe comments, this change is not needed and should not be merged as it is
Key (probably the best option here would have been "name", since "key" is also reserved in mysql) and value (here, even value is a reserved word in latest sql standard, but significantly not reserved by all major vendors, I would keep the current name even if designing the system from scratch) are column names that makes sense for this table.
But even if they were objectively wrong, we should not change them if we do not have other alternative, and the fact is that we have.
The right (in my opinion) solution for the flyway scripts (since the problem does not affect runtime, because we are using JPA, which proprely handle this out of box) is to use the property Gonzalo mentions for H2. For mysql, qoutes will have to be used.
Please note that, in my opinion, it is perfectly fine to have different flyway scripts for different dbs (in fact we already have) and the expectation if that we will have one directory for each db we support. (if we can reuse the same dir for several dbs, the better, but it should not be a constraint and definitely it should not trigger changes in existing scripts or db tables)
|
@fjtirado @gmunozfe @yesamer @pefernan
|
| * under the License. | ||
| */ | ||
|
|
||
| alter table definitions_nodes_metadata alter column key rename to metadata_key; |
There was a problem hiding this comment.
If we are going to rename, I think we should rename key to name or id
|
|
||
| alter table definitions_nodes_metadata alter column key rename to metadata_key; | ||
| alter table definitions_nodes_metadata alter column value rename to metadata_value; | ||
| alter table definitions_metadata alter column key rename to metadata_key; |
| */ | ||
|
|
||
| alter table definitions_nodes_metadata alter column key rename to metadata_key; | ||
| alter table definitions_nodes_metadata alter column value rename to metadata_value; |
There was a problem hiding this comment.
I think value should be ok, but since it was reserved by standard, maybe stringValue ;)?
Closes apache/incubator-kie-issues#1606
The flyway migration sql scripts creates few tables with column names like 'key' and 'value' which are reserved words in H2. So in order to avoid that
Create new flyway scripts to alter the existing column names
Verify/modify any jpa mappings if required.
Many thanks for submitting your Pull Request ❤️!
Please make sure that your PR meets the following requirements:
KOGITO-XYZ Subject[0.9.x] KOGITO-XYZ SubjectHow to replicate CI configuration locally?
Build Chain tool does "simple" maven build(s), the builds are just Maven commands, but because the repositories relates and depends on each other and any change in API or class method could affect several of those repositories there is a need to use build-chain tool to handle cross repository builds and be sure that we always use latest version of the code for each repository.
build-chain tool is a build tool which can be used on command line locally or in Github Actions workflow(s), in case you need to change multiple repositories and send multiple dependent pull requests related with a change you can easily reproduce the same build by executing it on Github hosted environment or locally in your development environment. See local execution details to get more information about it.
How to retest this PR or trigger a specific build:
for pull request checks
Please add comment: Jenkins retest this
for a specific pull request check
Please add comment: Jenkins (re)run [kogito-apps|kogito-examples] tests
for quarkus branch checks
Run checks against Quarkus current used branch
Please add comment: Jenkins run quarkus-branch
for a quarkus branch specific check
Run checks against Quarkus current used branch
Please add comment: Jenkins (re)run [kogito-apps|kogito-examples] quarkus-branch
for quarkus main checks
Run checks against Quarkus main branch
Please add comment: Jenkins run quarkus-main
for a specific quarkus main check
Run checks against Quarkus main branch
Please add comment: Jenkins (re)run [kogito-apps|kogito-examples] quarkus-main
for quarkus lts checks
Run checks against Quarkus lts branch
Please add comment: Jenkins run quarkus-lts
for a specific quarkus lts check
Run checks against Quarkus lts branch
Please add comment: Jenkins (re)run [kogito-apps|kogito-examples] quarkus-lts
for native checks
Run native checks
Please add comment: Jenkins run native
for a specific native check
Run native checks
Please add comment: Jenkins (re)run [kogito-apps|kogito-examples] native
for native lts checks
Run native checks against quarkus lts branch
Please add comment: Jenkins run native-lts
for a specific native lts check
Run native checks against quarkus lts branch
Please add comment: Jenkins (re)run [kogito-apps|kogito-examples] native-lts
How to backport a pull request to a different branch?
In order to automatically create a backporting pull request please add one or more labels having the following format
backport-<branch-name>, where<branch-name>is the name of the branch where the pull request must be backported to (e.g.,backport-7.67.xto backport the original PR to the7.67.xbranch).Once the original pull request is successfully merged, the automated action will create one backporting pull request per each label (with the previous format) that has been added.
If something goes wrong, the author will be notified and at this point a manual backporting is needed.
Quarkus-3 PR check is failing ... what to do ?
The Quarkus 3 check is applying patches from the `.ci/environments/quarkus-3/patches`.The first patch, called
0001_before_sh.patch, is generated from Openrewrite.ci/environments/quarkus-3/quarkus3.ymlrecipe. The patch is created to speed up the check. But it may be that some changes in the PR broke this patch.No panic, there is an easy way to regenerate it. You just need to comment on the PR:
and it should, after some minutes (~20/30min) apply a commit on the PR with the patch regenerated.
Other patches were generated manually. If any of it fails, you will need to manually update it... and push your changes.