fix(main): sync Name2ColIndex when appending __mo_rowid to index table TableDef - #24669
Merged
XuPeng-SH merged 3 commits intoMay 28, 2026
Merged
Conversation
…able TableDef Root cause: txn_table.go GetTableDef() appends __mo_rowid to Cols array but did not update Name2ColIndex map. For index tables created within the same transaction, Name2ColIndex[Row_ID] returned 0 (default) instead of the actual column position, causing ExpectedEOB during commit when DELETE operations on index tables used the wrong column as rowid. Fix: add name2index[catalog.Row_ID] = int32(len(cols)-1) when appending __mo_rowid, ensuring Cols and Name2ColIndex stay in sync. Remove all workaround scans in bind_update.go, bind_delete.go, bind_insert.go, bind_replace.go, and query_builder.go that were working around the inconsistency. Add BVT test covering CREATE INDEX + UPDATE/DELETE in same transaction to prevent regression. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
7 tasks
XuPeng-SH
approved these changes
May 28, 2026
… tests The DropDatabase function has a defer block that calls txnOp.SetSnapshotTS(origSnapshotTS) to restore the snapshot timestamp. Two tests were missing this mock expectation, causing gomock to fail with "unexpected call to SetSnapshotTS". This is a pre-existing issue on the main branch caused by two PRs (matrixorigin#24645 and matrixorigin#24461) being merged independently without cross-updating mock expectations. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
XuPeng-SH
added a commit
that referenced
this pull request
May 28, 2026
…able TableDef (#24671) ## What type of PR is this? - [ ] API-change - [x] BUG - [ ] Improvement - [ ] Documentation - [ ] Feature - [ ] Test and CI - [ ] Code Refactoring ## Which issue(s) this PR fixes: issue #24621 issue #24635 ## What this PR does / why we need it: ### Root Cause `txn_table.go` `GetTableDef()` appends `__mo_rowid` to the `Cols` array but did not update the `Name2ColIndex` map. For index tables created within the same transaction, `Name2ColIndex[catalog.Row_ID]` returned `0` (the default int32 value) instead of the actual column position. This causes `ExpectedEOB` during commit when DELETE operations on index tables use the wrong column as rowid. The bug triggers in same-transaction DDL + DML scenarios: - `CREATE INDEX` + `UPDATE`/`DELETE` on the indexed table - `ALTER TABLE ADD INDEX` + DML - `CREATE TABLE` with inline index + DML ### Fix Add `name2index[catalog.Row_ID] = int32(len(cols) - 1)` when appending `__mo_rowid` in `txn_table.go`, ensuring `Cols` and `Name2ColIndex` stay in sync. This allows removal of all workaround scans in `bind_update.go`, `bind_delete.go`, `bind_insert.go`, `bind_replace.go`, and `query_builder.go` that were compensating for the inconsistency. ### Verification - Added BVT test `index_table_rowid_eob` covering `CREATE INDEX` + `UPDATE`/`DELETE` in same transaction - Without the fix: test fails with `ExpectedEOB` on COMMIT (75% success rate) - With the fix: test passes 100% ### Note Same fix as #24668 (targeting 3.0-dev) and #24669 (targeting main), cherry-picked to 4.0-dev. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: XuPeng-SH <xupeng3112@163.com>
VioletQwQ-0
pushed a commit
to VioletQwQ-0/matrixone
that referenced
this pull request
May 29, 2026
…e TableDef (matrixorigin#24669) ## What type of PR is this? - [ ] API-change - [x] BUG - [ ] Improvement - [ ] Documentation - [ ] Feature - [ ] Test and CI - [ ] Code Refactoring ## Which issue(s) this PR fixes: issue matrixorigin#24621 issue matrixorigin#24635 ## What this PR does / why we need it: ### Root Cause `txn_table.go` `GetTableDef()` appends `__mo_rowid` to the `Cols` array but did not update the `Name2ColIndex` map. For index tables created within the same transaction, `Name2ColIndex[catalog.Row_ID]` returned `0` (the default int32 value) instead of the actual column position. This causes `ExpectedEOB` during commit when DELETE operations on index tables use the wrong column as rowid. The bug triggers in same-transaction DDL + DML scenarios: - `CREATE INDEX` + `UPDATE`/`DELETE` on the indexed table - `ALTER TABLE ADD INDEX` + DML - `CREATE TABLE` with inline index + DML ### Fix Add `name2index[catalog.Row_ID] = int32(len(cols) - 1)` when appending `__mo_rowid` in `txn_table.go`, ensuring `Cols` and `Name2ColIndex` stay in sync. ### Verification - Added BVT test `index_table_rowid_eob` covering `CREATE INDEX` + `UPDATE`/`DELETE` in same transaction - Without the fix: test fails with `ExpectedEOB` on COMMIT (75% success rate) - With the fix: test passes 100% ### Note Same fix as matrixorigin#24668 (targeting 3.0-dev), cherry-picked to main. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: XuPeng-SH <xupeng3112@163.com>
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
3 participants
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 type of PR is this?
Which issue(s) this PR fixes:
issue #24621
issue #24635
What this PR does / why we need it:
Root Cause
txn_table.goGetTableDef()appends__mo_rowidto theColsarray but did not update theName2ColIndexmap. For index tables created within the same transaction,Name2ColIndex[catalog.Row_ID]returned0(the default int32 value) instead of the actual column position.This causes
ExpectedEOBduring commit when DELETE operations on index tables use the wrong column as rowid. The bug triggers in same-transaction DDL + DML scenarios:CREATE INDEX+UPDATE/DELETEon the indexed tableALTER TABLE ADD INDEX+ DMLCREATE TABLEwith inline index + DMLFix
Add
name2index[catalog.Row_ID] = int32(len(cols) - 1)when appending__mo_rowidintxn_table.go, ensuringColsandName2ColIndexstay in sync.Verification
index_table_rowid_eobcoveringCREATE INDEX+UPDATE/DELETEin same transactionExpectedEOBon COMMIT (75% success rate)Note
Same fix as #24668 (targeting 3.0-dev), cherry-picked to main.