You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org> on 2020/02/13 00:15:11 UTC
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Anurag Mantripragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15213
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
IMPALA-9256: Refactor constraint information into a separate class.
This change refactors the primary keys and foreign keys into a
SqlConstraints class since they are almost always used together. This
work also helps extend the constraints class to include other
constraints we may support in the future. (Ex: Unique constraints.)
This patch also:
- Fixes a bug in foreign key constraint name generation that was
causing foreign keys corresponding to a composite primary key get
different foreign key constraint names instead of the same name.
- Introduces a canonical representation for foreign keys to prevent
bugs like IMPALA-9372 which can occur due to HMS returning results
in inconsistent ways.
Testing:
- Fixed the tests to work with the new behavior.
- Ran all the PK/FK tests.
Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M common/thrift/CMakeLists.txt
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
A common/thrift/SqlConstraints.thrift
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
A fe/src/main/java/org/apache/impala/catalog/SqlConstraints.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
M tests/hs2/test_hs2.py
24 files changed, 382 insertions(+), 342 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/15213/1
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5329/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 13 Feb 2020 08:45:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 15 Feb 2020 12:50:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
IMPALA-9256: Refactor constraint information into a separate class.
This change refactors the primary keys and foreign keys into a
SqlConstraints class since they are almost always used together. This
work also helps extend the constraints class to include other
constraints we may support in the future. (Ex: Unique constraints.)
This patch also:
- Fixes a bug in the MetadataOp.getPrimaryKeys() and getForeignKeys()
which returned incorrect results. The tests did not catch this
before beacuse we did not have tests to verify individual resultset
rows. The patch modifies these tests.
- Fixes a bug in foreign key constraint name generation that was
causing foreign keys corresponding to a composite primary key get
different foreign key constraint names instead of the same name.
- Introduces a canonical representation for foreign keys to prevent
bugs like IMPALA-9372 which can occur due to HMS returning results
in inconsistent ways.
Testing:
- Fixed the tests to work with the new behavior.
- Ran all the PK/FK tests.
Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M common/thrift/CMakeLists.txt
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
A common/thrift/SqlConstraints.thrift
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
A fe/src/main/java/org/apache/impala/catalog/SqlConstraints.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
M tests/hs2/test_hs2.py
24 files changed, 395 insertions(+), 344 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/15213/4
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 1:
(3 comments)
Hi, left some comments.
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/main/java/org/apache/impala/catalog/Table.java
File fe/src/main/java/org/apache/impala/catalog/Table.java:
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/main/java/org/apache/impala/catalog/Table.java@117
PS1, Line 117: sql
nit: SQL
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java:
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@544
PS1, Line 544: primary
nit: or foreign keys/SQL constraints
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
File fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java:
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java@213
PS1, Line 213: return new SqlConstraints(new ArrayList<>(), new ArrayList<>());
Since IMPALA-9158 is merged, can't we return the SQL constraints here with primary and foreign keys?
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 14 Feb 2020 13:03:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/main/java/org/apache/impala/analysis/TableDef.java
File fe/src/main/java/org/apache/impala/analysis/TableDef.java:
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/main/java/org/apache/impala/analysis/TableDef.java@578
PS1, Line 578: if (fk.getFkConstraintName() == null) {
: if (i == 0){
nit: can combine these two. BTW, can we move it outside the loop?
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@169
PS1, Line 169: sqlConstraints = new SqlConstraints(c.getHiveClient().getPrimaryKeys(
Should we check whether IMetaStoreClient.getPrimaryKeys and IMetaStoreClient.getForeignKeys return nulls here? Just worry on any bugs in Hive or future versions of Hive.
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java
File fe/src/main/java/org/apache/impala/service/MetadataOp.java:
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/main/java/org/apache/impala/service/MetadataOp.java@648
PS1, Line 648: TResultRow row = new TResultRow();
Oops... Is this a bug? I'm curious why it didn't cause test failures before this patch. May worth to mention it in the commit message.
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/test/java/org/apache/impala/service/JdbcTest.java
File fe/src/test/java/org/apache/impala/service/JdbcTest.java:
http://gerrit.cloudera.org:8080/#/c/15213/1/fe/src/test/java/org/apache/impala/service/JdbcTest.java@437
PS1, Line 437: pkCount - 1
Need assertion to avoid index overflow
http://gerrit.cloudera.org:8080/#/c/15213/1/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:
http://gerrit.cloudera.org:8080/#/c/15213/1/tests/hs2/test_hs2.py@508
PS1, Line 508: results = fetch_results_resp.results
nit: this should be moved outside since it's used outside the loop
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 14 Feb 2020 08:26:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15213/3/fe/src/main/java/org/apache/impala/analysis/TableDef.java
File fe/src/main/java/org/apache/impala/analysis/TableDef.java:
http://gerrit.cloudera.org:8080/#/c/15213/3/fe/src/main/java/org/apache/impala/analysis/TableDef.java@520
PS3, Line 520: if (fk.getForeignKeyColNames().size() != fk.getPrimaryKeyColNames().size()){
> The parser will not allow such a statement. empty foreignKeysList_ will pas
Thanks for your explanation!
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 15 Feb 2020 08:02:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
IMPALA-9256: Refactor constraint information into a separate class.
This change refactors the primary keys and foreign keys into a
SqlConstraints class since they are almost always used together. This
work also helps extend the constraints class to include other
constraints we may support in the future. (Ex: Unique constraints.)
This patch also:
- Fixes a bug in the MetadataOp.getPrimaryKeys() and getForeignKeys()
which returned incorrect results. The tests did not catch this
before beacuse we did not have tests to verify individual resultset
rows. The patch modifies these tests.
- Fixes a bug in foreign key constraint name generation that was
causing foreign keys corresponding to a composite primary key get
different foreign key constraint names instead of the same name.
- Introduces a canonical representation for foreign keys to prevent
bugs like IMPALA-9372 which can occur due to HMS returning results
in inconsistent ways.
Testing:
- Fixed the tests to work with the new behavior.
- Ran all the PK/FK tests.
Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M common/thrift/CMakeLists.txt
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
A common/thrift/SqlConstraints.thrift
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
A fe/src/main/java/org/apache/impala/catalog/SqlConstraints.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
M tests/hs2/test_hs2.py
24 files changed, 395 insertions(+), 344 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/15213/3
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 3
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 1: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5329/
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 13 Feb 2020 13:39:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15213/3/fe/src/main/java/org/apache/impala/analysis/TableDef.java
File fe/src/main/java/org/apache/impala/analysis/TableDef.java:
http://gerrit.cloudera.org:8080/#/c/15213/3/fe/src/main/java/org/apache/impala/analysis/TableDef.java@520
PS3, Line 520: if (fk.getForeignKeyColNames().size() != fk.getPrimaryKeyColNames().size()){
> Is it possible that these two are both 0? If so, should we throw AnalysisEx
The parser will not allow such a statement. empty foreignKeysList_ will pass the analysis and will not create any FKs for the table. This check is done in line 517.
http://gerrit.cloudera.org:8080/#/c/15213/3/fe/src/main/java/org/apache/impala/analysis/TableDef.java@578
PS3, Line 578: fk
> nit: should be two spaces indention
Done
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 15 Feb 2020 07:22:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5232/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 15 Feb 2020 08:11:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
IMPALA-9256: Refactor constraint information into a separate class.
This change refactors the primary keys and foreign keys into a
SqlConstraints class since they are almost always used together. This
work also helps extend the constraints class to include other
constraints we may support in the future. (Ex: Unique constraints.)
This patch also:
- Fixes a bug in the MetadataOp.getPrimaryKeys() and getForeignKeys()
which returned incorrect results. The tests did not catch this
before beacuse we did not have tests to verify individual resultset
rows. The patch modifies these tests.
- Fixes a bug in foreign key constraint name generation that was
causing foreign keys corresponding to a composite primary key get
different foreign key constraint names instead of the same name.
- Introduces a canonical representation for foreign keys to prevent
bugs like IMPALA-9372 which can occur due to HMS returning results
in inconsistent ways.
Testing:
- Fixed the tests to work with the new behavior.
- Ran all the PK/FK tests.
Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Reviewed-on: http://gerrit.cloudera.org:8080/15213
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/generated-sources/gen-cpp/CMakeLists.txt
M common/thrift/CMakeLists.txt
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
A common/thrift/SqlConstraints.thrift
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
A fe/src/main/java/org/apache/impala/catalog/SqlConstraints.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/MetadataOp.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
M tests/hs2/test_hs2.py
24 files changed, 395 insertions(+), 344 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 6
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 15 Feb 2020 08:02:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5229/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 3
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 15 Feb 2020 01:38:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 1: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 14 Feb 2020 10:28:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5214/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 13 Feb 2020 01:01:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5338/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 14 Feb 2020 05:37:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5344/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 15 Feb 2020 08:02:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 1:
The build errors are due to row filtering. Unrelated to this change.
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 13 Feb 2020 18:17:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15213 )
Change subject: IMPALA-9256: Refactor constraint information into a separate class.
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15213/3/fe/src/main/java/org/apache/impala/analysis/TableDef.java
File fe/src/main/java/org/apache/impala/analysis/TableDef.java:
http://gerrit.cloudera.org:8080/#/c/15213/3/fe/src/main/java/org/apache/impala/analysis/TableDef.java@520
PS3, Line 520: if (fk.getForeignKeyColNames().size() != fk.getPrimaryKeyColNames().size()){
Is it possible that these two are both 0? If so, should we throw AnalysisException as well?
http://gerrit.cloudera.org:8080/#/c/15213/3/fe/src/main/java/org/apache/impala/analysis/TableDef.java@578
PS3, Line 578:
nit: should be two spaces indention
--
To view, visit http://gerrit.cloudera.org:8080/15213
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f1c441c24df84d2d0791ffe94dff60d039a3341
Gerrit-Change-Number: 15213
Gerrit-PatchSet: 3
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 15 Feb 2020 01:38:02 +0000
Gerrit-HasComments: Yes