Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21142 )
Change subject: IMPALA-12737: Query columns in workload management tables. ...................................................................... Patch Set 40: (7 comments) http://gerrit.cloudera.org:8080/#/c/21142/40/be/src/service/workload-management-flags.cc File be/src/service/workload-management-flags.cc: http://gerrit.cloudera.org:8080/#/c/21142/40/be/src/service/workload-management-flags.cc@174 PS40, Line 174: workload_mgmt_schema_version, "1.1.0" Question: if there is a bug exist in a specific version (mistake in collecting column path, etc), can user set this flag backward to disable code related to the new version? http://gerrit.cloudera.org:8080/#/c/21142/38/be/src/service/workload-management-init.cc File be/src/service/workload-management-init.cc: http://gerrit.cloudera.org:8080/#/c/21142/38/be/src/service/workload-management-init.cc@166 PS38, Line 166: : VLOG(2) << "Upgrading workload management table '" << table_name << "' from schema " : << "version '" << current_version.ToString() << "' to '" : << target_versi > My original implementation had the upgrade broken out by version. I moved Ack http://gerrit.cloudera.org:8080/#/c/21142/38/be/src/statestore/statestore.h File be/src/statestore/statestore.h: http://gerrit.cloudera.org:8080/#/c/21142/38/be/src/statestore/statestore.h@242 PS38, Line 242: : int32_t port() { return thrift_server_->port(); } > Good catch. Not sure how this snuck back in, most likely due to a conflict Done http://gerrit.cloudera.org:8080/#/c/21142/38/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/21142/38/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@4662 PS38, Line 4662: sourceExpr.hasImportantChildren()) { : List<SlotRef> childSlotRefs = new ArrayList<>(); : sourceExpr.collect(Predicates.instanceOf(SlotRef.class), childSlotRefs); : addColumnsTo(clause, childSlotRefs) > I tested this by adding an else clause and running the entire tpcds suite o Done http://gerrit.cloudera.org:8080/#/c/21142/40/fe/src/main/java/org/apache/impala/analysis/Expr.java File fe/src/main/java/org/apache/impala/analysis/Expr.java: http://gerrit.cloudera.org:8080/#/c/21142/40/fe/src/main/java/org/apache/impala/analysis/Expr.java@2041 PS40, Line 2041: hasImportantChildren 'isCandidateForQueryLog' might fit better. http://gerrit.cloudera.org:8080/#/c/21142/38/fe/src/main/java/org/apache/impala/analysis/Path.java File fe/src/main/java/org/apache/impala/analysis/Path.java: http://gerrit.cloudera.org:8080/#/c/21142/38/fe/src/main/java/org/apache/impala/analysis/Path.java@442 PS38, Line 442: /** > Done Done http://gerrit.cloudera.org:8080/#/c/21142/38/fe/src/main/java/org/apache/impala/analysis/Path.java@450 PS38, Line 450: individual piece of the path. Element { > The ColumnsTest junits revealed that Iceberg delete tables caused an error Ack. Might be good to have Iceberg expert to look and confirm. Or run related iceberg tests to ensure no breakage. -- To view, visit http://gerrit.cloudera.org:8080/21142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I78f3670b067c0c192ee8a212fba95466fbcb51d7 Gerrit-Change-Number: 21142 Gerrit-PatchSet: 40 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Wed, 28 Aug 2024 18:42:02 +0000 Gerrit-HasComments: Yes
