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

Reply via email to