paleolimbot commented on code in PR #16053: URL: https://github.com/apache/datafusion/pull/16053#discussion_r2103559755
########## 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: Keeping with the literial is OK as long as we are also prepared to make any other breaking changes required to pipe metadata through to ensure that it is not dropped/it is possible to implement extension logic in other places without rewriting large chunks of DataFusion code. A few places I've noted in passing are the Statistics, the physical and logical literals, casting logic, and some conversions like protobuf and to/from Python via ffi. Perhaps those places should be using a `Literal` instead of a `ScalarValue`? This is possibly a good illustration of the difference between "metadata" and extension types...an extension type scalar should not be blindly/implicitly cast to its storage type; however, for an integer that happened to have some field metadata attached to it, it would be surprising if that metadata caused an error somewhere in the engine. -- 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