You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/07/05 09:58:25 UTC

[GitHub] [flink] fsk119 commented on a diff in pull request #19218: [FLINK-27376][sql] Support current_database function

fsk119 commented on code in PR #19218:
URL: https://github.com/apache/flink/pull/19218#discussion_r913552152


##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/calls/StringCallGen.scala:
##########
@@ -233,6 +233,10 @@ object StringCallGen {
             isCharacterString(operands(2).resultType) =>
         methodGen(BuiltInMethods.CONVERT_TZ)
 
+      case CURRENT_DATABASE =>
+        val timestamp = ctx.addReusableQueryLevelCurrentDatabase()

Review Comment:
   timestamp -> database



##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/utils/InternalConfigOptions.java:
##########
@@ -52,6 +52,13 @@ public final class InternalConfigOptions {
                                     + " some temporal functions like LOCAL_TIMESTAMP in batch job to make sure these"
                                     + " temporal functions has query-start semantics.");
 
+    public static final ConfigOption<String> TABLE_QUERY_CURRENT_DATABASE =
+            key("__table.query-start.current_database__")
+                    .stringType()
+                    .noDefaultValue()
+                    .withDescription(
+                            "The config used to save the current database at query start.");

Review Comment:
   Also, document who will use the value?



##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/utils/InternalConfigOptions.java:
##########
@@ -52,6 +52,13 @@ public final class InternalConfigOptions {
                                     + " some temporal functions like LOCAL_TIMESTAMP in batch job to make sure these"
                                     + " temporal functions has query-start semantics.");
 
+    public static final ConfigOption<String> TABLE_QUERY_CURRENT_DATABASE =
+            key("__table.query-start.current_database__")

Review Comment:
   Other options use the "-" rather than "_". How about renaming to `__table.query-start.current-database__`?



##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java:
##########
@@ -1268,6 +1268,16 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL)
                     .outputTypeStrategy(nullableIfArgs(SpecificTypeStrategies.ROUND))
                     .build();
 
+    // --------------------------------------------------------------------------------------------
+    // Catalog functions
+    // --------------------------------------------------------------------------------------------
+    public static final BuiltInFunctionDefinition CURRENT_DATABASE =

Review Comment:
   Leave a blank line before the `CURRENT_DATABASE`. We should align with others.



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