tobixdev commented on code in PR #16053: URL: https://github.com/apache/datafusion/pull/16053#discussion_r2098428747
########## datafusion/physical-expr/src/expressions/literal.rs: ########## @@ -34,15 +36,37 @@ use datafusion_expr_common::interval_arithmetic::Interval; use datafusion_expr_common::sort_properties::{ExprProperties, SortProperties}; /// Represents a literal value -#[derive(Debug, PartialEq, Eq, Hash)] +#[derive(Debug, PartialEq, Eq)] pub struct Literal { value: ScalarValue, + metadata: Option<HashMap<String, String>>, Review Comment: Thanks! That's an interesting idea! When i experimented with logical scalars, I did not go down that path as I thought that it is very easy to miss handling metadata that only adds little information to a scalar. To elaborate a bit on that: Let's assume I have a `DataType::Utf8` field and a `len()` function that returns the length of a value. For strings, that could be the number of unicode characters. Now, given the following "implementation" of `len`: ```rust match value { ScalarValue::Utf8(value) => count_chars(value), _ => panic!("bye bye") } ``` Now I'd like to call this function with `ScalarValue::WithMetadata(my_string, LANGUAGE_INFO)`. This call will panic, as the actual scalar is hidden behind the metadata wrapper. Initially I though this is a deal-breaker, as many functions just want to work on the physical data. After all, that's how it is with arrays, as the Metadata can be simply ignored. However, in a scenario where the metadata indicates an extension type, it may be a good thing that Metadata cannot be ignored that easily. For example, the `len()` function should probably return the number of keys/entries in the top-level JSON object/array. Carrying the metadata in a separate `ScalarValue` variant then may be a good thing! Especially if there is a function that unwraps the metadata so that some functions can operate on the scalar without metadata. Hope this does make some sense. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org