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: [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]
