fix: Optimize !~ '.*' case to col IS NULL AND Boolean(NULL) instead of Eq ""

[petern48]

Which issue does this PR close?

• Closes bug: regexp simplify optimization incorrect simplifies .* pattern to Eq "" operation #20701

Rationale for this change

What changes are included in this PR?

A pre-existing optimization rule for the !~ .* (regexp not match) case rewrote the plan to Eq "", which would return empty strings as part of the result. This is incorrect and doesn't match the output without the optimization rule.

Instead, this PR rewrites the plan to simply col IS NULL AND Boolean(NULL) or, in other words, "NULL if col is NULL else false."

I've confirmed this behavior matches the result of running queries manually with the optimization rule turned off.

Are these changes tested?

Fixed expected output in tests. Added new tests for nulls

Are there any user-facing changes?

Yes, a minor bug fix. When querying s !~ .*, empty strings will no longer be included in the result which is consistent with the behavior without the optimization rule.

[alamb]

Thanks @petern48

[alamb]

I think it is only false in the context of filters (and this code can currently be used for both filters and regular expressions)

I think null !~ '.*' is actually null (not false)

I think a correct rewrite is

CASE 
  WHEN col IS NOT NULL THEN FALSE 
  ELSE NULL
END

[petern48]

ah thanks, i didn't realize this code applied to cases outside of filters. I've fixed it and updated the PR title and description.

note: the optimizer would further rewrite the CASE expr into IS NOT NULL AND NULL when I ran the tests, so I rewrote it directly to that instead

edit: should've been IS NULL AND NULL 😬

[alamb]

Thank you @petern48

[alamb]

Thanks again for working on this @petern48 -- here is a proposed PR to fix the logic in this PR

• Fix correctness petern48/datafusion#1

What do you think?

[petern48]

Appreciate the little bump to get this moving again. So sorry for the delay. I didn't mean to keep you waiting. I definitely agree with your suggestion. I was cramming for a deadline, and wanted to wait until I had more time to investigate something a bit further.

I vaguely remember just copy-pasting the IS NOT NULL part from the optimized output, so I had a small suspicion that the CASE statement was rewritten incorrectly to IS NOT NULL. Though I just manually checked it again, and it seems to optimize properly to IS NULL. Maybe I just wrote the case statement incorrectly, too 🤷. Anyway, lesson learned. I need to slow down a bit nowadays 🤦 , haha. Thanks for bearing with me.

[alamb]

No worries -- thank you so much for helping push this along (crazy corner cases are always tricky)