timsaucer commented on code in PR #24670:
URL: https://github.com/apache/datafusion/pull/24670#discussion_r3896905406


##########
datafusion/proto-models/src/generated/prost.rs:
##########
@@ -1178,6 +1178,8 @@ pub struct CastNode {
     >,
     #[prost(bool, optional, tag = "4")]
     pub nullable: ::core::option::Option<bool>,
+    #[prost(message, optional, tag = "5")]
+    pub target_field: ::core::option::Option<super::datafusion_common::Field>,

Review Comment:
   This and other places in this file are adding `pub` fields also making this 
a breaking change.



##########
datafusion/expr/src/expr.rs:
##########
@@ -984,26 +983,76 @@ pub enum GetFieldAccess {
     },
 }
 
+/// Target of a cast expression.
+///
+/// A type-only target inherits metadata from the source expression. An 
explicit
+/// field supplies its own metadata, including an empty map that clears source
+/// metadata.
+#[derive(Clone, PartialEq, Eq, PartialOrd, Hash, Debug)]
+pub enum CastTarget {
+    /// Cast to a data type while inheriting source metadata.
+    DataType(DataType),
+    /// Cast to an explicit field, including its metadata policy.
+    Field(FieldRef),
+}
+
+impl CastTarget {
+    /// Create a target that inherits source metadata.
+    pub fn type_only(data_type: DataType) -> Self {
+        Self::DataType(data_type)
+    }
+
+    /// Create an explicit field target.
+    pub fn explicit(field: FieldRef) -> Self {
+        Self::Field(field)
+    }
+
+    /// Return the target data type.
+    pub fn data_type(&self) -> &DataType {
+        match self {
+            Self::DataType(data_type) => data_type,
+            Self::Field(field) => field.data_type(),
+        }
+    }
+
+    /// Return the explicit target field, or `None` for a type-only target.
+    pub fn explicit_field(&self) -> Option<&FieldRef> {
+        match self {
+            Self::DataType(_) => None,
+            Self::Field(field) => Some(field),
+        }
+    }
+
+    /// Return explicit target metadata, or `None` when metadata is inherited.
+    pub fn metadata(&self) -> Option<&std::collections::HashMap<String, 
String>> {
+        self.explicit_field().map(|field| field.metadata())
+    }
+}
+
 /// Cast expression
 #[derive(Clone, PartialEq, Eq, PartialOrd, Hash, Debug)]
 pub struct Cast {
     /// The expression being cast
     pub expr: Box<Expr>,
-    /// The `DataType` the expression will yield
-    pub field: FieldRef,
+    /// The target type and metadata policy.
+    pub field: CastTarget,

Review Comment:
   This is a breaking change of a public type, so if we go down this route this 
PR will not be eligible for a backport to 55. 
https://datafusion.apache.org/contributor-guide/release_management.html#backport-criteria



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

Reply via email to