Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21142 )
Change subject: IMPALA-12737: Query columns in workload management tables. ...................................................................... Patch Set 27: (5 comments) http://gerrit.cloudera.org:8080/#/c/21142/27/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/21142/27/be/src/service/impala-server.cc@3235 PS27, Line 3235: VLOG(2) << "Internal server ready"; > Seems redundant with LOG(INFO) below. I agree. Deleted this line. http://gerrit.cloudera.org:8080/#/c/21142/27/be/src/service/internal-server.h File be/src/service/internal-server.h: http://gerrit.cloudera.org:8080/#/c/21142/27/be/src/service/internal-server.h@149 PS27, Line 149: Intended for use as a convenience method when query results are small > Will be great if we can LOG(WARNING) if the query results turns out not sma That line came from copying and pasting the comments from the ExecuteAndFetchAllText function and it not applicable. Removed. http://gerrit.cloudera.org:8080/#/c/21142/27/be/src/service/internal-server.cc File be/src/service/internal-server.cc: http://gerrit.cloudera.org:8080/#/c/21142/27/be/src/service/internal-server.cc@305 PS27, Line 305: i=0; i<results_metadata->columns.size() > nit: add space between '=' and '<' operator, Done http://gerrit.cloudera.org:8080/#/c/21142/27/be/src/service/workload-management-fields.cc File be/src/service/workload-management-fields.cc: http://gerrit.cloudera.org:8080/#/c/21142/27/be/src/service/workload-management-fields.cc@454 PS27, Line 454: }, VERSION_2_0_0), > nit: major version usually signifies a breaking change, would this make mor Good point. I agree that 1.1.0 makes more sense. http://gerrit.cloudera.org:8080/#/c/21142/27/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java: http://gerrit.cloudera.org:8080/#/c/21142/27/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@380 PS27, Line 380: // Register all columns referenced in this statement, starting with select clause. : // Joins will be added during planning. > Can you add comment explaining about how column alias is treated? Is the al Done -- 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: 27 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, 21 Aug 2024 21:53:44 +0000 Gerrit-HasComments: Yes
