Skip to content

MDEV-24813 Signal full scan to storage engines.#3487

Open
mariadb-YuchenPei wants to merge 2 commits into
11.4from
mdev-24813
Open

MDEV-24813 Signal full scan to storage engines.#3487
mariadb-YuchenPei wants to merge 2 commits into
11.4from
mdev-24813

Conversation

@mariadb-YuchenPei
Copy link
Copy Markdown
Contributor

@mariadb-YuchenPei mariadb-YuchenPei commented Aug 29, 2024

This is an initial patch for discussion.

Will work in the following minimal example

--source include/have_innodb.inc
CREATE TABLE t (a INT PRIMARY KEY) ENGINE=InnoDB;
insert into t values (42);

BEGIN;
SELECT * FROM t LOCK IN SHARE MODE; # will acquire S lock
COMMIT;

DROP TABLE t;

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Aug 29, 2024

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.
0 out of 2 committers have signed the CLA.

❌ Thirunarayanan
❌ mariadb-YuchenPei
You have signed the CLA already but the status is still pending? Let us recheck it.

@mariadb-YuchenPei mariadb-YuchenPei marked this pull request as draft August 29, 2024 05:57
Comment thread storage/innobase/include/row0mysql.h Outdated
Comment thread sql/records.cc Outdated
@spetrunia
Copy link
Copy Markdown
Member

Also, I've tried this testcase

create table t10 (a int, b varchar(100), index(a));
insert into t10 select seq, uuid() from seq_1_to_10000;
create table t11(a int);
explain insert into t11 select a from t10;
insert into t11 select a from t10;

and init_read_record() isn't invoked for it.

The index scan on t10 is initialized here:

#0  ha_innobase::index_init (this=0x7fffb80b6eb0, keynr=0)   at /home/psergey/dev-git/12.3-no-lock/storage/innobase/handler/ha_innodb.cc:8767
#1  0x0000555555f0a5a0 in handler::ha_index_init (this=0x7fffb80b6eb0,     idx=0, sorted=true)    at /home/psergey/dev-git/12.3-no-lock/sql/handler.h:3527
#2  0x0000555556125194 in join_read_first (tab=0x7fffb80cae40)    at /home/psergey/dev-git/12.3-no-lock/sql/sql_select.cc:24768

@spetrunia
Copy link
Copy Markdown
Member

With this patch, we have "Using where with pushed condition" all over the place:

MariaDB [test]> explain select * from t1 where b <1;
+------+-------------+-------+------+---------------+------+---------+------+-------+-----------------------------------+
| id   | select_type | table | type | possible_keys | key  | key_len | ref  | rows  | Extra                             |
+------+-------------+-------+------+---------------+------+---------+------+-------+-----------------------------------+
|    1 | SIMPLE      | t1    | ALL  | NULL          | NULL | NULL    | NULL | 10157 | Using where with pushed condition |
+------+-------------+-------+------+---------------+------+---------+------+-------+-----------------------------------+
1 row in set (0.002 sec)

@mariadb-YuchenPei
Copy link
Copy Markdown
Contributor Author

Also, I've tried this testcase

create table t10 (a int, b varchar(100), index(a));
insert into t10 select seq, uuid() from seq_1_to_10000;
create table t11(a int);
explain insert into t11 select a from t10;
insert into t11 select a from t10;

and init_read_record() isn't invoked for it.

The index scan on t10 is initialized here:

#0  ha_innobase::index_init (this=0x7fffb80b6eb0, keynr=0)   at /home/psergey/dev-git/12.3-no-lock/storage/innobase/handler/ha_innodb.cc:8767
#1  0x0000555555f0a5a0 in handler::ha_index_init (this=0x7fffb80b6eb0,     idx=0, sorted=true)    at /home/psergey/dev-git/12.3-no-lock/sql/handler.h:3527
#2  0x0000555556125194 in join_read_first (tab=0x7fffb80cae40)    at /home/psergey/dev-git/12.3-no-lock/sql/sql_select.cc:24768

You are right. I had forgotten to make the call in join_read_first (and join_read_last). Added the calls to ha_extra in a new commit now.

@mariadb-YuchenPei
Copy link
Copy Markdown
Contributor Author

With this patch, we have "Using where with pushed condition" all over the place:

No longer the case with the new patch that does not comment out the cond_push if condition

@mariadb-YuchenPei mariadb-YuchenPei changed the base branch from main to 11.4 March 1, 2026 23:22
@mariadb-YuchenPei mariadb-YuchenPei marked this pull request as ready for review March 12, 2026 02:04
@mariadb-YuchenPei mariadb-YuchenPei changed the title MDEV-24813 [poc] Signal full table lock when cond is NULL MDEV-24813 Signal full scan to storage engines. Mar 12, 2026
@Thirunarayanan Thirunarayanan requested review from iMineLink and removed request for Thirunarayanan June 2, 2026 12:24
When starting to do a full table/index scan without a WHERE or JOIN
condition, tell the storage engine so and the corresponding
ulong-truncated LIMIT.

Include an innodb implementation: added an innodb switch
table_lock_on_full_scan, so that when the switch is on, on receiving
the full scan signal from the sql layer, if the truncated LIMIT is
ULONG_MAX (likely no LIMIT), attempt to acquire a table lock.

Updated tests that have different results with the switch on.

(Comment and code edited by Sergei Petrunia)
Copy link
Copy Markdown
Contributor

@iMineLink iMineLink left a comment

Choose a reason for hiding this comment

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

Please check my few minor comments, especially the ones around innodb_full_scan.test, thanks.

Comment thread mysql-test/main/innodb_full_scan.test Outdated
Comment thread mysql-test/main/innodb_full_scan.test Outdated
Comment thread storage/innobase/handler/ha_innodb.cc Outdated
Comment thread sql/sql_select.cc
int join_init_read_record(JOIN_TAB *tab)
{
bool need_unpacking= FALSE;
/* TODO: s/tab->join/join/g in this function */
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.

What is this about?

Comment thread storage/innobase/handler/ha_innodb.cc Outdated
The three deadlock_*_race tests cannot reach their DEBUG_SYNC race
under a table lock (it degenerates to a timeout), and the three I_S
tests only restate a lock-mode change already covered
by innodb_full_scan.test.

- Addressed review comments
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.

7 participants