Jimexist commented on pull request #1199: URL: https://github.com/apache/arrow-datafusion/pull/1199#issuecomment-955121817
> THanks @Jimexist ! > > 1. I think this is a nice code cleanup PR > 2. I am not sure it helps directly with null constant support, however. > > There is already a `ScalarValue::is_null()` function that returns true if any particular instance of `ScalarValue` represents Null. The core issue I encountered was that `ScalarValue::Null` is "typed" and we still need to teach DataFusion it can cast such values to/from any other types. > > This PR retains the "a Null ScalarValue has a type" property, so I am not sure this PR will make remedying that easier (or harder) Thanks for the comment @alamb, I realize I haven't been clear in my intention for this pull request. I intended to address those null related issues, by introducing a `Null` leg in the `ScalarValue` enum, then that null value specifically represents an untyped null. But I also added `Null(DataType)` because `DataType::Null` itself can represent null type information, so in this case all the other option wrapper in other legs of `ScalarValue` can be removed. But after so many changes to the current code, I started to wonder if I should stop at `Null` instead of `Null(DataType)` or even `Null(Box<DataType>)` because the changes to the code is not worth it. -- 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]
