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

Change subject: IMPALA-15394: Add missing scan properties to the HBO scan key
......................................................................


Patch Set 5:

(4 comments)

Thanks for adding the e2e tests. Do you mind adding a test for partition scan 
queries as well?

Just have some nits on comments which seem too verbose.

http://gerrit.cloudera.org:8080/#/c/24913/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/24913/3//COMMIT_MSG@20
PS3, Line 20:   input rows count every partition with a sampled file (or all 
files of
> Agreed, input_rows is too high there. But I'd keep <SAMPLE> in the key: wit
Yeah, we can do it in a follow-up JIRA.


http://gerrit.cloudera.org:8080/#/c/24913/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

http://gerrit.cloudera.org:8080/#/c/24913/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1941
PS5, Line 1941:
nit: add "returns"


http://gerrit.cloudera.org:8080/#/c/24913/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1943
PS5, Line 1943: and without numRows its runs are matched by the catalog version 
alone
nit: remove this? It's a bit confusing. We also match total file bytes and it's 
the same mechanism in matching runs of other keys.


http://gerrit.cloudera.org:8080/#/c/24913/5/tests/query_test/test_hbo.py
File tests/query_test/test_hbo.py:

http://gerrit.cloudera.org:8080/#/c/24913/5/tests/query_test/test_hbo.py@246
PS5, Line 246: The table has stats, so the
             :     # expected cardinality doesn't depend on the size of the 
data files.
nit: "Use a parquet table with stats."



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03183aa998f90bf834dba4ce9ad2f2e57fef1140
Gerrit-Change-Number: 24913
Gerrit-PatchSet: 5
Gerrit-Owner: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Tue, 29 Sep 2026 14:18:03 +0000
Gerrit-HasComments: Yes

Reply via email to