Repository navigation
[fix](fe) Preserve non-UTF-8 prepared statement parameters - #69805
Merged
Merged
Conversation
### What problem does this PR solve? Related PR: apache#69314 Problem Summary: JDBC setBytes and ODBC binary bindings can use MySQL string wire types. Java UTF-8 replacement decoding silently corrupted their binary payloads. Decode strictly, retain invalid UTF-8 as binary bytes and bind it through a BE-side string cast. Preserve valid text behavior and use value-based binary child hashes for casts reconstructed during reuse and forwarding. ### Release note Preserve non-UTF-8 bytes in MySQL server-prepared string parameters, including JDBC setBytes and Connector/ODBC binary bindings. ### Check List (For Author) - Test: Regression tests and actual JDBC/ODBC driver checks passed; FE build and Checkstyle passed. All 17 focused FE unit tests passed. - Behavior changed: Yes. Preserve malformed UTF-8 bytes instead of replacing them while retaining string expression semantics. - Does this need documentation: No
CalvinKirs
requested review from
924060929,
englefly,
morrySnow and
starocean999
as code owners
October 29, 2026 03:46
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
run buildall |
CalvinKirs
force-pushed
the
fix-mysql-binary-parameters
branch
from
October 29, 2026 06:16
1a91318 to
fa69b45
Compare
Member
Author
|
run buildall |
1 similar comment
Member
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28369 ms |
Contributor
TPC-DS: Total hot run time: 151487 ms |
Contributor
ClickBench: Total hot run time: 24.25 s |
### What problem does this PR solve? Issue Number: None Related PR: apache#69805 Problem Summary: The PR merge commit contained a duplicate normalizeStreamRead helper after master added the same method. Master removed the duplicate in apache#69809. Sync master so the PR merge passes CheckStyle. ### Release note None ### Check List (For Author) - Test: Manual test: mvn clean checkstyle:check in fe (passed) - Behavior changed: No - Does this need documentation: No
Member
Author
|
run buildall |
924060929
requested changes
Nov 2, 2026
924060929
left a comment
924060929
left a comment
Contributor
There was a problem hiding this comment.
This change preserves non-UTF-8 prepared string parameters by retaining their bytes in a binary literal and casting to the original string category on BE.
Static FE review of f4402db found one blocking P2: the new expression-valued bindings reach literal-only cached point-query and Group Commit paths, causing ClassCastException. Please adapt their admission/fallback logic and cover both execution paths. Normal translation, raw-byte forwarding, and value-based child hashing showed no additional blocking issues.
Scope: FE and the necessary BE cast consumer. No builds or tests were run. The supplied current-head CI snapshot is queued and does not establish that required P0 checks have passed. Recommendation: REQUEST_CHANGES.
Automated FE-focused static review (gpt-6.1-sol, xhigh). No build or tests were run.
| switch (type) { | ||
| case MYSQL_TYPE_STRING: | ||
| case MYSQL_TYPE_VARSTRING: | ||
| l = new Cast(binary, StringType.INSTANCE); |
Contributor
There was a problem hiding this comment.
[P2] Keep expression-valued parameters out of literal-only fast paths
This introduces non-Literal bindings without adapting the prepared-execution fast paths. On a point-query-eligible MOW UNIQUE table with a VARCHAR key, execute SELECT ... WHERE k=? with ASCII first, then bind FF using the same string wire type. ExecuteCommand.run reuses the cached point-query plan before analysis, and PointQueryExecutor:165 casts this Cast to Literal, throwing ClassCastException. Prepared INSERTs with group_commit=sync_mode and the default enable_group_commit_full_prepare=true hit the same assumption at GroupCommitPlanner:242. The base produced literals for these bindings. Gate these fast paths on the current bindings' capabilities and reset/fall back to full planning and BE evaluation when necessary. Add coverage for both paths; the new binding tests mock StmtExecutor and do not exercise them.
Contributor
TPC-H: Total hot run time: 28580 ms |
Contributor
TPC-DS: Total hot run time: 151399 ms |
Contributor
ClickBench: Total hot run time: 24.37 s |
### What problem does this PR solve? Related PR: apache#69805 Problem Summary: Non-UTF-8 prepared string parameters are bound as binary-to-string casts. Cached point queries and fully prepared Group Commit inserts require literal bindings and would throw ClassCastException for these expressions. Check the current bindings before entering either fast path, refresh cached point-query plans when bindings require evaluation, and clear inherited point-query flags before normal planning. Keep literal bindings eligible for existing fast paths. ### Release note Avoid prepared-statement execution failures when binary string parameters follow text parameters in point queries or Group Commit inserts. ### Check List (For Author) - Test: Unit Test / ExecuteCommandTest, MysqlParameterLiteralTest, MysqlConnectProcessorLongDataTest, PointQueryExecutorTest and GroupCommitPlannerTest via run-fe-ut.sh; FE Checkstyle - Behavior changed: Yes. Expression-valued parameters use normal planning and BE evaluation instead of literal-only prepared fast paths. - Does this need documentation: No
Member
Author
|
run buildall |
Member
Author
Local pipeline review — ✅ PASSschema: doris-repo-review/v1
status: PASS
pr: apache/doris#69805
commit: 96e2a12c6bee44c44647f52c542967be5f402b1f
base: 0976b2cd5020607205969bf7986dcb551b6a7325
reviewed_at: 2026-11-03T12:37+08:00
reviewer: CalvinKirs
model: gpt-6.1-sol
effort: xhigh
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: trueNotes for maintainers
Reviewed locally with the |
Contributor
TPC-H: Total hot run time: 28279 ms |
Contributor
TPC-DS: Total hot run time: 151388 ms |
Contributor
ClickBench: Total hot run time: 24.33 s |
924060929
approved these changes
Nov 3, 2026
924060929
left a comment
924060929
left a comment
Contributor
There was a problem hiding this comment.
String-coded prepared parameters can contain arbitrary bytes. This change preserves invalid UTF-8 as binary literals and casts them to the original string category during normal BE evaluation, while retaining existing valid-text and BLOB behavior.
Independent static review of 96e2a12 against merge-base 0976b2c found no blocking FE issues. I checked decoding, placeholder binding, constant-folding boundaries, translation, raw-byte forwarding, content-based hashing, and prepared-execution fallback. The previous P2 concerning literal-only point-query and Group Commit fast paths is fixed by the binding admission checks and state reset/replanning logic, with targeted routing tests added.
Recommendation: APPROVE within the FE scope and necessary BE consumers. No builds or tests were run during this review. The supplied head snapshot reports successful compilation, FE UT and P0 regressions, but failed FE coverage. Required-check rules were not supplied; merge eligibility is separate from this code recommendation.
Automated FE-focused static review (gpt-6.1-sol, high). No build or tests were run.
This was referenced Nov 3, 2026
3 of 16 tasks
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.
What problem does this PR solve?
Related PR: #69314
JDBC
setBytes()and Connector/ODBC binary bindings can sendMYSQL_TYPE_VAR_STRING/MYSQL_TYPE_STRINGrather than BLOB. FE decoded those bytes with Java's replacement UTF-8 decoder, silently changing arbitrary binary data. For example,0080FF4100C328became00EFBFBDEFBFBD4100EFBFBD28inSELECT HEX(?). The BLOB handling added by #69314 did not cover these wire types.Decode string parameters strictly and retain non-UTF-8 payloads as binary literals. Bind those payloads through a BE-side cast to the original string category, preserving string function and comparison typing without replacing bytes. Valid UTF-8 retains the existing text literal types, and BLOB types retain their existing binary behavior. Use value-based binary literal child hashes so equivalent casts remain equal after rebinding and forwarding.
Release note
Preserve non-UTF-8 bytes in MySQL server-prepared string parameters, including JDBC
setBytes()and Connector/ODBC binary bindings.Check List (For Author)
prepared_stmt_p0/binary_parameters; replayedprepared_stmt_p0/send_long_data. Covers all byte values, malformed UTF-8, parameter reuse, stored HEX/MD5, string expression predicates, NULL/empty values and ordinary Unicode text.MysqlParameterLiteralTestandMysqlConnectProcessorLongDataTest, including string wire aliases, cached types, uploads and forwarding.setBytes,setBinaryStreamandsetBlobpreserve0080FF4100C328.Runtime regression checks used the newly built master FE with an existing 4.1.4 BE and condition cache disabled. The predicate regression uses
CONCAT(value, '') = ?; direct constant predicate pushdown on a current master BE was not validated. No BE, storage-format or protocol-field changes are included.