This optimizer transformation doesn't look right for projected columns like

create table t1(v1 int);
insert into t1 values (null);

select v1=v1 from t1;
-- Should return `NULL`, but get `False`

However it makes sense if it is only applied on filter expressions.

This optimizer transformation doesn't look right for projected columns like

create table t1(v1 int);
insert into t1 values (null);

select v1=v1 from t1;
-- Should return `NULL`, but get `False`

However it makes sense if it is only applied on filter expressions.

Thank you for review. I'll make the change

@2010YOUY01 Instead of applying transformation on filter expression, I adjusted the rule to transform x=x into x IS NOT NULL OR NULL. This preserves the behavior where NULL = NULL evaluates to NULL, which is treated as FALSE in filter expressions.

About the performance, I'm not 100% sure whether this rule worth the change, but I made this change because IS NOT NULL OR NULL was a bit better than actual comparision.

About the performance, I'm not 100% sure whether this rule worth the change, but I made this change because IS NOT NULL OR NULL was a bit better than actual comparision.

I recommend writing a simple benchmark by criterion or other tools to get more reliable results instead of measuring the CLI execution. But I agree that this should be more performant

We could also use a CASE

CASE x IS NOT NULL THEN true ELSE null END

I expect this to make a large performance difference when x is a string type (as string comparisons are fairly expensive)

Thank you for this PR @ding-young and the great reviews @2010YOUY01 and @berkaysynnada

I wrote a simple criterion bench (https://github.com/ding-young/datafusion/blob/simplify-trivial-eq-bench/datafusion/core/benches/simplify_trivial_eq.rs) and the result is as follows.
It showed performance improvement, especially for comparing long strings.

select_f32_eq_f32       time:   [366.19 µs 366.42 µs 366.69 µs]
                        change: [-79.571% -79.180% -78.787%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  2 (2.00%) high mild
  6 (6.00%) high severe

select_str_eq_str       time:   [372.53 µs 373.02 µs 373.61 µs]
                        change: [-85.333% -85.117% -84.884%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  4 (4.00%) high mild
  6 (6.00%) high severe

select_str_eq_str_long  time:   [372.49 µs 372.85 µs 373.25 µs]
                        change: [-98.351% -98.316% -98.280%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  3 (3.00%) high mild
  6 (6.00%) high severe

Plus, since x IS NOT NULL OR NULL is slightly better than CASE, I'll leave the pr as is :)

select_CASE_str_eq_str_long
                        time:   [396.36 µs 396.73 µs 397.15 µs]
Found 10 outliers among 100 measurements (10.00%)
  2 (2.00%) high mild
  8 (8.00%) high severe

Thanks for the review! Trying out the bench was also a great experience for me.

I wrote a simple criterion bench (https://github.com/ding-young/datafusion/blob/simplify-trivial-eq-bench/datafusion/core/benches/simplify_trivial_eq.rs) and the result is as follows. It showed performance improvement, especially for comparing long strings.

Thank you for sharing the results. It doesn't effect much here but next time you can exclude the create_context() part out of bench_function :)

Thanks again @ding-young 🙏