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 <memory> 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<TNetworkAddress> 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 <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Tue, 12 Mar 2024 00:18:58 +0000 Gerrit-HasComments: Yes
