flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [flink] dawidwys commented on a change in pull request #10942: [FLINK-15487][table] Allow registering FLIP-65 functions in TableEnvironment
Date Tue, 28 Jan 2020 15:52:25 GMT
dawidwys commented on a change in pull request #10942: [FLINK-15487][table] Allow registering
FLIP-65 functions in TableEnvironment
URL: https://github.com/apache/flink/pull/10942#discussion_r371888078
 
 

 ##########
 File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/FunctionCatalog.java
 ##########
 @@ -81,8 +85,283 @@ public void setPlannerTypeInferenceUtil(PlannerTypeInferenceUtil plannerTypeInfe
 		this.plannerTypeInferenceUtil = plannerTypeInferenceUtil;
 	}
 
+	/**
+	 * Registers a temporary system function.
+	 */
+	public void registerTemporarySystemFunction(
+			String name,
+			FunctionDefinition definition,
+			boolean ignoreIfExists) {
+		final String normalizedName = FunctionIdentifier.normalizeName(name);
+
+		if (definition instanceof UserDefinedFunction) {
+			try {
+				UserDefinedFunctionHelper.prepareInstance(config, (UserDefinedFunction) definition);
+			} catch (Throwable t) {
+				throw new ValidationException(
+					String.format(
+						"Could not register temporary system function '%s' due to implementation errors.",
+						name),
+					t);
+			}
+		}
+
+		if (!tempSystemFunctions.containsKey(normalizedName)) {
+			tempSystemFunctions.put(normalizedName, definition);
+		} else if (!ignoreIfExists) {
+			throw new ValidationException(
+				String.format(
+					"Could not register temporary system function. A function named '%s' does already exist.",
+					name));
+		}
+	}
+
+	/**
+	 * Drops a temporary system function. Returns true if a function was dropped.
+	 */
+	public boolean dropTemporarySystemFunction(
+			String name,
+			boolean ignoreIfNotExist) {
+		final String normalizedName = FunctionIdentifier.normalizeName(name);
+		final FunctionDefinition definition = tempSystemFunctions.remove(normalizedName);
+
+		if (definition == null && !ignoreIfNotExist) {
+			throw new ValidationException(
+				String.format(
+					"Could not drop temporary system function. A function named '%s' doesn't exist.",
+					name));
+		}
+
+		return definition != null;
+	}
+
+	/**
+	 * Registers a temporary catalog function.
+	 */
+	public void registerTemporaryCatalogFunction(
+			UnresolvedIdentifier unresolvedIdentifier,
 
 Review comment:
   We're not being too consistent with the type of identifier that the `FunctionCatalog/CatalogManager`
accepts (I'm guilty of that myself).
   
   Some methods accept `UnresolvedIdentifier` e.g. `FunctionCatalog#registerTemporaryCatalogFunction`,
`CatalogManager#dropTemporaryView`. Some resolved `ObjectIdentifier` e.g. `CatalogManager#createTemporaryTable`,
`CatalogManager#createTable`.
   
   I am not sure which one should we prefer. If we go with the `UnresolvedIdentifier` the
benefit is that we always qualify in the `Catalog*`. The downside is that we would no longer
qualify in the `*Operations`, (e.g. CreateTableOperation etc.), whereas we said that all Operations
should be fully resolved...
   
   We can create a follow up for that.

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


With regards,
Apache Git Services

Mime
View raw message