morningman opened a new pull request, #70150:
URL: https://github.com/apache/doris/pull/70150
### What problem does this PR solve?
Issue Number: None
Related PR: #68162, #69314
Problem Summary:
**In short.** This ports `MysqlPreparedStatementForwardingTest` from
branch-4.1 #68162 to master. Master already has the fix the test guards,
through #69314. The port adds what master's own tests do not cover: a NULL
bitmap of two bytes, and the forward request that `FEOpExecutor` builds and
Thrift carries. The PR changes tests only.
**Background.** A client that runs a server-side prepared statement again
may leave out the parameter types (`new_params_bind_flag` = 0) and have the
server reuse the ones it cached. A follower forwards every COM_STMT_EXECUTE to
the master. The master prepares the statement again for each forwarded
execution, so it has no cached types and cannot bind the parameters. #68162
fixed this on branch-4.1, together with the Lance prepared vector search it was
written for. Master fixed it independently in #69314: when the client reuses
cached types, `MysqlConnectProcessor.handleExecute` writes the forwarded packet
again, with the NULL bitmap, `new_params_bind_flag` = 1, every type code
including the unsigned flag, and the values. #69314 also has the prepare stage
plan with the context of the re-parsed statement, which is the other generic
part of #68162. Its tests in `MysqlConnectProcessorLongDataTest` cover cached
types, unsigned flags, changing NULLs, changing types, all-NULL executions
and the prepare context.
**What those tests leave out.** They use at most four parameters, so the
NULL bitmap always fits in one byte. They also give the master the buffer kept
on the follower's context, rather than the request
`FEOpExecutor.buildStmtForwardParams` builds.
**What this PR adds.** The test runs a ten-parameter statement five times on
a follower connection:
- The parameter types are sent with the first and the fourth execution; the
other executions reuse the cached ones.
- One execution has a NULL parameter in each byte of the NULL bitmap.
- An unsigned INT parameter becomes a BIGINT in the fourth execution.
After each execution, the test builds the forward request with
`FEOpExecutor`, passes it through a Thrift round trip and executes it on a new
proxy context whose placeholders have no types. It then checks three things:
every parameter binds to the same value as on the follower, the unsigned flag
carries over, and the client's packet is read to its end and left unchanged.
The code is the 4.1 test adapted to master's structure: master's
`PrepareCommand`, query id and audit setup, as in
`MysqlConnectProcessorLongDataTest`, and expressions compared directly. The 4.1
assertion that the follower keeps the client's buffer was dropped, because
master writes a separate buffer for cached types by design.
**Results.**
| `handleExecute` on master | `MysqlConnectProcessorLongDataTest` (14) |
`MysqlPreparedStatementForwardingTest` (1) |
|---|---|---|
| as is | pass | pass |
| no rewrite of the forwarded packet for cached types (the bug #68162 fixed)
| 3 fail | fails: the master binds untyped placeholders (`No value present`) |
| the rewrite keeps only the first byte of the NULL bitmap | pass | fails:
the master reads the packet one byte off |
Original author: @Gabriel39
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
New `MysqlPreparedStatementForwardingTest` passes, together with
`MysqlConnectProcessorLongDataTest`, `FEOpExecutorMysqlProtocolTest`,
`MysqlConnectProcessorCursorFetchTest` and
`MysqlConnectProcessorExecuteCleanupTest` (28 tests). The two mutations in the
table above were run against it. This PR changes no production code.
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
