Skip to content

fix: preserve NULL semantics when simplifying col ~ '.*' - #24380

Merged
viirya merged 1 commit into
apache:mainfrom
viirya:simplify-regex-anychar-null
Aug 15, 2026
Merged

fix: preserve NULL semantics when simplifying col ~ '.*'#24380
viirya merged 1 commit into
apache:mainfrom
viirya:simplify-regex-anychar-null

Conversation

@viirya

@viirya viirya commented Aug 14, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

Rationale for this change

simplify_regex_expr rewrites col ~ '.*' to col IS NOT NULL. For a NULL input that returns false, but NULL ~ '.*' is NULL under three-valued logic — so the rewrite produces wrong results in a projection context:

SELECT s, s ~ '.*' FROM (VALUES (CAST(NULL AS VARCHAR)), ('x')) t(s);
-- NULL row currently returns `false`; it should be NULL

The !~ (RegexNotMatch) branch of the same rule is already NULL-aware (col IS NULL AND NULL); only the ~ branch dropped the NULL.

What changes are included in this PR?

  • Rewrite col ~ '.*' to col IS NOT NULL OR NULLtrue for a non-NULL string, NULL for a NULL input.
  • In a WHERE filter both FALSE and NULL reject the row, so filter results are unchanged; only the plan text and projection-context values differ. Existing filter-plan expectations in simplify_expr.slt and the test_simplify_regex_special_cases unit test are updated accordingly, and a projection regression test is added.

Are these changes tested?

Yes.

  • New projection regression test in simplify_expr.slt asserting col ~ '.*' returns true/true/NULL for 'foo'/''/NULL.
  • Updated the two filter-context plan expectations (logical + physical) that previously encoded the IS NOT NULL rewrite.
  • simplify_expr.slt, the regexp/* SLTs, and the optimizer simplify unit tests all pass.

Are there any user-facing changes?

col ~ '.*' in a projection now returns NULL for a NULL input instead of false, matching SQL semantics. No API changes.

`simplify_regex_expr` rewrote `col ~ '.*'` to `col IS NOT NULL`. For a NULL
input that returns `false`, but `NULL ~ '.*'` is `NULL` under three-valued
logic, so the rewrite produces wrong results in a projection context. (The
`!~` branch was already NULL-aware; only the `~` branch dropped the NULL.)

Rewrite to `col IS NOT NULL OR NULL`, which is `true` for a non-NULL string
and `NULL` for a NULL input. In a WHERE filter this is equivalent to the old
`IS NOT NULL` (both reject the NULL row), so filter results are unchanged;
only the plan text and projection-context values differ. Existing filter-plan
expectations updated and a projection regression test added.

Closes apache#24379.

Co-authored-by: Claude Code
@github-actions github-actions Bot added optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) labels Aug 14, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.18%. Comparing base (1b67f2e) to head (c4ecaef).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24380      +/-   ##
==========================================
- Coverage   81.19%   81.18%   -0.01%     
==========================================
  Files        1110     1110              
  Lines      388616   388740     +124     
  Branches   388616   388740     +124     
==========================================
+ Hits       315529   315617      +88     
- Misses      54506    54535      +29     
- Partials    18581    18588       +7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @viirya

@viirya

viirya commented Aug 15, 2026

Copy link
Copy Markdown
Member Author

Thank you @alamb for review!

@viirya
viirya added this pull request to the merge queue Aug 15, 2026
Merged via the queue into apache:main with commit 16b08db Aug 15, 2026
40 checks passed
@viirya
viirya deleted the simplify-regex-anychar-null branch August 15, 2026 16:20
imtherealnaska pushed a commit to imtherealnaska/datafusion that referenced this pull request Aug 16, 2026
## Which issue does this PR close?

- Closes apache#24379.

## Rationale for this change

`simplify_regex_expr` rewrites `col ~ '.*'` to `col IS NOT NULL`. For a
NULL input that returns `false`, but `NULL ~ '.*'` is `NULL` under
three-valued logic — so the rewrite produces wrong results in a
projection context:

```sql
SELECT s, s ~ '.*' FROM (VALUES (CAST(NULL AS VARCHAR)), ('x')) t(s);
-- NULL row currently returns `false`; it should be NULL
```

The `!~` (`RegexNotMatch`) branch of the same rule is already NULL-aware
(`col IS NULL AND NULL`); only the `~` branch dropped the NULL.

## What changes are included in this PR?

- Rewrite `col ~ '.*'` to `col IS NOT NULL OR NULL` — `true` for a
non-NULL string, `NULL` for a NULL input.
- In a WHERE filter both FALSE and NULL reject the row, so filter
*results* are unchanged; only the plan text and projection-context
values differ. Existing filter-plan expectations in `simplify_expr.slt`
and the `test_simplify_regex_special_cases` unit test are updated
accordingly, and a projection regression test is added.

## Are these changes tested?

Yes.

- New projection regression test in `simplify_expr.slt` asserting `col ~
'.*'` returns `true`/`true`/`NULL` for `'foo'`/`''`/`NULL`.
- Updated the two filter-context plan expectations (logical + physical)
that previously encoded the `IS NOT NULL` rewrite.
- `simplify_expr.slt`, the `regexp/*` SLTs, and the optimizer simplify
unit tests all pass.

## Are there any user-facing changes?

`col ~ '.*'` in a projection now returns `NULL` for a NULL input instead
of `false`, matching SQL semantics. No API changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expression simplification of col ~ '.*' to col IS NOT NULL drops NULL semantics

3 participants