alamb commented on code in PR #10221: URL: https://github.com/apache/datafusion/pull/10221#discussion_r1581375417
########## datafusion/expr/src/type_coercion/binary.rs: ########## @@ -638,22 +638,26 @@ fn dictionary_coercion( preserve_dictionaries: bool, ) -> Option<DataType> { use arrow::datatypes::DataType::*; + let maybe_preserve_dictionaries = + |index_type: &Box<DataType>, value_type: DataType| -> DataType { + if preserve_dictionaries { + Dictionary(index_type.clone(), Box::new(value_type)) + } else { + value_type + } + }; match (lhs_type, rhs_type) { ( Dictionary(_lhs_index_type, lhs_value_type), Dictionary(_rhs_index_type, rhs_value_type), ) => comparison_coercion(lhs_value_type, rhs_value_type), Review Comment: FWIW I tried the example from @jayzhan211 on main and it doesn't put a cast on the filter -- thus I agree this PR would be a regression if merged as is. I will dismiss my review ``` DataFusion CLI v37.1.0 > create table test as values ('row1', arrow_cast(1, 'Utf8')), ('row2', arrow_cast(2, 'Utf8')), ('row3', arrow_cast(3, 'Utf8')) ; 0 row(s) fetched. Elapsed 0.050 seconds. > explain SELECT * from test where column2 = arrow_cast(2, 'Dictionary(Int32, Int64)'); +---------------+---------------------------------------------------+ | plan_type | plan | +---------------+---------------------------------------------------+ | logical_plan | Filter: test.column2 = Utf8("2") | | | TableScan: test projection=[column1, column2] | | physical_plan | CoalesceBatchesExec: target_batch_size=8192 | | | FilterExec: column2@1 = 2 | | | MemoryExec: partitions=1, partition_sizes=[1] | | | | +---------------+---------------------------------------------------+ 2 row(s) fetched. Elapsed 0.053 seconds. ``` -- 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