imply-cheddar commented on code in PR #16708:
URL: https://github.com/apache/druid/pull/16708#discussion_r1705837069
##########
processing/src/main/java/org/apache/druid/query/rowsandcols/concrete/ColumnHolderRACColumn.java:
##########
@@ -91,7 +91,7 @@ public int numRows()
public boolean isNull(int rowNum)
{
offset.set(rowNum);
- return valueSelector.isNull();
+ return valueSelector.getObject() == null;
Review Comment:
The semantics as defined here is that a non-parseable String, when read from
`getLong` is `0` instead of `null`. If something wants that to be `null`, it
needs to do its own parsing/handling. It is entirely possible to, if you have
a String object, use the `getObject()` form and handle it yourself.
We could make this logic be generically accessible by having a
`LongAccessor` interface and ask the column to become one of those when it
wants to force-cast to `Long`. What I'm trying to say is that I understand the
semantics you are looking for and I think that it's totally valid, I don't
think it has to be delivered from this set of `getters` which just exist to
provide primitive values in the simplest possible way. That said, perhaps if
we had the type-specific accessors we would end up not needing these generic
`get` methods and then life would just be better. Either way, it's something
that can be adjusted in another PR.
--
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]
