MDEV-40167 - GTT created with the InnoDB incorrectly accept FULLTEXT/VECTOR indexes by pranavktiwari · Pull Request #5318 · MariaDB/server

pranavktiwari · 2026-07-01T12:22:20Z

Problem:

GLOBAL TEMPORARY tables were not subject to the same option/index restrictions as session TEMPORARY tables. Several InnoDB and server-layer checks tested only tmp_table(), so GLOBAL TEMPORARY tables could bypass validation for VECTOR/FULLTEXT indexes, DATA DIRECTORY, KEY_BLOCK_SIZE, and ROW_FORMAT=COMPRESSED.

Cause:

global_tmp_table() was added as a separate predicate from tmp_table(), but not all temp-table checks were updated to test both, so GLOBAL TEMPORARY tables fell through to "permanent table" logic in several places.

Fix:

Added global_tmp_table() alongside tmp_table() at each affected check:

Reject VECTOR and FULLTEXT indexes on GLOBAL TEMPORARY tables.
Reject/warn on DATA DIRECTORY, KEY_BLOCK_SIZE, and ROW_FORMAT=COMPRESSED for GLOBAL TEMPORARY tables, with accurate wording in the DATA DIRECTORY warning.
Fixed zip_allowed and related ut_ad assertions to exclude GLOBAL TEMPORARY tables.
Fixed m_use_file_per_table in set_tablespace_type() to exclude GLOBAL TEMPORARY tables (also fixes m_use_data_dir).

GLOBAL TEMPORARY tables now validate the same as session TEMPORARY tables across these options.

gemini-code-assist

Code Review

This pull request updates ha_innodb.cc to block FULLTEXT indexes on global temporary tables by checking is_global_temp. However, the review feedback points out that the implementation is incomplete, as it does not block VECTOR indexes on global temporary tables, which was also part of the pull request's objective.

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.

Copilot

Pull request overview

This PR tightens CREATE TABLE validation to treat GLOBAL TEMPORARY tables like TEMPORARY tables for certain InnoDB restrictions, addressing cases where InnoDB could incorrectly accept unsupported index/option combinations.

Changes:

Disallow DATA DIRECTORY and file-per-table-dependent options for GLOBAL TEMPORARY tables in InnoDB create option validation.
Reject FULLTEXT indexes on GLOBAL TEMPORARY tables in InnoDB table-flag derivation.
Reject VECTOR indexes on GLOBAL TEMPORARY tables during create-table finalization.

Reviewed changes

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

File	Description
storage/innobase/handler/ha_innodb.cc	Extends InnoDB create-option and FULLTEXT validation to include GLOBAL TEMPORARY tables.
sql/sql_table.cc	Extends VECTOR-index temporary-table restriction to also cover GLOBAL TEMPORARY tables.

Comments suppressed due to low confidence (1)

sql/sql_table.cc:3786

This change extends the VECTOR-index temporary-table restriction to GLOBAL TEMPORARY tables, but there doesn’t appear to be mysql-test coverage for the GLOBAL TEMPORARY case (e.g., existing mysql-test/main/vector.test only covers CREATE TEMPORARY TABLE). Adding a regression test for CREATE GLOBAL TEMPORARY TABLE ... VECTOR INDEX ... would help prevent future regressions.

        if (create_info->tmp_table() || create_info->global_tmp_table())
        {
          my_error(ER_NO_INDEX_ON_TEMPORARY, MYF(0), "VECTOR",
                   file->table_type());
          DBUG_RETURN(TRUE);
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

FooBarrior

Overall looks good, just requesting a little bit of style fixes.

Pressing Approve, but please fix those before pushing

FooBarrior · 2026-07-02T12:58:33Z

+        /* Do not use DATA DIRECTORY with TEMPORARY TABLE or
+        GLOBAL TEMPORARY TABLE. */
+        if (m_create_info->tmp_table() || m_create_info->global_tmp_table()) {
+		const char *msg = m_create_info->global_tmp_table()


Maybe:

fmt = "InnoDB: DATA DIRECTORY cannot be used for %s tables." push_warning(..., fmt, m_create_info->global_tmp_table() ? "GLOBAL TEMPORARY" : "TEMPORARY");

FooBarrior · 2026-07-02T13:01:32Z

-	/* Ignore the current innodb-file-per-table setting if we are
-	creating a temporary table. */
+        /* Ignore the current innodb-file-per-table setting if we are
+	creating a temporary table (session or global). */


AFAIK, term session temporary table doesn't exist. But looks pretty applicable, yes:)

We have either:

PRIVATE TEMPORARY TABLE (Oracle style)

LOCAL TEMPORARY TABLE (SQL Standard style and a wider use)

See, MDEV-35915 Section Documentation notes.

FooBarrior · 2026-07-02T13:13:44Z

Add MDEV in both commit message and PR title, squash the commits and remove DRAFT

FooBarrior · 2026-07-10T11:54:37Z

+                  m_thd, Sql_condition::WARN_LEVEL_WARN,
+                  ER_ILLEGAL_HA_CREATE_OPTION,
+                  "InnoDB: DATA DIRECTORY cannot be used for %s tables.",
+                  m_create_info->global_tmp_table() ? "GLOBAL TEMPORARY" : "TEMPORARY");


I'm afraid this line is longer than 80

FooBarrior · 2026-07-31T11:57:35Z

    share.db_plugin= ha_lock_engine(thd, create_info->db_type);

-    if (open_table_def(thd, &share))
+    if (open_table_def(thd, &share) || thd->killed)


Can you please explain, how thd->killed could be set here, while open_table_def returns 0? I guess it's set by memory overflow, right?

Yes, it is.

FooBarrior · 2026-07-31T11:58:23Z

                           &table_name.str, (size_t) table_name.length + 1,
                           &alias.str, (size_t) alias.length + 1,
-                           NullS))
+                           NullS) || thd->killed)


