MDEV-39384 Wrong result when selecting from precise-versioned table#5275
MDEV-39384 Wrong result when selecting from precise-versioned table#5275midenok wants to merge 4 commits into
Conversation
Followup for b337e14 as info_src takes time too. info_src does not make much sense without ABI check.
mtrr --mysqld=--debug=d,vers_trx_id,query:i:o,/tmp/good.log bug/v.trx_id,debug
|
|
There was a problem hiding this comment.
Code Review
This pull request optimizes transaction registry lookups in TR_table::query by utilizing indexes (IDX_TRX_ID and IDX_COMMIT_TS) for faster queries, falling back to a slow scan with a warning if the index definitions are unexpected. It also adds debug tracing and a helper method is_idx_correct. The review feedback correctly identifies critical safety issues where table->key_info[idx] is accessed before verifying that idx is within the bounds of table->s->keys, which can lead to out-of-bounds reads and undefined behavior. Additionally, the reviewer recommends removing overly restrictive debug assertions that would crash debug builds when unexpected indexes are encountered.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
This PR targets MDEV-39384 by changing how mysql.transaction_registry (TRT) rows are located, preferring direct index lookups (PK / commit_timestamp) with a slow-scan fallback when the TRT index layout is unexpected.
Changes:
- Add TRT index-id constants and introduce index-based lookup paths in
TR_table::query()for trx_id and commit_timestamp. - Add a new warning (
WARN_VERS_TRT_DEFINITION) when falling back to a slow scan due to unexpected TRT index definitions. - Extend
dbug_print_row()/dbug_print_table_row()to optionally print column names and add additional DBUG tracing around versioning visibility logic.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| sql/table.h | Add TRT index-id enum and helper for validating index properties. |
| sql/table.cc | Rework TRT queries to use index reads where possible; add warnings and DBUG tracing. |
| sql/sql_test.h | Extend debugger print helpers with optional print_names parameter. |
| sql/filesort.cc | Update debugger row-print helper implementations to match new signatures. |
| sql/share/errmsg-utf8.txt | Add new warning text for unexpected TRT index definitions. |
| sql/item_vers.cc | Add DBUG_ENTER/DBUG_PRINT instrumentation around TRT “sees” evaluation. |
| CMakeLists.txt | Gate INFO_* targets on WITHOUT_ABI_CHECK. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7555dee to
c24872c
Compare
6526c82 to
247d496
Compare
8b555d9 to
618c4c3
Compare
618c4c3 to
5116e57
Compare
e6c6b03 to
b27d713
Compare
b27d713 to
1f8d0c5
Compare
178b22e to
b40f8c5
Compare
c38829c to
ce00c8e
Compare
Const item behave wrongly in read_record loop. const_item() turns on cache in Arg_comparator::cache_converted_constant(). The fix manipulates table->map to force Item_field to be non-const.
ce00c8e to
94fdbfd
Compare
TR_table::query() used table scan. Now utilize the index if possible, with minimum validity detection. Getting trx_id by commit_ts is still suboptimal is it does two index accesses (as limited by fields in commit_ts index). When the index detection fails TR_table::query() falls back to original table scanning method.
94fdbfd to
8acdfd4
Compare
No description provided.