twalthr commented on code in PR #25810:
URL: https://github.com/apache/flink/pull/25810#discussion_r1890482335


##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/TableEnvironment.java:
##########
@@ -1040,6 +1051,17 @@ void createTemporarySystemFunction(
      */
     boolean dropTemporaryView(String path);
 
+    /**
+     * Drops a view registered in the given path.
+     *
+     * <p>Temporary objects can shadow permanent ones. If a permanent object 
in a given path exists,

Review Comment:
   Temporary objects can shadow permanent ones. If a temporary object exists in 
a given path, make sure to drop the temporary object first using {@link 
#dropTemporaryView}. This method can only drop permanent objects.



##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/CatalogManager.java:
##########
@@ -1350,6 +1356,7 @@ private void dropTableInternal(
                     objectIdentifier,
                     ignoreIfNotExists,
                     "DropTable");
+            return true;

Review Comment:
   there is no evidence that the table existed in the catalog as the catalog 
could have also considered the ignoreIfNotExists flag. if we want to return a 
correct value, we need to check in the catalog first.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to