Uh oh!
There was an error while loading. Please reload this page.
[fix](function) Align constant folding with BE results - #64881
Merged
Conversation
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: FE constant folding for string functions had two inconsistencies with backend execution. The array form of concat_ws appended separators while scanning every non-final element, so arrays ending in NULL could keep a trailing separator after folding. The literal vararg form with NULL was not folded with the same skip-NULL semantics as BE. The MD5 folding path also used the JVM default charset when converting Java strings to bytes, while BE hashes the UTF-8 bytes stored in string columns. This change joins only non-NULL concat_ws inputs with separators between retained values, falls back to BE for unsupported array varargs, and uses UTF-8 bytes for MD5 folding. ### Release note None ### Check List (For Author) - Test: Regression test / Manual test - mvn checkstyle:check -pl fe-core - mvn -pl fe-core -am -DskipTests compile - ./run-regression-test.sh --run -d query_p0/sql_functions/string_functions -s string_functions_all (failed against the already-running old FE, reproducing the pre-fix concat_ws mismatch: folded a, vs BE a) - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Issue Number: N/A Related PR: N/A Problem Summary: Constant folding for several FE executable functions produced results different from BE execution. String functions mixed Java UTF-16 indexes with SQL character positions, Unicode case conversion used per-char Java helpers instead of locale-independent string conversion or BE-like single-code-point initcap, strcmp used UTF-16 ordering instead of UTF-8 byte ordering, URL helpers decoded URI components instead of returning raw parser substrings, maketime dropped microsecond rounding carry, and floating field comparison distinguished -0.0 from 0.0. This change aligns those FE folding paths with BE behavior and adds testFoldConst coverage for the mismatched expressions. ### Release note None ### Check List (For Author) - Test: Unit Test - mvn checkstyle:check -pl fe-core - mvn -pl fe-core -am -DskipTests compile - Behavior changed: No - Does this need documentation: No
hello-stephen
commented
Jun 26, 2026
Contributor
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Issue Number: N/A Related PR: N/A Problem Summary: FE constant folding for find_in_set used Java split with trailing empty tokens, but BE does not match an empty token after the final comma because its scan loop stops when the next token starts at the string length. FE also made the floating field comparison clearer by explicitly comparing primitive float and double values, preserving BE numeric equality for -0.0 and 0.0. None - Test: Unit Test - mvn checkstyle:check -pl fe-core - mvn -pl fe-core -am -DskipTests compile - Behavior changed: No - Does this need documentation: No
morrySnowforce-pushed
the
fix-constant-folding
branch
from
June 26, 2026 06:46
e7bd52f to
ed5f4c6ComparemorrySnow
commented
Jun 26, 2026
ContributorAuthor
/review |
morrySnow
commented
Jun 26, 2026
ContributorAuthor
run buildall |
morrySnow
marked this pull request as ready for review
June 26, 2026 06:48
morrySnow
requested review from
924060929, englefly and starocean999
as code ownersJune 26, 2026 06:48
Contributor
There was a problem hiding this comment.
Automated review completed. I found no blocking issues and am leaving this as a summary-only review.
Critical checkpoints:
- Goal/test proof: The PR aligns FE executable constant folding for the touched string/time functions with BE behavior. I checked the FE implementations against BE
MakeTimeImpl,FunctionStringConcatWs,StringInStrImpl,FunctionRight, ICU case conversion,FunctionStrcmp,FindInSetOp, andUrlParser; the addedtestFoldConstcases compare folded vs non-folded execution. - Scope/parallel paths: The change is focused on FE constant-folding methods and matching regression calls. I did not find a missing parallel path in the reviewed scope.
- Concurrency/lifecycle/config/protocol/data writes: Not applicable; no concurrency, persistent state, config item, FE-BE protocol, storage format, transaction, or data-write behavior changed.
- Tests/validation: Added regression
testFoldConstcoverage for the fixed mismatches. I rangit diff --checkon the GitHub-scoped changed files. I could not run FE/regression tests in this runner becausethirdparty/installed/thirdparty/installed/bin/protocis absent. - User focus: No additional user-provided review focus was supplied.
Subagent conclusions:
optimizer-rewrite: no candidates in first pass; convergence round 1 returnedNO_NEW_VALUABLE_FINDINGS.tests-session-config: no candidates in first pass; convergence round 1 returnedNO_NEW_VALUABLE_FINDINGS.- No inline comments were proposed or submitted.
hello-stephen
commented
Jun 26, 2026
Contributor
TPC-H: Total hot run time: 29544 ms |
hello-stephen
commented
Jun 26, 2026
Contributor
TPC-DS: Total hot run time: 171880 ms |
hello-stephen
commented
Jun 26, 2026
Contributor
ClickBench: Total hot run time: 25.21 s |
starocean999
approved these changes
Jun 26, 2026
Uh oh!
There was an error while loading. Please reload this page.
puranjay2597 pushed a commit
to puranjay2597/doris
that referenced
this pull request
Sep 3, 2026
…rd with BE Reviewed the merged levenshtein/hamming_distance (apache#60412) and damerau_levenshtein_distance (apache#65278, apache#66236) PRs for consistency with this one. Found and fixed a real FE/BE divergence in the process: - The FE constant-fold length guard checked Java code point count, while the BE guard checks UTF-8 byte length. For multi-byte input the two disagree (e.g. ~30000 3-byte characters is under the FE's 65535 code-point cap but over BE's 65535-byte cap), so a literal expression could fold successfully on FE while the same value would be rejected by BE if read from a column. FE now measures UTF-8 bytes too, matching function_string_similarity.cpp exactly (mirrors the fix pattern in apache#64881, "Align constant folding with BE results"). - Added regression coverage for astral-plane (surrogate-pair/4-byte UTF-8) characters on all three functions, matching the precedent set for RIGHT/INSTR in apache#64881.
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 freeto 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.
What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: FE constant folding produced results different from BE execution for several string, URL, time, and floating-point expressions. This PR aligns the FE executable folding paths with BE behavior and adds
testFoldConstcoverage for the mismatched expressions.Mismatched expressions
CONCAT_WS(',', ['a', NULL])a,aNULL; BE skipsNULLarguments and only inserts separators between retained values.RIGHT('😀a', 1)😀aINSTR('😀a', 'a')32String.indexOf(...) + 1, which is a UTF-16 char offset; BE returns the 1-based character position.UPPER('éßi')ÉßIÉSSIcharCharacter.toUpperCase, which cannot perform Unicode string mappings such asß -> SS; BE uses ICU string case conversion.LOWER('ÉİA')éiaéi̇a(U+00E9 U+0069 U+0307 U+0061)charlowercasing and lost the combining dot fromİ; BE/ICU lowercasesİtoi + U+0307.INITCAP('ßETA İSTANBUL')ßeta İstanbulßeta İStanbul(İlowercases toi + U+0307, then the combining mark starts a new word forS)charcase conversion directly on the original string; BE lowercases the whole string first with ICU, then uppercases the first alphanumeric code point after each non-alphanumeric code point.STRCMP('😀','')-11😀sorts after.FIND_IN_SET('', 'a,')20split(',', -1), which preserves the trailing empty token; BE's scan loop stops when the next token would start at the string length, so the trailing empty token is not matched.PARSE_URL('http://h/p%20x?q=a+b%20c&k=v#r', 'PATH')/p x/p%20xjava.net.URI, which decodes escaped bytes; BEUrlParserreturns raw URL substrings.PARSE_URL('http://h/p%20x?q=a+b%20c&k=v#r', 'QUERY')q=a+b c&k=vq=a+b%20c&k=vEXTRACT_URL_PARAMETER('http://h/p%20x?q=a+b%20c&k=v#r', 'q')a+b ca+b%20cMAKETIME(1, 2, 3.9999995)01:02:03.00000001:02:04.0000001000000, losing the carry into seconds; BE rounds total seconds to microsecond precision before constructing the time value.MAKETIME(1, 2, 59.9999995)01:02:59.00000001:03:00.000000FIELD(CAST('-0.0' AS DOUBLE), CAST('0.0' AS DOUBLE), CAST('-0.0' AS DOUBLE))21-0.0from0.0; BE compares floating values numerically, so-0.0 == 0.0and the first candidate matches.The PR also makes
MD5/MD5SUMfolding explicitly use UTF-8 bytes, matching BE and avoiding JVM-default-charset dependent folding results.Release note
None
Check List (For Author)
mvn checkstyle:check -pl fe-coremvn -pl fe-core -am -DskipTests compile