Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20762 )

Change subject: IMPALA-12540: Query Live Table
......................................................................


Patch Set 44:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.cc
File be/src/exec/system-table-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.cc@157
PS41, Line 157:     switch (slot_desc->col_pos()) {
> Nit: for (auto slot_desc : tuple_desc->slots())
Done


http://gerrit.cloudera.org:8080/#/c/20762/41/be/src/exec/system-table-scanner.cc@214
PS41, Line 214:       }
> I'd be OK showing -1 here.
We had decided not to add END_TIME_UTC because it could be calculated from 
START_TIME_UTC and TOTAL_TIME_NS.

Also implemented this as 
https://github.com/apache/impala/blob/master/be/src/service/impala-http-handler.cc#L543-L551,
 because it's too useful not to include.


http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/SystemTables.thrift
File common/thrift/SystemTables.thrift:

http://gerrit.cloudera.org:8080/#/c/20762/41/common/thrift/SystemTables.thrift@21
PS41, Line 21: # Must be kept in-s
> TQueryTableColumn sounds good.
Done


http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java
File fe/src/main/java/org/apache/impala/catalog/SystemTable.java:

http://gerrit.cloudera.org:8080/#/c/20762/41/fe/src/main/java/org/apache/impala/catalog/SystemTable.java@52
PS41, Line 52:   private static final Map<String, TSystemTableName> 
SYSTEM_TABLE_NAME_MAP =
             :       ImmutableMap.of(QUERY_LIVE, TSystemTableName.QUERY_LIVE);
> Ah, nevermind. I misread it the first time.
I've added a FE test. SYSTEM_TABLE_NAME_MAP is private, but I'm asserting that 
the table we create with CreateQueryLiveTable returns 
TSystemTableName.QUERY_LIVE where relevant.


http://gerrit.cloudera.org:8080/#/c/20762/43/testdata/workloads/functional-planner/queries/PlannerTest/impala-query-live.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/impala-query-live.test:

http://gerrit.cloudera.org:8080/#/c/20762/43/testdata/workloads/functional-planner/queries/PlannerTest/impala-query-live.test@14
PS43, Line 14: # Error trying to create new sys.impala_query_live
             : create table sys.impala_query_live (i int)
> Please add test to show that insert/delete/update over impala_query_live wi
Done


http://gerrit.cloudera.org:8080/#/c/20762/43/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:

http://gerrit.cloudera.org:8080/#/c/20762/43/tests/custom_cluster/test_query_live.py@49
PS43, Line 49:
> Please add tests with group by and count.
Done


http://gerrit.cloudera.org:8080/#/c/20762/44/tests/custom_cluster/test_query_live.py
File tests/custom_cluster/test_query_live.py:

http://gerrit.cloudera.org:8080/#/c/20762/44/tests/custom_cluster/test_query_live.py@22
PS44, Line 22: from time import sleep
> flake8: F401 'time.sleep' imported but unused
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: 44
Gerrit-Owner: Michael Smith <michael.sm...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 19:13:18 +0000
Gerrit-HasComments: Yes

Reply via email to