same. This definitely needs explanation. Does it mean that every alloc_root needs thd->killed check

it does.
I explained over chat. Pasting the same content here-

Commit 5673838 includes a test case that passes on all builds except MSN.
Reason for the failure:
In MSN builds, malloc is used for memory allocation. Once max_session_mem_used is set to 8192, executing any query causes the alloc_query methods to exceed the memory limit. As a result, thd->killed is set to KILL_QUERY through the my_malloc_size_cb_func callback. Because of this, even SET max_session_mem_used = DEFAULT fails.

The main question for me was: if setting the memory limit to 8192 causes these queries to fail on MSN, why don't they fail on the other builds as well?
While investigating, I found that in the other builds, when the test case queries are executed, thd->killed is also set to KILL_QUERY through my_malloc_size_cb_func. However, those queries continue to execute successfully instead of being terminated.

FooBarrior · 2026-07-31T12:00:25Z

 set max_session_mem_used= 8192;
 --error ER_OPTION_PREVENTS_STATEMENT
 insert t values (0);
+--error ER_OPTION_PREVENTS_STATEMENT


The test was about to check ALTER TABLE under locked table. Now the table won't be locked. Maybe raise the memory?

Based one the discussion we had over chat, I am deleting these test cases.

FooBarrior · 2026-07-31T12:00:44Z

 lock table t write;
 --error ER_OPTION_PREVENTS_STATEMENT
 alter table t add z int;
+--error 0, ER_OPTION_PREVENTS_STATEMENT


Why it's sometimes 0, and sometimes ER_OPTION_PREVENTS_STATEMENT?

Due to different build envoronment- MSAN and normal builds.

FooBarrior · 2026-07-31T12:10:16Z

    share.db_plugin= ha_lock_engine(thd, create_info->db_type);

-    if (open_table_def(thd, &share))
+    if (open_table_def(thd, &share) || thd->killed)


Can you please explain, how thd->killed could be set here, while open_table_def returns 0? I guess it's set by memory overflow, right?

FooBarrior · 2026-07-31T12:10:29Z

                           &table_name.str, (size_t) table_name.length + 1,
                           &alias.str, (size_t) alias.length + 1,
-                           NullS))
+                           NullS) || thd->killed)


same. This definitely needs explanation. Does it mean that every alloc_root needs thd->killed check?

FooBarrior · 2026-07-31T12:11:40Z

 set max_session_mem_used= 8192;
 --error ER_OPTION_PREVENTS_STATEMENT
 insert t values (0);
+--error ER_OPTION_PREVENTS_STATEMENT


The test was about to check ALTER TABLE under locked table. Now the table won't be locked. Maybe raise the memory limit?

FooBarrior · 2026-07-31T12:11:50Z

 lock table t write;
 --error ER_OPTION_PREVENTS_STATEMENT
 alter table t add z int;
+--error 0, ER_OPTION_PREVENTS_STATEMENT


Why it's sometimes 0, and sometimes ER_OPTION_PREVENTS_STATEMENT?

