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 13:

(3 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: .*t2.
> Heh, I was thinking about this right before I read your comment.
I suppose you will file an upstream Impala ticket for the aliasing work in the 
future.


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: .*t2.
Interestingly, here the regex was changed to include t2 which is the alias 
name, not the table name.


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'
> I have had this problem in other explain plans.  While the number of explai
If we have done similar changes in previous commits, I am ok with that for now. 
 Yes, having the duplication in a separate Calcite specific section does have 
the cons of needing to update 2 plans instead of 1 if something changes in the 
plan shape. I do anticipate, though, that we would have to re-look at this if 
the changes get pervasive enough.



--
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: 13
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 05:11:45 +0000
Gerrit-HasComments: Yes

Reply via email to