Aman Sinha 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 12:

(2 comments)

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
> One general problem I've had with the explain plans is that Calcite does no
We need to think about the Calcite changes needed for introducing the aliases, 
otherwise the plan gets hard to understand  even though it is correct since 
Calcite logical plan resolves the table references correctly.
But this is a separate issue which involves Calcite community involvement.
For this test, I think the option 1 + a regex could work .. just like you did 
for runtime_filters.test line 333.  Even within the 
CALCITE_PLANNER_RUNTIME_PROFILE section, having the regex would be useful.


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'
> This is similar to my other comment.  Explain plan for this line looks like
Could we use CALCITE_PLANNER_RESULTS section for the Calcite plan ? The 
aliasing difference between original planner and Calcite would be pervasive, so 
how are we handling that elsewhere ?  It would be too intrusive to have to add 
a regex for many of the explain plans.



--
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: 12
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: Sun, 12 Jul 2026 23:56:22 +0000
Gerrit-HasComments: Yes

Reply via email to