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]