Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24528 )
Change subject: IMPALA-15135: Calcite planner: added support for Kudu tables ...................................................................... Patch Set 15: (5 comments) http://gerrit.cloudera.org:8080/#/c/24528/11/fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java File fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java: http://gerrit.cloudera.org:8080/#/c/24528/11/fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java@88 PS11, Line 88: default Iterable<Column> getStarColumns() { > Does it need to return a List? Lazy iteration would be nice. I think this is what you mean? Lemme know if you were thinking something different. http://gerrit.cloudera.org:8080/#/c/24528/11/fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java@89 PS11, Line 89: return getColumnsInHiveOrder().stream() > https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/im Done http://gerrit.cloudera.org:8080/#/c/24528/12/testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test File testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test: http://gerrit.cloudera.org:8080/#/c/24528/12/testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test@30 PS12, Line 30: .*.ts > I suppose you will file an upstream Impala ticket for the aliasing work in I thought I had done it before, but I couldn't find it. Created IMPALA-15180 http://gerrit.cloudera.org:8080/#/c/24528/13/testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test File testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test: http://gerrit.cloudera.org:8080/#/c/24528/13/testdata/workloads/functional-query/queries/QueryTest/kudu_runtime_filter_with_timestamp_conversion.test@30 PS13, Line 30: .*.ts > Interestingly, here the regex was changed to include t2 which is the alias This was an accidental rollback from patch 12 to patch 13. It fails with the "t2" in there. It is removed again to make the test work. http://gerrit.cloudera.org:8080/#/c/24528/12/testdata/workloads/functional-query/queries/QueryTest/partitioned-top-n-high-ndv.test File testdata/workloads/functional-query/queries/QueryTest/partitioned-top-n-high-ndv.test: http://gerrit.cloudera.org:8080/#/c/24528/12/testdata/workloads/functional-query/queries/QueryTest/partitioned-top-n-high-ndv.test@39 PS12, Line 39: row_regex:'| partition by: .*part1, .*part2, .*part3, .*col4, .*col5' > If we have done similar changes in previous commits, I am ok with that for ack -- To view, visit http://gerrit.cloudera.org:8080/24528 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie22a66f0edad195cade74a8447efc83a8ce526f7 Gerrit-Change-Number: 24528 Gerrit-PatchSet: 15 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Reviewer: Xuebin Su <[email protected]> Gerrit-Comment-Date: Tue, 14 Jul 2026 18:11:00 +0000 Gerrit-HasComments: Yes
