LakshSingla commented on code in PR #15869:
URL: https://github.com/apache/druid/pull/15869#discussion_r1482837027


##########
sql/src/main/java/org/apache/druid/sql/http/ResultFormat.java:
##########
@@ -133,7 +134,10 @@ public interface Writer extends Closeable
 
     void writeHeader(RelDataType rowType, boolean includeTypes, boolean 
includeSqlTypes) throws IOException;
 
-    void writeHeaderFromRowSignature(RowSignature rowSignature, boolean 
includeTypes) throws IOException;
+    default void writeHeaderFromRowSignature(RowSignature rowSignature, 
boolean includeTypes) throws IOException

Review Comment:
   The interface isn't marked as `PublicApi` or `ExtensionPoint`. Is there any 
reason, we want to add this default behavior on account of it being a no-op?
   
   The callers of the method would call this method indiscriminately since 
there's no check like ` boolean canWriteHeaderFromRowSignature()`, which would 
lead to a runtime error, instead of a compilation failure. Any extension or 
class modifying it should fail to compile, and after consideration, be 
no-op/throw exception/add implementation, depending on the implementation; than 
compiling, and throwing at runtime. (Since there's no flag that the callers can 
invoke to see if the caller can call the method safely or not)



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to