[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16060/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 12 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 30 Apr 2024 05:39:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 12: (16 comments) > Uploaded patch set 12. http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@9 PS11, Line 9: An > An error - we're introducing it now. Done http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@14 PS11, Line 14: matched one of th > matched one of the ... Done http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@15 PS11, Line 15: '\u00FF' > '\u00FF' is not a valid character literal in C++. "\u00FF" was included in Done http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@18 PS11, Line 18: A set > See the comment above, we should explain that it was a mistake from the beg Done http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@22 PS11, Line 22: > Parquet and Iceberg are not exclusive things, we also use Parquet in our Ic Done http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@22 PS11, Line 22: > It would be good to include in which file the new tests are. Done http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc File be/src/util/coding-util.cc: http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc@51 PS11, Line 51: '\x1A > Why adding space character here? Hive doesn't escape it in partition paths. Done http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc@57 PS11, Line 57: "upp > It would be clearer like this: "uppercase" and "hex" only affect the insert Done http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc@62 PS11, Line 62: // the character is not alphanumeric and it is not one of the characters specifically > This doesn't match the code. Try this: Done http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/insert.test File testdata/workloads/functional-query/queries/QueryTest/insert.test: http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/insert.test@450 PS11, Line 450: 'R > We just need double single quotes in the RESULTS section to escape a single Done http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/insert.test@452 PS11, Line 452: > I don't think need this. It's used for non-ascii characters. Done http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/insert.test@470 PS11, Line 470: > This will fail if we escape space. Ack http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test File testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test: http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@327 PS11, Line 327: TYPES > You don't need to drop the table, the tables are created in a unique databa Done http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@337 PS11, Line 337: > I don't think this is needed. Done http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@344 PS11, Line 344: > You should do the same insertions into the Iceberg table as into the other Done http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@354 PS11, Line 354: > See L327. Done -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 12 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 30 Apr 2024 05:14:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. IMPALA-11499: Refactor UrlEncode function to handle special characters An error came from an issue with URL encoding, where certain Unicode characters were being incorrectly encoded due to their UTF-8 representation matching characters in the set of characters to escape. For example, the string '运', which consists of three bytes 0xe8 0xbf 0x90 was wrongly getting encoded into '\E8%FFBF\90', because the middle byte matched one of the two bytes that represented the '\u00FF' literal.Inclusion of '\u00FF' was likely a mistake from the beginning and it should have been '\x7F'. A set containing all the special characters has been included and, '\xFF', which is an invalid UTF-8 byte whose inclusion was erroneous from the beginning, is replaced with '\x7F', which is a control character for DELETE, ensuring consistency and correctness in URL encoding. Testing: Tests on both traditional Hive tables and Iceberg tables are included in unicode-column-name.test, insert.test and coding-util-test.cc. Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb --- M be/src/util/coding-util-test.cc M be/src/util/coding-util.cc M testdata/workloads/functional-query/queries/QueryTest/insert.test M testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test 4 files changed, 178 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21131/12 -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 12 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-12935: First pass on Calcite planner functions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21357 ) Change subject: IMPALA-12935: First pass on Calcite planner functions .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16059/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21357 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2dd4e402d69ee10547abeeafe893164ffd789b88 Gerrit-Change-Number: 21357 Gerrit-PatchSet: 5 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Tue, 30 Apr 2024 04:39:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12934: Added Calcite parsing files to Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21194 ) Change subject: IMPALA-12934: Added Calcite parsing files to Impala .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16058/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767 Gerrit-Change-Number: 21194 Gerrit-PatchSet: 8 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Tue, 30 Apr 2024 04:38:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12934: Added Calcite parsing files to Impala
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21194 ) Change subject: IMPALA-12934: Added Calcite parsing files to Impala .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/21194/7/java/pom.xml File java/pom.xml: http://gerrit.cloudera.org:8080/#/c/21194/7/java/pom.xml@246 PS7, Line 246: : sonatype-nexus-snapshots : https://oss.sonatype.org/content/repositories/snapshots : : false : : : true : fail : : > When I removed this, it builds fine, so let's drop this. Done -- To view, visit http://gerrit.cloudera.org:8080/21194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767 Gerrit-Change-Number: 21194 Gerrit-PatchSet: 7 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Tue, 30 Apr 2024 04:15:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12934: Added Calcite parsing files to Impala
Hello Aman Sinha, Joe McDonnell, Csaba Ringhofer, Michael Smith, Impala Public
Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21194
to look at the new patch set (#8).
Change subject: IMPALA-12934: Added Calcite parsing files to Impala
..
IMPALA-12934: Added Calcite parsing files to Impala
Adding the framework to create our own parsing syntax for Impala using
the base Calcite Parser.jj file.
The Parser.jj file here was grabbed from Calcite 1.36. So with this commit,
we are using the same parsing analysis as Calcite 1.36. Any changes made
on top of the Parser.jj file or the config.fmpp file in the future are Impala
specific changes, so a diff can be done from this commit to see all the Impala
parsing changes.
The config.fmpp file was grabbed from Calcite 1.36 default_config.fmpp. It does
have two very small modifications from the original Calcite fmpp file in order
to fix compilation issues within Impala
1) the entire json is wrapped with a "data {}" tag
2) the class used is ImpalaSqlParserImpl as opposed to the Calcite SqlParserImpl
class to prevent naming collisions with Calcite.
There's no unit test needed since there is no functional change. The Calcite
planner will eventually make changes in the ".jj" file to support the
differences
between the Impala parser and the Calcite parser.
Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767
---
M java/calcite-planner/pom.xml
A
java/calcite-planner/src/main/java/org/apache/impala/calcite/parserimpl/codegen/config.fmpp
A
java/calcite-planner/src/main/java/org/apache/impala/calcite/parserimpl/codegen/templates/Parser.jj
M
java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteQueryParser.java
4 files changed, 9,616 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/21194/8
--
To view, visit http://gerrit.cloudera.org:8080/21194
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767
Gerrit-Change-Number: 21194
Gerrit-PatchSet: 8
Gerrit-Owner: Steve Carlin
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Steve Carlin
[Impala-ASF-CR] IMPALA-12935: First pass on Calcite planner functions
Hello Aman Sinha, Joe McDonnell, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21357 to look at the new patch set (#5). Change subject: IMPALA-12935: First pass on Calcite planner functions .. IMPALA-12935: First pass on Calcite planner functions This commit handles the first pass on getting functions to work through the Calcite planner. Only basic functions will work with this commit. Implicit conversions for parameters are not yet supported. Custom UDFs are also not supported yet. The functions are loaded in CalciteJniFrontend from the Impala "builtin" database. A "FunctionSignature" is created for each builtin function. Each function is loaded into Calcite's "ImpalaOperatorTable" object which is used in the validation process. Each Impala function maps to a Calcite "ImpalaOperator" object. For function names that are not an exact match with a Calcite operator, an entry is found in FunctionDetailStatics to do the conversion. When the Calcite validator tries to validate the function, it gets the return type through the "ImpalaOperator.inferReturnType()" method. In this method, a "FunctionSignatureForLookup" signature is created for looking up the stored signatures. The "Lookup" signature does not have a return type (yet), so the matching of signatures will be based on matching name and parameters. As mentioned above, implicit conversion is not yet supported in this commit; this will be added later. After validation is complete, the functions will be in a Calcite format. After the rest of compilation (relnode conversion, optimization) is complete, the function needs to be converted back into Impala form (the Expr object) to eventually get it into its thrift request. In this commit, all functions are converted into Expr starting in the ImpalaProjectRel, since this is the RelNode where functions do their thing. The RexCallConverter and RexLiteralConverter get called via the CreateExprVisitor for this conversion. Since Calcite is providing the analysis portion of the planning, there is no need to go through Impala's Analyzer object. However, the Impala planner requires Expr objects to be analyzed. To get around this, the AnalyzedFunctionCallExpr and AnalyzedNullLiteral objects exist which analyze the expression in the constructor. While this could potentially be combined with the existing FunctionCallExpr and NullLiteral objects, this fits in with the general plan to avoid changing "fe" Impala code as much as we can until much later in the commit cycle. Also, there will be other Analyzed*Expr classes created in the future, but this commit is intended for basic function call expressions only. One minor change to the parser is added with this commit. Calcite parser does not have acknowledge the "string" datatype, so this has been added here in Parser.jj and config.fmpp. Change-Id: I2dd4e402d69ee10547abeeafe893164ffd789b88 --- A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedFunctionCallExpr.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedNullLiteral.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionDetailStatics.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionSignature.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionSignatureForLookup.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionSignatureForStorage.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexCallConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaAggregateOperator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaHelperOperator.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaOperatorTable.java A java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaScalarOperator.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/parserimpl/codegen/config.fmpp M java/calcite-planner/src/main/java/org/apache/impala/calcite/parserimpl/codegen/templates/Parser.jj M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/CreateExprVisitor.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteValidator.java M java/calcite-planner/src/main/java/org/apache/impala/calcit
[Impala-ASF-CR] IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21304 ) Change subject: IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables .. Patch Set 17: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21304 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I44e8c1bb020e90559c7f22483a7ab7a151b8f48a Gerrit-Change-Number: 21304 Gerrit-PatchSet: 17 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: gaurav singh Gerrit-Comment-Date: Tue, 30 Apr 2024 03:34:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21372 ) Change subject: IMPALA-13045: Wait for impala_query_live to exist .. Patch Set 3: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10591/ -- To view, visit http://gerrit.cloudera.org:8080/21372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 Gerrit-Change-Number: 21372 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 30 Apr 2024 02:15:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 11: (4 comments) http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc File be/src/util/coding-util.cc: http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc@51 PS11, Line 51: '\x20' > For those characters that have a readable literal representation, e.g. " ", Why adding space character here? Hive doesn't escape it in partition paths. E.g. beeline -u "jdbc:hive2://localhost:11050" hive> create table my_part_tbl(i int) partitioned by (p string); hive> alter table my_part_tbl add partition(p='a b'); hive> show partitions my_part_tbl; ++ | partition | ++ | p=a b | ++ hive> describe formatted my_part_tbl partition(p='a b'); ... | Location: | hdfs://localhost:20500/test-warehouse/my_part_tbl/p=a b | NULL | ... http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/insert.test File testdata/workloads/functional-query/queries/QueryTest/insert.test: http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/insert.test@450 PS11, Line 450: '' We just need double single quotes in the RESULTS section to escape a single quote. Don't need to do this in the QUERY section. http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/insert.test@452 PS11, Line 452: RAW_STRING I don't think need this. It's used for non-ascii characters. http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/insert.test@470 PS11, Line 470: This will fail if we escape space. -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 11 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 30 Apr 2024 01:47:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21372 ) Change subject: IMPALA-13045: Wait for impala_query_live to exist .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 Gerrit-Change-Number: 21372 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Tue, 30 Apr 2024 00:38:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/21326 ) Change subject: IMPALA-13009: Fix catalogd not sending deletion updates for some dropped partitions .. Patch Set 6: Code-Review+1 (1 comment) LGTM http://gerrit.cloudera.org:8080/#/c/21326/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21326/5//COMMIT_MSG@14 PS5, Line 14: When catalogd collects the next : catalog update, they will be collected. HdfsTable then clears the set. > These are about CatalogServiceCatalog#addHdfsPartitionsToCatalogDelta(). Ad Ack -- To view, visit http://gerrit.cloudera.org:8080/21326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12a68158dca18ee48c9564ea16b7484c9f5b5d21 Gerrit-Change-Number: 21326 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 29 Apr 2024 22:39:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21304 ) Change subject: IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables .. Patch Set 17: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21304 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I44e8c1bb020e90559c7f22483a7ab7a151b8f48a Gerrit-Change-Number: 21304 Gerrit-PatchSet: 17 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: gaurav singh Gerrit-Comment-Date: Mon, 29 Apr 2024 22:27:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21304 ) Change subject: IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables .. Patch Set 17: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10592/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21304 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I44e8c1bb020e90559c7f22483a7ab7a151b8f48a Gerrit-Change-Number: 21304 Gerrit-PatchSet: 17 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: gaurav singh Gerrit-Comment-Date: Mon, 29 Apr 2024 22:23:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21304 ) Change subject: IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables .. Patch Set 17: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16057/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21304 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I44e8c1bb020e90559c7f22483a7ab7a151b8f48a Gerrit-Change-Number: 21304 Gerrit-PatchSet: 17 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Abhishek Rawat Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: gaurav singh Gerrit-Comment-Date: Mon, 29 Apr 2024 22:06:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables
Wenzhe Zhou has uploaded a new patch set (#17). (
http://gerrit.cloudera.org:8080/21304 )
Change subject: IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables
..
IMPALA-12910: Support running TPCH/TPCDS queries for JDBC tables
This patch adds script to create external JDBC tables for the dataset of
TPCH and TPCDS, and adds unit-tests to run TPCH and TPCDS queries for
external JDBC tables with Impala-Impala federation. Notes that JDBC
tables are mapping tables, they don't take additional disk spaces.
It fixes the race condition when caching of SQL DataSource objects by
using a new DataSourceObjectCache class, which checks reference count
before closing SQL DataSource.
Adds a new query-option 'clean_dbcp_ds_cache' with default value as
true. When it's set as false, SQL DataSource object will not be closed
when its reference count equals 0 and will be kept in cache until
the SQL DataSource is idle for more than 5 minutes.
java.sql.Connection.close() fails to remove a closed connection from
connection pool sometimes, which causes JDBC working threads to wait
for available connections from the connection pool for a long time.
The work around is to call BasicDataSource.invalidateConnection() API
to close a connection.
Two flag variables are added for DBCP configuration properties
'maxTotal' and 'maxWaitMillis'. Notes that 'maxActive' and 'maxWait'
properties are renamed to 'maxTotal' and 'maxWaitMillis' respectively
in apache.commons.dbcp v2.
Fixes a bug for database type comparison since the type strings
specified by user could be lower case or mix of upper/lower cases, but
the code compares the types with upper case string.
Fixes issue to close SQL DataSource object in JdbcDataSource.open()
and JdbcDataSource.getNext() when some errors returned from DBCP APIs
or JDBC drivers.
testdata/bin/create-tpc-jdbc-tables.py supports to create JDBC tables
for Impala-Impala, Postgres and MySQL.
Following sample commands creates TPCDS JDBC tables for Impala-Impala
federation with remote coordinator running at 10.19.10.86, and Postgres
server running at 10.19.10.86:
${IMPALA_HOME}/testdata/bin/create-tpc-jdbc-tables.py \
--jdbc_db_name=tpcds_jdbc --workload=tpcds \
--database_type=IMPALA --database_host=10.19.10.86 --clean
${IMPALA_HOME}/testdata/bin/create-tpc-jdbc-tables.py \
--jdbc_db_name=tpcds_jdbc --workload=tpcds \
--database_type=POSTGRES --database_host=10.19.10.86 \
--database_name=tpcds --clean
TPCDS tests for JDBC tables run only for release/exhaustive builds.
TPCH tests for JDBC tables run for core and exhaustive builds, except
Dockerized builds.
Remaining Issues:
- tpcds-decimal_v2-q80a failed with returned rows not matching expected
results for some decimal values. This will be fixed in IMPALA-13018.
Testing:
- Passed core tests.
- Passed query_test/test_tpcds_queries.py in release/exhaustive build.
- Manually verified that only one SQL DataSource object was created for
test_tpcds_queries.py::TestTpcdsQueryForJdbcTables since query option
'clean_dbcp_ds_cache' was set as false, and the SQL DataSource object
was closed by cleanup thread.
Change-Id: I44e8c1bb020e90559c7f22483a7ab7a151b8f48a
---
M be/src/exec/data-source-scan-node.cc
M be/src/service/frontend.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M common/thrift/ExternalDataSource.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/Query.thrift
M fe/src/main/java/org/apache/impala/extdatasource/jdbc/JdbcDataSource.java
M
fe/src/main/java/org/apache/impala/extdatasource/jdbc/conf/JdbcStorageConfigManager.java
A
fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DataSourceObjectCache.java
M
fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/DatabaseAccessor.java
M
fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
M
fe/src/main/java/org/apache/impala/extdatasource/jdbc/dao/JdbcRecordIterator.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M testdata/bin/create-load-data.sh
A testdata/bin/create-tpc-jdbc-tables.py
A testdata/datasets/tpcds/tpcds_jdbc_schema_template.sql
A testdata/datasets/tpch/tpch_jdbc_schema_template.sql
M tests/common/skip.py
M tests/query_test/test_tpcds_queries.py
M tests/query_test/test_tpch_queries.py
23 files changed, 1,914 insertions(+), 99 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/21304/17
--
To view, visit http://gerrit.cloudera.org:8080/21304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I44e8c1bb020e90559c7f22483a7ab7a151b8f48a
Gerrit-Change-Number: 21304
Gerrit-PatchSet: 17
Gerrit-Owner: Wenzhe Zhou
Gerrit-Reviewer: Abhishek Rawat
Gerrit-Reviewer: Anonymous
[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21372 ) Change subject: IMPALA-13045: Wait for impala_query_live to exist .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16056/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 Gerrit-Change-Number: 21372 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 29 Apr 2024 21:20:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21372 ) Change subject: IMPALA-13045: Wait for impala_query_live to exist .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10591/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 Gerrit-Change-Number: 21372 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 29 Apr 2024 21:14:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21372 ) Change subject: IMPALA-13045: Wait for impala_query_live to exist .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 Gerrit-Change-Number: 21372 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 29 Apr 2024 21:14:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12684: Enable IMPALA COMPRESSED DEBUG INFO by default
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20871 ) Change subject: IMPALA-12684: Enable IMPALA_COMPRESSED_DEBUG_INFO by default .. IMPALA-12684: Enable IMPALA_COMPRESSED_DEBUG_INFO by default IMPALA_COMPRESSED_DEBUG_INFO was introduced in IMPALA-11511 and reduces Impala binary sizes by >50%. Debug tools like gdb and our minidump processing scripts handle compressed debug information properly. There are slightly higher link times and additional overhead when doing debugging. Overall, the reduction in binary sizes seems worth it given the modest overhead. Compressing the debug information also avoids concerns that adding debug information to toolchain components would increase binary sizes. This changes the default for IMPALA_COMPRESSED_DEBUG_INFO to true. Testing: - Ran pstack on a Centos 7 machine running tests with IMPALA_COMPRESSED_DEBUG_INFO=true and verified that the symbols work properly - Forced the production of minidumps for a job using IMPALA_COMPRESSED_DEBUG_INFO=true and verified it is processed properly. - Used this locally for development for several months Change-Id: I31640f1453d351b11644bb46af3d2158b22af5b3 Reviewed-on: http://gerrit.cloudera.org:8080/20871 Reviewed-by: Quanlong Huang Tested-by: Impala Public Jenkins --- M bin/impala-config.sh 1 file changed, 4 insertions(+), 3 deletions(-) Approvals: Quanlong Huang: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/20871 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I31640f1453d351b11644bb46af3d2158b22af5b3 Gerrit-Change-Number: 20871 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-13046: Update Iceberg mixed format deletes test
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21373 ) Change subject: IMPALA-13046: Update Iceberg mixed format deletes test .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16055/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21373 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I87c23cc541983223c6b766372f4e582c33ae6836 Gerrit-Change-Number: 21373 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 29 Apr 2024 21:12:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21372 ) Change subject: IMPALA-13045: Wait for impala_query_live to exist .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 Gerrit-Change-Number: 21372 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 29 Apr 2024 21:08:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12684: Enable IMPALA COMPRESSED DEBUG INFO by default
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20871 ) Change subject: IMPALA-12684: Enable IMPALA_COMPRESSED_DEBUG_INFO by default .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/20871 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I31640f1453d351b11644bb46af3d2158b22af5b3 Gerrit-Change-Number: 20871 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 29 Apr 2024 21:06:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21372 ) Change subject: IMPALA-13045: Wait for impala_query_live to exist .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/21372/1/tests/custom_cluster/test_query_live.py File tests/custom_cluster/test_query_live.py: http://gerrit.cloudera.org:8080/#/c/21372/1/tests/custom_cluster/test_query_live.py@34 PS1, Line 34: def setup_method(self, method): > That wouldn't hurt. It needs to run after the cluster has been started, whi Done -- To view, visit http://gerrit.cloudera.org:8080/21372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 Gerrit-Change-Number: 21372 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 29 Apr 2024 20:56:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist
Hello Andrew Sherman, Riza Suminto, Jason Fehr, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21372 to look at the new patch set (#2). Change subject: IMPALA-13045: Wait for impala_query_live to exist .. IMPALA-13045: Wait for impala_query_live to exist Waits for creation of 'sys.impala_query_live' in tests to ensure it has been registered with HMS. Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 --- M tests/custom_cluster/test_query_live.py 1 file changed, 7 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/21372/2 -- To view, visit http://gerrit.cloudera.org:8080/21372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 Gerrit-Change-Number: 21372 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto
[Impala-ASF-CR] Refactor Workload Management
Michael Smith has posted comments on this change. (
http://gerrit.cloudera.org:8080/21358 )
Change subject: Refactor Workload Management
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21358/3/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:
http://gerrit.cloudera.org:8080/#/c/21358/3/tests/custom_cluster/test_query_log.py@66
PS3, Line 66: create_re = r'\]\s+(\w+:\w+)\]\s+Analyzing query: CREATE
TABLE IF NOT EXISTS {}' \
Put this in setup_method.
Clean up ordering in CustomClusterTestSuite.teardown_method.
--
To view, visit http://gerrit.cloudera.org:8080/21358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1127ef041a3e024bf2b262767d56ec5f29bf3855
Gerrit-Change-Number: 21358
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Riza Suminto
Gerrit-Comment-Date: Mon, 29 Apr 2024 20:55:37 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13046: Update Iceberg mixed format deletes test
Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21373 Change subject: IMPALA-13046: Update Iceberg mixed format deletes test .. IMPALA-13046: Update Iceberg mixed format deletes test Updates iceberg-mixed-format-position-deletes.test for HIVE-28069. Newer versions of Hive will now remove a data file if a delete would negate all rows in the data file to reduce the number of small files produced. The test now ensures every data file it expects to produce will have a row after delete (or circumvent the merge logic by using different formats). Change-Id: I87c23cc541983223c6b766372f4e582c33ae6836 --- M testdata/workloads/functional-query/queries/QueryTest/iceberg-mixed-format-position-deletes.test 1 file changed, 6 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/21373/1 -- To view, visit http://gerrit.cloudera.org:8080/21373 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I87c23cc541983223c6b766372f4e582c33ae6836 Gerrit-Change-Number: 21373 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith
[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21372 ) Change subject: IMPALA-13045: Wait for impala_query_live to exist .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/21372/1/tests/custom_cluster/test_query_live.py File tests/custom_cluster/test_query_live.py: http://gerrit.cloudera.org:8080/#/c/21372/1/tests/custom_cluster/test_query_live.py@34 PS1, Line 34: def wait_for_create_table(self, table_name): > Should this be an override of setup_method() instead, with table_name fixed That wouldn't hurt. It needs to run after the cluster has been started, which seems doable as long as it calls the parent setup_method first. -- To view, visit http://gerrit.cloudera.org:8080/21372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 Gerrit-Change-Number: 21372 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 29 Apr 2024 20:07:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21372 ) Change subject: IMPALA-13045: Wait for impala_query_live to exist .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/21372/1/tests/custom_cluster/test_query_live.py File tests/custom_cluster/test_query_live.py: http://gerrit.cloudera.org:8080/#/c/21372/1/tests/custom_cluster/test_query_live.py@34 PS1, Line 34: def wait_for_create_table(self, table_name): Should this be an override of setup_method() instead, with table_name fixed to 'sys.impala_query_live'? I'm guessing that this is a common requirement for all tests in TestQueryLive. -- To view, visit http://gerrit.cloudera.org:8080/21372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 Gerrit-Change-Number: 21372 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 29 Apr 2024 20:05:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21372 ) Change subject: IMPALA-13045: Wait for impala_query_live to exist .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16054/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 Gerrit-Change-Number: 21372 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 29 Apr 2024 19:53:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17551 ) Change subject: IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions .. Patch Set 3: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/764/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/17551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I379de6075af2168715e71edfec26265b6ff5eab7 Gerrit-Change-Number: 17551 Gerrit-PatchSet: 3 Gerrit-Owner: Shajini Thayasingh Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 29 Apr 2024 19:46:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/17551 ) Change subject: IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/17551/1/docs/topics/impala_functions.xml File docs/topics/impala_functions.xml: http://gerrit.cloudera.org:8080/#/c/17551/1/docs/topics/impala_functions.xml@70 PS1, Line 70: Unnecessary indentation here. I cleaned it up. http://gerrit.cloudera.org:8080/#/c/17551/3/docs/topics/impala_functions.xml File docs/topics/impala_functions.xml: http://gerrit.cloudera.org:8080/#/c/17551/3/docs/topics/impala_functions.xml@100 PS3, Line 100: It looks like we normally also add links to all functions in this table. -- To view, visit http://gerrit.cloudera.org:8080/17551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I379de6075af2168715e71edfec26265b6ff5eab7 Gerrit-Change-Number: 17551 Gerrit-PatchSet: 3 Gerrit-Owner: Shajini Thayasingh Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 29 Apr 2024 19:39:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17551 ) Change subject: IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions .. Patch Set 3: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/764/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/17551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I379de6075af2168715e71edfec26265b6ff5eab7 Gerrit-Change-Number: 17551 Gerrit-PatchSet: 3 Gerrit-Owner: Shajini Thayasingh Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 29 Apr 2024 19:37:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions
Michael Smith has uploaded a new patch set (#3) to the change originally created by Shajini Thayasingh. ( http://gerrit.cloudera.org:8080/17551 ) Change subject: IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions .. IMPALA-10729: [DOCS] Document SHA1 and SHA2 functions Introduced the new hash functions that were recently added. Change-Id: I379de6075af2168715e71edfec26265b6ff5eab7 --- M docs/impala.ditamap M docs/topics/impala_functions.xml A docs/topics/impala_hash_functions.xml 3 files changed, 79 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/51/17551/3 -- To view, visit http://gerrit.cloudera.org:8080/17551 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I379de6075af2168715e71edfec26265b6ff5eab7 Gerrit-Change-Number: 17551 Gerrit-PatchSet: 3 Gerrit-Owner: Shajini Thayasingh Gerrit-Reviewer: Amogh Margoor Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith
[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21372 ) Change subject: IMPALA-13045: Wait for impala_query_live to exist .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10590/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 Gerrit-Change-Number: 21372 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Riza Suminto Gerrit-Comment-Date: Mon, 29 Apr 2024 19:32:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13045: Wait for impala query live to exist
Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21372 Change subject: IMPALA-13045: Wait for impala_query_live to exist .. IMPALA-13045: Wait for impala_query_live to exist Waits for creation of 'sys.impala_query_live' in tests where it may not exist in HMS beforehand. Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 --- M tests/custom_cluster/test_query_live.py 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/21372/1 -- To view, visit http://gerrit.cloudera.org:8080/21372 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5cc3fa3c43be7af9a5f097359a0d4f20d057a207 Gerrit-Change-Number: 21372 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith
[Impala-ASF-CR] IMPALA-12934: Added Calcite parsing files to Impala
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21194 ) Change subject: IMPALA-12934: Added Calcite parsing files to Impala .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/21194/7/java/pom.xml File java/pom.xml: http://gerrit.cloudera.org:8080/#/c/21194/7/java/pom.xml@246 PS7, Line 246: : sonatype-nexus-snapshots : https://oss.sonatype.org/content/repositories/snapshots : : false : : : true : fail : : When I removed this, it builds fine, so let's drop this. -- To view, visit http://gerrit.cloudera.org:8080/21194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767 Gerrit-Change-Number: 21194 Gerrit-PatchSet: 7 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Mon, 29 Apr 2024 19:23:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 ) Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java File fe/src/main/java/org/apache/impala/service/BackendConfig.java: http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java@411 PS2, Line 411: return (int) Math.min(backendCfg_.thrift_rpc_max_message_size, Integer.MAX_VALUE); > No major reason. Mainly, I'm doing C++ because C++ is easy for us to patch That'd be useful to clarify in the commit message. -- To view, visit http://gerrit.cloudera.org:8080/21367 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87 Gerrit-Change-Number: 21367 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 29 Apr 2024 17:35:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13044: Upgrade bouncycastle to 1.78
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21371 ) Change subject: IMPALA-13044: Upgrade bouncycastle to 1.78 .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/21371/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21371/1//COMMIT_MSG@11 PS1, Line 11: jdk15on18, which is the most similar to the previous release format. Where did you find jdk15on18? I see jdk15to18, or jdk18on. jdk18on seems like a reasonable thing to upgrade to as we require Java 8+. -- To view, visit http://gerrit.cloudera.org:8080/21371 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8372916ab79b863e7a07d22e8333abd54492fa29 Gerrit-Change-Number: 21371 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Rozsa Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 29 Apr 2024 17:34:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13044: Upgrade bouncycastle to 1.78
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21371 ) Change subject: IMPALA-13044: Upgrade bouncycastle to 1.78 .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16053/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21371 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8372916ab79b863e7a07d22e8333abd54492fa29 Gerrit-Change-Number: 21371 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Rozsa Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 29 Apr 2024 17:36:56 + Gerrit-HasComments: No
[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 ) Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java File fe/src/main/java/org/apache/impala/service/BackendConfig.java: http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java@411 PS2, Line 411: return (int) Math.min(backendCfg_.thrift_rpc_max_message_size, Integer.MAX_VALUE); > What's the rationale for not increasing it? No major reason. Mainly, I'm doing C++ because C++ is easy for us to patch and the specific bug for IMPALA-13020 involves C++ to C++ communication. We should think about modifying the Java library to support a higher limit. We haven't established a way to patch the Java library yet. We also have to think about how Hive uses Thrift and whether it would need to be recompiled if we change the Thrift signature. -- To view, visit http://gerrit.cloudera.org:8080/21367 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87 Gerrit-Change-Number: 21367 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 29 Apr 2024 17:16:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13044: Upgrade bouncycastle to 1.78
Peter Rozsa has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21371 Change subject: IMPALA-13044: Upgrade bouncycastle to 1.78 .. IMPALA-13044: Upgrade bouncycastle to 1.78 This patch upgrades bouncycastle to 1.78. As of bouncycastle:1.71, the *-jdk15 artifact is no longer available. The artifact is changed to jdk15on18, which is the most similar to the previous release format. Tests: - core tests ran Change-Id: I8372916ab79b863e7a07d22e8333abd54492fa29 --- M bin/impala-config.sh M java/pom.xml 2 files changed, 3 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/21371/1 -- To view, visit http://gerrit.cloudera.org:8080/21371 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8372916ab79b863e7a07d22e8333abd54492fa29 Gerrit-Change-Number: 21371 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Rozsa
[Impala-ASF-CR] IMPALA-12934: Added Calcite parsing files to Impala
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21194 ) Change subject: IMPALA-12934: Added Calcite parsing files to Impala .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If756b5ea8beb85661a30fb5d029e74ebb6719767 Gerrit-Change-Number: 21194 Gerrit-PatchSet: 7 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Steve Carlin Gerrit-Comment-Date: Mon, 29 Apr 2024 17:02:05 + Gerrit-HasComments: No
[Impala-ASF-CR] PROTOTYPE: IMPALA-13020: Change thrift rpc max message size to int64 t
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21367 ) Change subject: PROTOTYPE: IMPALA-13020: Change thrift_rpc_max_message_size to int64_t .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java File fe/src/main/java/org/apache/impala/service/BackendConfig.java: http://gerrit.cloudera.org:8080/#/c/21367/2/fe/src/main/java/org/apache/impala/service/BackendConfig.java@411 PS2, Line 411: return (int) Math.min(backendCfg_.thrift_rpc_max_message_size, Integer.MAX_VALUE); What's the rationale for not increasing it? -- To view, visit http://gerrit.cloudera.org:8080/21367 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I681b1849cc565dcb25de8c070c18776ce69cbb87 Gerrit-Change-Number: 21367 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 29 Apr 2024 16:47:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12684: Enable IMPALA COMPRESSED DEBUG INFO by default
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20871 ) Change subject: IMPALA-12684: Enable IMPALA_COMPRESSED_DEBUG_INFO by default .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10589/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/20871 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I31640f1453d351b11644bb46af3d2158b22af5b3 Gerrit-Change-Number: 20871 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 29 Apr 2024 16:08:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12935: First pass on Calcite planner functions
Steve Carlin has posted comments on this change. (
http://gerrit.cloudera.org:8080/21357 )
Change subject: IMPALA-12935: First pass on Calcite planner functions
..
Patch Set 2:
(2 comments)
Rebase is done now. Will add a better comment in a bit.
http://gerrit.cloudera.org:8080/#/c/21357/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java
File
java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java:
http://gerrit.cloudera.org:8080/#/c/21357/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@119
PS2, Line 119: public static List
createRelDataTypesForArgs(List impalaTypes) {
> How does this differ from createRelDataTypes at line 318?
Unused, deleted it.
http://gerrit.cloudera.org:8080/#/c/21357/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java@130
PS2, Line 130: public static RelDataType getRelDataType(Type impalaType) {
> I'm not sure why this and createRelDataType both exist. They're very simila
The getRelDataType deals with datatypes where the precision and scale might not
be known. The "normalized" comment probably wasn't clear enough, and I know
"get" and "create" can be confusing. "get" was meant to convey that these
datatypes already exist in a map, whereas "create" will grab the precision and
scale from the Type passed in and "create" a new datatype.
I'll try to make the comment clearer.
--
To view, visit http://gerrit.cloudera.org:8080/21357
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dd4e402d69ee10547abeeafe893164ffd789b88
Gerrit-Change-Number: 21357
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Steve Carlin
Gerrit-Comment-Date: Mon, 29 Apr 2024 15:15:06 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 11: (17 comments) http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@9 PS11, Line 9: The An error - we're introducing it now. http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@14 PS11, Line 14: was matching with matched one of the ... http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@15 PS11, Line 15: '\u00FF' '\u00FF' is not a valid character literal in C++. "\u00FF" was included in a string, it was part of a string literal. At last I found out that it can actually be used in C++ to specify arbitrary unicode values, but of course it may be encoded on multiple bytes, see https://en.cppreference.com/w/cpp/language/escape. Also include in the commit message that including this character was probably a mistake from the beginning, it should have been '\x7F'. http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@18 PS11, Line 18: '\xFF' See the comment above, we should explain that it was a mistake from the beginning. http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@22 PS11, Line 22: Some It would be good to include in which file the new tests are. http://gerrit.cloudera.org:8080/#/c/21131/11//COMMIT_MSG@22 PS11, Line 22: for both parquet and iceberg Parquet and Iceberg are not exclusive things, we also use Parquet in our Iceberg tables. Parquet is a file format and Iceberg is a table format. We could say "tests on both traditional Hive tables and Iceberg tables ...". http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc File be/src/util/coding-util.cc: http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc@25 PS11, Line 25: #include The includes within a group should be in alphabetical order, so this should go before L23. http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc@31 PS11, Line 31: #include "sasl/saslutil.h" Includes from the standard library should be the first group after the header, i.e. this should come after #include . http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc@51 PS11, Line 51: '\x20' > Represents space character For those characters that have a readable literal representation, e.g. " ", "\n" etc., we should use that even if Hive doesn't. http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc@57 PS11, Line 57: Used It would be clearer like this: "uppercase" and "hex" only affect the insertion of integers, not that of char values. http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc@62 PS11, Line 62: // Hive-compat mode, and the character is not alphanumeric or is one This doesn't match the code. Try this: "... b) we are not in Hive-compat mode and the character is not alphanumeric and it is not one of the characters specifically excluded from escaping (see ShouldNotEscape())." http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test File testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test: http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@302 PS11, Line 302: show tables; I don't think it's needed. Also, if another table is added later before this query, this will have to be changed as well. http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@309 PS11, Line 309: insert into unicode_partition_values partition(p='运营业务数据') values (0); You don't need to put these inserts into separate QUERY segments. It could also go together with the select on L318. http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@327 PS11, Line 327: drop table unicode_partition_values; You don't need to drop the table, the tables are created in a unique database, see test_column_unicode.py::TestColumnUnicode::test_create_unicode_table. http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@337 PS11, Line 337: show tables; I don't think this is needed. http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@344 PS11, Line 344: insert into unicode_partition_values_iceberg values (0, '运'); You should do the same insertions into the Iceberg table as into the other one. http://gerrit.cloudera.org:8080/#/c/21131/11/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test@354 PS11, Line 354: drop table unicode_partition_values_iceber
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16052/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 11 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 29 Apr 2024 12:07:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21361 ) Change subject: IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16051/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21361 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7c25ed85a8813011537c73f0aaf72db1501f9ef Gerrit-Change-Number: 21361 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Mon, 29 Apr 2024 11:55:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has posted comments on this change. (
http://gerrit.cloudera.org:8080/21131 )
Change subject: IMPALA-11499: Refactor UrlEncode function to handle special
characters
..
Patch Set 11:
(5 comments)
> Patch Set 11:
>
> (2 comments)
http://gerrit.cloudera.org:8080/#/c/21131/10/be/src/util/coding-util-test.cc
File be/src/util/coding-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/21131/10/be/src/util/coding-util-test.cc@94
PS10, Line 94: TestUrl("/home/impala/directory/",
"%2Fhome%2Fimpala%2Fdirectory%2F", false);
: TestUrl("/home/impala/directory/",
"%2Fhome%2Fimpala%2Fdirectory%2F", true);
> Please add some tests like these for newly added marks like '\n', '\r', '^'
Done
http://gerrit.cloudera.org:8080/#/c/21131/10/be/src/util/coding-util.cc
File be/src/util/coding-util.cc:
http://gerrit.cloudera.org:8080/#/c/21131/10/be/src/util/coding-util.cc@a40
PS10, Line 40:
:
:
:
> Can you keep this comment? We should still mention common/src/java/org/apac
Done
http://gerrit.cloudera.org:8080/#/c/21131/10/be/src/util/coding-util.cc@45
PS10, Line 45: // characters it will encode.
: // See common/src/java/org/apache/hadoop/hive/common
> We are still missing several characters here, e.g. single quote, '\n' and '
Done
http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc
File be/src/util/coding-util.cc:
http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util.cc@51
PS11, Line 51: '\x20'
Represents space character
http://gerrit.cloudera.org:8080/#/c/21131/10/testdata/workloads/functional-query/queries/QueryTest/insert.test
File testdata/workloads/functional-query/queries/QueryTest/insert.test:
http://gerrit.cloudera.org:8080/#/c/21131/10/testdata/workloads/functional-query/queries/QueryTest/insert.test@452
PS10, Line 452: RESULTS: RAW_STRING
: '0','O''Reilly'
: TYPES
: STRING, STRING
:
: QUERY
: truncate insert_string_partitioned;
: INSERT INTO TABLE insert_string_partitioned
PARTITION(s2="Impala''#%*/:=?{}[]^") values ('0');
: SELECT * FROM insert_string_partitioned where s2=
"Impala''#%*/:=?{}[]^";
: RESULTS: RAW_STRING
: '0','Impala''#%*/:=?{}[]^'
: TYPES
> e2e tests for ASCII characters can be added here, e.g.
Done
--
To view, visit http://gerrit.cloudera.org:8080/21131
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Gerrit-Change-Number: 21131
Gerrit-PatchSet: 11
Gerrit-Owner: Anonymous Coward
Gerrit-Reviewer: Anonymous Coward
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Smith
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Mon, 29 Apr 2024 11:43:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21361 ) Change subject: IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16050/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21361 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7c25ed85a8813011537c73f0aaf72db1501f9ef Gerrit-Change-Number: 21361 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Mon, 29 Apr 2024 11:47:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
pranav.lo...@cloudera.com has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. IMPALA-11499: Refactor UrlEncode function to handle special characters The error came from an issue with URL encoding, where certain Unicode characters were being incorrectly encoded due to their UTF-8 representation matching characters in the set of characters to escape. For example, the string '运', which consists of three bytes 0xe8 0xbf 0x90 was wrongly getting encoded into '\E8%FFBF\90', because the middle byte was matching with one of the two bytes that represented the '\u00FF' literal. A set containing all the special characters has been included and, '\xFF', which is an invalid UTF-8 byte is replaced with '\x7F', which is a control character for DELETE, ensuring consistency and correctness in URL encoding. Testing: Some basic tests for both parquet and iceberg are provided in unicode-column-name.test. Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb --- M be/src/util/coding-util-test.cc M be/src/util/coding-util.cc M testdata/workloads/functional-query/queries/QueryTest/insert.test M testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test 4 files changed, 204 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21131/11 -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 11 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21131 ) Change subject: IMPALA-11499: Refactor UrlEncode function to handle special characters .. Patch Set 11: (2 comments) http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util-test.cc File be/src/util/coding-util-test.cc: http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util-test.cc@100 PS11, Line 100: "%08%03%04%05%06%07%09%0c%0d%0ebeginners%0f%10%11%12%13%14%15%16%17%18%19%10Amuststart" line too long (93 > 90) http://gerrit.cloudera.org:8080/#/c/21131/11/be/src/util/coding-util-test.cc@103 PS11, Line 103: "\x07\x09\x0C\r\x0E""beginners\x0F\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x10""" line too long (91 > 90) -- To view, visit http://gerrit.cloudera.org:8080/21131 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb Gerrit-Change-Number: 21131 Gerrit-PatchSet: 11 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 29 Apr 2024 11:42:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException
Daniel Becker has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/21361 ) Change subject: IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException .. IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException When attempting to query a metadata table of a non-Iceberg table the analyzer throws 'IllegalArgumentException'. The problem is that 'IcebergMetadataTable.isIcebergMetadataTable()' doesn't actually check whether the given path belongs to a valid metadata table, it only checks whether the path could syntactically refer to one. This is because it is called in 'Path.getCandidateTables()', at which point analysis has not been done yet. However, 'IcebergMetadataTable.isIcebergMetadataTable()' is also called in 'Analyzer.getTable()'. If 'isIcebergMetadataTable()' returns true, 'getTable()' tries to instantiate an 'IcebergMetadataTable' object with the table ref of the base table. If that table is not an Iceberg table, a precondition check fails. This change renames 'isIcebergMetadataTable()' to 'canBeIcebergMetadataTable()' and adds a new 'isIcebergMetadataTable()' function, which also takes an 'Analyzer' as a parameter. With the help of the 'Analyzer' it is possible to determine whether the base table is an Iceberg table. 'Analyzer.getTable()' then uses this new 'isIcebergMetadataTable()' function instead of canBeIcebergMetadataTable(). The constructor of 'IcebergMetadataTable' is also modified to take an 'FeIcebergTable' as the parameter for the base table instead of a general 'FeTable'. Testing: - Added a test query in iceberg-metadata-tables.test. Change-Id: Ia7c25ed85a8813011537c73f0aaf72db1501f9ef --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/Path.java M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java M fe/src/main/java/org/apache/impala/service/Frontend.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 6 files changed, 52 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/21361/4 -- To view, visit http://gerrit.cloudera.org:8080/21361 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia7c25ed85a8813011537c73f0aaf72db1501f9ef Gerrit-Change-Number: 21361 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa
[Impala-ASF-CR] IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21361 ) Change subject: IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/21361/2/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java: http://gerrit.cloudera.org:8080/#/c/21361/2/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@148 PS2, Line 148: // If the metadata table has already been analyzed in the query, the table cache will > If a metadata table is analyzed before and it's referenced in a different p Thanks, I added a comment for it. -- To view, visit http://gerrit.cloudera.org:8080/21361 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7c25ed85a8813011537c73f0aaf72db1501f9ef Gerrit-Change-Number: 21361 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Mon, 29 Apr 2024 11:23:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException
Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21361 Change subject: IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException .. IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException When attempting to query a metadata table of a non-Iceberg table the analyzer throws 'IllegalArgumentException'. The problem is that 'IcebergMetadataTable.isIcebergMetadataTable()' doesn't actually check whether the given path belongs to a valid metadata table, it only checks whether the path could syntactically refer to one. This is because it is called in 'Path.getCandidateTables()', at which point analysis has not been done yet. However, 'IcebergMetadataTable.isIcebergMetadataTable()' is also called in 'Analyzer.getTable()'. If 'isIcebergMetadataTable()' returns true, 'getTable()' tries to instantiate an 'IcebergMetadataTable' object with the table ref of the base table. If that table is not an Iceberg table, a precondition check fails. This change renames 'isIcebergMetadataTable()' to 'canBeIcebergMetadataTable()' and adds a new 'isIcebergMetadataTable()' function, which also takes an 'Analyzer' as a parameter. With the help of the 'Analyzer' it is possible to determine whether the base table is an Iceberg table. 'Analyzer.getTable()' then uses this new 'isIcebergMetadataTable()' function instead of canBeIcebergMetadataTable(). The constructor of 'IcebergMetadataTable' is also modified to take an 'FeIcebergTable' as the parameter for the base table instead of a general 'FeTable'. Testing: - Added a test query in iceberg-metadata-tables.test. Change-Id: Ia7c25ed85a8813011537c73f0aaf72db1501f9ef --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/Path.java M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java M fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java M fe/src/main/java/org/apache/impala/service/Frontend.java M testdata/workloads/functional-query/queries/QueryTest/iceberg-metadata-tables.test 6 files changed, 50 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/21361/3 -- To view, visit http://gerrit.cloudera.org:8080/21361 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia7c25ed85a8813011537c73f0aaf72db1501f9ef Gerrit-Change-Number: 21361 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Peter Rozsa
[Impala-ASF-CR] IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21361 ) Change subject: IMPALA-13035: Querying metadata tables from non-Iceberg tables throws IllegalArgumentException .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/21361/3/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/21361/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3672 PS3, Line 3672: Preconditions.checkArgument(IcebergMetadataTable.canBeIcebergMetadataTable(tblRefPath)); line too long (92 > 90) http://gerrit.cloudera.org:8080/#/c/21361/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java: http://gerrit.cloudera.org:8080/#/c/21361/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@150 PS3, Line 150: return catalogTable instanceof FeIcebergTable || catalogTable instanceof IcebergMetadataTable; line too long (98 > 90) -- To view, visit http://gerrit.cloudera.org:8080/21361 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7c25ed85a8813011537c73f0aaf72db1501f9ef Gerrit-Change-Number: 21361 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Mon, 29 Apr 2024 11:24:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12786: Optimize count(*) for JSON scans
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/21039 )
Change subject: IMPALA-12786: Optimize count(*) for JSON scans
..
Patch Set 10:
(5 comments)
The patch looks pretty good to me!
http://gerrit.cloudera.org:8080/#/c/21039/10//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/21039/10//COMMIT_MSG@53
PS10, Line 53:
+--++---+--++---+---+--+++---++---+
Nice improvement! Do you also get chance to test on nested data set, e.g.
tpch_nested?
http://gerrit.cloudera.org:8080/#/c/21039/10/be/src/exec/json/json-parser-test.cc
File be/src/exec/json/json-parser-test.cc:
http://gerrit.cloudera.org:8080/#/c/21039/10/be/src/exec/json/json-parser-test.cc@103
PS10, Line 103: }
: else {
nit: please put "else" to the line of "}"
http://gerrit.cloudera.org:8080/#/c/21039/10/be/src/exec/json/json-parser-test.cc@193
PS10, Line 193: TestSkip(R"({"a":null, "b":[1,true,false]})", TYPE_OBJECT);
Can we add more nested cases? E.g.
R"({"a":null, "b":{1,true,false}})" // will this fail?
R"({"a":null, "b":{"c":"d"}})"
R"({"a":null, "b":[{"k1":"v1"}, {"k2":"v2"}]})"
I think we need more coverage on the recursive code paths.
http://gerrit.cloudera.org:8080/#/c/21039/10/be/src/exec/json/json-parser-test.cc@247
PS10, Line 247: EXPECT_OK(js.Scan(max_rows, &num_rows));
Can we also check row count from this is the same as the final 'row_count'?
http://gerrit.cloudera.org:8080/#/c/21039/10/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/21039/10/be/src/service/query-options.cc@1259
PS10, Line 1259:
query_options->__set_enable_tuple_cache(enable_tuple_cache);
missing "break" after this
--
To view, visit http://gerrit.cloudera.org:8080/21039
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I97ff097661c3c577aeafeeb1518408ce7a8a255e
Gerrit-Change-Number: 21039
Gerrit-PatchSet: 10
Gerrit-Owner: Zihao Ye
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Riza Suminto
Gerrit-Reviewer: Zihao Ye
Gerrit-Comment-Date: Mon, 29 Apr 2024 09:16:20 +
Gerrit-HasComments: Yes
