zabetak commented on code in PR #5249: URL: https://github.com/apache/hive/pull/5249#discussion_r1670666746
########## ql/src/test/queries/clientpositive/perf/cbo_query11.q: ########## @@ -1,3 +1,4 @@ +set hive.optimize.cte.suggester.class=org.apache.hadoop.hive.ql.optimizer.calcite.CommonTableExpressionPrintSuggester; Review Comment: I came up with the `CommonTableExpressionPrintSuggester` as a convenient way to (unit) test the `CommonTableExpressionIdentitySuggester` on TPC-DS queries. I considered some other alternatives like https://github.com/apache/hive/pull/5154/commits/0ed2f5c45c852361c834c693f85758a107893246 but these required more changes and were less realistic. In terms of code duplication indeed the `set hive.optimize.cte.suggester.class` appears in ~100 files/lines. If we go for a separate suite the overall duplication may be larger. With the current design we would need to duplicate the driver class (`TestTezTPCDS30TBPerfCliDriver`), the respective config (`TezTPCDS30TBCliConfig`), potentially the queries that we want to test, xml config files, etc. Furthermore, if we want to test CTE specific configurations for certain TPC-DS queries we can always copy the respective file (e.g., cte_cbo_query11.q) and set additional properties as required. I am OK with all the options. If you feel a new CTE driver would be needed/beneficial for this work I can do the necessary changes. -- 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: gitbox-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
