alamb commented on code in PR #14270: URL: https://github.com/apache/datafusion/pull/14270#discussion_r1929542389
########## datafusion/sqllogictest/test_files/operator.slt: ########## @@ -110,5 +226,139 @@ from numeric_types; ---- Int8 Int16 Int32 Int64 UInt8 UInt16 UInt32 UInt64 Float32 Float64 Decimal128(11, 6) +# Divide with literal integer +query TTTTTTTTTTT +select + arrow_typeof(int8 / 2), + arrow_typeof(int16 / 2), + arrow_typeof(int32 / 2), + arrow_typeof(int64 / 2), + arrow_typeof(uint8 / 2), + arrow_typeof(uint16 / 2), + arrow_typeof(uint32 / 2), + arrow_typeof(uint64 / 2), + arrow_typeof(float32 / 2), + arrow_typeof(float64 / 2), + arrow_typeof(decimal / 2) +from numeric_types; +---- +Int64 Int64 Int64 Int64 Int64 Int64 Int64 Int64 Float32 Float64 Decimal128(9, 6) + +# Divide with literal decimal +query TTTTTTTTTTT +select + arrow_typeof(int8 / 2.0), + arrow_typeof(int16 / 2.0), + arrow_typeof(int32 / 2.0), + arrow_typeof(int64 / 2.0), + arrow_typeof(uint8 / 2.0), + arrow_typeof(uint16 / 2.0), + arrow_typeof(uint32 / 2.0), + arrow_typeof(uint64 / 2.0), + arrow_typeof(float32 / 2.0), + arrow_typeof(float64 / 2.0), + arrow_typeof(decimal / 2.0) +from numeric_types; +---- +Float64 Float64 Float64 Float64 Float64 Float64 Float64 Float64 Float64 Float64 Float64 + +############### +# Test for comparison with constants uses efficient types +# Expect the physical plans to compare with constants of the same type +# should have no casts of the column to a different type + +statement ok +set datafusion.explain.physical_plan_only = true; + +############### Less Than ############### + +## < positive integer (expect no casts) +query TT +EXPLAIN SELECT * FROM numeric_types +WHERE int64 < 5 AND uint64 < 5 AND float64 < 5 AND decimal < 5; +---- +physical_plan +01)CoalesceBatchesExec: target_batch_size=8192 +02)--FilterExec: int64@3 < 5 AND uint64@7 < 5 AND float64@9 < 5 AND decimal@10 < Some(500),5,2 Review Comment: This test shows that with a positive integer comparison does not result in any casting of the column ########## datafusion/sqllogictest/test_files/operator.slt: ########## @@ -110,5 +226,139 @@ from numeric_types; ---- Int8 Int16 Int32 Int64 UInt8 UInt16 UInt32 UInt64 Float32 Float64 Decimal128(11, 6) +# Divide with literal integer +query TTTTTTTTTTT +select + arrow_typeof(int8 / 2), + arrow_typeof(int16 / 2), + arrow_typeof(int32 / 2), + arrow_typeof(int64 / 2), + arrow_typeof(uint8 / 2), + arrow_typeof(uint16 / 2), + arrow_typeof(uint32 / 2), + arrow_typeof(uint64 / 2), + arrow_typeof(float32 / 2), + arrow_typeof(float64 / 2), + arrow_typeof(decimal / 2) +from numeric_types; +---- +Int64 Int64 Int64 Int64 Int64 Int64 Int64 Int64 Float32 Float64 Decimal128(9, 6) + +# Divide with literal decimal +query TTTTTTTTTTT +select + arrow_typeof(int8 / 2.0), + arrow_typeof(int16 / 2.0), + arrow_typeof(int32 / 2.0), + arrow_typeof(int64 / 2.0), + arrow_typeof(uint8 / 2.0), + arrow_typeof(uint16 / 2.0), + arrow_typeof(uint32 / 2.0), + arrow_typeof(uint64 / 2.0), + arrow_typeof(float32 / 2.0), + arrow_typeof(float64 / 2.0), + arrow_typeof(decimal / 2.0) +from numeric_types; +---- +Float64 Float64 Float64 Float64 Float64 Float64 Float64 Float64 Float64 Float64 Float64 + +############### +# Test for comparison with constants uses efficient types +# Expect the physical plans to compare with constants of the same type +# should have no casts of the column to a different type + +statement ok +set datafusion.explain.physical_plan_only = true; + +############### Less Than ############### + +## < positive integer (expect no casts) +query TT +EXPLAIN SELECT * FROM numeric_types +WHERE int64 < 5 AND uint64 < 5 AND float64 < 5 AND decimal < 5; +---- +physical_plan +01)CoalesceBatchesExec: target_batch_size=8192 +02)--FilterExec: int64@3 < 5 AND uint64@7 < 5 AND float64@9 < 5 AND decimal@10 < Some(500),5,2 +03)----MemoryExec: partitions=1, partition_sizes=[1] + +## < negative integer (expect no casts) +query TT +EXPLAIN SELECT * FROM numeric_types +WHERE int64 < -5 AND uint64 < -5 AND float64 < -5 AND decimal < -5; +---- +physical_plan +01)CoalesceBatchesExec: target_batch_size=8192 +02)--FilterExec: int64@3 < -5 AND CAST(uint64@7 AS Decimal128(20, 0)) < Some(-5),20,0 AND float64@9 < -5 AND decimal@10 < Some(-500),5,2 Review Comment: This test show that with a negative integer, the comparison to `int64` does not cast. However, a comparison with `-5` does cast to `Decimal128`: `CAST(uint64@7 AS Decimal128(20, 0)) < Some(-5),20,0` When I reverted the changes in https://github.com/apache/datafusion/pull/14223 locally, this is the difference (cast to Int64 rather than Decimal): ```diff [Diff] (-expected|+actual) physical_plan 01)CoalesceBatchesExec: target_batch_size=8192 - 02)--FilterExec: int64@3 < -5 AND CAST(uint64@7 AS Decimal128(20, 0)) < Some(-5),20,0 AND float64@9 < -5 AND decimal@10 < Some(-500),5,2 + 02)--FilterExec: int64@3 < -5 AND CAST(uint64@7 AS Int64) < -5 AND float64@9 < -5 AND decimal@10 < Some(-500),5,2 03)----MemoryExec: partitions=1, partition_sizes=[1] at test_files/operator.slt:286 ``` However, note that in this case the predicate will never be true anyways (a `uint64` can never be less than -5 as it is by definition always greater than zero -- something @gatesn pointed out on another ticket) -- 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
