tobixdev commented on code in PR #18552:
URL: https://github.com/apache/datafusion/pull/18552#discussion_r2617264949
##########
datafusion/common/src/metadata.rs:
##########
@@ -192,6 +195,7 @@ impl FieldMetadata {
pub fn new_empty() -> Self {
Self {
inner: Arc::new(BTreeMap::new()),
+ logical_type: None,
Review Comment:
This is the vehicle that carries the logical type information around.
Currently, we have the following definitions:
`Expr::Literal(ScalarValue, Option<FieldMetadata>)`
I wish that this becomes
`Expr::Literal(ScalarValue, <InsertNameHere>)` (without options)
Then the second element will have an optional metadata and always provide a
reference to a `LogicalType` (will use `NativeType` if no metadata exists).
For `<InsertNameHere>` we need a name. I think `FieldMetadata` suggests that
its only the metadata map that is wrapped. However, now we are doing more and
this should be reflected in the name. Some ideas: `MetadataAndType` /
`ExprField` / `...`. Happy for further ideas.
--
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]
