jonahgao commented on code in PR #14223:
URL: https://github.com/apache/datafusion/pull/14223#discussion_r1923910753
##########
datafusion/expr-common/src/type_coercion/binary.rs:
##########
@@ -777,29 +777,20 @@ pub fn binary_numeric_coercion(
(_, Float32) | (Float32, _) => Some(Float32),
// The following match arms encode the following logic: Given the two
// integral types, we choose the narrowest possible integral type that
- // accommodates all values of both types. Note that some information
- // loss is inevitable when we have a signed type and a `UInt64`, in
- // which case we use `Int64`;i.e. the widest signed integral type.
-
- // TODO: For i64 and u64, we can use decimal or float64
- // Postgres has no unsigned type :(
- // DuckDB v.0.10.0 has double (double precision floating-point number
(8 bytes))
- // for largest signed (signed sixteen-byte integer) and unsigned
integer (unsigned sixteen-byte integer)
+ // accommodates all values of both types. Note that to avoid
information
+ // loss when combining UInt64 with signed integers we use
Decimal128(20, 0).
+ (Decimal128(20, 0), _)
+ | (_, Decimal128(20, 0))
Review Comment:
I rechecked it, and `decimal_coercion` already covers them in
https://github.com/apache/datafusion/blob/2f283277d523404401b206e5b98819a0126341b8/datafusion/expr-common/src/type_coercion/binary.rs#L932-L940
Although it doesn't handle unsigned integer types, we can supplement it
there, maybe as a follow-up PR.
--
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
