[Impala-ASF-CR] IMPALA-9980: Remove jersey* jars from maven exclusions
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/16225 ) Change subject: IMPALA-9980: Remove jersey* jars from maven exclusions .. Patch Set 2: Makes sense. Thanks. -- To view, visit http://gerrit.cloudera.org:8080/16225 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I796a9ff4c1beff776147266c2f2649c2d02a8dd8 Gerrit-Change-Number: 16225 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Jul 2020 05:09:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9980: Remove jersey* jars from maven exclusions
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/16225 ) Change subject: IMPALA-9980: Remove jersey* jars from maven exclusions .. Patch Set 2: Could we just add jersey-server.jar and jersey-servlet.jar since these were the missing jars instead of the jersey-bundle which seems to be a large jar? -- To view, visit http://gerrit.cloudera.org:8080/16225 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I796a9ff4c1beff776147266c2f2649c2d02a8dd8 Gerrit-Change-Number: 16225 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Jul 2020 04:53:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9980: Remove jersey* jars from maven exclusions
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/16225 ) Change subject: IMPALA-9980: Remove jersey* jars from maven exclusions .. Patch Set 2: (1 comment) Instead of removing all jersey* exclusions, just removing the jersey-bundle dependency from ranger worked in my local testing. I will look into why our dockerized tests did not catch this. http://gerrit.cloudera.org:8080/#/c/16225/1/fe/pom.xml File fe/pom.xml: http://gerrit.cloudera.org:8080/#/c/16225/1/fe/pom.xml@189 PS1, Line 189: rocksdbjni > This doesn't make a lot of sense to me - if the problem is with Ranger, why The original idea was to revert parts of IMPALA-9679 but I missed this exclusion in Ranger dependency. Instead of reverting all jeresey* changes, just removing this exclusion also worked in my local testing. Do you think it is a good idea? -- To view, visit http://gerrit.cloudera.org:8080/16225 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I796a9ff4c1beff776147266c2f2649c2d02a8dd8 Gerrit-Change-Number: 16225 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Jul 2020 04:49:16 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9980: Remove jersey* jars from maven exclusions
Anurag Mantripragada has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/16225 ) Change subject: IMPALA-9980: Remove jersey* jars from maven exclusions .. IMPALA-9980: Remove jersey* jars from maven exclusions IMPALA-9679 added jersey* jars to maven exclusions. These jars are required by Impala Ranger plugin to instantiate RuntimeDelegateImpl As a result of the exclusions, ClassNotFound exceptions are thrown in Impala docker containers when ranger plugin is enabled. This change removes jersey-bundle exclusion from Ranger dependency. Testing: - Built and ran Impala containers locally with ranger enabled. - Ran dockerized tests in precommit. Change-Id: I796a9ff4c1beff776147266c2f2649c2d02a8dd8 --- M fe/pom.xml 1 file changed, 0 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/16225/2 -- To view, visit http://gerrit.cloudera.org:8080/16225 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I796a9ff4c1beff776147266c2f2649c2d02a8dd8 Gerrit-Change-Number: 16225 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9980: Remove jersey* jars from maven exclusions
Anurag Mantripragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16225 Change subject: IMPALA-9980: Remove jersey* jars from maven exclusions .. IMPALA-9980: Remove jersey* jars from maven exclusions IMPALA-9679 added jersey* jars to maven exclusions. These jars are required by Impala Ranger plugin to instantiate RuntimeDelegateImpl As a result of the exclusions, ClassNotFound exceptions are thrown in Impala docker containers when ranger plugin is enabled. This change removes jersey-server.jar and jersey-servlet.jar from exclusions. Testing: Ran dockerized tests in precommit. Change-Id: I796a9ff4c1beff776147266c2f2649c2d02a8dd8 --- M fe/pom.xml 1 file changed, 0 insertions(+), 27 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/16225/1 -- To view, visit http://gerrit.cloudera.org:8080/16225 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I796a9ff4c1beff776147266c2f2649c2d02a8dd8 Gerrit-Change-Number: 16225 Gerrit-PatchSet: 1 Gerrit-Owner: Anurag Mantripragada
[Impala-ASF-CR] IMPALA-9913: Use table id to match the table in drop table event
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/16129 ) Change subject: IMPALA-9913: Use table id to match the table in drop table event .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16129 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id6a80bbf5757e46318af1b57911fc127d7dd1f01 Gerrit-Change-Number: 16129 Gerrit-PatchSet: 2 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 30 Jun 2020 19:22:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9858: Fix wrong partition metrics in LocalCatalog profile
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/16080 ) Change subject: IMPALA-9858: Fix wrong partition metrics in LocalCatalog profile .. Patch Set 1: Code-Review+2 (2 comments) Minor nits but LGTM. Feel free to carry forward the +2. http://gerrit.cloudera.org:8080/#/c/16080/1/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java File fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java: http://gerrit.cloudera.org:8080/#/c/16080/1/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java@263 PS1, Line 263: // Load all partitions ids. Miss the partition list. Nit: Change the line to "Load all partition ids. This will create a PartitionLists miss." http://gerrit.cloudera.org:8080/#/c/16080/1/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java@265 PS1, Line 265: // Load all partitions. All of them are missing. Nit: Change the line to "Load all partitions. This will create one partition miss per partition." -- To view, visit http://gerrit.cloudera.org:8080/16080 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I10cabce2908f1d252b90390978e679d31003e89d Gerrit-Change-Number: 16080 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Mon, 15 Jun 2020 23:53:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Filter out "Checksum validation failed" messages during the maven build
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15775 ) Change subject: Filter out "Checksum validation failed" messages during the maven build .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15775 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19afbd157533e52ef3157730c7ec5159241749bc Gerrit-Change-Number: 15775 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 05 Jun 2020 18:26:35 + Gerrit-HasComments: No
[Impala-ASF-CR] Filter out "Checksum validation failed" messages during the maven build
Anurag Mantripragada has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15775 ) Change subject: Filter out "Checksum validation failed" messages during the maven build .. Filter out "Checksum validation failed" messages during the maven build Some Impala dependencies come from repositories that don't have checksums available. During the build, this produces a large number of messages like: [WARNING] Checksum validation failed, no checksums available from the repository for ... or: [WARNING] Checksum validation failed, could not read expected checksum ... These messages are not very useful, and they make it harder to search the console output for failed tests. This filters them out of the maven output. Differet versions of maven structure the messsages differently, so this filters all the "Checksum validation failed" messages that happen at WARNING level. Testing: - Ran core tests, verified the messages are gone Change-Id: I19afbd157533e52ef3157730c7ec5159241749bc Reviewed-on: http://gerrit.cloudera.org:8080/15775 Tested-by: Impala Public Jenkins Reviewed-by: Anurag Mantripragada --- M bin/mvn-quiet.sh 1 file changed, 7 insertions(+), 1 deletion(-) Approvals: Impala Public Jenkins: Verified Anurag Mantripragada: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15775 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I19afbd157533e52ef3157730c7ec5159241749bc Gerrit-Change-Number: 15775 Gerrit-PatchSet: 5 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR](asf-site) Add link to slack channel on community
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/16024 ) Change subject: Add link to slack channel on community .. Patch Set 1: Also, we need to add this url. Thanks. -- To view, visit http://gerrit.cloudera.org:8080/16024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: Ibc05665607cc35a967c01660b6f4890ddd7e6a40 Gerrit-Change-Number: 16024 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 03 Jun 2020 19:38:38 + Gerrit-HasComments: No
[Impala-ASF-CR](asf-site) Add link to slack channel on community
Anurag Mantripragada has removed a vote on this change. Change subject: Add link to slack channel on community .. Removed Code-Review+2 by Anurag Mantripragada -- To view, visit http://gerrit.cloudera.org:8080/16024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ibc05665607cc35a967c01660b6f4890ddd7e6a40 Gerrit-Change-Number: 16024 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR](asf-site) Add link to slack channel on community
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/16024 ) Change subject: Add link to slack channel on community .. Patch Set 1: Code-Review+2 This worked for me, thanks. Can we make it never expire? -- To view, visit http://gerrit.cloudera.org:8080/16024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: Ibc05665607cc35a967c01660b6f4890ddd7e6a40 Gerrit-Change-Number: 16024 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 03 Jun 2020 19:26:35 + Gerrit-HasComments: No
[Impala-ASF-CR](asf-site) Add link to slack channel on community
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/16024 ) Change subject: Add link to slack channel on community .. Patch Set 1: Is this an "invite-only" workspace? I couldn't log in with my personal email. -- To view, visit http://gerrit.cloudera.org:8080/16024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: comment Gerrit-Change-Id: Ibc05665607cc35a967c01660b6f4890ddd7e6a40 Gerrit-Change-Number: 16024 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 03 Jun 2020 17:56:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9791: Support validWriteIdList in getPartialCatalogObject API
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/16008 ) Change subject: IMPALA-9791: Support validWriteIdList in getPartialCatalogObject API .. Patch Set 2: (8 comments) I did the first round, I will still have to go through the tests. http://gerrit.cloudera.org:8080/#/c/16008/2/common/thrift/CatalogService.thrift File common/thrift/CatalogService.thrift: http://gerrit.cloudera.org:8080/#/c/16008/2/common/thrift/CatalogService.thrift@497 PS2, Line 497: valid_write_id_list Nit: Could we use the same variable name for this field to avoid confusion in code? http://gerrit.cloudera.org:8080/#/c/16008/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/16008/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1851 PS2, Line 1851: Nit: Indent 4 spaces. http://gerrit.cloudera.org:8080/#/c/16008/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1868 PS2, Line 1868: Nit: Indent 4 spaces. http://gerrit.cloudera.org:8080/#/c/16008/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3027 PS2, Line 3027: String dbName = objectDesc.getTable().db_name == null ? "default" : : objectDesc.getTable().db_name; I could not find the code for it but does a similar conversion to "default" db happen in the later getOrLoadTable() call? I just want to make sure this conversion is consistent. http://gerrit.cloudera.org:8080/#/c/16008/2/fe/src/main/java/org/apache/impala/catalog/FileMetadataLoader.java File fe/src/main/java/org/apache/impala/catalog/FileMetadataLoader.java: http://gerrit.cloudera.org:8080/#/c/16008/2/fe/src/main/java/org/apache/impala/catalog/FileMetadataLoader.java@196 PS2, Line 196: Nit: indent with 4 spaces. http://gerrit.cloudera.org:8080/#/c/16008/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/16008/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1536 PS2, Line 1536: LoadStats stats = new LoadStats(getHdfsBaseDirPath()); If LoadStats is only needed if `req.table_info_selector.want_partition_files` is true, could we move this initilization inside the if-statement on L1561 below? http://gerrit.cloudera.org:8080/#/c/16008/2/fe/src/main/java/org/apache/impala/util/AcidUtils.java File fe/src/main/java/org/apache/impala/util/AcidUtils.java: http://gerrit.cloudera.org:8080/#/c/16008/2/fe/src/main/java/org/apache/impala/util/AcidUtils.java@324 PS2, Line 324: public static List Could you add a method description? http://gerrit.cloudera.org:8080/#/c/16008/2/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java File fe/src/test/java/org/apache/impala/catalog/CatalogTest.java: http://gerrit.cloudera.org:8080/#/c/16008/2/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java@138 PS2, Line 138: Nit: 4 space indentation needed here and other places in this file. -- To view, visit http://gerrit.cloudera.org:8080/16008 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ied2c7c3cb2009c407e8fbc3af4722b0d34f57c4a Gerrit-Change-Number: 16008 Gerrit-PatchSet: 2 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 01 Jun 2020 22:33:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9749: ASAN builds should not run FE tests.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15921 ) Change subject: IMPALA-9749: ASAN builds should not run FE tests. .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/15921/4/bin/run-all-tests.sh File bin/run-all-tests.sh: http://gerrit.cloudera.org:8080/#/c/15921/4/bin/run-all-tests.sh@206 PS4, Line 206: if [[ "$CODE_COVERAGE" == true ]]; then > I don't think this works - it means that you'll be running the FE custom cl Done. http://gerrit.cloudera.org:8080/#/c/15921/4/bin/run-all-tests.sh@211 PS4, Line 211: MVN_ARGS_TEMP=$MVN_ARGS > You should refactor this out into its own function in this file so that we' Done -- To view, visit http://gerrit.cloudera.org:8080/15921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I26c469a20032bdc1f4f0bb3938d9f1c50163c99a Gerrit-Change-Number: 15921 Gerrit-PatchSet: 5 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Sat, 30 May 2020 00:22:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9749: ASAN builds should not run FE tests.
Anurag Mantripragada has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/15921 ) Change subject: IMPALA-9749: ASAN builds should not run FE tests. .. IMPALA-9749: ASAN builds should not run FE tests. https://gerrit.cloudera.org/#/c/15778/ inadvertently changed the behaviour of ASAN builds to to run FE tests. After this change, FE custom cluster tests run immediately after other FE tests when FE_TEST is true. Testing: Ran private parametrized job with ASAN. Change-Id: I26c469a20032bdc1f4f0bb3938d9f1c50163c99a --- M bin/run-all-tests.sh 1 file changed, 25 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/15921/5 -- To view, visit http://gerrit.cloudera.org:8080/15921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I26c469a20032bdc1f4f0bb3938d9f1c50163c99a Gerrit-Change-Number: 15921 Gerrit-PatchSet: 5 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-9787: fix spinning thread with memory-based table invalidation
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15994 ) Change subject: IMPALA-9787: fix spinning thread with memory-based table invalidation .. Patch Set 2: Code-Review+2 Hmm, looks like this happens when memory based eviction is configured and time based eviction is not. LGTM. -- To view, visit http://gerrit.cloudera.org:8080/15994 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I47d0cfc40ed7247e96322f9533fa594a03d7a8a3 Gerrit-Change-Number: 15994 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 28 May 2020 17:02:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9749: ASAN builds should not run FE tests.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15921 ) Change subject: IMPALA-9749: ASAN builds should not run FE tests. .. Patch Set 4: The precommits as well as ASAN private parameterized jobs have passed. Will merge after a +2. -- To view, visit http://gerrit.cloudera.org:8080/15921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I26c469a20032bdc1f4f0bb3938d9f1c50163c99a Gerrit-Change-Number: 15921 Gerrit-PatchSet: 4 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Sat, 23 May 2020 14:53:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9749: ASAN builds should not run FE tests.
Anurag Mantripragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/15921 )
Change subject: IMPALA-9749: ASAN builds should not run FE tests.
..
Patch Set 4:
(1 comment)
Made the changes suggested by Thomas. I'm started a private-parametrized asan
build. I will wait for the job to finish.
http://gerrit.cloudera.org:8080/#/c/15921/3/bin/run-all-tests.sh
File bin/run-all-tests.sh:
http://gerrit.cloudera.org:8080/#/c/15921/3/bin/run-all-tests.sh@41
PS3, Line 41: # Parametrized Test Options
: # Run FE Tests
: : ${FE_TEST:=true}
: # Run Backend Tests
: : ${BE_TEST:=true}
> Yeah of course, this is happening because we previously required both CLUST
As per Thomas's suggestion above, I moved the FE custom cluster tests to the
branch in FE_TESTS = true. Also added restart script from L166 after the tests.
--
To view, visit http://gerrit.cloudera.org:8080/15921
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26c469a20032bdc1f4f0bb3938d9f1c50163c99a
Gerrit-Change-Number: 15921
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Sahil Takiar
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Comment-Date: Fri, 22 May 2020 21:43:24 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9749: ASAN builds should not run FE tests.
Anurag Mantripragada has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/15921 ) Change subject: IMPALA-9749: ASAN builds should not run FE tests. .. IMPALA-9749: ASAN builds should not run FE tests. https://gerrit.cloudera.org/#/c/15778/ inadvertently changed the behaviour of ASAN builds to to run FE tests. After this change, FE custom cluster tests require only CLUSTER_TESTS to be true. Testing: Ran private parametrized job with ASAN. Change-Id: I26c469a20032bdc1f4f0bb3938d9f1c50163c99a --- M bin/run-all-tests.sh 1 file changed, 12 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/15921/4 -- To view, visit http://gerrit.cloudera.org:8080/15921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I26c469a20032bdc1f4f0bb3938d9f1c50163c99a Gerrit-Change-Number: 15921 Gerrit-PatchSet: 4 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-9749: ASAN builds should not run FE tests.
Anurag Mantripragada has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/15921 ) Change subject: IMPALA-9749: ASAN builds should not run FE tests. .. IMPALA-9749: ASAN builds should not run FE tests. https://gerrit.cloudera.org/#/c/15778/ inadvertently changed the behaviour of ASAN builds to to run FE tests. This change skips FE tests if it is an ASAN build. Testing: Ran private parametrized job with ASAN. Change-Id: I26c469a20032bdc1f4f0bb3938d9f1c50163c99a --- M bin/run-all-tests.sh 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/15921/3 -- To view, visit http://gerrit.cloudera.org:8080/15921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I26c469a20032bdc1f4f0bb3938d9f1c50163c99a Gerrit-Change-Number: 15921 Gerrit-PatchSet: 3 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] [WIP] IMPALA-9433: File handle cache improvements.
Anurag Mantripragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15699 Change subject: [WIP] IMPALA-9433: File handle cache improvements. .. [WIP] IMPALA-9433: File handle cache improvements. Change-Id: Idf4f79d67d8727fb5175532eb9b5495553027e8c --- M be/src/runtime/io/handle-cache.h M be/src/runtime/io/handle-cache.inline.h 2 files changed, 109 insertions(+), 87 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/15699/2 -- To view, visit http://gerrit.cloudera.org:8080/15699 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Idf4f79d67d8727fb5175532eb9b5495553027e8c Gerrit-Change-Number: 15699 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-9749: ASAN builds should not run FE tests.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15921 ) Change subject: IMPALA-9749: ASAN builds should not run FE tests. .. Patch Set 2: Looks like the environment variable is "CMAKE_BUILD_TYPE" on our GVD hence it could not recognize "BUILD_TYPE". -- To view, visit http://gerrit.cloudera.org:8080/15921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I26c469a20032bdc1f4f0bb3938d9f1c50163c99a Gerrit-Change-Number: 15921 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 18 May 2020 18:33:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9749: ASAN builds should not run FE tests.
Anurag Mantripragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15921 Change subject: IMPALA-9749: ASAN builds should not run FE tests. .. IMPALA-9749: ASAN builds should not run FE tests. https://gerrit.cloudera.org/#/c/15778/ inadvertently changed the behaviour of ASAN builds to to run FE tests. This change skips FE tests if it is an ASAN build. Testing: Ran private parametrized job with ASAN. Change-Id: I26c469a20032bdc1f4f0bb3938d9f1c50163c99a --- M bin/run-all-tests.sh 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/15921/1 -- To view, visit http://gerrit.cloudera.org:8080/15921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I26c469a20032bdc1f4f0bb3938d9f1c50163c99a Gerrit-Change-Number: 15921 Gerrit-PatchSet: 1 Gerrit-Owner: Anurag Mantripragada
[Impala-ASF-CR] IMPALA-9669: Fix wrong table types for GET TABLES in LocalCatalog
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15887 ) Change subject: IMPALA-9669: Fix wrong table types for GET_TABLES in LocalCatalog .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/15887/1/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@693 PS1, Line 693: cached table nit: I think it will be useful to mention the table name and database name in the logs. -- To view, visit http://gerrit.cloudera.org:8080/15887 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2180c603f061838347936f718cd4a0257d82e633 Gerrit-Change-Number: 15887 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 12 May 2020 19:14:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9365: Disable acid-negative.test on non-HDFS filesystems
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15862 ) Change subject: IMPALA-9365: Disable acid-negative.test on non-HDFS filesystems .. Patch Set 1: Code-Review+1 Thanks for the quick fix. LGTM! -- To view, visit http://gerrit.cloudera.org:8080/15862 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I10c6711c39825b648acfe59cb39a435e034dbc92 Gerrit-Change-Number: 15862 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 05 May 2020 01:05:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9677: Fix frontend tests using a non-existent S3 bucket
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15799 ) Change subject: IMPALA-9677: Fix frontend tests using a non-existent S3 bucket .. Patch Set 1: Code-Review+1 (1 comment) Looks good to me. http://gerrit.cloudera.org:8080/#/c/15799/1/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.py File testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.py: http://gerrit.cloudera.org:8080/#/c/15799/1/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.py@114 PS1, Line 114: use_cdp_components: Does setting USE_CDP_HIVE=true also change Hadoop version? -- To view, visit http://gerrit.cloudera.org:8080/15799 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id61ffbf686f8b7827e7fbf13167cfc1dfc06a325 Gerrit-Change-Number: 15799 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 24 Apr 2020 16:42:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9574: support ubuntu 18.04 base image
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15765 ) Change subject: IMPALA-9574: support ubuntu 18.04 base image .. Patch Set 2: Code-Review+1 Looks good to me. -- To view, visit http://gerrit.cloudera.org:8080/15765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8dfdb349e78fd76b91138a70449d51b0ef0021df Gerrit-Change-Number: 15765 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Apr 2020 06:11:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9574: support ubuntu 18.04 base image
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15765 ) Change subject: IMPALA-9574: support ubuntu 18.04 base image .. Patch Set 1: Is this a part of a bigger change? I do not see L22 - L48 in asf master branch. -- To view, visit http://gerrit.cloudera.org:8080/15765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8dfdb349e78fd76b91138a70449d51b0ef0021df Gerrit-Change-Number: 15765 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 21 Apr 2020 03:39:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9663: Fix for NPE in fire listener events
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15745 ) Change subject: IMPALA-9663: Fix for NPE in fire listener events .. Patch Set 2: Code-Review+1 (1 comment) Thanks for the change. Looks good to me. http://gerrit.cloudera.org:8080/#/c/15745/2/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/15745/2/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@1062 PS2, Line 1062: Collections.EMPTY_LIST; Is this same as Collections.emptyList();? -- To view, visit http://gerrit.cloudera.org:8080/15745 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d Gerrit-Change-Number: 15745 Gerrit-PatchSet: 2 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Xiaomeng Zhang Gerrit-Comment-Date: Fri, 17 Apr 2020 01:21:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9663: Fix for NPE in fire listener events
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15745 ) Change subject: IMPALA-9663: Fix for NPE in fire listener events .. Patch Set 2: Thanks. -- To view, visit http://gerrit.cloudera.org:8080/15745 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibfcc5acd598fb0354a5a8288df7c495359f9e53d Gerrit-Change-Number: 15745 Gerrit-PatchSet: 2 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Xiaomeng Zhang Gerrit-Comment-Date: Fri, 17 Apr 2020 01:30:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8795 : Enable event polling by default in dockerized tests.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14272 ) Change subject: IMPALA-8795 : Enable event polling by default in dockerized tests. .. Patch Set 13: A couple of important events processor patches have landed in master. I will try another dry-run of this patch. -- To view, visit http://gerrit.cloudera.org:8080/14272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I222b64236060b3c4c2d554e2f10e129984ebe972 Gerrit-Change-Number: 14272 Gerrit-PatchSet: 13 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Mon, 13 Apr 2020 02:24:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8795 : Enable event polling by default in dockerized tests.
Anurag Mantripragada has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/14272 ) Change subject: IMPALA-8795 : Enable event polling by default in dockerized tests. .. IMPALA-8795 : Enable event polling by default in dockerized tests. Most of the fixes for event processing are committed. This change enables the feature by default for all the tests so that eventually we can turn it on out of the box. Testing [WIP]: 1. Ran core tests with USE_CDP_HIVE=true 2. Running exhaustive tests with USE_CDP_HIVE=false Change-Id: I222b64236060b3c4c2d554e2f10e129984ebe972 --- M docker/catalogd/Dockerfile M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M tests/common/environ.py M tests/common/impala_test_suite.py M tests/custom_cluster/test_event_processing.py M tests/custom_cluster/test_hive_parquet_codec_interop.py M tests/metadata/test_hms_integration.py M tests/metadata/test_metadata_query_statements.py M tests/metadata/test_refresh_partition.py M tests/util/event_processor_utils.py 12 files changed, 202 insertions(+), 133 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/14272/13 -- To view, visit http://gerrit.cloudera.org:8080/14272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I222b64236060b3c4c2d554e2f10e129984ebe972 Gerrit-Change-Number: 14272 Gerrit-PatchSet: 13 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-8632: Add support for self-event detection for insert events
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15648 ) Change subject: IMPALA-8632: Add support for self-event detection for insert events .. Patch Set 5: Code-Review+1 Thanks for addressing my comments. I will let Vihang +2 the change. -- To view, visit http://gerrit.cloudera.org:8080/15648 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7873fbb2c159343690f93b9d120f6b425b983dcf Gerrit-Change-Number: 15648 Gerrit-PatchSet: 5 Gerrit-Owner: Xiaomeng Zhang Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Xiaomeng Zhang Gerrit-Comment-Date: Wed, 08 Apr 2020 05:17:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8632: Add support for self-event detection for insert events
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15648 ) Change subject: IMPALA-8632: Add support for self-event detection for insert events .. Patch Set 4: (15 comments) Thanks for the change, I have added some comments, most of them are style-related and some nits. http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@54 PS4, Line 54: import org.apache.impala.catalog.events.NoOpEventProcessor; Nit: Duplicate import, could you remove this? http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@934 PS4, Line 934: Nit: Incorrect indentation. Remove the empty spaces. http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@937 PS4, Line 937: Incorrect indentation. Remove the empty spaces. http://gerrit.cloudera.org:8080/#/c/15648/4/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/15648/4/fe/src/main/java/org/apache/impala/catalog/Table.java@807 PS4, Line 807: Nit: Incorrect indentation. Remove the empty spaces. http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/catalog/Table.java@826 PS4, Line 826: Nit: Incorrect indentation. Remove the empty spaces. http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/catalog/events/InFlightEvents.java File fe/src/main/java/org/apache/impala/catalog/events/InFlightEvents.java: http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/catalog/events/InFlightEvents.java@65 PS4, Line 65: eventIds_ Here and everywhere else, if this means "only" insert events, may I suggest using insertEventIds_? http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/catalog/events/InFlightEvents.java@69 PS4, Line 69: @param isInsertEvent if true, return list of versions for in-flight Insert events :* if false, return list of eventIds for in-flight DDL events Should this be the other way around? i.e., return eventIds for in-flight insert events if it is insert events and versions if it is not. Also, indentation is incorrect, please remove the extra spaces. http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/catalog/events/InFlightEvents.java@87 PS4, Line 87: Nit: Indentation incorrect, remove extra spaces. http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/catalog/events/InFlightEvents.java@107 PS4, Line 107: Nit: Indentation incorrect, remove extra spaces. http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java: http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@773 PS4, Line 773: // long eventId_test = catalog_.public_eventId.consumeId(); Could you remove this comment? http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@787 PS4, Line 787: /** : * Check self-event for in flight Insert event using eventId : */ Nit: Not sure this is needed since this is an overridden method and its purpose is the same as described in L318. http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@75 PS4, Line 75: *; We should be careful with *. Are you sure we are importing everything from org.apache.impala.catalog? http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4424 PS4, Line 4424: List of all catalog object that was insert into Did you mean "List of all partitions we insert into"? http://gerrit.cloudera.org:8080/#/c/15648/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4487 PS4, Line 4487: // Firing insert events by making calls to HMS APIs can be slow for tables with : // large number of partitions. > this comment should be updated with the bulk API it should not be a problem I see that MetastoreShim.fireInsertEvent() for hive-2 does not insert in bulk and is still making one RPC per partition. We should keep the earlier asynchronous fireInsertEvent() for hive-2.
[Impala-ASF-CR] IMPALA-8857: Fix flaky Kudu tests with external inserts
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15633 ) Change subject: IMPALA-8857: Fix flaky Kudu tests with external inserts .. Patch Set 1: Change looks good to me. Just curious as to what happens during reads from Impala client after setting the timestamp, do the reads happen at the snapshot that has been set? -- To view, visit http://gerrit.cloudera.org:8080/15633 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5b787f6542dc31dcd846f19576a060a89aec891d Gerrit-Change-Number: 15633 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 07 Apr 2020 19:14:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8857: Fix flaky Kudu tests with external inserts
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15633 ) Change subject: IMPALA-8857: Fix flaky Kudu tests with external inserts .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15633 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5b787f6542dc31dcd846f19576a060a89aec891d Gerrit-Change-Number: 15633 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 07 Apr 2020 19:14:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8005: Randomize partitioning exchanges.
Anurag Mantripragada has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/15497 ) Change subject: IMPALA-8005: Randomize partitioning exchanges. .. IMPALA-8005: Randomize partitioning exchanges. Currently, we use the same hash seed for partitioning exchanges at the sender. For a table with skew in distribution in the shuffling keys, multiple queries using the same shuffling keys for exchanges will end up hashing to the same destination fragments running on a particular host and potentially overloading that host. This patch seeds the hash with query id. This will ensure that the partitioning exchanges do not always hash to the same destination with same shuffling keys. Testing: Added a test to data-stream-test to verify the data values at destination are different for different queries. Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468 --- M be/src/runtime/data-stream-test.cc M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/krpc-data-stream-sender.h 3 files changed, 93 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/15497/5 -- To view, visit http://gerrit.cloudera.org:8080/15497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468 Gerrit-Change-Number: 15497 Gerrit-PatchSet: 5 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-8005: Randomize partitioning exchanges.
Anurag Mantripragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/15497 )
Change subject: IMPALA-8005: Randomize partitioning exchanges.
..
Patch Set 5:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/15497/4/be/src/runtime/data-stream-test.cc
File be/src/runtime/data-stream-test.cc:
http://gerrit.cloudera.org:8080/#/c/15497/4/be/src/runtime/data-stream-test.cc@496
PS4, Line 496: // each sender sent all values
: EXPECT_EQ(k / num_senders, *j);
: if (k/num_senders !=
> Couple small style things:
Thanks for the pointers. Since you did not have a preference and the Google
style guide says "Prefer using return values over output parameters: they
improve readability and often provide the same or better performance." I went
ahead and used return values.
http://gerrit.cloudera.org:8080/#/c/15497/4/be/src/runtime/data-stream-test.cc@516
PS4, Line 516: StartSender(TPartitionType::HASH_PARTITIONED, buffer_size,
: reset_hash_seed);
: }
: JoinSenders();
: CheckSenders();
: JoinReceivers();
: receiver_data_map.clear();
:
: for (int i = 0; i < receiver_info_.size(); i++) {
:
> Nit: For the output parameter receiver_data_map, go ahead and do receiver_d
Done
http://gerrit.cloudera.org:8080/#/c/15497/4/be/src/runtime/data-stream-test.cc@596
PS4, Line 596: eset the hash seed with a new query id. Useful for testing h
> Since you did this above for the 'config' variable, I think it would be cle
Done
http://gerrit.cloudera.org:8080/#/c/15497/4/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:
http://gerrit.cloudera.org:8080/#/c/15497/4/be/src/runtime/krpc-data-stream-sender.cc@739
PS4, Line 739: }
:
> Do we need this? We set exchange_hash_seed_ above.
Thanks for the catch. Removed.
--
To view, visit http://gerrit.cloudera.org:8080/15497
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468
Gerrit-Change-Number: 15497
Gerrit-PatchSet: 5
Gerrit-Owner: Anurag Mantripragada
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Comment-Date: Thu, 26 Mar 2020 23:13:12 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9549: Handle catalogd startup delays when using local catalog
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15561 ) Change subject: IMPALA-9549: Handle catalogd startup delays when using local catalog .. Patch Set 2: Code-Review+1 Thanks for the test coverage. Change looks good to me. -- To view, visit http://gerrit.cloudera.org:8080/15561 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b5a94c59f25927a169dcb58f310ce6b1044 Gerrit-Change-Number: 15561 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Thu, 26 Mar 2020 16:10:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8005: Randomize partitioning exchanges.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15497 ) Change subject: IMPALA-8005: Randomize partitioning exchanges. .. Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/15497/3/be/src/runtime/data-stream-test.cc File be/src/runtime/data-stream-test.cc: http://gerrit.cloudera.org:8080/#/c/15497/3/be/src/runtime/data-stream-test.cc@514 PS3, Line 514: JoinReceivers(); > nit: add a newline between this and next method Done http://gerrit.cloudera.org:8080/#/c/15497/3/be/src/runtime/data-stream-test.cc@725 PS3, Line 725: int num_receivers = 4; > Seems like this code block can be consolidated with the identical block on Done. http://gerrit.cloudera.org:8080/#/c/15497/3/be/src/runtime/data-stream-test.cc@742 PS3, Line 742: ASSERT_EQ(result, true); > Is the non-match guaranteed though ? Given that there are only 4 receivers, Agreed, we should have a more deterministic way to test this. Any thoughts? http://gerrit.cloudera.org:8080/#/c/15497/3/be/src/runtime/krpc-data-stream-sender.cc File be/src/runtime/krpc-data-stream-sender.cc: http://gerrit.cloudera.org:8080/#/c/15497/3/be/src/runtime/krpc-data-stream-sender.cc@88 PS3, Line 88: exchange_hash_seed_ = > Suggest keeping the constant factor as a static constexpr in the header fil Done -- To view, visit http://gerrit.cloudera.org:8080/15497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468 Gerrit-Change-Number: 15497 Gerrit-PatchSet: 4 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Sat, 21 Mar 2020 23:55:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8005: Randomize partitioning exchanges.
Anurag Mantripragada has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/15497 ) Change subject: IMPALA-8005: Randomize partitioning exchanges. .. IMPALA-8005: Randomize partitioning exchanges. Currently, we use the same hash seed for partitioning exchanges at the sender. For a table with skew in distribution in the shuffling keys, multiple queries using the same shuffling keys for exchanges will end up hashing to the same destination fragments running on a particular host and potentially overloading that host. This patch seeds the hash with query id. This will ensure that the partitioning exchanges do not always hash to the same destination with same shuffling keys. Testing: Added a test to data-stream-test to verify the data values at destination are different for different queries. Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468 --- M be/src/runtime/data-stream-test.cc M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/krpc-data-stream-sender.h 3 files changed, 94 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/15497/4 -- To view, visit http://gerrit.cloudera.org:8080/15497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468 Gerrit-Change-Number: 15497 Gerrit-PatchSet: 4 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-8005: Randomize partitioning exchanges.
Anurag Mantripragada has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/15497 ) Change subject: IMPALA-8005: Randomize partitioning exchanges. .. IMPALA-8005: Randomize partitioning exchanges. Currently, we use the same hash seed for partitioning exchanges at the sender. For a table with skew in distribution in the shuffling keys, multiple queries using the same shuffling keys for exchanges will end up hashing to the same destination fragments running on a particular host and potentially overloading that host. This patch seeds the hash with query id. This will ensure that the partitioning exchanges do not always hash to the same destination with same shuffling keys. Testing: Added a test to data-stream-test to verify the data values at destination are different for different queries. Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468 --- M be/src/runtime/data-stream-test.cc M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/krpc-data-stream-sender.h 3 files changed, 96 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/15497/3 -- To view, visit http://gerrit.cloudera.org:8080/15497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468 Gerrit-Change-Number: 15497 Gerrit-PatchSet: 3 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-8005: Randomize partitioning exchanges.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15497 ) Change subject: IMPALA-8005: Randomize partitioning exchanges. .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15497/1/be/src/runtime/data-stream-test.cc File be/src/runtime/data-stream-test.cc: http://gerrit.cloudera.org:8080/#/c/15497/1/be/src/runtime/data-stream-test.cc@489 PS1, Line 489: EXPECT_EQ(k / num_senders, *j); > This is causing your clang-tidy problem. Oops! Don't know how that crept into the code. Thanks for the catch. -- To view, visit http://gerrit.cloudera.org:8080/15497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468 Gerrit-Change-Number: 15497 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Thu, 19 Mar 2020 20:40:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8005: Randomize partitioning exchanges.
Anurag Mantripragada has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/15497 ) Change subject: IMPALA-8005: Randomize partitioning exchanges. .. IMPALA-8005: Randomize partitioning exchanges. Currently, we use the same hash seed for partitioning exchanges at the sender. For a table with skew in distribution in the shuffling keys, multiple queries using the same shuffling keys for exchanges will end up hashing to the same destination fragments running on a particular host and potentially overloading that host. This patch seeds the hash with query id. This will ensure that the partitioning exchanges do not always hash to the same destination with same shuffling keys. Testing: Added a test to data-stream-test to verify the data values at destination are different for different queries. Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468 --- M be/src/runtime/data-stream-test.cc M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/krpc-data-stream-sender.h 3 files changed, 97 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/15497/2 -- To view, visit http://gerrit.cloudera.org:8080/15497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468 Gerrit-Change-Number: 15497 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell
[Impala-ASF-CR] IMPALA-8005: Randomize partitioning exchanges.
Anurag Mantripragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15497 Change subject: IMPALA-8005: Randomize partitioning exchanges. .. IMPALA-8005: Randomize partitioning exchanges. Currently, we use the same hash seed for partitioning exchanges at the sender. For a table with skew in distribution in the shuffling keys, multiple queries using the same shuffling keys for exchanges will end up hashing to the same destination fragments running on a particular host and potentially overloading that host. This patch seeds the hash with query id. This will ensure that the partitioning exchanges do not always hash to the same destination with same shuffling keys. Testing: Added a test to data-stream-test to verify the data values at destination are different for different queries. Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468 --- M be/src/runtime/data-stream-test.cc M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/krpc-data-stream-sender.h 3 files changed, 97 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/15497/1 -- To view, visit http://gerrit.cloudera.org:8080/15497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1936e6cc3e8d66420a5a9301f49221ca38f3e468 Gerrit-Change-Number: 15497 Gerrit-PatchSet: 1 Gerrit-Owner: Anurag Mantripragada
[Impala-ASF-CR] IMPALA-9369: Make createInsertEvents() async.
Anurag Mantripragada has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/15263 ) Change subject: IMPALA-9369: Make createInsertEvents() async. .. IMPALA-9369: Make createInsertEvents() async. This patch makes the createInsertEvents() method async to avoid blocking the insert code path for long periods for tables with large number of partitions and files. Currently the createInsertEvents() method fires the HMS insert event one partition at a time. This makes insert statements with thousands of new files significantly slower. This change makes the createInsertEvent() call asynchronous by making it run in a separate thread. Testing: - Ran MetastoreEventsProcessorTest#testInsertEvents. - Ran test_events_processing::test_insert_events. Change-Id: I97802a5c03abc067fccf9e3a9d0047324626706e --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java 3 files changed, 84 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/15263/3 -- To view, visit http://gerrit.cloudera.org:8080/15263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I97802a5c03abc067fccf9e3a9d0047324626706e Gerrit-Change-Number: 15263 Gerrit-PatchSet: 3 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9369: Make createInsertEvents() async.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15263 ) Change subject: IMPALA-9369: Make createInsertEvents() async. .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/15263/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/15263/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4393 PS2, Line 4393: } > This implementation will spin up a singleThreadExecutor for each partition Oops! Thanks for the catch. Changed it as suggested. -- To view, visit http://gerrit.cloudera.org:8080/15263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I97802a5c03abc067fccf9e3a9d0047324626706e Gerrit-Change-Number: 15263 Gerrit-PatchSet: 3 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Mon, 09 Mar 2020 19:00:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9357: Fix race condition in alter database event
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15260 ) Change subject: IMPALA-9357: Fix race condition in alter_database event .. Patch Set 5: Code-Review+1 (1 comment) Thanks! http://gerrit.cloudera.org:8080/#/c/15260/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/15260/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@449 PS5, Line 449: table Did you mean DB? -- To view, visit http://gerrit.cloudera.org:8080/15260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e Gerrit-Change-Number: 15260 Gerrit-PatchSet: 5 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 05 Mar 2020 18:19:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8738: "show extended tables" to return more than the table name
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14422 ) Change subject: IMPALA-8738: "show extended tables" to return more than the table name .. Patch Set 8: Should we abandon this change? -- To view, visit http://gerrit.cloudera.org:8080/14422 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I63057c9d2fc453f95c6890bdc90e11c61a98a419 Gerrit-Change-Number: 14422 Gerrit-PatchSet: 8 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 04 Mar 2020 19:26:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9369: Make createInsertEvents() async.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15263 ) Change subject: IMPALA-9369: Make createInsertEvents() async. .. Patch Set 2: Thanks for researching CompletableFuture :) I added the shutdown for the thread. I changed the patch to run only the firing HMS API asynchronous. This should also prevent the issues of table lock being released. Regarding modifying the state, my understanding is that from Impala's perspective this async function does not change any state except logging in case of any exceptions. Please correct me if I'm wrong. -- To view, visit http://gerrit.cloudera.org:8080/15263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I97802a5c03abc067fccf9e3a9d0047324626706e Gerrit-Change-Number: 15263 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Mon, 02 Mar 2020 01:53:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9369: Make createInsertEvents() async.
Anurag Mantripragada has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/15263 ) Change subject: IMPALA-9369: Make createInsertEvents() async. .. IMPALA-9369: Make createInsertEvents() async. This patch makes the createInsertEvents() method async to avoid blocking the insert code path for long periods for tables with large number of partitions and files. Currently the createInsertEvents() method fires the HMS insert event one partition at a time. This makes insert statements with thousands of new files significantly slower. This change makes the createInsertEvent() call asynchronous by making it run in a separate thread. Testing: - Ran MetastoreEventsProcessorTest#testInsertEvents. - Ran test_events_processing::test_insert_events. Change-Id: I97802a5c03abc067fccf9e3a9d0047324626706e --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java 1 file changed, 38 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/15263/2 -- To view, visit http://gerrit.cloudera.org:8080/15263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I97802a5c03abc067fccf9e3a9d0047324626706e Gerrit-Change-Number: 15263 Gerrit-PatchSet: 2 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9357: Fix race condition in alter database event
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15260 ) Change subject: IMPALA-9357: Fix race condition in alter_database event .. Patch Set 1: Code-Review+1 LGTM. -- To view, visit http://gerrit.cloudera.org:8080/15260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I472fd8a55740769ee5cdb84e48422a4ab39a8d1e Gerrit-Change-Number: 15260 Gerrit-PatchSet: 1 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 21 Feb 2020 22:28:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8852: Skip short-circuit config check for dedicated coordinator
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15173 ) Change subject: IMPALA-8852: Skip short-circuit config check for dedicated coordinator .. Patch Set 10: Code-Review+1 LGTM! -- To view, visit http://gerrit.cloudera.org:8080/15173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I373d4037f4cee203322a398b77b75810ba708bb5 Gerrit-Change-Number: 15173 Gerrit-PatchSet: 10 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Fri, 21 Feb 2020 18:26:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9369: Make createInsertEvents() async.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15263 ) Change subject: IMPALA-9369: Make createInsertEvents() async. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15263/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/15263/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4340 PS1, Line 4340: table, affectedExistingPartitions, : isInsertOverwrite Should we make local copies of these in this method since this method can take a while? -- To view, visit http://gerrit.cloudera.org:8080/15263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I97802a5c03abc067fccf9e3a9d0047324626706e Gerrit-Change-Number: 15263 Gerrit-PatchSet: 1 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Fri, 21 Feb 2020 03:43:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9369: Make createInsertEvents() async.
Anurag Mantripragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15263 Change subject: IMPALA-9369: Make createInsertEvents() async. .. IMPALA-9369: Make createInsertEvents() async. This patch makes the createInsertEvents() method async to avoid blocking the insert code path for long periods for tables with large number of partitions and files. Currently the createInsertEvents() method fires the HMS insert event one partition at a time. This makes insert statements with thousands of new files significantly slower. This change makes the createInsertEvent() call asynchronous by making it run in a separate thread. This is okay because we only log a message in case of failure of this method. Testing: - Ran MetastoreEventsProcessorTest#testInsertEvents. - Ran test_events_processing::test_insert_events. Change-Id: I97802a5c03abc067fccf9e3a9d0047324626706e --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java 2 files changed, 20 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/15263/1 -- To view, visit http://gerrit.cloudera.org:8080/15263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I97802a5c03abc067fccf9e3a9d0047324626706e Gerrit-Change-Number: 15263 Gerrit-PatchSet: 1 Gerrit-Owner: Anurag Mantripragada
[Impala-ASF-CR] IMPALA-6689: Speed up point lookup for Kudu primary key
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15250 ) Change subject: IMPALA-6689: Speed up point lookup for Kudu primary key .. Patch Set 2: I agree that these optimizations could also be made in HDFS for better cardinality estimations. However, it can be tricky in the case of HDFS tables as we do not enforce these constraints, we just rely on user hints about the existence of PKs. Ignoring that, I think Tim is right, for single-column PK's we already have pretty good estimates. In theory, We could improve the estimates for composite PKs. Apart from Csaba's comments, the patch looks good to me. -- To view, visit http://gerrit.cloudera.org:8080/15250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4631cd4d1a528a1152b5cdcb268426f2ba1a0c08 Gerrit-Change-Number: 15250 Gerrit-PatchSet: 2 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 20 Feb 2020 21:18:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9386: Remove hive-metastore dependency from hive-3 profile
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15223 ) Change subject: IMPALA-9386: Remove hive-metastore dependency from hive-3 profile .. Patch Set 2: Abandon this? -- To view, visit http://gerrit.cloudera.org:8080/15223 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iab7de6a7f11ef0bff85c0fb03f004756c3cde784 Gerrit-Change-Number: 15223 Gerrit-PatchSet: 2 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Thu, 20 Feb 2020 07:56:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
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
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Norbert Luksa
Gerrit-Reviewer: Quanlong Huang
Gerrit-Comment-Date: Sat, 15 Feb 2020 07:22:04 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
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 Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
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 Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-9256: Refactor constraint information into a separate class.
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 Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 13 Feb 2020 18:17:58 + Gerrit-HasComments: No
[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
[Impala-ASF-CR] IMPALA-8852: Skip short-circuit config check for coordinator-only mode
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15173 ) Change subject: IMPALA-8852: Skip short-circuit config check for coordinator-only mode .. Patch Set 4: Code-Review+1 LGTM. -- To view, visit http://gerrit.cloudera.org:8080/15173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I373d4037f4cee203322a398b77b75810ba708bb5 Gerrit-Change-Number: 15173 Gerrit-PatchSet: 4 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Mon, 10 Feb 2020 05:20:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8852: Skip short-circuit config check for coordinator-only mode
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15173 ) Change subject: IMPALA-8852: Skip short-circuit config check for coordinator-only mode .. Patch Set 3: Code-Review+1 (1 comment) LGTM. Okay for Andrew to +2 after code style nit is addressed. http://gerrit.cloudera.org:8080/#/c/15173/3/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java File fe/src/test/java/org/apache/impala/service/JniFrontendTest.java: http://gerrit.cloudera.org:8080/#/c/15173/3/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java@79 PS3, Line 79: Line 79 and 80 should have 4 space indent. I know these can be difficult to catch but the best way is to teach the IDE . You can use something like https://github.com/google/styleguide/blob/gh-pages/intellij-java-google-style.xml and modify it to comply with our guidelines. (I only had to change line wrap at 90.) If I'm unsure, I usually look at the surrounding code and follow that style. -- To view, visit http://gerrit.cloudera.org:8080/15173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I373d4037f4cee203322a398b77b75810ba708bb5 Gerrit-Change-Number: 15173 Gerrit-PatchSet: 3 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Sat, 08 Feb 2020 02:30:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9358: Query slowdown with inline views and hundreds of columns
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15157 ) Change subject: IMPALA-9358: Query slowdown with inline views and hundreds of columns .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77423d9c10e1edbb505cb210b5c072281b5d7cfc Gerrit-Change-Number: 15157 Gerrit-PatchSet: 3 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Fri, 07 Feb 2020 18:41:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8795 : Enable event polling by default in dockerized tests.
Anurag Mantripragada has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/14272 ) Change subject: IMPALA-8795 : Enable event polling by default in dockerized tests. .. IMPALA-8795 : Enable event polling by default in dockerized tests. Most of the fixes for event processing are committed. This change enables the feature by default for all the tests so that eventually we can turn it on out of the box. Testing [WIP]: 1. Ran core tests with USE_CDP_HIVE=true 2. Running exhaustive tests with USE_CDP_HIVE=false Change-Id: I222b64236060b3c4c2d554e2f10e129984ebe972 --- M docker/catalogd/Dockerfile M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M tests/common/environ.py M tests/common/impala_test_suite.py M tests/custom_cluster/test_event_processing.py M tests/custom_cluster/test_hive_parquet_codec_interop.py M tests/metadata/test_hms_integration.py M tests/metadata/test_metadata_query_statements.py M tests/metadata/test_refresh_partition.py M tests/util/event_processor_utils.py 12 files changed, 204 insertions(+), 140 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/14272/12 -- To view, visit http://gerrit.cloudera.org:8080/14272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I222b64236060b3c4c2d554e2f10e129984ebe972 Gerrit-Change-Number: 14272 Gerrit-PatchSet: 12 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-8852: Skipping short-circuit config check for coordinator only
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15173 ) Change subject: IMPALA-8852: Skipping short-circuit config check for coordinator only .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/15173/2/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java File fe/src/test/java/org/apache/impala/service/JniFrontendTest.java: http://gerrit.cloudera.org:8080/#/c/15173/2/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java@66 PS2, Line 66: testCheckShortCircuitReadShouldReturnErrorForInvalidConfig() Would it be possible to consolidate this and the next into a single test? Say something like "testCheckShortCircuitConfigs()". -- To view, visit http://gerrit.cloudera.org:8080/15173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I373d4037f4cee203322a398b77b75810ba708bb5 Gerrit-Change-Number: 15173 Gerrit-PatchSet: 2 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 06 Feb 2020 17:42:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8852: Skipping short-circuit config check for coordinator only
Anurag Mantripragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/15173 )
Change subject: IMPALA-8852: Skipping short-circuit config check for
coordinator only
..
Patch Set 2:
(5 comments)
Thanks for the change. Just a few nits.
http://gerrit.cloudera.org:8080/#/c/15173/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15173/2//COMMIT_MSG@7
PS2, Line 7: Skipping
Nit: Change to Skip?
http://gerrit.cloudera.org:8080/#/c/15173/2//COMMIT_MSG@13
PS2, Line 13:
Usually, our commit messages also have a section "Testing" to mention the tests
added by this commit. This will help reviewers understand the testing done for
the patch. See this for example. https://gerrit.cloudera.org/#/c/15157/
You could add something like:
Added tests in JNIFrontendTest.java to verify the short-circuit check is
skipped if it is coordinator-only mode.
http://gerrit.cloudera.org:8080/#/c/15173/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:
http://gerrit.cloudera.org:8080/#/c/15173/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java@719
PS2, Line 719: ,
Could you also mention here that this will skip the check if it is
coordinator-only mode?
http://gerrit.cloudera.org:8080/#/c/15173/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java@806
PS2, Line 806:
Nit: Extra space.
http://gerrit.cloudera.org:8080/#/c/15173/2/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java
File fe/src/test/java/org/apache/impala/service/JniFrontendTest.java:
http://gerrit.cloudera.org:8080/#/c/15173/2/fe/src/test/java/org/apache/impala/service/JniFrontendTest.java@90
PS2, Line 90: conf.setStrings("dfs.domain.socket.path", "");
Don't we need to set dfs.client.read.shortcircuit = true here? Is the default
true?
--
To view, visit http://gerrit.cloudera.org:8080/15173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I373d4037f4cee203322a398b77b75810ba708bb5
Gerrit-Change-Number: 15173
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Thu, 06 Feb 2020 17:38:00 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9358: Query slowdown with inline views and hundreds of columns
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15157 ) Change subject: IMPALA-9358: Query slowdown with inline views and hundreds of columns .. Patch Set 2: Code-Review+1 LGTM! I ran the FE tests successfully. -- To view, visit http://gerrit.cloudera.org:8080/15157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77423d9c10e1edbb505cb210b5c072281b5d7cfc Gerrit-Change-Number: 15157 Gerrit-PatchSet: 2 Gerrit-Owner: Kurt Deschler Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 06 Feb 2020 04:25:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9358: Query slowdown with inline views and hundreds of columns
Anurag Mantripragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/15157 )
Change subject: IMPALA-9358: Query slowdown with inline views and hundreds of
columns
..
Patch Set 1:
(2 comments)
Thanks for making this quick-fix. Comments are more for my understanding than
about the code change.
This change seems small enough but did you have a chance to run any EE/FE tests
that touch this path as a sanity check that nothing went there?
http://gerrit.cloudera.org:8080/#/c/15157/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15157/1//COMMIT_MSG@25
PS1, Line 25: Performance testing with a query with 1000 expressions of the
Not a blocking comment, feel free to skip it but would it be possible to
include the performance improvement you observed here?
http://gerrit.cloudera.org:8080/#/c/15157/1/fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java
File fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java:
http://gerrit.cloudera.org:8080/#/c/15157/1/fe/src/main/java/org/apache/impala/analysis/ExprSubstitutionMap.java@77
PS1, Line 77: for (int i = hint; i < lhs_.size(); ++i) {
: if (lhsExpr.equals(lhs_.get(i))) return rhs_.get(i);
: }
Just trying to understand this. Is it safe to assume that most equality
searches end in this for loop?
--
To view, visit http://gerrit.cloudera.org:8080/15157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77423d9c10e1edbb505cb210b5c072281b5d7cfc
Gerrit-Change-Number: 15157
Gerrit-PatchSet: 1
Gerrit-Owner: Kurt Deschler
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Wed, 05 Feb 2020 03:55:25 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9336: [DOCS] constraints
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15146 ) Change subject: IMPALA-9336: [DOCS] constraints .. Patch Set 4: Code-Review+1 Looks good to me. -- To view, visit http://gerrit.cloudera.org:8080/15146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee12da322fbdab7c671c17ceb8436bc3ace2b820 Gerrit-Change-Number: 15146 Gerrit-PatchSet: 4 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Mon, 03 Feb 2020 18:04:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9336: [DOCS] constraints
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15146 ) Change subject: IMPALA-9336: [DOCS] constraints .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/15146/3/docs/topics/impala_create_table.xml File docs/topics/impala_create_table.xml: http://gerrit.cloudera.org:8080/#/c/15146/3/docs/topics/impala_create_table.xml@147 PS3, Line 147: My apologies! I missed a "," after primary key spec. Could you please add a "," here? Thanks! -- To view, visit http://gerrit.cloudera.org:8080/15146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee12da322fbdab7c671c17ceb8436bc3ace2b820 Gerrit-Change-Number: 15146 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 03 Feb 2020 04:39:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9330: Resolve nested types of masked tables
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15108 ) Change subject: IMPALA-9330: Resolve nested types of masked tables .. Patch Set 7: Code-Review+1 (1 comment) The patch makes sense to me. I will let someone with more expertise in the analyzer take another look at it. http://gerrit.cloudera.org:8080/#/c/15108/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/15108/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@912 PS7, Line 912: Resolved Nit: Did you mean Resolves? -- To view, visit http://gerrit.cloudera.org:8080/15108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1cc5565c64c1a4a56445b8edde59b1168f387791 Gerrit-Change-Number: 15108 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Mon, 03 Feb 2020 04:36:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9336: [DOCS] constraints
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15146 ) Change subject: IMPALA-9336: [DOCS] constraints .. Patch Set 2: (1 comment) Thanks for addressing my comments. Another minor change is needed in the spec. I added the comment below. http://gerrit.cloudera.org:8080/#/c/15146/2/docs/topics/impala_create_table.xml File docs/topics/impala_create_table.xml: http://gerrit.cloudera.org:8080/#/c/15146/2/docs/topics/impala_create_table.xml@147 PS2, Line 147: [DISABLE] [NOVALIDATE] [RELY] This should immediately follow primary key specification like so: PRIMARY KEY (col_name, ...) [DISABLE] [NOVALIDATE] [RELY] [foreign_key_specification, ...] -- To view, visit http://gerrit.cloudera.org:8080/15146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee12da322fbdab7c671c17ceb8436bc3ace2b820 Gerrit-Change-Number: 15146 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sun, 02 Feb 2020 22:50:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9336: [DOCS] constraints
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15146 ) Change subject: IMPALA-9336: [DOCS] constraints .. Patch Set 1: (5 comments) Thanks for making these changes to the docs. I have a few comments and nits. http://gerrit.cloudera.org:8080/#/c/15146/1/docs/topics/impala_create_table.xml File docs/topics/impala_create_table.xml: http://gerrit.cloudera.org:8080/#/c/15146/1/docs/topics/impala_create_table.xml@145 PS1, Line 145: Nit: I see a few places where there is extra indentation. In Gerrit, this is marked with red. Could you please remove these for consistency? http://gerrit.cloudera.org:8080/#/c/15146/1/docs/topics/impala_create_table.xml@147 PS1, Line 147: [DISABLE] [NOVALIDATE] [RELY] DISABLE NOVALIDATE RELY is not common to the entire PK/FK definition, it has to be mentioned in primary key spec as well as EACH foreign key spec. Maybe this will be more accurate?: constraint_specification: PRIMARY KEY (col_name, ...) [DISABLE] [NOVALIDATE] [RELY],[foreign_key_specification, ...] foreign_key_specification: FOREIGN KEY (col_name, ...) REFERENCES table_name(col_name, ...) [DISABLE] [NOVALIDATE] [RELY] http://gerrit.cloudera.org:8080/#/c/15146/1/docs/topics/impala_create_table.xml@150 PS1, Line 150: Nit: Extra indentation http://gerrit.cloudera.org:8080/#/c/15146/1/docs/topics/impala_create_table.xml@151 PS1, Line 151: Nit: Extra indentation http://gerrit.cloudera.org:8080/#/c/15146/1/docs/topics/impala_create_table.xml@291 PS1, Line 291: Nit: Extra indentation -- To view, visit http://gerrit.cloudera.org:8080/15146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee12da322fbdab7c671c17ceb8436bc3ace2b820 Gerrit-Change-Number: 15146 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sun, 02 Feb 2020 02:35:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9311: Store SQLPrimaryKeys in canonical order.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15106 ) Change subject: IMPALA-9311: Store SQLPrimaryKeys in canonical order. .. Patch Set 1: Could you also please merge this, Tim? -- To view, visit http://gerrit.cloudera.org:8080/15106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0f798d7a2659c6cd061002db151f3fa787eb6370 Gerrit-Change-Number: 15106 Gerrit-PatchSet: 1 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Mon, 27 Jan 2020 21:16:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9329: Don't add nested columns in TableMaskingView
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/15108 ) Change subject: IMPALA-9329: Don't add nested columns in TableMaskingView .. Patch Set 4: Code-Review+1 The patch looks good to me. Thanks for adding the tests. Just for my understanding, do the nested columns never appear if any of the primitive columns are masked? -- To view, visit http://gerrit.cloudera.org:8080/15108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1cc5565c64c1a4a56445b8edde59b1168f387791 Gerrit-Change-Number: 15108 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Mon, 27 Jan 2020 17:53:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9311: Store SQLPrimaryKeys in canonical order.
Anurag Mantripragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15106 Change subject: IMPALA-9311: Store SQLPrimaryKeys in canonical order. .. IMPALA-9311: Store SQLPrimaryKeys in canonical order. HMS seems to be returning SQLPrimaryKeys in inconsistent orders. This makes some of the primary keys tests flaky. This change sorts the list of primary keys and stores them in canonical order within Impala. Testing: - Modified the tests that were relying on HMS to return same order every time. - Ran parametrized job. Change-Id: I0f798d7a2659c6cd061002db151f3fa787eb6370 --- M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.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 testdata/workloads/functional-query/queries/QueryTest/show-create-table.test 7 files changed, 31 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/15106/1 -- To view, visit http://gerrit.cloudera.org:8080/15106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I0f798d7a2659c6cd061002db151f3fa787eb6370 Gerrit-Change-Number: 15106 Gerrit-PatchSet: 1 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode.
Anurag Mantripragada has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/14731 ) Change subject: IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode. .. IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode. This change add a new method 'loadConstraints()' to the MetaProvider interface. 1. In CatalogdMetaProvider implementation, we fetch the primary key (PK) and foreign key(FK) information via the GetPartialCatalogObject() RPC to the catalogd. This is modified to include PK/FK information. This is because, on catalog side we eagerly load PK/FK information which can be sent over to local catalog in a single RPC to Catalog. This information is then stored in TableMetaRef object for future consumers. 2. In the DirectMetaProvider implementation, we make two RPCs to HMS to directly get PK/FK information. Load constraints can be extended to include other constraints later (for ex: unique constraints.) Testing: - Added tests in LocalCatalogTest, CatalogTest and PartialCatalogInfoTest - This change also modifies the toSqlUtil for show create table statements. Added a test for the same. Change-Id: I7ea7e1bacf6eb502c67caf310a847b32687e0d58 --- M common/thrift/CatalogService.thrift M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.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 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/MetaProvider.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 testdata/workloads/functional-query/queries/QueryTest/show-create-table.test 14 files changed, 375 insertions(+), 67 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/14731/7 -- To view, visit http://gerrit.cloudera.org:8080/14731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7ea7e1bacf6eb502c67caf310a847b32687e0d58 Gerrit-Change-Number: 14731 Gerrit-PatchSet: 7 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode.
Anurag Mantripragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14731 )
Change subject: IMPALA-9158: Support loading primary key/foreign key
constraints in LocalCatalog Mode.
..
Patch Set 7:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14731/6/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/14731/6/fe/src/main/java/org/apache/impala/analysis/TableDef.java@555
PS6, Line 555: } catch (TException e) {
> Looks like the TException can only be thrown when using local catalog mode
Done
http://gerrit.cloudera.org:8080/#/c/14731/6/fe/src/main/java/org/apache/impala/analysis/TableDef.java@556
PS6, Line 556: In local cata
> I recommend using "legacy catalog mode" / "local catalog mode" instead of "
Done
http://gerrit.cloudera.org:8080/#/c/14731/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
http://gerrit.cloudera.org:8080/#/c/14731/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1660
PS6, Line 1660:
> nit: redundant blank line
Done
http://gerrit.cloudera.org:8080/#/c/14731/6/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/14731/6/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@691
PS6, Line 691: le
> Let's try avoid using V1. I think "legacy catalog mode" is better.
Done
http://gerrit.cloudera.org:8080/#/c/14731/6/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
File fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java:
http://gerrit.cloudera.org:8080/#/c/14731/6/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java@271
PS6, Line 271: // Test constraints of child_table.
> nit: redundant blank line
Done
--
To view, visit http://gerrit.cloudera.org:8080/14731
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7ea7e1bacf6eb502c67caf310a847b32687e0d58
Gerrit-Change-Number: 14731
Gerrit-PatchSet: 7
Gerrit-Owner: Anurag Mantripragada
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Fri, 17 Jan 2020 16:04:05 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14731 ) Change subject: IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode. .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/14731/5/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test File testdata/workloads/functional-query/queries/QueryTest/show-create-table.test: http://gerrit.cloudera.org:8080/#/c/14731/5/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test@565 PS5, Line 565: ) > nit: empty line Done -- To view, visit http://gerrit.cloudera.org:8080/14731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ea7e1bacf6eb502c67caf310a847b32687e0d58 Gerrit-Change-Number: 14731 Gerrit-PatchSet: 6 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Fri, 17 Jan 2020 00:39:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode.
Anurag Mantripragada has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/14731 ) Change subject: IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode. .. IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode. This change add a new method 'loadConstraints()' to the MetaProvider interface. 1. In CatalogdMetaProvider implementation, we fetch the primary key (PK) and foreign key(FK) information via the GetPartialCatalogObject() RPC to the catalogd. This is modified to include PK/FK information. This is because, on catalog side we eagerly load PK/FK information which can be sent over to local catalog in a single RPC to Catalog. This information is then stored in TableMetaRef object for future consumers. 2. In the DirectMetaProvider implementation, we make two RPCs to HMS to directly get PK/FK information. Load constraints can be extended to include other constraints later (for ex: unique constraints.) Testing: - Added tests in LocalCatalogTest, CatalogTest and PartialCatalogInfoTest - This change also modifies the toSqlUtil for show create table statements. Added a test for the same. Change-Id: I7ea7e1bacf6eb502c67caf310a847b32687e0d58 --- M common/thrift/CatalogService.thrift M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.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 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/MetaProvider.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 testdata/workloads/functional-query/queries/QueryTest/show-create-table.test 14 files changed, 376 insertions(+), 66 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/14731/6 -- To view, visit http://gerrit.cloudera.org:8080/14731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7ea7e1bacf6eb502c67caf310a847b32687e0d58 Gerrit-Change-Number: 14731 Gerrit-PatchSet: 6 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9101: Add support for detecting self-events on partition events
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14799 ) Change subject: IMPALA-9101: Add support for detecting self-events on partition events .. Patch Set 5: Code-Review+1 (1 comment) Did another round. This patch looks good to me. http://gerrit.cloudera.org:8080/#/c/14799/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java: http://gerrit.cloudera.org:8080/#/c/14799/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@924 PS5, Line 924: full : // invalidate Should this be refresh? -- To view, visit http://gerrit.cloudera.org:8080/14799 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9b4148f6be0f9f946c8ad8f314d64b095731744c Gerrit-Change-Number: 14799 Gerrit-PatchSet: 5 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 09 Jan 2020 04:12:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode.
Anurag Mantripragada has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/14731 ) Change subject: IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode. .. IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode. This change add a new method 'loadConstraints()' to the MetaProvider interface. 1. In CatalogdMetaProvider implementation, we fetch the primary key (PK) and foreign key(FK) information via the GetPartialCatalogObject() RPC to the catalogd. This is modified to include PK/FK information. This is because, on catalog side we eagerly load PK/FK information which can be sent over to local catalog in a single RPC to Catalog. This information is then stored in TableMetaRef object for future consumers. 2. In the DirectMetaProvider implementation, we make two RPCs to HMS to directly get PK/FK information. Load constraints can be extended to include other constraints later (for ex: unique constraints.) Testing: - Added tests in LocalCatalogTest, CatalogTest and PartialCatalogInfoTest - This change also modifies the toSqlUtil for show create table statements. Added a test for the same. Change-Id: I7ea7e1bacf6eb502c67caf310a847b32687e0d58 --- M common/thrift/CatalogService.thrift M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.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 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/MetaProvider.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 testdata/workloads/functional-query/queries/QueryTest/show-create-table.test 14 files changed, 377 insertions(+), 66 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/14731/5 -- To view, visit http://gerrit.cloudera.org:8080/14731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7ea7e1bacf6eb502c67caf310a847b32687e0d58 Gerrit-Change-Number: 14731 Gerrit-PatchSet: 5 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode.
Anurag Mantripragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14731 )
Change subject: IMPALA-9158: Support loading primary key/foreign key
constraints in LocalCatalog Mode.
..
Patch Set 4:
(8 comments)
Thanks for comments. I added tests in show-create-table.test.
http://gerrit.cloudera.org:8080/#/c/14731/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14731/3//COMMIT_MSG@27
PS3, Line 27: - Added tests in LocalCatalogTest, CatalogTest and
PartialCatalogInfoTest
> Can you add some end-to-end tests for show create table, since this is addi
Done.
http://gerrit.cloudera.org:8080/#/c/14731/3/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/14731/3/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@167
PS3, Line 167: public Pair, List>
loadConstraints(
> Optional: it seems like basically everywhere that we want one list, we also
That is a good point. In fact, Hive 4 onwards already does this:
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/ddl/table/constraint/Constraints.java
I would like to do this refactoring in a separate JIRA as the scope is bigger
than this particular change. I have filed IMPALA-9256 for this.
http://gerrit.cloudera.org:8080/#/c/14731/3/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/14731/3/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@114
PS3, Line 114:*/
> Can you document the invariants around these two variables - i.e. when is i
Done. Modified this so empty means either the table does not have the keys or
it is not loaded.
http://gerrit.cloudera.org:8080/#/c/14731/3/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@554
PS3, Line 554:
> Shouldn't an empty list mean that the pks are loaded and that there are no
Modified this so the empty list could be either no primary keys or the table is
not loaded.
http://gerrit.cloudera.org:8080/#/c/14731/3/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@558
PS3, Line 558: } catch (TException e) {
> Is this code reachable? If so, shouldn't we load the table instead of retur
This change was made to be consistent with the behavior that statements such as
"SHOW CREATE" do not do any RPCs and show only available information as pointed
out by Quanlong. I'm leaning towards loading constraints if unavailable so I
reverted this behavior. So you think there is a significant performance penalty?
http://gerrit.cloudera.org:8080/#/c/14731/3/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@569
PS3, Line 569: loadConstraints();
> Same comments apply here.
Addressed in the above comment.
http://gerrit.cloudera.org:8080/#/c/14731/3/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
File fe/src/test/java/org/apache/impala/catalog/CatalogTest.java:
http://gerrit.cloudera.org:8080/#/c/14731/3/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java@452
PS3, Line 452: for (SQLPrimaryKey pk: primaryKeys) {
> It's worth thinking about putting the primary and foreign keys into a canon
Looks like HMS always returns the reverse order of the primary keys specified
in DDLs. I tested on different examples locally. I also added a comment in the
test. Changed the tests accordingly.
Hive seems to have added canonical way starting Hive 4:
https://github.com/apache/hive/blob/706c1d44f7734ed36900a02c7255c1d0ce0ad45a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L4892
http://gerrit.cloudera.org:8080/#/c/14731/3/testdata/data/child_table.txt
File testdata/data/child_table.txt:
PS3:
> Please add description of these data files to the README in this directory
These files are added as part of IMPALA-9104 and removed from this change.
Added README in the other change.
--
To view, visit http://gerrit.cloudera.org:8080/14731
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7ea7e1bacf6eb502c67caf310a847b32687e0d58
Gerrit-Change-Number: 14731
Gerrit-PatchSet: 4
Gerrit-Owner: Anurag Mantripragada
Gerrit-Reviewer: Anurag Mantripragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Thu, 09 Jan 2020 00:19:50 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode.
Anurag Mantripragada has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/14731 ) Change subject: IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode. .. IMPALA-9158: Support loading primary key/foreign key constraints in LocalCatalog Mode. This change add a new method 'loadConstraints()' to the MetaProvider interface. 1. In CatalogdMetaProvider implementation, we fetch the primary key (PK) and foreign key(FK) information via the GetPartialCatalogObject() RPC to the catalogd. This is modified to include PK/FK information. This is because, on catalog side we eagerly load PK/FK information which can be sent over to local catalog in a single RPC to Catalog. This information is then stored in TableMetaRef object for future consumers. 2. In the DirectMetaProvider implementation, we make two RPCs to HMS to directly get PK/FK information. Load constraints can be extended to include other constraints later (for ex: unique constraints.) Testing: - Added tests in LocalCatalogTest, CatalogTest and PartialCatalogInfoTest - This change also modifies the toSqlUtil for show create table statements. Added a test for the same. Change-Id: I7ea7e1bacf6eb502c67caf310a847b32687e0d58 --- M common/thrift/CatalogService.thrift M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.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 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/MetaProvider.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 testdata/workloads/functional-query/queries/QueryTest/show-create-table.test 14 files changed, 377 insertions(+), 66 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/14731/4 -- To view, visit http://gerrit.cloudera.org:8080/14731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7ea7e1bacf6eb502c67caf310a847b32687e0d58 Gerrit-Change-Number: 14731 Gerrit-PatchSet: 4 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9257: Last event id should be advanced if all events are skipped
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14916 ) Change subject: IMPALA-9257: Last event id should be advanced if all events are skipped .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/14916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f94c1a8e8c221f504262d5591cda8c3a25c0c32 Gerrit-Change-Number: 14916 Gerrit-PatchSet: 2 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 17 Dec 2019 19:42:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9257: Last event id should be advanced if all events are skipped
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14916 ) Change subject: IMPALA-9257: Last event id should be advanced if all events are skipped .. Patch Set 1: (1 comment) Looks good to me. Will let Quanlong approve it after you address his comments. http://gerrit.cloudera.org:8080/#/c/14916/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java: http://gerrit.cloudera.org:8080/#/c/14916/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@588 PS1, Line 588: @return the last Notification event which was processed. Remove this? -- To view, visit http://gerrit.cloudera.org:8080/14916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f94c1a8e8c221f504262d5591cda8c3a25c0c32 Gerrit-Change-Number: 14916 Gerrit-PatchSet: 1 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 17 Dec 2019 18:52:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9101: Add support for detecting self-events on partition events
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14799 ) Change subject: IMPALA-9101: Add support for detecting self-events on partition events .. Patch Set 2: (3 comments) Thanks for making this change. Overall, the patch looks good to me. I just posted some questions to understand a few things better. http://gerrit.cloudera.org:8080/#/c/14799/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java: http://gerrit.cloudera.org:8080/#/c/14799/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1383 PS2, Line 1383: reloadPartition(tPartSpec, "ADD_PARTITION"); Just for my understanding, here and in DropPartitionEvents, are we changing the behavior from bailing out of refreshing the rest of the partitions when we encounter a failed refresh to a best-effort (refresh as many partitions in the event as possible) refresh? http://gerrit.cloudera.org:8080/#/c/14799/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java: http://gerrit.cloudera.org:8080/#/c/14799/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@210 PS2, Line 210: NUMBER_OF_PARTITION_REFRESHES Should this metric be initialized similarly to table refreshes in line 316? http://gerrit.cloudera.org:8080/#/c/14799/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/14799/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@631 PS2, Line 631: catalog_.addVersionsForInflightEvents(tbl, newCatalogVersion); Is there a reason why we are not doing this for DROP_PARTITION case below on L650? -- To view, visit http://gerrit.cloudera.org:8080/14799 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9b4148f6be0f9f946c8ad8f314d64b095731744c Gerrit-Change-Number: 14799 Gerrit-PatchSet: 2 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 17 Dec 2019 18:44:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9188: Composite primary keys should use same constraint name
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14792 ) Change subject: IMPALA-9188: Composite primary keys should use same constraint name .. Patch Set 1: Code-Review+1 Thanks for fixing this. Looks good to me. -- To view, visit http://gerrit.cloudera.org:8080/14792 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7a91658945b0355753c3cf05be195757b37edaf4 Gerrit-Change-Number: 14792 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Sun, 24 Nov 2019 23:35:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14720 ) Change subject: IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. .. Patch Set 12: The test failures in GVD were because we started returning Immutable lists for PKs/FKs. In IMPALA-2112 we were using dummy tables for tests which used to addPrimaryKeys in Frontend fixture. After we started returning Immutable list, this code broke. However, since we added new dataset in this patch, I modified the PK/FK tests to use the newly added dataset and removed the line where pk's had to be added in FrontendFixture. The changes are in FrontEndFixture, AnalyzeDDLTest and ToSqlTest files. -- To view, visit http://gerrit.cloudera.org:8080/14720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 Gerrit-Change-Number: 14720 Gerrit-PatchSet: 12 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 21 Nov 2019 17:41:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server.
Anurag Mantripragada has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/14720 ) Change subject: IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. .. IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. The goal is to let JDBC clients get constraint information from Impala tables. We implement two new metadata operations in impala-hs2-server, GetPrimaryKeys and GetCrossReference, which are already implemented in Hive's HS2. The thrift definitions are copied from Hive's TCLIService.thrift. In FE, these two operations are implemented to get the information from tables in the catalog. Much like GetColumns(), tables need to be loaded in order to be able to get PK/FK information. We wait for the PK table/FK table to load. In the implementation, PK/FK information is returned ONLY if the user has access to ALL the columns involved in the PK/FK relationship. Testing: - Added three test tables to our test datasets since most of our FE tests relied on dummy tables or testdata. It was difficult to test PK/FK with these methods. Also, we can build on this testdata in future when we make optimizer improvements. - Added unit tests in AuthorizationTest and JDBCtest. - Added e2e test in test_hs2.py - This patch modifies AnalyzeDDLTests and ToSqlTests to rely on the newly added dataset instead of dummy tables for pk/fk tests. Caveats: - Ranger needs OWNER user information for authorization. Since this is HMS metadata that we do not aggresively load, this information is not available for IncompleteTables. Some foreign key tables (fact tables for example) might have FK/PK relationships with several PK tables some of which might not be loaded in catalog. Currently we have no way to check column previleges without owner user information tables. We do not return keys involving such columns. Therefore, when Ranger is used, there maybe missing PK/FK relationships for parent tables that are not loaded. This can be tracked in IMPALA-9172. - Retrieval of constraints is not yet supported in LocalCatalog mode. See IMPALA-9158. Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 --- M be/src/rpc/kerberos-test.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.h M bin/rat_exclude_files.txt M common/thrift/Frontend.thrift M common/thrift/hive-1-api/TCLIService.thrift M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.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/service/Frontend.java M fe/src/main/java/org/apache/impala/service/MetadataOp.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java M fe/src/test/java/org/apache/impala/common/FrontendFixture.java M fe/src/test/java/org/apache/impala/service/JdbcTest.java M testdata/data/README A testdata/data/child_table.txt A testdata/data/parent_table.txt A testdata/data/parent_table_2.txt M testdata/datasets/functional/functional_schema_template.sql M tests/hs2/test_hs2.py 25 files changed, 835 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/14720/12 -- To view, visit http://gerrit.cloudera.org:8080/14720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 Gerrit-Change-Number: 14720 Gerrit-PatchSet: 12 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server.
Anurag Mantripragada has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/14720 ) Change subject: IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. .. IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. The goal is to let JDBC clients get constraint information from Impala tables. We implement two new metadata operations in impala-hs2-server, GetPrimaryKeys and GetCrossReference, which are already implemented in Hive's HS2. The thrift definitions are copied from Hive's TCLIService.thrift. In FE, these two operations are implemented to get the information from tables in the catalog. Much like GetColumns(), tables need to be loaded in order to be able to get PK/FK information. We wait for the PK table/FK table to load. In the implementation, PK/FK information is returned ONLY if the user has access to ALL the columns involved in the PK/FK relationship. Testing: - Added three test tables to our test datasets since most of our FE tests relied on dummy tables or testdata. It was difficult to test PK/FK with these methods. Also, we can build on this testdata in future when we make optimizer improvements. - Added unit tests in AuthorizationTest and JDBCtest. - Added e2e test in test_hs2.py Caveats: - Ranger needs OWNER user information for authorization. Since this is HMS metadata that we do not aggresively load, this information is not available for IncompleteTables. Some foreign key tables (fact tables for example) might have FK/PK relationships with several PK tables some of which might not be loaded in catalog. Currently we have no way to check column previleges without owner user information tables. We do not return keys involving such columns. Therefore, when Ranger is used, there maybe missing PK/FK relationships for parent tables that are not loaded. This can be tracked in IMPALA-9172. - Retrieval of constraints is not yet supported in LocalCatalog mode. See IMPALA-9158. Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 --- M be/src/rpc/kerberos-test.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.h M bin/rat_exclude_files.txt M common/thrift/Frontend.thrift M common/thrift/hive-1-api/TCLIService.thrift M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.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/service/Frontend.java M fe/src/main/java/org/apache/impala/service/MetadataOp.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java M fe/src/test/java/org/apache/impala/service/JdbcTest.java M testdata/data/README A testdata/data/child_table.txt A testdata/data/parent_table.txt A testdata/data/parent_table_2.txt M testdata/datasets/functional/functional_schema_template.sql M tests/hs2/test_hs2.py 22 files changed, 809 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/14720/11 -- To view, visit http://gerrit.cloudera.org:8080/14720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 Gerrit-Change-Number: 14720 Gerrit-PatchSet: 11 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14720 ) Change subject: IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/14720/10//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14720/10//COMMIT_MSG@13 PS10, Line 13: definitions are copied from Hive's TCLIService.thrift. In FE, these > Apologies for the drive-by comment, but I think it'd be worth mentioning th Done. -- To view, visit http://gerrit.cloudera.org:8080/14720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 Gerrit-Change-Number: 14720 Gerrit-PatchSet: 11 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 21 Nov 2019 02:56:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14720 ) Change subject: IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. .. Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/14720/8/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/14720/8/fe/src/main/java/org/apache/impala/service/Frontend.java@888 PS8, Line 888: .any() : .onColumn(table.getTableName().getDb(), table.getTableName().getTbl(), : fk.getFkcolumn_name(), table.getOwnerUser()).build(); : > May be I don't understand what you mean by sequence here. Can you give an e The confusion is because of a "foreign key" for a table is different from a "SQLForiegnKey" structures stored in HMS. A table can have a foreign key referencing two columns in a parent table. This "foreign key" is represent as two SQLForeignKeys, one for each column. These two share the same FKName but will have key_sequence numbers like 1, 2, .. respectively for each column. For authorization, if we find that the user doesn't have privileges on one column in this FK, we will omit all the SQLForeignKeys that have the same fkName. This is because the entire sequence is together considered a "foreign key" for that table and we don't want to return just one in the sequence. I have written a more concrete example in the comments in code. Please let me know if doesn't make sense. :) http://gerrit.cloudera.org:8080/#/c/14720/8/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/14720/8/fe/src/main/java/org/apache/impala/service/MetadataOp.java@393 PS8, Line 393:} : columns.addAll(fe.getColumns(table, columnPatternMa > Actually, it looks like if the column matcher is NONE, we get the table if Hmm, I think you are right, it is unnecessary to load pks/fks if matcher is NONE. Added a condition. -- To view, visit http://gerrit.cloudera.org:8080/14720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 Gerrit-Change-Number: 14720 Gerrit-PatchSet: 10 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 21 Nov 2019 01:56:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server.
Anurag Mantripragada has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/14720 ) Change subject: IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. .. IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. The goal is to let JDBC clients get constraint information from Impala tables. We implement two new metadata operations in impala-hs2-server, GetPrimaryKeys and GetCrossReference, whose thrift definitions are taken from Hive. For these to work, new TCLIService methods were added. In FE, these two operations are implemented to get the information from tables in the catalog. Much like GetColumns(), tables need to be loaded in order to be able to get PK/FK information. We wait for the PK table/FK table to load. In the implementation, PK/FK information is returned ONLY if the user has access to ALL the columns involved in the PK/FK relationship. Testing: - Added three test tables to our test datasets since most of our FE tests relied on dummy tables or testdata. It was difficult to test PK/FK with these methods. Also, we can build on this testdata in future when we make optimizer improvements. - Added unit tests in AuthorizationTest and JDBCtest. - Added e2e test in test_hs2.py Caveats: - Ranger needs OWNER user information for authorization. Since this is HMS metadata that we do not aggresively load, this information is not available for IncompleteTables. Some foreign key tables (fact tables for example) might have FK/PK relationships with several PK tables some of which might not be loaded in catalog. Currently we have no way to check column previleges without owner user information tables. We do not return keys involving such columns. Therefore, when Ranger is used, there maybe missing PK/FK relationships for parent tables that are not loaded. This can be tracked in IMPALA-9172. - Retrieval of constraints is not yet supported in LocalCatalog mode. See IMPALA-9158. Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 --- M be/src/rpc/kerberos-test.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.h M bin/rat_exclude_files.txt M common/thrift/Frontend.thrift M common/thrift/hive-1-api/TCLIService.thrift M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.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/service/Frontend.java M fe/src/main/java/org/apache/impala/service/MetadataOp.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java M fe/src/test/java/org/apache/impala/service/JdbcTest.java M testdata/data/README A testdata/data/child_table.txt A testdata/data/parent_table.txt A testdata/data/parent_table_2.txt M testdata/datasets/functional/functional_schema_template.sql M tests/hs2/test_hs2.py 22 files changed, 809 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/14720/10 -- To view, visit http://gerrit.cloudera.org:8080/14720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 Gerrit-Change-Number: 14720 Gerrit-PatchSet: 10 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server.
Anurag Mantripragada has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/14720 ) Change subject: IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. .. IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server. The goal is to let JDBC clients get constraint information from Impala tables. We implement two new metadata operations in impala-hs2-server, GetPrimaryKeys and GetCrossReference, whose thrift definitions are taken from Hive. For these to work, new TCLIService methods were added. In FE, these two operations are implemented to get the information from tables in the catalog. Much like GetColumns(), tables need to be loaded in order to be able to get PK/FK information. We wait for the PK table/FK table to load. In the implementation, PK/FK information is returned ONLY if the user has access to ALL the columns involved in the PK/FK relationship. Testing: - Added three test tables to our test datasets since most of our FE tests relied on dummy tables or testdata. It was difficult to test PK/FK with these methods. Also, we can build on this testdata in future when we make optimizer improvements. - Added unit tests in AuthorizationTest and JDBCtest. - Added e2e test in test_hs2.py Caveats: - Ranger needs OWNER user information for authorization. Since this is HMS metadata that we do not aggresively load, this information is not available for IncompleteTables. Some foreign key tables (fact tables for example) might have FK/PK relationships with several PK tables some of which might not be loaded in catalog. Currently we have no way to check column previleges without owner user information tables. We do not return keys involving such columns. Therefore, when Ranger is used, there maybe missing PK/FK relationships for parent tables that are not loaded. This can be tracked in IMPALA-9172. - Retrieval of constraints is not yet supported in LocalCatalog mode. See IMPALA-9158. Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 --- M be/src/rpc/kerberos-test.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.h M bin/rat_exclude_files.txt M common/thrift/Frontend.thrift M common/thrift/hive-1-api/TCLIService.thrift M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.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/service/Frontend.java M fe/src/main/java/org/apache/impala/service/MetadataOp.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java M fe/src/test/java/org/apache/impala/service/JdbcTest.java M testdata/data/README A testdata/data/child_table.txt A testdata/data/parent_table.txt A testdata/data/parent_table_2.txt M testdata/datasets/functional/functional_schema_template.sql M tests/hs2/test_hs2.py 22 files changed, 793 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/14720/9 -- To view, visit http://gerrit.cloudera.org:8080/14720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8942dfbbd4a3be244eed1c61ac2ce17069960477 Gerrit-Change-Number: 14720 Gerrit-PatchSet: 9 Gerrit-Owner: Anurag Mantripragada Gerrit-Reviewer: Anurag Mantripragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server.
Anurag Mantripragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14720 )
Change subject: IMPALA-9104: Support retrieval of PK/FK information through
impala-hs2-server.
..
Patch Set 9:
(13 comments)
Thanks for the comments Vihang.
http://gerrit.cloudera.org:8080/#/c/14720/8/common/thrift/Frontend.thrift
File common/thrift/Frontend.thrift:
http://gerrit.cloudera.org:8080/#/c/14720/8/common/thrift/Frontend.thrift@594
PS8, Line 594:
> hmm, adding fields in the middle generally is considered a bad practice sin
Okay, will add new fields instead of reusing old ids.
http://gerrit.cloudera.org:8080/#/c/14720/8/common/thrift/hive-1-api/TCLIService.thrift
File common/thrift/hive-1-api/TCLIService.thrift:
http://gerrit.cloudera.org:8080/#/c/14720/8/common/thrift/hive-1-api/TCLIService.thrift@954
PS8, Line 954: catalogName
> Just curious to know if this is the Hive's catalog name or the catalog serv
This is the catalog name.
https://github.com/apache/hive/blob/df8e185aeb555f522345d5703cd5375aad2ae4b4/service/src/java/org/apache/hive/service/cli/operation/GetPrimaryKeysOperation.java#L79
http://gerrit.cloudera.org:8080/#/c/14720/8/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/14720/8/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java@226
PS8, Line 226: TGetSchemasReq req = request.getGet_schemas_req();
: return MetadataOp.getSchemas(
: frontend, req.getCatalogName(), req.getSchemaName(),
user);
: }
:
: /**
:* Supported HMS-2 types
:*/
: public static final EnumSet
IMPALA_SUPPORTED_TABLE_TYPES = EnumSet
: .of(TableType.EXTERNAL_TABLE, TableType.MANAGED_TABLE,
TableType.VIRTUAL_VIEW);
:
: /**
:* mapping between the HMS-2 type the Impala types
:*/
: p
> Are these methods are exactly the same as in case of hive-3 shim? If yes, w
Removed the shim methods. Used MetadataOp directly.
http://gerrit.cloudera.org:8080/#/c/14720/8/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/14720/8/fe/src/main/java/org/apache/impala/catalog/Table.java@610
PS8, Line 610:
: @Override // FeTable
: public List getPrimaryKeys() {
: // Prevent clients from modifying the primary keys list.
: return ImmutableList.copyOf(primaryKeys_);
> Generally a good practice is to make these methods return Immutable objects
Done
http://gerrit.cloudera.org:8080/#/c/14720/8/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/14720/8/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java@215
PS8, Line 215: // TODO: return primary k
> replace with Collection.emptyList()?
Done.
http://gerrit.cloudera.org:8080/#/c/14720/8/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java@221
PS8, Line 221: / TODO: return foreign k
> replace with Collection.emptyList()?
Done
http://gerrit.cloudera.org:8080/#/c/14720/8/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/14720/8/fe/src/main/java/org/apache/impala/service/Frontend.java@888
PS8, Line 888: if (!authzChecker_.get().hasAccess(user,
fkPrivilegeRequest) ||
: !authzChecker_.get().hasAccess(user,
pkPrivilegeRequest)) {
: omitList.add(fkName);
: }
> Isn't this logic same as
There are these cases:
```
if(auth enabled) {
if (any of the keys in sequence doesn't have access) {
don't return the entire sequence but continue checking other fks.
} else {
return all fks;
} else {
return all fks as is.
}
```
A sequence of keys has a common fk name. We are storing all fk names in which
at least one FK has no permissions and skip the entire sequence in the end.
Does this make sense?
http://gerrit.cloudera.org:8080/#/c/14720/8/fe/src/main/java/org/apache/impala/service/Frontend.java@1649
PS8, Line 1649: case GET_PRIMARY_KEYS: return
MetadataOp.getPrimaryKeys(this, request,
: user);
: case GET_CROSS_REFE
> If the methods are exactly the same in both the shims, we should directly u
Done
http://gerrit.cloudera.org:8080/#/c/14720/8/fe/src/main/java/org/apache/impala/service/MetadataOp.java
File fe/src/main/java/org/apache/impala/service/MetadataOp.java:
[Impala-ASF-CR] IMPALA-9104: Support retrieval of PK/FK information through impala-hs2-server.
Anurag Mantripragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14720 )
Change subject: IMPALA-9104: Support retrieval of PK/FK information through
impala-hs2-server.
..
Patch Set 8:
(13 comments)
The developer docs page says clang can be used only with the C++ part of the
code. I was manually doing this and still miss a few code style errors. Clang
documentation says it can be used for Java too. Maybe I should try and modify
our clang tool to work with Java. Thank you for catching these errors.
http://gerrit.cloudera.org:8080/#/c/14720/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14720/7//COMMIT_MSG@38
PS7, Line 38: This can be
: tracked in IMPAL
> Is there a JIRA for it?
This was first mentioned in https://gerrit.cloudera.org/#/c/14106/. I could not
find a JIRA, so I filed IMPALA-9172. Updated the commit message.
http://gerrit.cloudera.org:8080/#/c/14720/7/common/thrift/Frontend.thrift
File common/thrift/Frontend.thrift:
http://gerrit.cloudera.org:8080/#/c/14720/7/common/thrift/Frontend.thrift@576
PS7, Line 576: GET_CROSS_REFERENC
> GET_CROSS_REFERENCE
Done
http://gerrit.cloudera.org:8080/#/c/14720/7/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/14720/7/fe/src/main/java/org/apache/impala/analysis/TableDef.java@547
PS7, Line 547: i
> nit: extra whitespace
Done
http://gerrit.cloudera.org:8080/#/c/14720/7/fe/src/main/java/org/apache/impala/catalog/FeTable.java
File fe/src/main/java/org/apache/impala/catalog/FeTable.java:
http://gerrit.cloudera.org:8080/#/c/14720/7/fe/src/main/java/org/apache/impala/catalog/FeTable.java@99
PS7, Line 99:
> I don't think this is necessary, here and elsewhere
This is obsolete. I reverted the NotImplementedException change.
http://gerrit.cloudera.org:8080/#/c/14720/7/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/14720/7/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@56
PS7, Line 56: import org.apache.impala.thrift.CatalogObjectsConstants;
> I don't think you need this
Done
http://gerrit.cloudera.org:8080/#/c/14720/7/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/14720/7/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java@216
PS7, Line 216:
:
> nit: you can get rid of the '+' by putting this all on the second line
Not needed anymore since I reverted the NotImplementedException change.
http://gerrit.cloudera.org:8080/#/c/14720/7/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/14720/7/fe/src/main/java/org/apache/impala/service/Frontend.java@907
PS7, Line 907: blic List getDbs(PatternMatcher matcher, User
user)
: throws InternalException {
: List dbs = getCatalog().getDbs(matcher);
: // If authorization is enabled
> My thinking in suggesting the NotImplementedException was that we would bub
I see your point. Since the IMPALA-9158 patch is in review, depending on which
patch lands first, I will either create a cleanup patch or modify this patch to
clean things. If it is okay with you, to avoid changing the tests more than
once, I will revert to the earlier behavior of returning an empty PK/FK list in
LocalCatlogMode.
http://gerrit.cloudera.org:8080/#/c/14720/5/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/14720/5/fe/src/main/java/org/apache/impala/service/MetadataOp.java@736
PS5, Line 736: eName == null) {
> Good question - I just know if it as our usual convention, eg. if you look
Thanks for the explanation. I reverted it to use the "diamond" style.
http://gerrit.cloudera.org:8080/#/c/14720/7/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/14720/7/fe/src/main/java/org/apache/impala/service/MetadataOp.java@650
PS7, Line 650: if (LOG.isTraceEnabled()) {
: LOG.trace("Returning {} primary k
> formatting
Done
http://gerrit.cloudera.org:8080/#/c/14720/7/fe/src/main/java/org/apache/impala/service/MetadataOp.java@676
PS7, Line 676: tableM
> formatting
Done
http://gerrit.cloudera.org:8080/#/c/14720/7/fe/src/main/java/org/apache/impala/service/MetadataOp.java@720
PS7, Line 720: }
: if (LOG.isTraceEnabled()) {
> formatting
Done
