tarun11Mavani commented on code in PR #18368:
URL: https://github.com/apache/pinot/pull/18368#discussion_r3199507054
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/datasource/MapDataSource.java:
##########
@@ -32,9 +32,22 @@ public interface MapDataSource extends DataSource {
/**
* Get the Data Source representation of a single key within this map column.
+ * Only call after confirming the key exists via {@link
#containsKey(String)}.
*/
DataSource getKeyDataSource(String key);
Review Comment:
For @Nullable: I'd prefer to keep the current non-null contract backed by
NullDataSource. The callers in ProjectionBlock and ItemTransformFunction
dereference the result immediately without a null-check — NullDataSource lets
them do that safely by returning the column's default value for every doc.
If we switch to @Nullable, those two callers need null-guards, and so does
any future caller. NullDataSource gives the same semantic (absent key →
null/default for all rows) without pushing null-handling into every call site.
Happy to revisit this if you strognly feel we should add @nullable.
Let me know your thoughts.
--
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]
