MDEV-40167 - GTT created with the InnoDB incorrectly accept FULLTEXT/VECTOR indexes#5318
MDEV-40167 - GTT created with the InnoDB incorrectly accept FULLTEXT/VECTOR indexes#5318pranavktiwari wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 DIRECTORYand 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.testonly coversCREATE TEMPORARY TABLE). Adding a regression test forCREATE 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
left a comment
FooBarrior
left a comment
There was a problem hiding this comment.
Overall looks good, just requesting a little bit of style fixes.
Pressing Approve, but please fix those before pushing
| /* 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() |
There was a problem hiding this comment.
Maybe:
fmt = "InnoDB: DATA DIRECTORY cannot be used for %s tables."
push_warning(..., fmt, m_create_info->global_tmp_table() ? "GLOBAL TEMPORARY" : "TEMPORARY");
| /* 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). */ |
There was a problem hiding this comment.
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.
|
Add MDEV in both commit message and PR title, squash the commits and remove DRAFT |
a1ec4fc to
76f3413
Compare
pranavktiwari
changed the title
pranavktiwari
changed the title
ca2b64e to
4b28744
Compare
| 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"); |
There was a problem hiding this comment.
I'm afraid this line is longer than 80
1cf021a to
d50659b
Compare
d50659b to
10bb8cc
Compare
…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.
10bb8cc to
a2a100a
Compare
fixes MDEV-40167
Problem:
GLOBAL TEMPORARYtables were not subject to the same option/index restrictions as sessionTEMPORARYtables. Several InnoDB and server-layer checks tested onlytmp_table(), soGLOBAL TEMPORARYtables could bypass validation for VECTOR/FULLTEXT indexes,DATA DIRECTORY,KEY_BLOCK_SIZE, andROW_FORMAT=COMPRESSED.Cause:
global_tmp_table()was added as a separate predicate fromtmp_table(), but not all temp-table checks were updated to test both, soGLOBAL TEMPORARYtables fell through to "permanent table" logic in several places.Fix:
Added
global_tmp_table()alongsidetmp_table()at each affected check:GLOBAL TEMPORARYtables.DATA DIRECTORY,KEY_BLOCK_SIZE, andROW_FORMAT=COMPRESSEDforGLOBAL TEMPORARYtables, with accurate wording in the DATA DIRECTORY warning.zip_allowedand relatedut_adassertions to excludeGLOBAL TEMPORARYtables.m_use_file_per_tableinset_tablespace_type()to excludeGLOBAL TEMPORARYtables (also fixesm_use_data_dir).GLOBAL TEMPORARYtables now validate the same as sessionTEMPORARYtables across these options.