twalthr commented on code in PR #25810: URL: https://github.com/apache/flink/pull/25810#discussion_r1891354425
########## flink-python/pyflink/table/table_environment.py: ########## @@ -657,19 +657,50 @@ def drop_temporary_table(self, table_path: str) -> bool: """ return self._j_tenv.dropTemporaryTable(table_path) + def drop_table(self, table_path: str) -> bool: + """ + Drops a table registered in the given path. + + Temporary objects can shadow permanent ones. If a temporary object exists in a given path, + make sure to drop the temporary object first using :func:`drop_temporary_view`. Review Comment: ```suggestion make sure to drop the temporary object first using :func:`drop_temporary_table`. ``` ########## flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/TableEnvironmentImpl.java: ########## @@ -649,6 +649,13 @@ public boolean dropTemporaryTable(String path) { } } + @Override + public boolean dropTable(String path) { + UnresolvedIdentifier unresolvedIdentifier = getParser().parseIdentifier(path); + ObjectIdentifier identifier = catalogManager.qualifyIdentifier(unresolvedIdentifier); + return catalogManager.dropTable(identifier, true); Review Comment: just to clarify: this is different from SQL right? `DROP TABLE` in SQL will use `ifNotExists=false`. If yes we should also document this in the JavaDoc to avoid confusion. ``` Compared to SQL, this method will not throw an error if the table does not exist. Use {@link #dropTable(.., ignoreIfExists)} to change the default behavior. ``` ########## flink-python/pyflink/table/table_environment.py: ########## @@ -657,19 +657,50 @@ def drop_temporary_table(self, table_path: str) -> bool: """ return self._j_tenv.dropTemporaryTable(table_path) + def drop_table(self, table_path: str) -> bool: + """ + Drops a table registered in the given path. + + Temporary objects can shadow permanent ones. If a temporary object exists in a given path, + make sure to drop the temporary object first using :func:`drop_temporary_view`. + This method can only drop permanent objects. + + :param table_path: The path of the registered table. + :return: True if a table existed in the given path and was removed. + + .. versionadded:: 2.0.0 + """ + return self._j_tenv.dropTable(table_path) + def drop_temporary_view(self, view_path: str) -> bool: """ Drops a temporary view registered in the given path. If a permanent table or view with a given path exists, it will be used from now on for any queries that reference this path. + :param view_path: The path of the registered temporary view. :return: True if a view existed in the given path and was removed. .. versionadded:: 1.10.0 """ return self._j_tenv.dropTemporaryView(view_path) + def drop_view(self, view_path: str) -> bool: + """ + Drops a view registered in the given path. + + Temporary objects can shadow permanent ones. If a temporary object exists in a given path, + make sure to drop the temporary object first using :func:`drop_temporary_table`. Review Comment: ```suggestion make sure to drop the temporary object first using :func:`drop_temporary_view`. ``` ########## flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/TableEnvironment.java: ########## @@ -1030,6 +1030,17 @@ void createTemporarySystemFunction( */ boolean dropTemporaryTable(String path); + /** + * Drops a table registered in the given path. Review Comment: ```suggestion * Drops a table registered in the given path. Since views are considered virtual tables, it can also drop a registered view. ``` Is that correct? If yes, this sentence might avoid confusion. ########## flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/TableEnvironmentImpl.java: ########## @@ -649,6 +649,13 @@ public boolean dropTemporaryTable(String path) { } } + @Override + public boolean dropTable(String path) { + UnresolvedIdentifier unresolvedIdentifier = getParser().parseIdentifier(path); + ObjectIdentifier identifier = catalogManager.qualifyIdentifier(unresolvedIdentifier); + return catalogManager.dropTable(identifier, true); Review Comment: Same for views -- 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-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org