[GitHub] [flink] wuchong commented on a change in pull request #17438: [FLINK-24389] [Table SQL / API] Fix NPE in CatalogTableImpl#getDescription

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

wuchong commented on a change in pull request #17438:
URL: https://github.com/apache/flink/pull/17438#discussion_r725427333



##########
File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/AbstractCatalogTable.java
##########
@@ -37,18 +39,18 @@
     // Properties of the table
     private final Map<String, String> options;
     // Comment of the table
-    private final String comment;
+    @Nullable private final String comment;
 
     public AbstractCatalogTable(
-            TableSchema tableSchema, Map<String, String> options, String comment) {
+            TableSchema tableSchema, Map<String, String> options, @Nullable String comment) {
         this(tableSchema, new ArrayList<>(), options, comment);
     }
 
     public AbstractCatalogTable(
             TableSchema tableSchema,
             List<String> partitionKeys,
             Map<String, String> options,
-            String comment) {
+            @Nullable String comment) {

Review comment:
       According to the Javadoc of `getComment()`, it doesn't say the return value can be null. There is no `@Nullable` annotation on the return type as well. The default implementation of `CatalogTable#getComment` and `CatalogView#getComment` (`DefaultCatalogTable`, `DefaultCatalogView`) returns empty string instead of null. 
   
   Therefore, would be better to return `comment != null ? comment : ""` in `AbstractCatalogTable#getComment` as well. 




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