amogh-jahagirdar commented on code in PR #6598:
URL: https://github.com/apache/iceberg/pull/6598#discussion_r1071424411


##########
api/src/main/java/org/apache/iceberg/view/SQLViewRepresentation.java:
##########
@@ -36,17 +38,21 @@ default Type type() {
   String dialect();
 
   /** The default catalog when the view is created. */
+  @Nullable
   String defaultCatalog();
 
   /** The default namespace when the view is created. */
+  @Nullable
   Namespace defaultNamespace();
 
-  /** The query output schema at version create time, without aliases. */
-  Schema schema();
+  /** The query output schema id at version create time */
+  int schemaId();
 
   /** The view field comments. */
+  @Nullable

Review Comment:
   Sure, I've updated so that we just won't serialize if null or empty for 
field aliases and field comments but at the API level we return an empty list. 
I think default catalog and namespace should still be nullable. 
   
   One benefit is that we just avoid callers having to do null checks (and they 
would have to do empty list checks anyways) so it simplifies for callers. 
   
   @nastra I thought that this was the case, but it seems like for collections, 
Immutables will initialize an empty collection in the builder state. So it 
seems not required to explicitly set those (if it's never set, it ends up being 
an empty collection). let me know if you have any concern on not setting these 
explicitly! 



-- 
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...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to