Skip to content

MDEV-39384 Wrong result when selecting from precise-versioned table#5275

Open
midenok wants to merge 4 commits into
bb-10.11-midenokfrom
midenok-MDEV-39384
Open

MDEV-39384 Wrong result when selecting from precise-versioned table#5275
midenok wants to merge 4 commits into
bb-10.11-midenokfrom
midenok-MDEV-39384

Conversation

@midenok

@midenok midenok commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

No description provided.

midenok added 2 commits June 25, 2026 01:11
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
Copilot AI review requested due to automatic review settings June 24, 2026 22:14
@CLAassistant

CLAassistant commented Jun 24, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread sql/table.h Outdated
Comment thread sql/table.cc
Comment thread sql/table.cc

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread sql/table.h Outdated
Comment thread sql/table.cc
Comment thread sql/table.cc
Comment thread sql/table.cc Outdated
Comment thread sql/table.cc
Comment thread sql/table.cc Outdated
Comment thread sql/table.cc
Comment thread sql/table.cc Outdated
Comment thread sql/table.cc

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Comment thread sql/table.h Outdated
Comment thread sql/table.cc
Comment thread sql/table.cc
Comment thread sql/table.cc

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Comment thread sql/table.cc Outdated
Comment thread sql/table.cc Outdated
Comment thread sql/share/errmsg-utf8.txt Outdated
Comment thread mysql-test/suite/versioning/r/trx_id,scan.rdiff Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.

Comment thread sql/table.cc Outdated
Comment thread sql/table.cc Outdated
Comment thread sql/table.cc
Comment thread sql/table.cc Outdated
Comment thread sql/table.cc Outdated
Comment thread mysql-test/suite/versioning/r/trx_id,scan.rdiff Outdated
Comment thread mysql-test/suite/versioning/r/trx_id,scan.rdiff Outdated
Comment thread mysql-test/suite/versioning/r/trx_id,scan.rdiff Outdated
Comment thread mysql-test/suite/versioning/r/trx_id,scan.rdiff Outdated
Comment thread mysql-test/suite/versioning/r/trx_id,scan.rdiff Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comment thread sql/table.cc Outdated
Comment thread sql/table.cc Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Comment thread sql/table.cc Outdated
Comment thread sql/table.cc
Comment thread CMakeLists.txt

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comment thread sql/table.cc Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comment thread sql/table.cc Outdated
Comment thread sql/table.cc Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comment thread sql/table.cc Outdated
Comment thread sql/table.cc Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Comment thread sql/table.cc Outdated
Comment thread sql/filesort.cc
Comment thread sql/filesort.cc

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Comment thread sql/table.cc
Comment thread sql/table.cc
Comment thread sql/table.cc
Comment thread sql/table.cc
Comment thread sql/table.cc
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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comment thread sql/table.cc
Comment thread sql/table.cc
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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

@midenok midenok requested a review from sanja-byelkin June 26, 2026 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants