[GitHub] [iceberg] wuwenchi commented on a diff in pull request #5120: Flink: add sql partition functions

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

wuwenchi commented on code in PR #5120:
URL: https://github.com/apache/iceberg/pull/5120#discussion_r905784983


##########
flink/v1.15/flink/src/main/java/org/apache/iceberg/flink/FlinkCatalog.java:
##########
@@ -619,12 +631,17 @@ public void alterPartition(ObjectPath tablePath, CatalogPartitionSpec partitionS
 
   @Override
   public List<String> listFunctions(String dbName) throws CatalogException {
-    return Collections.emptyList();
+    return Lists.newArrayList(partitionFunctions.keySet());
   }
 
   @Override
   public CatalogFunction getFunction(ObjectPath functionPath) throws FunctionNotExistException, CatalogException {
-    throw new FunctionNotExistException(getName(), functionPath);
+    CatalogFunction catalogFunction = partitionFunctions.get(functionPath.getObjectName());

Review Comment:
   > Are there any situations where this might get used for other reasons than just partition UDFs? Like accessing some other UDF that the user might have registered?
   
   Here, we mainly use these udfs to create, modify, and delete partitionSpec through flink.
   
   > From the existing code, it looks fine. But for my own understanding, I’m still wondering if it’s possible for a user to register their own UDF that would be associated with this Catalog, and if so are we currently making it not possible to do that?
   
   Currently, it is not supported for users to register UDF under the flink catalog. But in the next step (just next PR), I will add this feature.
   
   But after we support users to add UDFs, I still want to add these two functions, so that users do not need to manually add them again when they want to query related partitions, I would like to provide them by default. What do you think about this?



-- 
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@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org