Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18635 )

Change subject: IMPALA-886: Support displaying HBase cols in the order from HMS
......................................................................


Patch Set 4: Code-Review+1

(4 comments)

Thanks for fixing this legacy bug! The patch LGTM.

http://gerrit.cloudera.org:8080/#/c/18635/4/be/src/util/backend-gflag-util.cc
File be/src/util/backend-gflag-util.cc:

http://gerrit.cloudera.org:8080/#/c/18635/4/be/src/util/backend-gflag-util.cc@189
PS4, Line 189:
nit: duplicated blank line


http://gerrit.cloudera.org:8080/#/c/18635/4/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
File fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java:

http://gerrit.cloudera.org:8080/#/c/18635/4/fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java@136
PS4, Line 136: table.getColumns()
nit: use 'columns' directly


http://gerrit.cloudera.org:8080/#/c/18635/4/testdata/workloads/functional-query/queries/QueryTest/hbase-hms-column-order.test
File 
testdata/workloads/functional-query/queries/QueryTest/hbase-hms-column-order.test:

http://gerrit.cloudera.org:8080/#/c/18635/4/testdata/workloads/functional-query/queries/QueryTest/hbase-hms-column-order.test@5
PS4, Line 5:  =
nit: needs a space after '='


http://gerrit.cloudera.org:8080/#/c/18635/4/testdata/workloads/functional-query/queries/QueryTest/hbase-hms-column-order.test@31
PS4, Line 31: ====
It'd be nice if we can copy some tests from 
testdata/workloads/functional-planner/queries/PlannerTest/hbase.test. Just make 
sure string predicates on the key column is pushed down as expected.



--
To view, visit http://gerrit.cloudera.org:8080/18635
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc5df8b803f2ae3b93951765326cdaea706e3563
Gerrit-Change-Number: 18635
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Wed, 22 Jun 2022 03:54:57 +0000
Gerrit-HasComments: Yes

Reply via email to