Skip to content

[3.0-dev] Backport prepared-statement / connector fixes (Fixes #24186)#24191

Open
jiangxinmeng1 wants to merge 10 commits intomatrixorigin:3.0-devfrom
jiangxinmeng1:fix-issue-24186
Open

[3.0-dev] Backport prepared-statement / connector fixes (Fixes #24186)#24191
jiangxinmeng1 wants to merge 10 commits intomatrixorigin:3.0-devfrom
jiangxinmeng1:fix-issue-24186

Conversation

@jiangxinmeng1
Copy link
Copy Markdown
Contributor

@jiangxinmeng1 jiangxinmeng1 commented Apr 24, 2026

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

fixes #24186

What this PR does / why we need it:

Backports the prepared-statement / connector compatibility fixes from main to 3.0-dev listed in #24186:

PR Title
#23413 Fix BOOL MySQL protocol output and refresh BVT results
#23578 Add server_id system variable for ODBC
#23864 Reset vector class to FLAT in ResetWithSameType to fix prepared stmt null json panic
#23887 Fix NULL on duplicate key for unsigned columns (already on 3.0-dev — skipped as empty)
#23982 Support JSON prepared params
#23984 Clear prepared binary param state
#23980 Harden prepare long data parsing

Additional changes required to make the cherry-picks compile and pass tests on 3.0-dev:

  • Pulled in the prerequisite fix: harden prepared binary reset cleanup #23978 "harden prepared binary reset cleanup" (the COM_STMT_EXECUTE error path now returns the prepareStmt so clearBinaryParamState runs; doReset now clears binary state; COM_STMT_RESET / COM_STMT_CLOSE error paths return early instead of dereferencing a nil prepareStmt; parseStmtExecute returns preStmt on error).
  • Added a missing MultiUpdateCtx.clone() helper (with PartitionCols initialization) on 3.0-dev that fix: harden prepare long data parsing #23980 expects.
  • Adjusted the cherry-picked unit tests (deletion_partition_test.go, insert_partition_test.go) to use the 2-argument testutil.MakeInt64Vector/MakeRowIdVector signatures present on 3.0-dev, and renamed the partition ExprWithRowID field reference to Expr.
  • De-duplicated TestResetWithSameTypeResetsClass / TestFunctionResultAppendNullAfterToConst (already present on 3.0-dev).
  • Added support for DATE input under MYSQL_TYPE_DATETIME/MYSQL_TYPE_TIMESTAMP in appendResultSetBinaryRow2 and brought scale/timezone-aware DATE/DATETIME/TIME/TIMESTAMP formatting into appendResultSetTextRow (matches main).
  • A handful of refreshed BVT result files from Fix BOOL MySQL protocol output and refresh BVT results #23413 were dropped because the underlying BVT cases don't exist on 3.0-dev.

Special notes for your reviewer:

CI on this branch will validate the BVT delta. Tested locally with go test -tags matrixone_test ./pkg/frontend/... ./pkg/sql/colexec/{insert,deletion,preinsert,multi_update}/... ./pkg/container/vector/... — all green (with TZ=UTC for the pre-existing TZ-sensitive TestRowToString).

gouhongshen and others added 7 commits April 24, 2026 11:43
…23413)

- Return BOOL as TINYINT(1) (0/1) instead of VARCHAR ("true/false") in MySQL protocol
- Adjust column definition (type/length/charset) and result row encoding (text/binary)
- Update related unit test and add jdbc-type-compatibility case
- Refresh BVT expected results

Approved by: @XuPeng-SH, @heni02
Adds the missing server_id system variable to MatrixOne so ODBC clients can read @@server_id during connection initialization without error. Also adds regression coverage in the distributed system variable tests. This prevents Tableau/ODBC connections from failing with “system variable does not exist.”

Approved by: @fengttt, @heni02, @XuPeng-SH
…stmt null json panic (matrixorigin#23864)

When a prepared statement inserts NULL into a JSON column multiple times, the second execution causes a panic during SELECT:

```
panic runtime error: index out of range [0] with length 0
ByteJson.Unmarshal (bytejson.go:96)
DecodeJson (encoding.go:93)
ColumnSlices.GetStringBased (output.go:538)
```

**Reproduce:**
```sql
CREATE TABLE test_ps (id INT, meta JSON);
PREPARE stmt FROM 'INSERT INTO test_ps (id, meta) VALUES (?, ?)';
SET @A=1, @b=NULL; EXECUTE stmt USING @A, @b;
SET @A=2, @b=NULL; EXECUTE stmt USING @A, @b;
DEALLOCATE PREPARE stmt;
SELECT meta FROM test_ps where id = 1;  -- ok
SELECT meta FROM test_ps where id = 2;  -- panic
```

**Root cause:** `FunctionExpressionExecutor.doFold()` calls `ToConst()` on the result vector after constant folding. On the second execution, `PreExtendAndReset()` calls `ResetWithSameType()` which clears nsp (null bitmap) but preserves the CONSTANT class. When `FunctionResult.AppendBytes(nil, true)` is called on a CONSTANT vector, it skips setting the null marker entirely, resulting in a non-null empty byte slice being stored instead of NULL.

**Fix:** Reset vector class to FLAT in `ResetWithSameType()`, so the vector is in a clean state for re-population. `doFold()` will call `ToConst()` again after evaluation if needed.

Approved by: @heni02, @aunjgr
This PR fixes a missing `MYSQL_TYPE_JSON` branch in the prepared-statement binary execute parser on `main`.

Without this change, `COM_STMT_EXECUTE` falls through to `unsupport parameter type` when a client sends a JSON-typed parameter through the binary protocol.

Focused validation:
- `go test ./pkg/frontend -run 'TestParseExecuteDataWithJSONParam'`
- `go test ./pkg/frontend -run 'TestParseExecuteDataWithJSONParam|Test_StripPassword'`

Approved by: @LeftHandCold
This PR reclaims prepared-statement binary parameter state after `COM_STMT_EXECUTE` and on malformed execute cleanup.

It fixes two closely related problems on `main`:
1. repeated binary executes with varlen parameters reused the same params vector and kept appending to its area, growing session memory until statement close;
2. malformed `COM_STMT_EXECUTE` packets could leave a partially populated params vector attached to the prepared statement because the parse-error path dropped the statement pointer before cleanup.

Focused validation:
- `go test ./pkg/frontend -run 'TestPrepareStmtClearBinaryParamStateReleasesParamArea|Test_ExecRequestStmtExecuteErrorClearsPreparedParamState'`
- `go test ./pkg/frontend -run 'TestPrepareStmtClearBinaryParamStateReleasesParamArea|Test_ExecRequestStmtExecuteErrorClearsPreparedParamState|Test_parseStmtSendLongData'`

Approved by: @LeftHandCold
This PR fixes three categories of issues:

- Repeated COM_STMT_SEND_LONG_DATA chunks for the same parameter now **append** instead of overwriting, matching MySQL protocol semantics.
- Truncated COM_STMT_EXECUTE packets now return a malformed-packet error instead of risking a panic while reading the new parameter bound flag.

- PreInsert.refreshAutoIncrementTableID() wrote directly to the shared *plan.TableDef.TblId during Prepare().
- MultiUpdate.Prepare() read the same TblId concurrently in a different ants worker pool goroutine.
- **Fix**: Store the refreshed TblId in a local container field (ctr.tblId) instead of mutating the shared plan object.

- PartitionInsert and PartitionDelete temporarily mutated the shared ObjRef.ObjName during partition iteration (save/restore via defer) -- replaced with stack-local copies.
- PartitionMultiUpdate.writeTable() directly mutated shared MultiUpdateCtx entries -- now uses clone() to match the writeS3 code path.
- Also fixed a pre-existing copy-paste bug in MultiUpdateCtx.clone() where PartitionCols was incorrectly copying DeleteCols.

Approved by: @ouyuanning
This PR hardens prepared-statement binary protocol cleanup on `main`.

It fixes three closely related problems in the frontend prepare path:
1. `PrepareStmt.Close()` now keeps a valid process context for freeing binary parameter vectors after binary parsing.
2. `COM_STMT_RESET` / `reset prepare` now clears accumulated binary prepared-statement state instead of acting like a no-op.
3. `COM_STMT_CLOSE`, `COM_STMT_RESET`, and malformed `COM_STMT_EXECUTE` error paths now cleanly return and clean up instead of dereferencing nil state or leaving dirty binary state behind.

Focused validation:
- `go test ./pkg/frontend -run 'Test_doResetClearsPreparedBinaryState|Test_ExecRequestPrepareCommandMissingStmt|Test_ExecRequestStmtExecuteErrorClearsPreparedBinaryState|TestPrepareStmtCloseAfterBinaryParamParsing|TestMysqlProtocolImpl_Close|Test_parseStmtSendLongData'`

Approved by: @LeftHandCold
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 24, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
4 out of 5 committers have signed the CLA.

✅ gouhongshen
✅ XuPeng-SH
✅ ck89119
✅ jiangxinmeng1
❌ robll-v1
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working size/XL Denotes a PR that changes [1000, 1999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants