davidradl commented on code in PR #25810:
URL: https://github.com/apache/flink/pull/25810#discussion_r1894119512
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/TableEnvironment.java:
##########
@@ -1026,20 +1026,86 @@ void createTemporarySystemFunction(
* <p>If a permanent table with a given path exists, it will be used from
now on for any queries
* that reference this path.
*
+ * @param path The given path under which the temporary table will be
dropped. See also the
+ * {@link TableEnvironment} class description for the format of the
path.
* @return true if a table existed in the given path and was removed
*/
boolean dropTemporaryTable(String path);
+ /**
+ * Drops a table registered in the given path.
+ *
+ * <p>This method can only drop permanent objects. 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 #dropTemporaryTable}.
+ *
+ * <p>Compared to SQL, this method will not throw an error if the table
does not exist. Use
+ * {@link #dropTable(java.lang.String, boolean)} to change the default
behavior.
+ *
+ * @param path The given path under which the table will be dropped. See
also the {@link
+ * TableEnvironment} class description for the format of the path.
+ * @return true if a table existed in the given path and was removed
+ */
+ boolean dropTable(String path);
+
+ /**
+ * Drops a table registered in the given path.
+ *
+ * <p>This method can only drop permanent objects. 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 #dropTemporaryTable}.
+ *
+ * @param path The given path under which the given table will be dropped.
See also the {@link
+ * TableEnvironment} class description for the format of the path.
+ * @param ignoreIfNotExists whether to ignore if table does not exist.
+ * @return true if a table existed in the given path and was removed,
throws {@link
Review Comment:
Can we add when false is returned, so it is explicit , maybe add "return
false if the table was not dropped, because the table does not exist and
ignoreIfNotExists is true."
In Catalog manager we say:
" * @return true if table existed in the given path and was dropped, false
if table didn't exist
* in the given path and ignoreIfNotExists was true."
*
I think the javadoc should be the same in each case, I would suggest
that the return only documents when it returns true and false - like
CatalogManager does.
I think if we want to mention the ValidationException this should be in the
body of the javadoc, or as an @Exception if we want to declare the
ValidationException on the method. Though I see it is mentioned in the @param
ignoreIfNotExists javadoc in some of the methods - but not consistently.
--
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
