konstantinb commented on code in PR #6423: URL: https://github.com/apache/hive/pull/6423#discussion_r3169272298
########## ql/src/test/queries/clientpositive/cte_materialize.q: ########## @@ -0,0 +1,46 @@ +-- Test CTE materialization with both CBO enabled and disabled +-- Verifies DDLSemanticAnalyzerFactory is used for CTE materialization +-- Also ensures that an NPE is no longer triggered with CBO off (HIVE-28724 regression) + +-- Test with CBO enabled (default) +explain +WITH cte AS ( + SELECT COUNT(*) as cnt FROM (SELECT 1 as id) t +) +SELECT * FROM cte +UNION ALL +SELECT * FROM cte +UNION ALL +SELECT * FROM cte; + +-- Test with CBO disabled +set hive.cbo.enable=false; + +explain +WITH cte AS ( + SELECT COUNT(*) as cnt FROM (SELECT 1 as id) t +) +SELECT * FROM cte +UNION ALL +SELECT * FROM cte +UNION ALL +SELECT * FROM cte; + +-- Test the recompile-without-CBO auto-trigger path. Review Comment: > TBH, I’m not sure it is necessary to add this .q file @kasakrisz, while I can agree that testing the non-CBO configured path becomes redundant once full CBO deprecation gets addressed, I believe that having at least one new query that guards against similar regressions could be beneficial. The last query of the test file generates the following when executed against master: [cte_materialize.q.test.log.txt](https://github.com/user-attachments/files/27250177/cte_materialize.q.test.log.txt) > Based on the scope of this patch, this path does not need to be covered because the previous test case already covers the non-CBO path. The very first test in the file is **not** broken in the current master. I thought it would be useful to leave it in to confirm that my changes introduce no additional regressions for the CBO path, but you helped me to see that it is indeed redundant now -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
