andygrove commented on PR #65:
URL: https://github.com/apache/datafusion-spark/pull/65#issuecomment-6823887733

   Thanks, the fix looks right to me: hoisting `with_data_type` so it covers 
the non-ANSI `unary` branch too is exactly what was missing. Two small 
non-blocking nits on the tests.
   
   1. The new tests check values but never assert the output type. A result 
with the wrong precision or scale could still print the same values. The 
`DOUBLE` cast catches that for scale 2, but not for the zero and negative scale 
cases. An `arrow_typeof` check would pin the type directly, for example:
   
   ```
   query TTTT
   SELECT arrow_typeof(negative(d32)), arrow_typeof(negative(d64)),
          arrow_typeof(negative(d128)), arrow_typeof(negative(d256))
   FROM decimal_negative_scales LIMIT 1;
   ----
   Decimal32(7, -2) Decimal64(10, 0) Decimal128(10, -2) Decimal256(42, 0)
   ```
   
   2. `decimal_negative` and `decimal_negative_scales` are only dropped in the 
ANSI section, so they stay around for the rest of the non-ANSI tests. It's 
harmless, but dropping them right after their last use in each section would 
keep the file tidier.


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