[GitHub] [flink] twalthr commented on a change in pull request #15303: [FLINK-21713][table-api/table-planner] Correct function CURRENT_TIMESTAMP/CURRENT_TIME/CURRENT_DATE/NOW/LOCALTIME/LOCALTIMESTAMP

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

twalthr commented on a change in pull request #15303:
URL: https://github.com/apache/flink/pull/15303#discussion_r598667617



##########
File path: flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/planner/codegen/CodeGeneratorContext.scala
##########
@@ -505,8 +598,27 @@ class CodeGeneratorContext(val tableConfig: TableConfig) {
   }
 
   /**
-    * Adds a reusable local time to the beginning of the SAM of the generated class.
-    */
+   * Adds a reusable session-level local date time to the beginning of
+   * the SAM of the generated class.
+   */
+  def addReusableSessionLocalDateTime(): String = {

Review comment:
       Are you sure that this is enough? A session-level local time should be the same for multiple calcs in the pipeline. Ideally, across parallel instances. As far as I can see this would only ensure per-calc operator constant time here. We should find a solution that has better semantics.




-- 
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.

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