Skip to content

Fix lazy quantifier priority handling in Glushkov regex engine - #23381

Merged
rapids-bot[bot] merged 2 commits into
rapidsai:release/26.08from
davidwendt:lazy-glushkov-fix
Jul 21, 2026
Merged

Fix lazy quantifier priority handling in Glushkov regex engine#23381
rapids-bot[bot] merged 2 commits into
rapidsai:release/26.08from
davidwendt:lazy-glushkov-fix

Conversation

@davidwendt

Copy link
Copy Markdown
Contributor

Description

Closes #23287.

The Glushkov eligibility checker previously rejected an ACCEPT item only when it appeared before the first character-consuming frontier item. This missed Thompson-priority frontiers such as [LF, ACCEPT, CR-repeat], where a successful accept has higher priority than a later continuation. Flattening that ordering into Glushkov bit positions caused a reluctant \r+? delimiter to consume a second \r, producing greedy behavior in split_record_re.

This PR:

  • rejects a Glushkov frontier whenever an ACCEPT item is followed by a later CHAR_POS, conservatively falling back to the Thompson engine;
  • preserves safe frontiers that end in ACCEPT;
  • adds StringsSplitTest.SplitRecordRegexLazyQuantifier to verify the delimiter length and resulting split records.

The change affects only patterns whose Thompson-priority ordering cannot be represented faithfully by the Glushkov fast path. Supported patterns continue to use Glushkov.

Validation

  • Focused StringsSplitTest.SplitRecordRegexLazyQuantifier: 1/1 passed.
  • Focused regression with LIBCUDF_DISABLE_GLUSHKOV=1: 1/1 passed.
  • Full STRINGS_TEST: 540/540 passed.
  • Clean local spark-rapids-jni package using this cuDF checkout: BUILD SUCCESS; a second same-toolchain rebuild also completed successfully.
  • NVIDIA/cudf-spark, Scala 2.13 / Spark 4.0.1, RegularExpressionTranspilerSuite: 97 succeeded, 0 failed, 6 pre-existing canceled tests; Maven BUILD SUCCESS. The original string split fuzz - anchor focused failure passed.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@davidwendt davidwendt self-assigned this Jul 21, 2026
@davidwendt davidwendt added the bug Something isn't working label Jul 21, 2026
@davidwendt
davidwendt requested a review from a team as a code owner July 21, 2026 21:27
@davidwendt davidwendt added the 3 - Ready for Review Ready for review by team label Jul 21, 2026
@davidwendt davidwendt added libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Jul 21, 2026
@davidwendt davidwendt moved this to Burndown in libcudf Jul 21, 2026
@davidwendt
davidwendt requested a review from igorpeshansky July 21, 2026 21:29
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: cfe76e2b-417e-4879-be75-57c99d9bf592

📥 Commits

Reviewing files that changed from the base of the PR and between 56f5503 and 79422b6.

📒 Files selected for processing (3)
  • cpp/src/strings/regex/glushkov_regcomp.cpp
  • cpp/src/strings/regex/glushkov_regcomp.hpp
  • cpp/tests/strings/split_tests.cpp

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved regular expression priority handling for patterns where acceptance occurs before a later character match.
    • Added clearer documentation for patterns that cannot be represented faithfully by the regex engine.
  • Tests

    • Added coverage for lazy versus greedy quantifiers when splitting records with regular expressions.

Walkthrough

The Glushkov frontier eligibility check now rejects ACCEPT entries followed by later character positions. Its failure condition is documented, and split-record regex tests cover differing lazy and greedy quantifier results.

Changes

Glushkov lazy quantifier handling

Layer / File(s) Summary
Frontier conflict detection
cpp/src/strings/regex/glushkov_regcomp.cpp, cpp/src/strings/regex/glushkov_regcomp.hpp
Rule 1 detects an ACCEPT before a later CHAR position, and the compiler documentation records this frontier incompatibility as a null-return condition.
Split-record regression coverage
cpp/tests/strings/split_tests.cpp
Adds lazy-versus-greedy quantifier assertions for carriage-return input and updates the SPDX copyright year.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

  • rapidsai/cudf issue 23290 — Covers the same lazy-quantifier regression and Glushkov frontier conflict detection changes.

Possibly related PRs

  • rapidsai/cudf#21936 — Introduced the Glushkov fast path and its frontier conflict-detection machinery.

Suggested labels: improvement

Suggested reviewers: bdice, lamarrr

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes an unrelated 2019-2026 copyright year update in the test file. Remove the standalone copyright year change or split it into a separate maintenance PR.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main fix: lazy quantifier priority handling in the Glushkov regex engine.
Description check ✅ Passed The description matches the PR and explains the bug, fix, and validation.
Linked Issues check ✅ Passed The code and test changes implement the linked bug fix, fallback behavior, and regression coverage described in #23287.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@igorpeshansky igorpeshansky 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.

LGTM :shipit:

@davidwendt

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit 9a6de59 into rapidsai:release/26.08 Jul 21, 2026
256 of 261 checks passed
@davidwendt
davidwendt deleted the lazy-glushkov-fix branch July 22, 2026 03:16
@GregoryKimball GregoryKimball moved this from Burndown to Landed in libcudf Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Ready for Review Ready for review by team bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

Status: Landed

Development

Successfully merging this pull request may close these issues.

6 participants