gianm commented on PR #14020: URL: https://github.com/apache/druid/pull/14020#issuecomment-1497783579
> Should the null handling be done in the StringFieldWriters as well? Consider the following test case in CalciteQueryTest ([permalink](https://github.com/apache/druid/blob/d21babc5b831a90b904e499f4435cd60d168e99a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java#L8176)) which works with the native engine, however doesn't with MSQ. I didn't update the StringFieldWriter (or other writers, generally) but I did update the StringFieldReader to return `null` rather than `""` in default-value mode. That's consistent with how selectors behave on regular segments. However, this didn't help with the test `testGroupByLimitPushDownWithHavingOnLong`, at least in default-value mode. I think it's because `coerce` is turning the `null` into `""` in the native path, but not the MSQ path. I have another PR I'm working on to update that, which may unlock the ability to run the test case in MSQ. I _did_ update the test to run with MSQ in SQL-compat mode though, by adding: ``` if (NullHandling.sqlCompatible()) { msqCompatible(); } ``` -- 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]
