Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24075 )
Change subject: IMPALA-14789: Modify some tests for Calcite planner ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/24075/3/testdata/workloads/functional-query/queries/QueryTest/explain-level0.test File testdata/workloads/functional-query/queries/QueryTest/explain-level0.test: http://gerrit.cloudera.org:8080/#/c/24075/3/testdata/workloads/functional-query/queries/QueryTest/explain-level0.test@37 PS3, Line 37: # Calcite planner has no rewrite, so this section is blank > What happens then? Calcite planner fails? Or it produces a plan, and you're This will automatically succeed since there is nothing to check. I'm open to suggestions if this seems like too big of a hack, but basically there is no point of running this test because there is nothing to test here for the Calcite planner http://gerrit.cloudera.org:8080/#/c/24075/3/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-scheduling.test File testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-scheduling.test: http://gerrit.cloudera.org:8080/#/c/24075/3/testdata/workloads/functional-query/queries/QueryTest/mt-dop-parquet-scheduling.test@334 PS3, Line 334: # Calcite plan does not have union nodes > Why not check for something else in the profile? Ack...will add this soon, wanted to post other comments but signal that I still need to do this. http://gerrit.cloudera.org:8080/#/c/24075/1/testdata/workloads/functional-query/queries/QueryTest/subquery.test File testdata/workloads/functional-query/queries/QueryTest/subquery.test: http://gerrit.cloudera.org:8080/#/c/24075/1/testdata/workloads/functional-query/queries/QueryTest/subquery.test@1108 PS1, Line 1108: ---- CALCITE_PLANNER_CATCH: ANY_OF > I'm still confused what it's doing that's different. Why didn't the prior b So before the Calcite planner existed, there is no reason at all to support a CATCH section that contains __NO_ERROR__. Because...why would you even bother to put a CATCH section if there is no error? So it turns out the code just didn't work because there was no need to support it. The code for __NO_ERROR__ was added in the ANY_OF situation. There is a test somewhere that allows for both an exception to be thrown and it allows the case where no exception is thrown. So all that stuff is in the original code. So with the Calcite Planner, there now exists a case (specifically this one) where you can have a CATCH section that has __NO_ERROR__ But that's where I got lazy :). Since the code already exists with the ANY_OF, I cheated and used that. If you think this is too much of a hack, I can go make the change. -- To view, visit http://gerrit.cloudera.org:8080/24075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8cc0982fe2ece5323cfc77ee8aee4c55bac6062f Gerrit-Change-Number: 24075 Gerrit-PatchSet: 1 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Wed, 18 Mar 2026 03:51:41 +0000 Gerrit-HasComments: Yes