…ECTOR indexes Problem: GLOBAL TEMPORARY tables were not subject to the same option/index restrictions as session TEMPORARY tables. Several InnoDB and server-layer checks tested only tmp_table(), so GLOBAL TEMPORARY tables could bypass validation for VECTOR/FULLTEXT indexes, DATA DIRECTORY, KEY_BLOCK_SIZE, and ROW_FORMAT=COMPRESSED. Cause: global_tmp_table() was added as a separate predicate from tmp_table(), but not all temp-table checks were updated to test both, so GLOBAL TEMPORARY tables fell through to "permanent table" logic in several places. Fix: Added global_tmp_table() alongside tmp_table() at each affected check: Reject VECTOR and FULLTEXT indexes on GLOBAL TEMPORARY tables. Reject/warn on DATA DIRECTORY, KEY_BLOCK_SIZE, and ROW_FORMAT=COMPRESSED for GLOBAL TEMPORARY tables, with accurate wording in the DATA DIRECTORY warning. Fixed zip_allowed and related ut_ad assertions to exclude GLOBAL TEMPORARY tables. Fixed m_use_file_per_table in set_tablespace_type() to exclude GLOBAL TEMPORARY tables (also fixes m_use_data_dir). GLOBAL TEMPORARY tables now validate the same as session TEMPORARY tables across these options.

FooBarrior

Thanks, Pranav, that'll do.

Move the test removal into a separate commit. No MDEV for it is necessary IMO, or you can add the test's MDEV
amd64-ubuntu-2204-debug-ps is failing, and our MTR is the only one :

Failing test(s): binlog.global_temporary_table rpl.global_temporary_table main.global_temporary_table

likely unrelated again, but please check. If unrelated indeed, then ignore and push, but please make an MDEV about it and tell Sanja about it. That's the thing that needs to be fixed before the release anyway.

gemini-code-assist Bot reviewed Jul 1, 2026

View reviewed changes

Comment thread storage/innobase/handler/ha_innodb.cc

pranavktiwari requested review from Copilot July 1, 2026 15:06

Copilot started reviewing on behalf of pranavktiwari July 1, 2026 15:07 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

Comment thread storage/innobase/handler/ha_innodb.cc Outdated

Comment thread storage/innobase/handler/ha_innodb.cc Outdated

Comment thread storage/innobase/handler/ha_innodb.cc Outdated

gkodinov added the MariaDB Corporation label Jul 2, 2026

FooBarrior approved these changes Jul 2, 2026

View reviewed changes

pranavktiwari force-pushed the 13.1-MDEV-40167 branch from a1ec4fc to 76f3413 Compare July 2, 2026 13:56

pranavktiwari changed the title ~~DRAFT - GTT created with the InnoDB incorrectly accept FULLTEXT/VECTOR indexes~~ GTT created with the InnoDB incorrectly accept FULLTEXT/VECTOR indexes Jul 2, 2026

pranavktiwari changed the title ~~GTT created with the InnoDB incorrectly accept FULLTEXT/VECTOR indexes~~ MDEV-40167 - GTT created with the InnoDB incorrectly accept FULLTEXT/VECTOR indexes Jul 2, 2026

pranavktiwari force-pushed the 13.1-MDEV-40167 branch 6 times, most recently from ca2b64e to 4b28744 Compare July 7, 2026 05:04

FooBarrior requested changes Jul 10, 2026

View reviewed changes

pranavktiwari force-pushed the 13.1-MDEV-40167 branch 3 times, most recently from 1cf021a to d50659b Compare July 15, 2026 11:45

FooBarrior approved these changes Jul 15, 2026

View reviewed changes

pranavktiwari force-pushed the 13.1-MDEV-40167 branch 2 times, most recently from 10bb8cc to a2a100a Compare July 16, 2026 05:58

pranavktiwari force-pushed the 13.1-MDEV-40167 branch 5 times, most recently from aa6ce08 to 3ae5edd Compare July 27, 2026 07:48

FooBarrior requested changes Jul 31, 2026

View reviewed changes

pranavktiwari force-pushed the 13.1-MDEV-40167 branch 2 times, most recently from 2fb460d to dad5624 Compare August 3, 2026 06:38

pranavktiwari force-pushed the 13.1-MDEV-40167 branch from dad5624 to 22f9b6f Compare August 3, 2026 07:56

FooBarrior approved these changes Aug 3, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

pranavktiwari commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem:

Cause:

Fix:

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

FooBarrior left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FooBarrior commented Jul 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FooBarrior left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

pranavktiwari commented Jul 1, 2026 •

edited

Loading