[GitHub] [flink] slinkydeveloper commented on a change in pull request #17256: [FLINK-23385][table] Implement COALESCE function

[GitBox <gi...@apache.org>]

slinkydeveloper commented on a change in pull request #17256:
URL: https://github.com/apache/flink/pull/17256#discussion_r707327177



##########
File path: flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/calcite/FlinkPlannerImpl.scala
##########
@@ -97,6 +97,7 @@ class FlinkPlannerImpl(
       typeFactory,
       SqlValidator.Config.DEFAULT
         .withIdentifierExpansion(true)
+        .withCallRewrite(false) // Disables the rewrite of COALESCE

Review comment:
       Seems like the getter for that flag is used only at `SqlOperatorImpl` line 1193, which invokes `SqlOperator#rewriteCall` that is used only by `SqlProcedureCallOperator`, `SqlJdbcFunctionCall`, `SqlCoalesceFunction`, `SqlDatePartFunction`, `SqlNewOperator` and `SqlNullifFunction`. Among all of these, my understanding is that we use on coalesce, sql data part function and null if (which seems to me the same of if_null). And in fact, this change breaks a test here: https://dev.azure.com/apache-flink/apache-flink/_build/results?buildId=23992&view=ms.vss-test-web.build-test-results-tab&runId=1115000&resultId=100738&paneView=debug
   
   The way to hack around this is to remove this line change and add a condition to the if at `SqlOperatorImpl` line 1193, but I don't really like it :) Do you have any other alternative?




-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org