[Impala-ASF-CR] IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21083 ) Change subject: IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax .. Patch Set 4: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/755/ 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/21083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb348d8395a6140f0be200d73e2f22fded9a5116 Gerrit-Change-Number: 21083 Gerrit-PatchSet: 4 Gerrit-Owner: Noemi Pap-Takacs Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Comment-Date: Mon, 11 Mar 2024 08:58:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax
Hello Andrew Sherman, Daniel Becker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21083 to look at the new patch set (#4). Change subject: IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax .. IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax Extended the ALTER TABLE documentation with the SORT BY clause. Also added more information about the available and the deafult sort orders to the CREATE TABLE description. Testing: Built docs locally. Change-Id: Ieb348d8395a6140f0be200d73e2f22fded9a5116 --- M docs/topics/impala_alter_table.xml M docs/topics/impala_create_table.xml 2 files changed, 35 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/21083/4 -- To view, visit http://gerrit.cloudera.org:8080/21083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ieb348d8395a6140f0be200d73e2f22fded9a5116 Gerrit-Change-Number: 21083 Gerrit-PatchSet: 4 Gerrit-Owner: Noemi Pap-Takacs Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs
[Impala-ASF-CR] IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21083 ) Change subject: IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax .. Patch Set 4: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/755/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/21083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb348d8395a6140f0be200d73e2f22fded9a5116 Gerrit-Change-Number: 21083 Gerrit-PatchSet: 4 Gerrit-Owner: Noemi Pap-Takacs Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Comment-Date: Mon, 11 Mar 2024 09:06:15 + Gerrit-HasComments: No
[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 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/21131/1/be/src/util/coding-util.cc File be/src/util/coding-util.cc: http://gerrit.cloudera.org:8080/#/c/21131/1/be/src/util/coding-util.cc@75 PS1, Line 75: static inline void UrlEncode(const char* in, int in_len, std::string* out, bool hive_compat) { line too long (94 > 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: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 11 Mar 2024 09:58:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax
Noemi Pap-Takacs has posted comments on this change. ( http://gerrit.cloudera.org:8080/21083 ) Change subject: IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax .. Patch Set 4: (1 comment) Thank you for the review! I think that both LEXICAL and ZORDER are common knowledge. I found a blog post about ZORDER in Impala and the description aligns with what one can find on Wikipedia. http://gerrit.cloudera.org:8080/#/c/21083/3/docs/topics/impala_create_table.xml File docs/topics/impala_create_table.xml: http://gerrit.cloudera.org:8080/#/c/21083/3/docs/topics/impala_create_table.xml@456 PS3, Line 456: > > Nit: comma before "which". Done -- To view, visit http://gerrit.cloudera.org:8080/21083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb348d8395a6140f0be200d73e2f22fded9a5116 Gerrit-Change-Number: 21083 Gerrit-PatchSet: 4 Gerrit-Owner: Noemi Pap-Takacs Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Comment-Date: Mon, 11 Mar 2024 10:00:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21083 ) Change subject: IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ieb348d8395a6140f0be200d73e2f22fded9a5116 Gerrit-Change-Number: 21083 Gerrit-PatchSet: 4 Gerrit-Owner: Noemi Pap-Takacs Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Comment-Date: Mon, 11 Mar 2024 10:10:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax
Daniel Becker has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21083 ) Change subject: IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax .. IMPALA-12774: [DOCS] Document ALTER TABLE SORT BY syntax Extended the ALTER TABLE documentation with the SORT BY clause. Also added more information about the available and the deafult sort orders to the CREATE TABLE description. Testing: Built docs locally. Change-Id: Ieb348d8395a6140f0be200d73e2f22fded9a5116 Reviewed-on: http://gerrit.cloudera.org:8080/21083 Tested-by: Impala Public Jenkins Reviewed-by: Daniel Becker --- M docs/topics/impala_alter_table.xml M docs/topics/impala_create_table.xml 2 files changed, 35 insertions(+), 7 deletions(-) Approvals: Impala Public Jenkins: Verified Daniel Becker: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/21083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ieb348d8395a6140f0be200d73e2f22fded9a5116 Gerrit-Change-Number: 21083 Gerrit-PatchSet: 5 Gerrit-Owner: Noemi Pap-Takacs Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs
[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 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15474/ : 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: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 11 Mar 2024 10:26:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12487: Skip reloading file metadata for ALTER TABLE events with trivial changes in StorageDescriptor
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21019 ) Change subject: IMPALA-12487: Skip reloading file metadata for ALTER_TABLE events with trivial changes in StorageDescriptor .. Patch Set 3: (1 comment) > Patch Set 2: > > > Patch Set 2: > > > > (2 comments) > > > > Skip reloading file metadata looks good to me. But I'm uncertain whether > > skip reloading table schema is safe. Maybe we should compare the whole > > msTable object instead of just checking several fields. > > Comparing the whole msTable object is not ideal because certain fields like > change columns, change owner, may require required table metadata reload but > certain other properties in serde like change location, and change row/file > format don't need table metadata reload. This is much more evident when we > use this patch in conjunction with IMPALA-10976. I found a bug that changing the fileformat to AVRO does require table schema reload. Details in the comments. I think the main purpose for this JIRA is to skip reloading file metadata which is expensive. Skip reloading the table schema is a minor optimization and brings risks. I'm not sure if there are other bugs like IMPALA-12889. So keeps reloading the table schema seems safer to me. When testing IMPALA-10976, did you find any test failure if we don't skip reloading table schema? http://gerrit.cloudera.org:8080/#/c/21019/1/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/21019/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1788 PS1, Line 1788: !Objects.equals(beforeSd.getSerdeInfo(), afterSd.getSerdeInfo())) { > This behavior is consistent with the CatalogOpExecutor (i.e., command execu Thanks for pointing to the existing codes. I found it's an existing issue that avro schema is not updated correctly when changing file format to AVRO. Filed IMPALA-12889. The following scenario works before this patch: 1. Create a non-avro table with 'avro.schema.url' set to a valid schema file. impala> create table my_part_tbl(i int) partitioned by (p int) stored as parquet; impala> alter table my_part_tbl set tblproperties('avro.schema.url'='hdfs:test-warehouse/avro_schemas/functional/alltypes.json'); 2. In HIVE, change the file format to AVRO hive> alter table my_part_tbl set fileformat avro; 3. After Impala applies the ALTER_TABLE event, the schema should be the same: hive> describe my_part_tbl; +--++--+ | col_name | data_type | comment | +--++--+ | id | int| | | bool_col | boolean| | | tinyint_col | int| | | smallint_col | int| | | int_col | int| | | bigint_col | bigint | | | float_col| float | | | double_col | double | | | date_string_col | string | | | string_col | string | | | timestamp_col| string | | | p| int| | | | NULL | NULL | | # Partition Information | NULL | NULL | | # col_name | data_type | comment | | p| int| | +--++--+ If I apply the current patch, Impala will keep using the old schema even after applying the ALTER_TABLE event. impala> describe my_part_tbl +--+--+-+ | name | type | comment | +--+--+-+ | i| int | | | p| int | | +--+--+-+ -- To view, visit http://gerrit.cloudera.org:8080/21019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac Gerrit-Change-Number: 21019 Gerrit-PatchSet: 3 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 11 Mar 2024 12:27:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12693: [DOCS] Typo in link for ltrim in string functions docs
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21123 ) Change subject: IMPALA-12693: [DOCS] Typo in link for ltrim in string functions docs .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/21123/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21123/1//COMMIT_MSG@9 PS1, Line 9: f Nit: should be capitalised: "Fixed". http://gerrit.cloudera.org:8080/#/c/21123/1//COMMIT_MSG@9 PS1, Line 9: LTRIM Nit: full stop at the end of the sentence ("LTRIM."). -- To view, visit http://gerrit.cloudera.org:8080/21123 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If4345fc6d19f04d0c0c6feef3e0c8598271224fe Gerrit-Change-Number: 21123 Gerrit-PatchSet: 1 Gerrit-Owner: Saurabh Katiyal Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 11 Mar 2024 12:32:00 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12831: Fix HdfsTable.toMinimalTCatalogObject() failed by concurrent modification
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21072 ) Change subject: IMPALA-12831: Fix HdfsTable.toMinimalTCatalogObject() failed by concurrent modification .. Patch Set 3: Code-Review+2 upgrading to +2 after Sai's +1 -- To view, visit http://gerrit.cloudera.org:8080/21072 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie9f8e65c0bd24000241eedf8ca765c1e4e14fdb3 Gerrit-Change-Number: 21072 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 11 Mar 2024 15:27:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 ) Change subject: IMPALA-12883: Support updating the charge on an entry in the cache .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10370/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/21122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025 Gerrit-Change-Number: 21122 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Mon, 11 Mar 2024 16:17:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 ) Change subject: IMPALA-12883: Support updating the charge on an entry in the cache .. Patch Set 3: Test failure is not related -- To view, visit http://gerrit.cloudera.org:8080/21122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025 Gerrit-Change-Number: 21122 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Mon, 11 Mar 2024 16:17:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 ) Change subject: IMPALA-12883: Support updating the charge on an entry in the cache .. Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/cache-test.cc File be/src/util/cache/cache-test.cc: http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/cache-test.cc@180 PS3, Line 180: // Updating the charge for the erased element evicts something Why do you call h1 the erased element? http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/cache.h File be/src/util/cache/cache.h: http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/cache.h@96 PS2, Line 96: // this makes it easier to embed in containers. > Done Ack http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/cache.h@130 PS2, Line 130: // this makes it easier to embed in containers. > Done Ack http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/lirs-cache-test.cc File be/src/util/cache/lirs-cache-test.cc: http://gerrit.cloudera.org:8080/#/c/21122/3/be/src/util/cache/lirs-cache-test.cc@290 PS3, Line 290: cache_->UpdateCharge(handle, cache_->MaxCharge()); Do you want to validate the handle's charge doesn't change too? http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/lirs-cache.cc File be/src/util/cache/lirs-cache.cc: http://gerrit.cloudera.org:8080/#/c/21122/2/be/src/util/cache/lirs-cache.cc@975 PS2, Line 975: handle->set_charge(charge); > Fixed this and added some tests Done -- To view, visit http://gerrit.cloudera.org:8080/21122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025 Gerrit-Change-Number: 21122 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Mon, 11 Mar 2024 18:53:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12872: Use Calcite for ...
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21109 to look at the new patch set (#4). Change subject: IMPALA-12872: Use Calcite for ... .. IMPALA-12872: Use Calcite for ... ...query analysis/optimization for simple queries. This is the first commit to use the Calcite library to parse, analyze, and optimize queries. The hook for the planner is through an override of the JniFrontend. The CalciteJniFrontend class is the driver that walks through each of the Calcite steps which are as follows: CalciteQueryParser: Takes the string query and outputs an AST in the form of Calcite's SqlNode object. CalciteMetadataHandler: Iterate through the SqlNode from the previous step and make sure all essential table metadata is retrieved from catalogd. CalciteValidator: Validate the SqlNode tree, akin to the Impala Analyzer. CalciteRelNodeConverter: Change the AST into a logical plan. In this first commit, the only logical nodes used are LogicalTableScan and LogicalProject. The LogicalTableScan will serve as the node that reads from an Hdfs Table and the LogicalProject will only project out the used columns in the query. In later versions, the LogicalProject will also handle function changes. CalciteOptimizer: This step is to optimize the query. In this cut, it will be a nop, but in later versions, it will perform logical optimizations via Calcite's rule mechanism. CalcitePhysPlanCreator: Converts the Calcite RelNode logical tree into Impala's PlanNode physical tree ExecRequestCreator: Implement the existing Impala steps that turn a Single Node Plan into a Distributed Plan. It will also create the TExecRequest object needed by the runtime server. Only some very basic queries will work with this commit. These include: select * from tbl <-- only needs the LogicalTableScan select c1 from tbl <-- Also uses the LogicalProject In the CalciteJniFrontend, there is some basic checks to make sure only select statements will get processed. Any non-query statement will revert back to the current Impala planner. In this iteration, any queries besides the minimal ones listed above will result in a caught exception which will then be run through the current Impala planner. The tests that do work can be found in calcite.test and run through the custom cluster test test_experimental_planner.py Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 --- M bin/impala-config.sh M fe/pom.xml M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/planner/PlannerContext.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A java/experimental-planner/pom.xml A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ConvertToImpalaRelRules.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaPlanRel.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/NodeWithExprs.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/phys/ImpalaHdfsScanNode.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/util/CreateExprVisitor.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCalciteCatalogReader.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalcitePhysPlanCreator.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteQueryParser.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteValidator.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CompilerStep.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator2.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeSystemImpl.java A java/exper
[Impala-ASF-CR] IMPALA-12872: Use Calcite for ...
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21109 ) Change subject: IMPALA-12872: Use Calcite for ... .. Patch Set 4: (54 comments) http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java File java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java: http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java@88 PS4, Line 88:* location for the column will remain null. line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java@103 PS4, Line 103: int position = (slotDesc.getColumn().getPosition() + nonPartitionedCols) % totalCols; line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java File java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java: http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java@40 PS4, Line 40: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java File java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java: http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java@50 PS4, Line 50: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java File java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java: http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@67 PS4, Line 67: public class CalciteTable extends RelOptAbstractTable implements Table, Prepare.PreparingTable { line too long (96 > 90) http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@117 PS4, Line 117: SlotRef slotref = new SlotRef(Path.createRawPath(baseTblRef.getUniqueAlias(), fieldName)); line too long (96 > 90) http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@122 PS4, Line 122: + "is a complex type (array/map/struct) column. This is not currently supported.")); line too long (96 > 90) http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@181 PS4, Line 181: @Override line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@182 PS4, Line 182: public boolean columnHasDefaultValue(RelDataType rowType, int ordinal, InitializerContext initializerContext) { line too long (113 > 90) http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java@186 PS4, Line 186: @Override line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCalciteCatalogReader.java File java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCalciteCatalogReader.java: http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCalciteCatalogReader.java@46 PS4, Line 46: return stmtTableCache; line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java File java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java: http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java@53 PS4, Line 53: * to walk through all the steps of compiling the query (e.g. parsing, validating, etc... ) line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java@58 PS4, Line 58: protected static final Logger LOG = LoggerFactory.getLogger(CalciteJniFrontend.class.getName()); line too long (98 > 90) http://gerrit.cloudera.org:8080/#/c/21109/4/java/experimental-planner/sr
[Impala-ASF-CR] IMPALA-12872: Use Calcite for ...
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21109 ) Change subject: IMPALA-12872: Use Calcite for ... .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15475/ : 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/21109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 Gerrit-Change-Number: 21109 Gerrit-PatchSet: 4 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 11 Mar 2024 20:03:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11499: Refactor UrlEncode function to handle special characters
Michael Smith 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 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/21131/1/be/src/util/coding-util.cc File be/src/util/coding-util.cc: http://gerrit.cloudera.org:8080/#/c/21131/1/be/src/util/coding-util.cc@47 PS1, Line 47: static function HiveShouldEscape = is_any_of("\"#%\\*/:=?\u00FF"); With this change it doesn't look like we need this list or ShouldNotEscape anymore, specialCharacterMap covers it. http://gerrit.cloudera.org:8080/#/c/21131/1/be/src/util/coding-util.cc@84 PS1, Line 84: boost::replace_all(input_str, std::string(1, entry.first), entry.second); So this works because it only replaces specific chars - that won't appear in multi-byte characters - rather than anything that's not isalnum? http://gerrit.cloudera.org:8080/#/c/21131/1/be/src/util/coding-util.cc@95 PS1, Line 95: } We could return unused capacity with https://en.cppreference.com/w/cpp/string/basic_string/shrink_to_fit. -- 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: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Mon, 11 Mar 2024 20:29:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12883: Support updating the charge on an entry in the cache
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21122 ) Change subject: IMPALA-12883: Support updating the charge on an entry in the cache .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I34f54fb3a91a77821651c25d8d3bc3a2a3945025 Gerrit-Change-Number: 21122 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Yida Wu Gerrit-Comment-Date: Mon, 11 Mar 2024 20:53:06 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12886: Bump GoogleTest version to 1.14.0
Laszlo Gaal has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21133 Change subject: IMPALA-12886: Bump GoogleTest version to 1.14.0 .. IMPALA-12886: Bump GoogleTest version to 1.14.0 This is to unblock clang-tidy runs on ARM. The current version of gtest seems to suffer from a heap corruption problem that causes unpredictable segfaults in unified-be-tests. Upgrading to GoogleTest 1.14.0 fixed this problem in test runs. Change-Id: Iaa52b4809a7c969e863518b182d2df63256f4c43 --- M buildall.sh 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/33/21133/1 -- To view, visit http://gerrit.cloudera.org:8080/21133 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iaa52b4809a7c969e863518b182d2df63256f4c43 Gerrit-Change-Number: 21133 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal
[Impala-ASF-CR] IMPALA-12872: Use Calcite for ...
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21109 to look at the new patch set (#5). Change subject: IMPALA-12872: Use Calcite for ... .. IMPALA-12872: Use Calcite for ... ...query analysis/optimization for simple queries. This is the first commit to use the Calcite library to parse, analyze, and optimize queries. The hook for the planner is through an override of the JniFrontend. The CalciteJniFrontend class is the driver that walks through each of the Calcite steps which are as follows: CalciteQueryParser: Takes the string query and outputs an AST in the form of Calcite's SqlNode object. CalciteMetadataHandler: Iterate through the SqlNode from the previous step and make sure all essential table metadata is retrieved from catalogd. CalciteValidator: Validate the SqlNode tree, akin to the Impala Analyzer. CalciteRelNodeConverter: Change the AST into a logical plan. In this first commit, the only logical nodes used are LogicalTableScan and LogicalProject. The LogicalTableScan will serve as the node that reads from an Hdfs Table and the LogicalProject will only project out the used columns in the query. In later versions, the LogicalProject will also handle function changes. CalciteOptimizer: This step is to optimize the query. In this cut, it will be a nop, but in later versions, it will perform logical optimizations via Calcite's rule mechanism. CalcitePhysPlanCreator: Converts the Calcite RelNode logical tree into Impala's PlanNode physical tree ExecRequestCreator: Implement the existing Impala steps that turn a Single Node Plan into a Distributed Plan. It will also create the TExecRequest object needed by the runtime server. Only some very basic queries will work with this commit. These include: select * from tbl <-- only needs the LogicalTableScan select c1 from tbl <-- Also uses the LogicalProject In the CalciteJniFrontend, there is some basic checks to make sure only select statements will get processed. Any non-query statement will revert back to the current Impala planner. In this iteration, any queries besides the minimal ones listed above will result in a caught exception which will then be run through the current Impala planner. The tests that do work can be found in calcite.test and run through the custom cluster test test_experimental_planner.py Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 --- M bin/impala-config.sh M fe/pom.xml M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/planner/PlannerContext.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A java/experimental-planner/pom.xml A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ConvertToImpalaRelRules.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaHdfsScanRel.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaPlanRel.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/NodeWithExprs.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/phys/ImpalaHdfsScanNode.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/rel/util/CreateExprVisitor.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteDb.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/CalciteTable.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/schema/ImpalaCalciteCatalogReader.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalcitePhysPlanCreator.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteQueryParser.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CalciteValidator.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/CompilerStep.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/service/ExecRequestCreator.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java A java/experimental-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeSystemImpl.java A java/experi
[native-toolchain-CR] IMPALA-12697: Set FAIL ON PUBLISH to true by default
Laszlo Gaal has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21134 Change subject: IMPALA-12697: Set FAIL_ON_PUBLISH to true by default .. IMPALA-12697: Set FAIL_ON_PUBLISH to true by default Ensure that a package upload failure fails the complete build process instead of just producing an incomplete toolchain in silence. This changes the default initialized value of the FAIL_ON_PUBLISH environment variable to 1. This still allows Jenkins jobs to supply a different value if it ever becomes necessary. Change-Id: I156269e8b3b1fa5d743a8ab5a83810001f7dd648 --- M init.sh 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/34/21134/1 -- To view, visit http://gerrit.cloudera.org:8080/21134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I156269e8b3b1fa5d743a8ab5a83810001f7dd648 Gerrit-Change-Number: 21134 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal
[native-toolchain-CR] IMPALA-12885: Log configuration values when starting a toolchain build
Laszlo Gaal has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21135 Change subject: IMPALA-12885: Log configuration values when starting a toolchain build .. IMPALA-12885: Log configuration values when starting a toolchain build Report the main environment variables before the build starts, in a way similar to what happens at the end of Impala's bin/impala-config.sh Change-Id: Ie6549bb01acce51771fb796c89826b6bbdc54494 --- M init.sh 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/35/21135/1 -- To view, visit http://gerrit.cloudera.org:8080/21135 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie6549bb01acce51771fb796c89826b6bbdc54494 Gerrit-Change-Number: 21135 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal
[native-toolchain-CR] IMPALA-12697: Set FAIL ON PUBLISH to true by default
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21134 ) Change subject: IMPALA-12697: Set FAIL_ON_PUBLISH to true by default .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/21134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156269e8b3b1fa5d743a8ab5a83810001f7dd648 Gerrit-Change-Number: 21134 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 11 Mar 2024 21:13:18 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-12885: Log configuration values when starting a toolchain build
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21135 ) Change subject: IMPALA-12885: Log configuration values when starting a toolchain build .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/21135/1/init.sh File init.sh: http://gerrit.cloudera.org:8080/#/c/21135/1/init.sh@208 PS1, Line 208: echo "PRODUCTION = ${PRODUCTION}" Curious, why does this get curly braces? -- To view, visit http://gerrit.cloudera.org:8080/21135 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie6549bb01acce51771fb796c89826b6bbdc54494 Gerrit-Change-Number: 21135 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 11 Mar 2024 21:13:48 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-12886: Bump GoogleTest version to 1.14.0
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21133 ) Change subject: IMPALA-12886: Bump GoogleTest version to 1.14.0 .. Patch Set 1: Code-Review+1 Does this require much work in Impala? -- To view, visit http://gerrit.cloudera.org:8080/21133 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa52b4809a7c969e863518b182d2df63256f4c43 Gerrit-Change-Number: 21133 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 11 Mar 2024 21:14:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12426: Query History Table
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20770 ) Change subject: IMPALA-12426: Query History Table .. Patch Set 41: Code-Review+1 (7 comments) http://gerrit.cloudera.org:8080/#/c/20770/39/be/src/service/workload-management.h File be/src/service/workload-management.h: http://gerrit.cloudera.org:8080/#/c/20770/39/be/src/service/workload-management.h@42 PS39, Line 42: /// Name of the database where all workload management tables will be stored. > This is going to allocate storage every place the header is included. Do we Done http://gerrit.cloudera.org:8080/#/c/20770/39/be/src/service/workload-management.h@51 PS39, Line 51: /// this constant is slightly smaller to keep well away from the actual max. > This should still be extern, or it won't use the right value in all cases. Done http://gerrit.cloudera.org:8080/#/c/20770/39/be/src/service/workload-management.h@105 PS39, Line 105: FieldDefinition(const std::string db_col_name, const std::string db_col_type, > These should be `std::move`'d in to place. Right now it's making 2 copies o Done http://gerrit.cloudera.org:8080/#/c/20770/39/be/src/service/workload-management.cc File be/src/service/workload-management.cc: http://gerrit.cloudera.org:8080/#/c/20770/39/be/src/service/workload-management.cc@206 PS39, Line 206: > This is still evaluated during static initialization, before query_log_writ Done http://gerrit.cloudera.org:8080/#/c/20770/39/be/src/service/workload-management.cc@354 PS39, Line 354: > Does this actually need to be a global? Looks like it's only used in this f Done http://gerrit.cloudera.org:8080/#/c/20770/41/be/src/service/workload-management.cc File be/src/service/workload-management.cc: http://gerrit.cloudera.org:8080/#/c/20770/41/be/src/service/workload-management.cc@186 PS41, Line 186: // Definition of constants/variables declared in workload-management.h nit: this could all be static variables local to this file since they're not used elsewhere. http://gerrit.cloudera.org:8080/#/c/20770/41/be/src/service/workload-management.cc@368 PS41, Line 368: max_values_length = static_cast(FLAGS_query_log_max_query_length) - We should use this and the batch size to SET MAX_STATEMENT_LENGTH_BYTES for the query, and only need to truncate if the total is close to 2GB. This may benefit from some testing with multiple large queries to ensure writing to the table doesn't get overwhelmed. -- To view, visit http://gerrit.cloudera.org:8080/20770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 Gerrit-Change-Number: 20770 Gerrit-PatchSet: 41 Gerrit-Owner: Jason Fehr 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, 11 Mar 2024 21:23:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12426: Query History Table
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20770 ) Change subject: IMPALA-12426: Query History Table .. Patch Set 41: (24 comments) http://gerrit.cloudera.org:8080/#/c/20770/41/be/src/service/workload-management.h File be/src/service/workload-management.h: http://gerrit.cloudera.org:8080/#/c/20770/41/be/src/service/workload-management.h@40 PS41, Line 40: wm Can this renamed with something more explanatory? Perhaps "management" or "history"? http://gerrit.cloudera.org:8080/#/c/20770/41/be/src/service/workload-management.h@102 PS41, Line 102: std::string Looks like most columns are primitive types. So this can be replaced with TPrimitiveType. Also clarify in comment if these column types are nullable or not. http://gerrit.cloudera.org:8080/#/c/20770/41/be/src/service/workload-management.h@248 PS41, Line 248: if (ctx.record->base_state->stmt_type == TStmtType::SET) { : ctx.sql << "SET"; : } else if (ctx.record->base_state->stmt_type == TStmtType::QUERY) { : ctx.sql << "QUERY"; : } else if (ctx.record->base_state->stmt_type == TStmtType::DML) { : ctx.sql << "DML"; : } else if (ctx.record->base_state->stmt_type == TStmtType::DDL) { : ctx.sql << "DDL"; : } else if (ctx.record->base_state->stmt_type == TStmtType::EXPLAIN) { : ctx.sql << "EXPLAIN"; : } else if (ctx.record->base_state->stmt_type == TStmtType::ADMIN_FN) { : ctx.sql << "ADMIN"; : } else if (ctx.record->base_state->stmt_type == TStmtType::CONVERT) { : ctx.sql << "CONVERT"; : } else if (ctx.record->base_state->stmt_type == TStmtType::LOAD) { : ctx.sql << "LOAD"; : } else if (ctx.record->base_state->stmt_type == TStmtType::TESTCASE) { : ctx.sql << "TESTCASE"; : } else { : ctx.sql << "N/A"; : } This can be feed directly to StringStream sql. This the how generated Types_types.cpp looks like: const char* _kTStmtTypeNames[] = { "QUERY", "DDL", "DML", "EXPLAIN", "LOAD", "SET", "ADMIN_FN", "TESTCASE", "CONVERT", "UNKNOWN"
[Impala-ASF-CR] IMPALA-12872: Use Calcite for ...
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21109 ) Change subject: IMPALA-12872: Use Calcite for ... .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15476/ : 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/21109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 Gerrit-Change-Number: 21109 Gerrit-PatchSet: 5 Gerrit-Owner: Steve Carlin Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 11 Mar 2024 21:36:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12426: Query History Table
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20770 ) Change subject: IMPALA-12426: Query History Table .. Patch Set 41: (1 comment) http://gerrit.cloudera.org:8080/#/c/20770/41/tests/custom_cluster/test_query_log.py File tests/custom_cluster/test_query_log.py: http://gerrit.cloudera.org:8080/#/c/20770/41/tests/custom_cluster/test_query_log.py@87 PS41, Line 87: with_args( For with_args, consider adding line break after open parentheses. -- To view, visit http://gerrit.cloudera.org:8080/20770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 Gerrit-Change-Number: 20770 Gerrit-PatchSet: 41 Gerrit-Owner: Jason Fehr 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, 11 Mar 2024 21:36:39 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-12885: Log configuration values when starting a toolchain build
Laszlo Gaal has posted comments on this change. ( http://gerrit.cloudera.org:8080/21135 ) Change subject: IMPALA-12885: Log configuration values when starting a toolchain build .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/21135/1/init.sh File init.sh: http://gerrit.cloudera.org:8080/#/c/21135/1/init.sh@208 PS1, Line 208: echo "PRODUCTION = ${PRODUCTION}" > Curious, why does this get curly braces? IIRC that was the first variable I added, the rest was added in a second step. IOW pure inconsistency; I'll upload a fix. -- To view, visit http://gerrit.cloudera.org:8080/21135 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie6549bb01acce51771fb796c89826b6bbdc54494 Gerrit-Change-Number: 21135 Gerrit-PatchSet: 1 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 11 Mar 2024 22:10:59 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-12885: Log configuration values when starting a toolchain build
Hello Michael Smith, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21135 to look at the new patch set (#2). Change subject: IMPALA-12885: Log configuration values when starting a toolchain build .. IMPALA-12885: Log configuration values when starting a toolchain build Report the main environment variables before the build starts, in a way similar to what happens at the end of Impala's bin/impala-config.sh Change-Id: Ie6549bb01acce51771fb796c89826b6bbdc54494 --- M init.sh 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/35/21135/2 -- To view, visit http://gerrit.cloudera.org:8080/21135 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie6549bb01acce51771fb796c89826b6bbdc54494 Gerrit-Change-Number: 21135 Gerrit-PatchSet: 2 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith
[native-toolchain-CR] IMPALA-12885: Log configuration values when starting a toolchain build
Laszlo Gaal has posted comments on this change. ( http://gerrit.cloudera.org:8080/21135 ) Change subject: IMPALA-12885: Log configuration values when starting a toolchain build .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/21135/1/init.sh File init.sh: http://gerrit.cloudera.org:8080/#/c/21135/1/init.sh@208 PS1, Line 208: echo "PRODUCTION = $PRODUCTION" > IIRC that was the first variable I added, the rest was added in a second st Done -- To view, visit http://gerrit.cloudera.org:8080/21135 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie6549bb01acce51771fb796c89826b6bbdc54494 Gerrit-Change-Number: 21135 Gerrit-PatchSet: 2 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 11 Mar 2024 22:13:00 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-12885: Log configuration values when starting a toolchain build
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21135 ) Change subject: IMPALA-12885: Log configuration values when starting a toolchain build .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21135 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie6549bb01acce51771fb796c89826b6bbdc54494 Gerrit-Change-Number: 21135 Gerrit-PatchSet: 2 Gerrit-Owner: Laszlo Gaal Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 11 Mar 2024 22:26:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12426: Query History Table
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/20770 ) Change subject: IMPALA-12426: Query History Table .. Patch Set 41: (1 comment) http://gerrit.cloudera.org:8080/#/c/20770/41/tests/custom_cluster/test_query_log.py File tests/custom_cluster/test_query_log.py: http://gerrit.cloudera.org:8080/#/c/20770/41/tests/custom_cluster/test_query_log.py@33 PS41, Line 33: class TestQueryLogTable(CustomClusterTestSuite): : """Tests to assert the query log table is correctly populated.""" : : WM_DB = "sys" : QUERY_TBL = "{0}.impala_query_log".format(WM_DB) : PROTOCOL_BEESWAX = ["beeswax"] : PROTOCOL_HS2 = ["hs2"] : PROTOCOL_ALL = [PROTOCOL_BEESWAX[0], PROTOCOL_HS2[0]] : : @CustomClusterTestSuite.with_args(impalad_args="--enable_workload_mgmt " : "--query_log_write_interval_s=1 " : "--cluster_id=test_max_select " : "--shutdown_grace_period_s=10 " : "--shutdown_deadline_s=60", : catalogd_args="--enable_workload_mgmt", : impalad_graceful_shutdown=True) : @pytest.mark.parametrize("client_protocol", PROTOCOL_BEESWAX) : def test_query_log_table_almost_max_select(self, client_protocol): > Impala test framework use ImpalaTestMatrix to parameterize its test functio Looks like ImpalaTestSuite already has 'protocol' dimension. https://github.com/apache/impala/blob/70c35425d3f8ac68b23fb8e0d08e12ee763965d7/tests/common/impala_test_suite.py#L185 In that case, TestQueryLogTableHS2Client and TestQueryLogTableAllClients can simply override that 'protocol' dimension. -- To view, visit http://gerrit.cloudera.org:8080/20770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 Gerrit-Change-Number: 20770 Gerrit-PatchSet: 41 Gerrit-Owner: Jason Fehr 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, 11 Mar 2024 22:53:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12487: Skip reloading file metadata for ALTER TABLE events with trivial changes in StorageDescriptor
Hello Quanlong Huang, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/21019 to look at the new patch set (#4). Change subject: IMPALA-12487: Skip reloading file metadata for ALTER_TABLE events with trivial changes in StorageDescriptor .. IMPALA-12487: Skip reloading file metadata for ALTER_TABLE events with trivial changes in StorageDescriptor IMPALA-11534 skips reloading file metadata for some trivial ALTER_TABLE events. However, ALTER_TABLE events that have trivial changes in StorageDescriptor are not handled in IMPALA-11534. The only changes that require file metadata reload are: location, rowformat, fileformat, serde, and storedAsSubDirectories=true. The file metadata reload can be skipped for all other changes in SD. Also introduced a small optimization to skip reloading of table schema when ALTER_TABLE changes location, rowformat, fileformat, and serde in the Storage Descriptor. Testing: 1) Manual testing by changing SD parameters in local environment. 2) Added unit tests for the same in MetastoreEventsProcessorTest class. Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac --- M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/Table.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 5 files changed, 156 insertions(+), 46 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/21019/4 -- To view, visit http://gerrit.cloudera.org:8080/21019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac Gerrit-Change-Number: 21019 Gerrit-PatchSet: 4 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala
[Impala-ASF-CR] IMPALA-12487: Skip reloading file metadata for ALTER TABLE events with trivial changes in StorageDescriptor
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/21019 ) Change subject: IMPALA-12487: Skip reloading file metadata for ALTER_TABLE events with trivial changes in StorageDescriptor .. Patch Set 4: (1 comment) > When testing IMPALA-10976, did you find any test failure if we don't skip > reloading table schema? I have seen some failures but I don't remember now. I think what we can do for now is to keep the skipping file metadata reload logic and remove the table schema skipping reload logic and we'll address that in IMPALA-10976. Is that ok with you? http://gerrit.cloudera.org:8080/#/c/21019/1/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/21019/1/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1788 PS1, Line 1788: } > Thanks for pointing to the existing codes. I found it's an existing issue t Thanks for pointing out this issue. -- To view, visit http://gerrit.cloudera.org:8080/21019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac Gerrit-Change-Number: 21019 Gerrit-PatchSet: 4 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 11 Mar 2024 22:56:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12831: Fix HdfsTable.toMinimalTCatalogObject() failed by concurrent modification
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21072 ) Change subject: IMPALA-12831: Fix HdfsTable.toMinimalTCatalogObject() failed by concurrent modification .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21072 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie9f8e65c0bd24000241eedf8ca765c1e4e14fdb3 Gerrit-Change-Number: 21072 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 11 Mar 2024 23:04:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12831: Fix HdfsTable.toMinimalTCatalogObject() failed by concurrent modification
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21072 ) Change subject: IMPALA-12831: Fix HdfsTable.toMinimalTCatalogObject() failed by concurrent modification .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10371/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21072 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie9f8e65c0bd24000241eedf8ca765c1e4e14fdb3 Gerrit-Change-Number: 21072 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 11 Mar 2024 23:04:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12831: Fix HdfsTable.toMinimalTCatalogObject() failed by concurrent modification
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21072 ) Change subject: IMPALA-12831: Fix HdfsTable.toMinimalTCatalogObject() failed by concurrent modification .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/21072/1/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/21072/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2133 PS1, Line 2133: /** > I tried adding a lock for the partition map and it seems not that straightf Filed IMPALA-12891 for this. -- To view, visit http://gerrit.cloudera.org:8080/21072 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie9f8e65c0bd24000241eedf8ca765c1e4e14fdb3 Gerrit-Change-Number: 21072 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 11 Mar 2024 23:04:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12487: Skip reloading file metadata for ALTER TABLE events with trivial changes in StorageDescriptor
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21019 ) Change subject: IMPALA-12487: Skip reloading file metadata for ALTER_TABLE events with trivial changes in StorageDescriptor .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15477/ : 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/21019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6fd9a9504bf93d2529dc7accbf436ad83e51d8ac Gerrit-Change-Number: 21019 Gerrit-PatchSet: 4 Gerrit-Owner: Sai Hemanth Gantasala Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Mon, 11 Mar 2024 23:20:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12426: Query History Table
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20770 ) Change subject: IMPALA-12426: Query History Table .. Patch Set 41: (1 comment) http://gerrit.cloudera.org:8080/#/c/20770/41/tests/util/workload_management.py File tests/util/workload_management.py: http://gerrit.cloudera.org:8080/#/c/20770/41/tests/util/workload_management.py@259 PS41, Line 259: root_mem = re.search(r'\nPLAN-ROOT SINK.*?mem-estimate=(\S+?) mem-reservation', This doesn't work when I try to write tests using some existing tables, like 'select * from functional.alltypes'. I ended up modifying it to root_mem = re.search( r'\n\nF\d+:PLAN FRAGMENT.*?mem-estimate=(\S+?) mem-reservation', profile_text, re.DOTALL) There's a fragment above PLAN-ROOT SINK, and I suspect that's what we actually want. -- To view, visit http://gerrit.cloudera.org:8080/20770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844 Gerrit-Change-Number: 20770 Gerrit-PatchSet: 41 Gerrit-Owner: Jason Fehr 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, 12 Mar 2024 00:03:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12626: Capture tables in query for log
Hello Andrew Sherman, Jason Fehr, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20886 to look at the new patch set (#12). Change subject: IMPALA-12626: Capture tables in query for log .. IMPALA-12626: Capture tables in query for log Currently requires 'drop table sys.impala_query_log' to recreate it with the new column. Change-Id: I9c9c80b2adf7f3e44225a191fe8eb9df3c4bc5aa --- M be/src/exec/system-table-scanner.cc M be/src/service/client-request-state.h M be/src/service/query-state-record.cc M be/src/service/query-state-record.h M be/src/service/workload-management.h M common/thrift/Frontend.thrift M common/thrift/SystemTables.thrift M fe/src/main/java/org/apache/impala/service/Frontend.java M tests/custom_cluster/test_query_live.py M tests/util/workload_management.py 10 files changed, 54 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/20886/12 -- To view, visit http://gerrit.cloudera.org:8080/20886 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9c9c80b2adf7f3e44225a191fe8eb9df3c4bc5aa Gerrit-Change-Number: 20886 Gerrit-PatchSet: 12 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith
[Impala-ASF-CR] IMPALA-12540: Add system.impala query live table
Hello Andrew Sherman, Jason Fehr, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20762 to look at the new patch set (#29). Change subject: IMPALA-12540: Add system.impala_query_live table .. IMPALA-12540: Add system.impala_query_live table Defines SystemTable which are in-memory tables that can provide access to Impala state. Adds the 'impala_query_live' to the database 'sys', which already exists for 'sys.impala_query_log'. Implements the 'impala_query_live' table to view active queries across all coordinators sharing the same statestore. SystemTables create new SystemTableScanNodes for their scan node implementation. When computing scan range locations, SystemTableScanNodes creates a scan range for each in the cluster (identified via ClusterMembershipMgr). This produces a plan that looks like: Query: explain select * from sys.impala_query_live ++ | Explain String | ++ | Max Per-Host Resource Reservation: Memory=4.00MB Threads=2 | | Per-Host Resource Estimates: Memory=11MB | | WARNING: The following tables are missing relevant table | | and/or column statistics. | | sys.impala_query_live | || | PLAN-ROOT SINK | | | | | 01:EXCHANGE [UNPARTITIONED]| | | | | 00:SCAN SYSTEM_TABLE [sys.impala_query_live]| |row-size=72B cardinality=20 | ++ Execution will pull data from ImpalaServer on the backend via a SystemTableScanner implementation based on table name. In the query profile, SYSTEM_TABLE_SCAN_NODE includes ActiveQueryCollectionTime and PendingQueryCollectionTime to track time spent collecting QueryState from ImpalaServer. Grants QueryScanner private access to ImpalaServer, identical to how ImpalaHttpHandler access internal server state. Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 --- M be/src/exec/CMakeLists.txt M be/src/exec/exec-node.cc M be/src/exec/scan-node.cc A be/src/exec/system-table-scan-node.cc A be/src/exec/system-table-scan-node.h A be/src/exec/system-table-scanner.cc A be/src/exec/system-table-scanner.h M be/src/runtime/descriptors.cc M be/src/runtime/descriptors.h M be/src/scheduling/scheduler.cc M be/src/service/fe-support.cc M be/src/service/frontend.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-state-record.h M be/src/util/sharded-query-map-util.h M common/thrift/CMakeLists.txt M common/thrift/CatalogObjects.thrift M common/thrift/Descriptors.thrift M common/thrift/PlanNodes.thrift M common/thrift/StatestoreService.thrift A common/thrift/SystemTables.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/Db.java A fe/src/main/java/org/apache/impala/catalog/SystemTable.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java A fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/Frontend.java A tests/custom_cluster/test_query_live.py M tests/util/workload_management.py 35 files changed, 1,278 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/20762/29 -- To view, visit http://gerrit.cloudera.org:8080/20762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 Gerrit-Change-Number: 20762 Gerrit-PatchSet: 29 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith
[Impala-ASF-CR] IMPALA-12540: Add system.impala query live table
Hello Andrew Sherman, Jason Fehr, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20762 to look at the new patch set (#30). Change subject: IMPALA-12540: Add system.impala_query_live table .. IMPALA-12540: Add system.impala_query_live table Defines SystemTable which are in-memory tables that can provide access to Impala state. Adds the 'impala_query_live' to the database 'sys', which already exists for 'sys.impala_query_log'. Implements the 'impala_query_live' table to view active queries across all coordinators sharing the same statestore. SystemTables create new SystemTableScanNodes for their scan node implementation. When computing scan range locations, SystemTableScanNodes creates a scan range for each in the cluster (identified via ClusterMembershipMgr). This produces a plan that looks like: Query: explain select * from sys.impala_query_live ++ | Explain String | ++ | Max Per-Host Resource Reservation: Memory=4.00MB Threads=2 | | Per-Host Resource Estimates: Memory=11MB | | WARNING: The following tables are missing relevant table | | and/or column statistics. | | sys.impala_query_live | || | PLAN-ROOT SINK | | | | | 01:EXCHANGE [UNPARTITIONED]| | | | | 00:SCAN SYSTEM_TABLE [sys.impala_query_live]| |row-size=72B cardinality=20 | ++ Execution will pull data from ImpalaServer on the backend via a SystemTableScanner implementation based on table name. In the query profile, SYSTEM_TABLE_SCAN_NODE includes ActiveQueryCollectionTime and PendingQueryCollectionTime to track time spent collecting QueryState from ImpalaServer. Grants QueryScanner private access to ImpalaServer, identical to how ImpalaHttpHandler access internal server state. Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 --- M be/src/exec/CMakeLists.txt M be/src/exec/exec-node.cc M be/src/exec/scan-node.cc A be/src/exec/system-table-scan-node.cc A be/src/exec/system-table-scan-node.h A be/src/exec/system-table-scanner.cc A be/src/exec/system-table-scanner.h M be/src/runtime/descriptors.cc M be/src/runtime/descriptors.h M be/src/scheduling/scheduler.cc M be/src/service/fe-support.cc M be/src/service/frontend.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-state-record.h M be/src/util/sharded-query-map-util.h M common/thrift/CMakeLists.txt M common/thrift/CatalogObjects.thrift M common/thrift/Descriptors.thrift M common/thrift/PlanNodes.thrift M common/thrift/StatestoreService.thrift A common/thrift/SystemTables.thrift M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/Db.java A fe/src/main/java/org/apache/impala/catalog/SystemTable.java M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java A fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java M fe/src/main/java/org/apache/impala/service/FeSupport.java M fe/src/main/java/org/apache/impala/service/Frontend.java A tests/custom_cluster/test_query_live.py M tests/util/workload_management.py 35 files changed, 1,278 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/20762/30 -- To view, visit http://gerrit.cloudera.org:8080/20762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 Gerrit-Change-Number: 20762 Gerrit-PatchSet: 30 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith
[Impala-ASF-CR] IMPALA-12540: Add system.impala query live table
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/20762 ) Change subject: IMPALA-12540: Add system.impala_query_live table .. Patch Set 30: (8 comments) http://gerrit.cloudera.org:8080/#/c/20762/28/be/src/exec/system-table-scanner.cc File be/src/exec/system-table-scanner.cc: http://gerrit.cloudera.org:8080/#/c/20762/28/be/src/exec/system-table-scanner.cc@19 PS28, Line 19: > Please #include since make_unique<> is used Done http://gerrit.cloudera.org:8080/#/c/20762/28/be/src/exec/system-table-scanner.cc@57 PS28, Line 57: > The docs on this constructor says it should not be used for status that are Honestly shouldn't be visible, but NOT_IMPLEMENTED_ERROR also seems fine. Done. http://gerrit.cloudera.org:8080/#/c/20762/28/be/src/exec/system-table-scanner.cc@57 PS28, Line 57: > Nit: a message of "Unknown table name" would be clearer. Done http://gerrit.cloudera.org:8080/#/c/20762/28/be/src/exec/system-table-scanner.cc@147 PS28, Line 147: if (!query.events_timeline_empty()) { > Since this file does "using namespace boost::algorithm", can the "boost::al I may try to re-use helpers from workload-management.h, or move this logic to QueryStateExpanded. http://gerrit.cloudera.org:8080/#/c/20762/28/be/src/service/fe-support.cc File be/src/service/fe-support.cc: http://gerrit.cloudera.org:8080/#/c/20762/28/be/src/service/fe-support.cc@651 PS28, Line 651: vector coordinators; > Note: TODO Removed, no clear argument for it. http://gerrit.cloudera.org:8080/#/c/20762/28/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java File fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java: http://gerrit.cloudera.org:8080/#/c/20762/28/fe/src/main/java/org/apache/impala/planner/SystemTableScanNode.java@59 PS28, Line 59: assignConjuncts(analyzer); > I'll look into it, just copied this from other ScanNodes so I didn't look i Removed. http://gerrit.cloudera.org:8080/#/c/20762/28/tests/custom_cluster/test_query_live.py File tests/custom_cluster/test_query_live.py: http://gerrit.cloudera.org:8080/#/c/20762/28/tests/custom_cluster/test_query_live.py@34 PS28, Line 34: result1 = self.client.execute("select * from functional.alltypes", > Yes, I'll work on that in the next rebase. Done http://gerrit.cloudera.org:8080/#/c/20762/28/tests/custom_cluster/test_query_live.py@409 PS28, Line 409: > Nit: need space Done -- To view, visit http://gerrit.cloudera.org:8080/20762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 Gerrit-Change-Number: 20762 Gerrit-PatchSet: 30 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Tue, 12 Mar 2024 00:18:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12626: Capture tables in query for log
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20886 ) Change subject: IMPALA-12626: Capture tables in query for log .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15479/ : 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/20886 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c9c80b2adf7f3e44225a191fe8eb9df3c4bc5aa Gerrit-Change-Number: 20886 Gerrit-PatchSet: 12 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Tue, 12 Mar 2024 00:32:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12540: Add system.impala query live table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20762 ) Change subject: IMPALA-12540: Add system.impala_query_live table .. Patch Set 29: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15478/ : 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/20762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 Gerrit-Change-Number: 20762 Gerrit-PatchSet: 29 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Tue, 12 Mar 2024 00:32:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12540: Add system.impala query live table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20762 ) Change subject: IMPALA-12540: Add system.impala_query_live table .. Patch Set 30: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15480/ : 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/20762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2f9a449f0e5502078931e7f1c5df6e0b762c743 Gerrit-Change-Number: 20762 Gerrit-PatchSet: 30 Gerrit-Owner: Michael Smith Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Tue, 12 Mar 2024 00:42:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12831: Fix HdfsTable.toMinimalTCatalogObject() failed by concurrent modification
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21072 ) Change subject: IMPALA-12831: Fix HdfsTable.toMinimalTCatalogObject() failed by concurrent modification .. Patch Set 4: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10371/ -- To view, visit http://gerrit.cloudera.org:8080/21072 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie9f8e65c0bd24000241eedf8ca765c1e4e14fdb3 Gerrit-Change-Number: 21072 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Tue, 12 Mar 2024 04:00:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12543: Detect self-events before finishing DDL
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21029 ) Change subject: IMPALA-12543: Detect self-events before finishing DDL .. Patch Set 15: ps15 is just a rebase to pick up IMPALA-12678 -- To view, visit http://gerrit.cloudera.org:8080/21029 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8365c934349ad21a4d9327fc11594d2fc3445f79 Gerrit-Change-Number: 21029 Gerrit-PatchSet: 15 Gerrit-Owner: Riza Suminto Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jason Fehr Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Riza Suminto Gerrit-Reviewer: Sai Hemanth Gantasala Gerrit-Comment-Date: Tue, 12 Mar 2024 04:03:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12771: Impala catalogd events-skipped may mark the wrong number
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21045 ) Change subject: IMPALA-12771: Impala catalogd events-skipped may mark the wrong number .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/21045/3/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/21045/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2347 PS3, Line 2347: metrics_.getCounter(MetastoreEventsProcessor.EVENTS_SKIPPED_METRIC).getCount()); line too long (92 > 90) -- To view, visit http://gerrit.cloudera.org:8080/21045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7aeb04e999b82187eb138c0b643ead259da22f1a Gerrit-Change-Number: 21045 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 12 Mar 2024 05:14:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12771: Impala catalogd events-skipped may mark the wrong number
cclive1...@gmail.com has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/21045 ) Change subject: IMPALA-12771: Impala catalogd events-skipped may mark the wrong number .. IMPALA-12771: Impala catalogd events-skipped may mark the wrong number The description of events-skipped metric is wrong. Some cases in Add partition event ,the metric will also be increased, besides for some other cases like alter partition the event is skipped and the log is printed but the events-skipped metric is not increased. Change-Id: I7aeb04e999b82187eb138c0b643ead259da22f1a --- M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java 4 files changed, 165 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/21045/3 -- To view, visit http://gerrit.cloudera.org:8080/21045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7aeb04e999b82187eb138c0b643ead259da22f1a Gerrit-Change-Number: 21045 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-12771: Impala catalogd events-skipped may mark the wrong number
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21045 ) Change subject: IMPALA-12771: Impala catalogd events-skipped may mark the wrong number .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15481/ : 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/21045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7aeb04e999b82187eb138c0b643ead259da22f1a Gerrit-Change-Number: 21045 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 12 Mar 2024 05:38:31 + 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 (#3). ( 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 In this commit, a comprehensive mapping of special characters, including Unicode, to their respective URL-encoded forms has been introduced. The UrlEncode function has been modified to dynamically iterate through this mapping, providing a more generic and scalable approach to special character encoding. This change also enhances code readability and maintainability by incorporating detailed comments explaining each section of the updated implementation. Testing: Some basic tests are provided in unicode-column-name.test. Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb --- M be/src/util/coding-util.cc M testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test 2 files changed, 78 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/31/21131/3 -- 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: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward 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
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 3: (4 comments) > Uploaded patch set 3. http://gerrit.cloudera.org:8080/#/c/21131/1/be/src/util/coding-util.cc File be/src/util/coding-util.cc: http://gerrit.cloudera.org:8080/#/c/21131/1/be/src/util/coding-util.cc@47 PS1, Line 47: // Mapping of special characters to their URL-encoded forms > With this change it doesn't look like we need this list or ShouldNotEscape Done http://gerrit.cloudera.org:8080/#/c/21131/1/be/src/util/coding-util.cc@75 PS1, Line 75: boost::replace_all(input_str, "%", "%25"); > line too long (94 > 90) Done http://gerrit.cloudera.org:8080/#/c/21131/1/be/src/util/coding-util.cc@84 PS1, Line 84: // Encode forward slashes to prevent misinterpretation in paths or URLs > So this works because it only replaces specific chars - that won't appear i Yes, that's correct. The UrlEncode function in the provided code is designed to work with UTF-8 encoded strings, where each character can be represented by one to four bytes. The key is that it replaces specific characters with their URL-encoded forms, and these replacements are carefully chosen to avoid interfering with multi-byte characters.The replacements are safe because they don't disrupt the structure of multi-byte characters in UTF-8 encoding. The % sign followed by two hexadecimal digits, used in URL encoding, ensures that the replacements won't be confused with the actual encoding of multi-byte characters. On the other hand, using a generic isalnum check to determine whether a character should be replaced might not be suitable for UTF-8 encoded strings. It could inadvertently affect multi-byte characters, potentially leading to incorrect or corrupted results. http://gerrit.cloudera.org:8080/#/c/21131/1/be/src/util/coding-util.cc@95 PS1, Line 95: void UrlEncode(const vector& in, string* out, bool hive_compat) { > We could return unused capacity with https://en.cppreference.com/w/cpp/stri 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: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 12 Mar 2024 05:56:40 + Gerrit-HasComments: Yes
[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 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/15482/ : 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: 3 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zihao Ye Gerrit-Comment-Date: Tue, 12 Mar 2024 06:22:06 + Gerrit-HasComments: No