alamb commented on code in PR #16317:
URL: https://github.com/apache/datafusion/pull/16317#discussion_r2133768464
##########
datafusion/expr/src/expr_rewriter/mod.rs:
##########
@@ -390,11 +390,7 @@ mod test {
} else {
utf8_val
};
- Ok(Transformed::yes(lit_with_metadata(
- utf8_val,
- metadata
- .map(|m| m.into_iter().collect::<HashMap<String,
String>>()),
- )))
+ Ok(Transformed::yes(lit_with_metadata(utf8_val, metadata)))
Review Comment:
this is a pretty good example of the kind of simplification this PR allows
(don't have to translate back/forth between HashMap / BTreeMap in various
places)
##########
datafusion/expr/src/expr.rs:
##########
@@ -284,8 +284,8 @@ pub enum Expr {
Column(Column),
/// A named reference to a variable in a registry.
ScalarVariable(DataType, Vec<String>),
- /// A constant value along with associated metadata
- Literal(ScalarValue, Option<BTreeMap<String, String>>),
+ /// A constant value along with associated [`FieldMetadata`].
+ Literal(ScalarValue, Option<FieldMetadata>),
Review Comment:
The point of this PR is to put all metadata handling in a Struct to make it
easier to work with (and more efficient)
##########
datafusion/physical-expr/src/planner.rs:
##########
@@ -114,22 +115,11 @@ pub fn create_physical_expr(
match e {
Expr::Alias(Alias { expr, metadata, .. }) => {
if let Expr::Literal(v, prior_metadata) = expr.as_ref() {
- let mut new_metadata = prior_metadata
- .as_ref()
- .map(|m| {
- m.iter()
- .map(|(k, v)| (k.clone(), v.clone()))
- .collect::<HashMap<String, String>>()
- })
- .unwrap_or_default();
- if let Some(metadata) = metadata {
- new_metadata.extend(metadata.clone());
- }
- let new_metadata = match new_metadata.is_empty() {
- true => None,
- false => Some(new_metadata),
- };
-
+ let metadata = metadata.as_ref().map(|m|
FieldMetadata::from(m.clone()));
Review Comment:
I remove this clone in a follow on PR:
- https://github.com/apache/datafusion/pull/16320
--
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
