jayzhan211 commented on code in PR #12536:
URL: https://github.com/apache/datafusion/pull/12536#discussion_r1779319730
##########
datafusion/expr-common/src/columnar_value.rs:
##########
@@ -89,7 +91,7 @@ pub enum ColumnarValue {
/// Array of values
Array(ArrayRef),
/// A single value
- Scalar(ScalarValue),
+ Scalar(Scalar),
Review Comment:
> > we can get the LogicalType from it if each DataType has at most one
LogicalType
>
> We should not assume this to be the case. I suppose #12644 may convey why,
but even if we don't do Extension Types, this assumption will be very limiting.
I'm still not convinced why my assumption is limited. I don't think it makes
sense to have more than 1 logical type for the same DataType. Let alone arrow's
extension type, it is more easier to map them to single logical type like
`LogicalType::UserDefined()` and carry to Datafusion. If we have two logical
type that has the same physical type, which means we should standardize them
into one type. Consider that in Datafusion we provide the native type like
rust's native type or arrow's builtin type. Other user-defined type and
extension types should be able to built on top of those native/builtin type.
--
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
