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 3: (7 comments) It'd be nice to add some e2e tests. 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 whole partitions (or all files of an Iceberg table). We use sampledPartitions_ in this case by getSampledOrRawPartitions(). But it's still incorrect if not all files in the sampledPartitions are selected. Maybe we can use the sample percent to fix this. I think the cardinality of a sampled scan can be used to estimate the corresponding full scan. Or if the full scan with more partition predicates have the same input cardinality of the sampled scan, they can share the cardinality. Then we don't need the sample clause in HBO key string. What do you think? http://gerrit.cloudera.org:8080/#/c/24913/3//COMMIT_MSG@24 PS3, Line 24: "a.item > 1" and 0 for "a.item > 100" under the same key. Nice catch! http://gerrit.cloudera.org:8080/#/c/24913/3/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/3/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1943 PS3, Line 1943: which has the same input stats. nit: sampled scan might not has the same input stats as the full scan's. http://gerrit.cloudera.org:8080/#/c/24913/3/fe/src/test/java/org/apache/impala/planner/HboKeyStringTest.java File fe/src/test/java/org/apache/impala/planner/HboKeyStringTest.java: http://gerrit.cloudera.org:8080/#/c/24913/3/fe/src/test/java/org/apache/impala/planner/HboKeyStringTest.java@149 PS3, Line 149: assertTrue(countStarScan.getCountStarSlot() != null); nit: I think we don't need to verify this since it's unrelevant to HBO. Then we can use scanNodeKey() as well in this test. http://gerrit.cloudera.org:8080/#/c/24913/3/fe/src/test/java/org/apache/impala/planner/HboKeyStringTest.java@164 PS3, Line 164: scanNodeKey nit: maybe use a more specific name like singleScanKey, uniqueScanKey, etc. http://gerrit.cloudera.org:8080/#/c/24913/3/fe/src/test/java/org/apache/impala/planner/HboKeyStringTest.java@213 PS3, Line 213: "functional_parquet.complextypestbl t, t.int_array x WHERE x.item > 1")); Can we also assert the actual key strings? HboKeyStringTest is a place to show how the HBO key strings look like. http://gerrit.cloudera.org:8080/#/c/24913/3/tests/query_test/test_hbo.py File tests/query_test/test_hbo.py: http://gerrit.cloudera.org:8080/#/c/24913/3/tests/query_test/test_hbo.py@195 PS3, Line 195: def test_collection_scan_cardinality(self): Can we improve this e2e test to cover the bug of missing collection conjuncts? -- 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: 3 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: Thu, 24 Sep 2026 10:50:55 +0000 Gerrit-HasComments: Yes
