Skip to content

MDEV-41351 Allow a covering index scan for a SELECT ... FOR UPDATE un… - #5847

Merged
mariadb-YuchenPei merged 1 commit into
13.1from
bb-13.1-mdev-41351
Oct 29, 2026
Merged

mariadb-YuchenPei merged 1 commit into
13.1from
bb-13.1-mdev-41351

Conversation

@mariadb-YuchenPei

Copy link
Copy Markdown
Contributor

…der a full-scan table lock

(TODO: improve commit message)

MDEV-24813 introduced a switch innodb_table_lock_on_full_scan that places a table lock on an innodb table for some unconditional SELECT statements.

With a full table lock, and when doing a covering scan, there is no need to acquire clustered locks. This patch does so for X-locks.

TODO: add code comments

@CLAassistant

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.

Comment thread storage/innobase/row/row0sel.cc Outdated
@mariadb-YuchenPei
mariadb-YuchenPei force-pushed the bb-13.1-mdev-41351 branch 2 times, most recently from 41cb62d to 399306e Compare October 20, 2026 02:23
Comment thread storage/innobase/include/row0mysql.h Outdated
Comment thread storage/innobase/row/row0sel.cc Outdated
Comment thread storage/innobase/row/row0sel.cc Outdated
@mariadb-YuchenPei
mariadb-YuchenPei force-pushed the bb-13.1-mdev-41351 branch 4 times, most recently from e35e688 to d75ffc7 Compare October 21, 2026 03:36

@dr-m dr-m 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.

OK to push after addressing my comments. I am pleased to see that this eliminates some conditional branches from the lock_table() call in row_search_mvcc().

Comment thread storage/innobase/handler/ha_innodb.cc Outdated
Comment thread storage/innobase/handler/ha_innodb.cc Outdated
Comment on lines 16451 to 16457
@@ -16454,7 +16455,30 @@ int ha_innobase::extra_opt(enum ha_extra_function operation, ulong arg)
case HA_EXTRA_FULL_SCAN:
if (THDVAR(ha_thd(), table_lock_on_full_scan) &&
!m_prebuilt->skip_locked && arg == ULONG_MAX)

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.

The cheapest conditions should be evaluated first. On any modern ISA, the function parameters should reside in registers, so they should be tested first:

int ha_innobase::extra_opt(enum ha_extra_function operation, ulong arg)
{
  switch (operation) {
    case HA_EXTRA_FULL_SCAN:
      if (arg == ULONG_MAX && !m_prebuilt->skip_locked &&
          THDVAR(m_user_thd, table_lock_on_full_scan))

Note: Our formatting does not deviate from K&R for the switch statement body.

Note 2: m_prebuilt would be zero-initialised in ha_innobase::ha_innobase() an initialised in ha_innobase::open(). At that point, also ha_innobase::update_thd() should be called, which is caching the value of ha_thd().

Comment thread storage/innobase/handler/ha_innodb.cc Outdated
if (THDVAR(ha_thd(), table_lock_on_full_scan) &&
!m_prebuilt->skip_locked && arg == ULONG_MAX)
{
m_prebuilt->full_table_scan = true;

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.

We are supposed to omit the space before any assignment operator.

Comment thread storage/innobase/include/row0mysql.h
Comment thread storage/innobase/row/row0sel.cc
Comment thread storage/innobase/row/row0sel.cc Outdated
…der a full-scan table lock

MDEV-24813 introduced a switch innodb_table_lock_on_full_scan that
requests a table lock on an innodb table for some unconditional
SELECT/UPDATE/DELETE statements.

With a full table lock, and when doing a covering scan, there is no
need to request clustered index reads. This patch does so for X-locks
in SELECT statements. Prior to this it is already the case for
S-locks.

There is one exception: we disable this optimization when inside a
transaction with innodb_snapshot_isolation and a read view already
open, in which case it needs trx_id obtained from the clustered index
to check if there is a committed change the transaction never saw.

Assisted-By: Claude Opus 5 <noreply@anthropic.com>
@mariadb-YuchenPei
mariadb-YuchenPei merged commit 8fe2322 into 13.1 Oct 29, 2026
16 of 20 checks passed
@mariadb-YuchenPei
mariadb-YuchenPei deleted the bb-13.1-mdev-41351 branch October 29, 2026 03:06
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.

4 participants