gianm commented on PR #14020: URL: https://github.com/apache/druid/pull/14020#issuecomment-1499549226
@LakshSingla — > To disambiguate the coerce mentioned above, I think you mean the one that is present in the NativeQueryMaker right? If so, this will also fix a few other test cases that I was seeing fail because of a type mismatch between DOUBLE and FLOAT, so I think that would be pretty cool. Yes, that's the one I mean. In a future PR I'm planning to use this same logic for MSQ results too. > Do we not require the changes in the StringFrameColumnReader for appropriate null handling as well? I see that there is something done in the getStringUtf8 method, so I might be wrong It's already there: `getStringUtf8` turns empty strings to nulls if `NullHandling.replaceWithDefault()`. > I think we should handle the nulls appropriately in the following line [(permalink)](https://github.com/apache/druid/blob/030ed911d4073843dc76a655e6fc8066b310af46/processing/src/main/java/org/apache/druid/frame/field/StringFieldReader.java#L269) as well, when the StringFieldReader has figured out that the field is a NULL_BYTE. WDYT? There's nothing special to do there, since in default-value mode, the convention is that nulls and empty strings are both returned as nulls from selectors. So the `NULL_BYTE` branch is already behaving as expected. > Unrelated to the change, while digging through that piece of code, I found the following condition ([permalink](https://github.com/apache/druid/blob/030ed911d4073843dc76a655e6fc8066b310af46/processing/src/main/java/org/apache/druid/frame/read/columnar/StringFrameColumnReader.java#L485)) It believe it's correct as-is. It's saying that we should return `null` in two cases: - `dataLength == 0 && NullHandling.replaceWithDefault()`: The frame contains an empty string, and `NullHandling.replaceWithDefault()` is true. (In this mode, by convention selectors return `null` instead of empty string. The method name is misleading; it refers to what happens _later_, in query results, when `null` are turned back into empty strings.) - `dataLength == 1 && memory.getByte(dataStart) == FrameWriterUtils.NULL_STRING_MARKER)`: The frame contains an actual null. Behavior here is the same in both modes: a `null` is returned. -- 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]
