Repository navigation
Conversation
added 3 commits
September 25, 2026 10:15
…lidatePattern
Adds translateJavaUnicodeToRe2() to cpp/velox/utils/Common.cc (.h) which
converts Java \uXXXX Unicode escapes (exactly 4 hex digits) to RE2's
\x{XXXX} syntax. The function is idempotent: patterns already in RE2 form
pass through unchanged.
validatePattern() now calls translateJavaUnicodeToRe2() before the RE2::RE2()
compile attempt, so patterns like [\u4e00-\u9fa5] no longer cause a spurious
compile failure in the native validator.
Also adds CommonTest.cc (registered in CMakeLists.txt) with unit tests covering
translateJavaUnicodeToRe2 and the updated validatePattern behaviour.
Generated-by: Claude claude-sonnet-4-5
…pressionConverter
Adds VeloxRegexUtils (gluten-substrait) with translateJavaPatternToRe2(),
which converts Java \uXXXX Unicode escapes to RE2's \x{XXXX} form before
the pattern is written into the Substrait protobuf literal.
ExpressionConverter gains a private withRe2PatternTranslation() helper that
is wired into the pattern argument of every regex expression:
RegExpReplace, RLike, RegExpExtract, RegExpExtractAll, RegExpInStr, StringSplit.
Previously, patterns such as [\u4e00-\u9fa5] caused a RE2 compile failure
in the native validator and fell back to the JVM engine. With this change they
are translated to [\x{4e00}-\x{9fa5}] before serialisation, so both the
validator and the Velox execution engine see RE2-compatible syntax.
Patterns that contain constructs with no RE2 equivalent (lookaheads,
lookbehinds, backreferences) are detected by containsUnsupportedConstruct()
and trigger GlutenNotSupportException, preserving the existing graceful
fallback behaviour for those cases.
Generated-by: Claude claude-sonnet-4-5
… patterns FallbackSuite: the test that asserted '\u4e00' patterns cause a fallback is inverted. The pattern now runs natively after the translation fix, so the test is renamed and asserts no 'rlike due to Pattern' fallback reason is present. VeloxStringFunctionsSuite: adds two regression tests: - 'rlike with Java Unicode escape runs natively' — asserts ProjectExecTransformer (native Velox path) is used for a \uXXXX pattern. - 'rlike with lookbehind falls back to JVM' — asserts (?<=x)y still falls back gracefully, confirming unsupported-construct detection is intact. Generated-by: Claude claude-sonnet-4-5
|
Run Gluten Clickhouse CI on x86 |
1 similar comment
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
Merge two continuation lines in translateJavaUnicodeToRe2's if-condition that fit within the 120-column limit, as required by clang-format-15.
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
1 similar comment
|
Run Gluten Clickhouse CI on x86 |
withRe2PatternTranslation now rejects lookahead/lookbehind patterns at the JVM level (GlutenNotSupportException) before the native validator is reached. The resulting fallback reason no longer contains the old native string 'regexp_extract due to Pattern ...'; it contains 'uses constructs (lookahead/lookbehind/backreference)' instead. Update the assertion in FallbackSuite to accept either message so the test passes regardless of which path fires. Generated-by: Claude claude-sonnet-4-6
|
Run Gluten Clickhouse CI on x86 |
Two non-ASCII characters crept into comments in files touched by this branch: backends-velox/src/test/scala/.../FallbackSuite.scala Lines 412, 444: em-dash (U+2014) -> ASCII double-hyphen (--) backends-velox/src/main/scala/.../ColumnarPartialProjectExec.scala Line 479: ideographic full stop (U+3002) -> ASCII period (.) Gluten's Scalastyle config has the NonASCIICharacters rule enabled globally, which rejects any byte > 0x7F outside a scalastyle:off nonascii guard. Generated-by: Claude claude-sonnet-4-6
|
Run Gluten Clickhouse CI on x86 |
added 3 commits
October 1, 2026 09:36
The utility lives in gluten-substrait (shared across all backends) and its behaviour is RE2-specific, not Velox-specific. Bolt also uses RE2; ClickHouse does not call this code at all. Rename the object and file to Re2PatternUtils to reflect the actual scope, and update the one call site in ExpressionConverter.
Change 'i + 5 < pattern.size()' to 'i + 6 <= pattern.size()' so that a \uXXXX sequence ending exactly at the last character of the pattern is correctly translated instead of silently left untouched. 'i + 6 <= size' mirrors the equivalent guard in Re2PatternUtils.translateUnicodeEscapes.
…ests FallbackSuite: switch forall->exists in the nested-loop-join fallback assertion and add 'GlutenNotSupportException' as a third accepted message variant, since the exact fallback string differs across Spark versions and AQE configurations. VeloxStringFunctionsSuite: use a plain (non-interpolated) string for the regex pattern literal in the Unicode escape regression test. An s-string silently resolves \u0041 to 'A' at compile time, so the translation path was never exercised. Mixing a plain string (pattern) with an s-string (table/limit variables) keeps the \uXXXX sequences intact.
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
2 similar comments
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
1 similar comment
|
Run Gluten Clickhouse CI on x86 |
KushShriv
force-pushed
the
regex-fix-1
branch
from
October 14, 2026 05:35
d1090fa to
0533d62
Compare
|
Run Gluten Clickhouse CI on x86 |
1 similar comment
|
Run Gluten Clickhouse CI on x86 |
Contributor
Author
|
Closing this as Opened #13278 with the same changes and more. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Gluten's Velox backend uses RE2 for regex execution. RE2 does not understand Java's
\uXXXXUnicode escape syntax, so patterns such as
[\u4e00-\u9fa5](CJK Unified Ideograph range) failto compile in RE2 and cause the entire operator to fall back to the Spark JVM engine. This was a
confirmed regression tracked in
FallbackSuite.Solution
Two-phase defence-in-depth translation of
\uXXXX→\x{XXXX}:Phase 1 — Scala (primary). Before a regex pattern literal is written into the Substrait
protobuf,
ExpressionConverternow calls a newwithRe2PatternTranslation()helper. This ensuresboth the C++ validator and Velox's execution engine receive the already-translated string. The
helper is wired into all six regex expression types:
RegExpReplace,RLike,RegExpExtract,RegExpExtractAll,RegExpInStr,StringSplit. A newVeloxRegexUtilsobject (ingluten-substrait) providestranslateJavaPatternToRe2()andtranslateUnicodeEscapes().Patterns with no RE2 equivalent (lookaheads, lookbehinds, backreferences) are detected and still
throw
GlutenNotSupportException, preserving the existing graceful fallback for those cases.Phase 2 — C++ (safeguard).
validatePattern()inCommon.ccnow callstranslateJavaUnicodeToRe2()before theRE2::RE2()compile attempt. This is a last line ofdefence for any future regex expression type added to
kRegexFunctionswhose Scala transformer isnot yet hooked into the Phase 1 helper. The function is idempotent — patterns already in RE2 syntax
pass through unchanged.
Changes
gluten-substrait/.../VeloxRegexUtils.scalatranslateJavaPatternToRe2,containsUnsupportedConstruct,translateUnicodeEscapesgluten-substrait/.../ExpressionConverter.scalawithRe2PatternTranslationhelper; explicit cases forRLike,RegExpExtract,RegExpExtractAll; wired into all 6 regex expressionscpp/velox/utils/Common.htranslateJavaUnicodeToRe2cpp/velox/utils/Common.cctranslateJavaUnicodeToRe2; updatedvalidatePatterncpp/velox/tests/CommonTest.cctranslateJavaUnicodeToRe2andvalidatePatterncpp/velox/tests/CMakeLists.txtcommon_testtargetbackends-velox/.../FallbackSuite.scala\u4e00test — asserts native execution now, not fallbackbackends-velox/.../VeloxStringFunctionsSuite.scalaTesting
FallbackSuite— existing test updated;\uXXXXpatterns in join post-filters now run nativelyVeloxStringFunctionsSuite— new test confirms[\u0041-\u005A]+usesProjectExecTransformer; new test confirms(?<=x)ystill falls back gracefullyCommonTest.cc— 13 unit tests covering translation correctness andvalidatePatternend-to-endNotes
GlutenNotSupportException[a[b]],[a&&[b]]) unaffected —ensureRegexIsCompatibleoperates on the translated string, which has identical bracket depth to the originalWas this patch authored or co-authored using generative AI tooling?
Generated-by: Claude claude-sonnet-4-5