[GitHub] [flink] fsk119 commented on a diff in pull request #22777: [FLINK-32332][sql-gateway] Show jars being used by function

["fsk119 (via GitHub)" <gi...@apache.org>]

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


##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/command/ShowJarsOperation.java:
##########
@@ -34,10 +48,53 @@ public String asSummaryString() {
 
     @Override
     public TableResultInternal execute(Context ctx) {
-        String[] jars =
-                ctx.getResourceManager().getResources().keySet().stream()
+        ResourceManager resourceManager = ctx.getResourceManager();
+        FunctionCatalog functionCatalog = ctx.getFunctionCatalog();
+        CatalogManager catalogManager = ctx.getCatalogManager();
+        Set<String> jars = getJars(resourceManager, functionCatalog, catalogManager);
+
+        return buildStringArrayResult("jars", jars.toArray(new String[0]));
+    }
+
+    public static Set<String> getJars(
+            ResourceManager resourceManager,
+            FunctionCatalog functionCatalog,
+            CatalogManager catalogManager) {
+        Set<String> jars =
+                resourceManager.getResources().keySet().stream()
                         .map(ResourceUri::getUri)
-                        .toArray(String[]::new);
-        return buildStringArrayResult("jars", jars);
+                        .collect(Collectors.toSet());
+        Set<FunctionIdentifier> identifiers =
+                functionCatalog.getUserDefinedFunctions(
+                        catalogManager.getCurrentCatalog(), catalogManager.getCurrentDatabase());
+        Catalog catalog = catalogManager.getCatalog(catalogManager.getCurrentCatalog()).get();
+        for (FunctionIdentifier identifier : identifiers) {

Review Comment:
   It seems it's a little heavy to check the catalog function one by one. I think it's better we register the resource URL into the ResourceManager. But I think we can leave this to the future work.



##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/SingleSessionManager.java:
##########
@@ -179,6 +186,12 @@ protected ResultFetcher callRemoveJar(OperationHandle operationHandle, String ja
                                         .getUserClassLoader())
                         .removeURL(jarURL);
             }
+            if (jars.contains(jarPath)) {
+                throw new UnsupportedOperationException(
+                        String.format(
+                                "Can not delete jar file [%s] while it is being used by function.",
+                                jarPath));
+            }

Review Comment:
   The jar list that is returned by the `SHOW JAR` syntax also contains the connector jars. It's a little hard to check if the jar is a connector jar or a function jar. If it is a connector jar, I think we can not check whether we have instances using this jar.



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