fix: Optimize !~ '.*' case to col IS NOT 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 NOT 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

[alamb]

can we please also add a test explicitly for a null input?

[alamb]

I think this case should be is_null rather than is_not_null

I think this because when left is null, The expression null !~ '.*' should also evaluate to null

However, as written (col IS NOT NULL AND NULL) will evaluate

col IS NOT NULL AND NULL
-->  FALSE AND NULL
-->  FALSE 

If the transformation is col IS NULL AND NULL 🤯 then:

col IS NULL AND NULL
-->  true AND NULL
-->  NULL 

As expected

[alamb]

Thank you @petern48

[alamb]

I think we should add a test that actually executes col !~ .* for a column that has 'foo', '', and null

Specifically like this (the ::text is needed for postgres):

-- Reproducer for regex simplification of `col !~ '.*'`
-- Input values: 'foo', '', NULL

WITH t(col) AS (
    VALUES
      ('foo'::text),
      (''::text),
      (NULL::text)
  )
  SELECT
    col,
    col !~ '.*' AS not_match_dot_star
  FROM t;

Should result in

+------+-----------------+
| a    | baseline_result |
+------+-----------------+
| foo  | false           |
|      | false           |
| NULL | NULL            |
+------+-----------------+

Here is what current datafusion does

andrewlamb@Andrews-MacBook-Pro-3:~/Software/datafusion2$ datafusion-cli
DataFusion CLI v52.1.0
> WITH t(col) AS (
    VALUES
      ('foo'::text),
      (''::text),
      (NULL::text)
  )
  SELECT
    col,
    col !~ '.*' AS not_match_dot_star
  FROM t;
+------+--------------------+
| col  | not_match_dot_star |
+------+--------------------+
| foo  | false              |
|      | true               |
| NULL | NULL               |
+------+--------------------+
3 row(s) fetched.
Elapsed 0.038 seconds.

Here is what postgres says

postgres=#   WITH t(col) AS (
    VALUES
      ('foo'::text),
      (''::text),
      (NULL::text)
  )
  SELECT
    col,
    col !~ '.*' AS not_match_dot_star
  FROM t;
 col | not_match_dot_star
-----+--------------------
 foo | f
     | f
     |
(3 rows)

I think this branch will not get the right value for col IS NULL