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

Reply via email to