MDEV-37493 I_S.ROUTINES stopped matching db name case-insensitively with lower_case_table_names=2#5347
MDEV-37493 I_S.ROUTINES stopped matching db name case-insensitively with lower_case_table_names=2#5347vaintroub wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes INFORMATION_SCHEMA lookup behavior when lower_case_table_names=2, restoring case-insensitive matching for ROUTINES (and related lookups) while still presenting schema/table names in INFORMATION_SCHEMA using the on-disk “true” letter case by resolving canonical names from the filesystem.
Changes:
- Lowercase I_S lookup values for any non-zero
lower_case_table_namesto restore correct index lookups (notably formysql.proc). - Add
my_canonical_case_name()and use it to resolve “true-case” db/table names for pinned lookups underlower_case_table_names=2. - Add new mysql-test coverage for true-case display and the ROUTINES case-insensitive regression (MDEV-37493).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| sql/sql_show.cc | Restores lowercasing for non-zero lower_case_table_names and resolves canonical db/table names for true-case output under lower_case_table_names=2. |
| mysys/my_lib.c | Adds my_canonical_case_name() implementation (Windows/macOS/other fallback). |
| include/my_sys.h | Exposes my_canonical_case_name() declaration for use outside mysys. |
| mysql-test/main/lowercase_truename.test | New test validating true-case display for SCHEMATA/TABLES under lower_case_table_names=2. |
| mysql-test/main/lowercase_truename.result | Expected output for the new true-case test. |
| mysql-test/main/lowercase_truename.opt | Runs the true-case test with --lower-case-table-names=2. |
| mysql-test/main/lowercase_routines.test | New regression test ensuring ROUTINES/PARAMETERS schema filtering matches case-insensitively again. |
| mysql-test/main/lowercase_routines.result | Expected output for the new routines regression test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #elif defined(__APPLE__) | ||
| struct attrlist attr; | ||
| struct | ||
| { | ||
| u_int32_t size; | ||
| attrreference_t name; | ||
| /* a file name is at most NAME_MAX UTF-16 units, 3 UTF-8 bytes each */ | ||
| char buf[NAME_MAX * 3 + 1]; | ||
| } info; | ||
| bzero((char*) &attr, sizeof(attr)); | ||
| attr.bitmapcount= ATTR_BIT_MAP_COUNT; | ||
| attr.commonattr= ATTR_CMN_NAME; | ||
| if (getattrlist(path, &attr, &info, sizeof(info), 0)) | ||
| return 0; | ||
| return (size_t) (strmake(to, (char*) &info.name + info.name.attr_dataoffset, | ||
| to_size - 1) - to); | ||
| #else |
There was a problem hiding this comment.
Per getattrlist(2), ATTR_CMN_NAME is a NUL-terminated UTF-8 C string (attr_length includes the NUL), so strmake() cannot over-read here.
| size_t len= build_table_filename(path, sizeof(path) - 1, | ||
| lookup_field_vals->db_value.str, | ||
| "", "", 0); | ||
| path[len - 1]= 0; // remove ending '\' | ||
| if (my_canonical_case_name(path, fname, sizeof(fname))) |
There was a problem hiding this comment.
build_table_filename() always emits the datadir prefix, so the length cannot be 0. Same pattern as verify_database_directory_exists().
![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 implements support for displaying the true on-disk letter case of database and table names in Information Schema queries when lower_case_table_names=2 is enabled. It introduces a utility function my_canonical_case_name to resolve canonical path casing across different platforms and integrates it into sql/sql_show.cc. Feedback on these changes highlights a potential buffer underflow and line-continuation comment issue in sql_show.cc, the use of the deprecated bzero function in my_lib.c, and a case-matching priority issue on case-sensitive filesystems within the directory scanning fallback.
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.
| size_t len= build_table_filename(path, sizeof(path) - 1, | ||
| lookup_field_vals->db_value.str, | ||
| "", "", 0); | ||
| path[len - 1]= 0; // remove ending '\' | ||
| if (my_canonical_case_name(path, fname, sizeof(fname))) | ||
| { | ||
| LEX_CSTRING *name; | ||
| uint name_len= filename_to_tablename(fname, name_buff, | ||
| sizeof(name_buff)); | ||
| if (!(name= thd->make_clex_string(name_buff, name_len)) || | ||
| files->append(name)) | ||
| return 1; | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
If build_table_filename returns 0 (e.g., due to an empty database name or an error), accessing path[len - 1] will result in a buffer underflow (out-of-bounds write at path[-1]). We should defensively check that len > 0 before modifying the buffer.
Additionally, the comment // remove ending '\' ends with a backslash. In C/C++, a trailing backslash at the end of a single-line comment acts as a line continuation character, which can comment out the subsequent if statement. We should change the comment to avoid ending with a backslash.
size_t len= build_table_filename(path, sizeof(path) - 1,
lookup_field_vals->db_value.str,
"", "", 0);
if (len > 0)
{
path[len - 1]= 0; // remove ending separator
if (my_canonical_case_name(path, fname, sizeof(fname)))
{
LEX_CSTRING *name;
uint name_len= filename_to_tablename(fname, name_buff,
sizeof(name_buff));
if (!(name= thd->make_clex_string(name_buff, name_len)) ||
files->append(name))
return 1;
return 0;
}
}There was a problem hiding this comment.
build_table_filename() always emits the datadir prefix, so the length cannot be 0 (same pattern as verify_database_directory_exists()). The comment ends with an apostrophe, not a backslash, so there is no line continuation.
| /* a file name is at most NAME_MAX UTF-16 units, 3 UTF-8 bytes each */ | ||
| char buf[NAME_MAX * 3 + 1]; | ||
| } info; | ||
| bzero((char*) &attr, sizeof(attr)); |
There was a problem hiding this comment.
bzero is deprecated in POSIX.1-2001 and removed in POSIX.1-2008. It is highly recommended to use standard memset instead for better portability and modern standards compliance.
memset(&attr, 0, sizeof(attr));There was a problem hiding this comment.
bzero() is the established style in this codebase, used throughout mysys and sql.
| while ((dp= readdir(dirp))) | ||
| { | ||
| if (!strcasecmp(dp->d_name, base)) | ||
| { | ||
| res= (size_t) (strmake(to, dp->d_name, to_size - 1) - to); | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
On case-sensitive filesystems, if both foobar and FooBar exist, and we call my_canonical_case_name with foobar, lstat will succeed because foobar exists. However, readdir might return FooBar first. Since strcasecmp("FooBar", "foobar") == 0, the function would return FooBar instead of the exact match foobar.
To make this robust on both case-sensitive and case-insensitive filesystems, we should prioritize an exact match (strcmp) over a case-insensitive match (strcasecmp). If an exact match is found, we can immediately break and return it. If only a case-insensitive match is found, we return that.
bool exact_match= false;
bool case_match= false;
while ((dp= readdir(dirp)))
{
if (!strcmp(dp->d_name, base))
{
res= (size_t) (strmake(to, dp->d_name, to_size - 1) - to);
exact_match= true;
break;
}
if (!case_match && !strcasecmp(dp->d_name, base))
{
res= (size_t) (strmake(to, dp->d_name, to_size - 1) - to);
case_match= true;
}
}There was a problem hiding this comment.
The function is documented to work on case insensitive file systems; the server only calls it with lower_case_table_names=2, which requires a case insensitive datadir.
…ith lower_case_table_names=2 Since MDEV-14432, get_lookup_field_values() lowercases the WHERE value only for lower_case_table_names=1, to preserve the letter case of table names in I_S output. But mysql.proc.db is utf8mb3_bin and always stores lowercase when lower_case_table_names is non-zero, so with lower_case_table_names=2 the index lookup in fill_schema_proc() missed. Fix: lowercase I_S lookup values for any non-zero lower_case_table_names again. To show db and table names in I_S in their on-disk letter case (MDEV-14432), resolve exactly-pinned names in make_db_list() and make_table_name_list() with a new mysys function my_canonical_case_name(), which returns the canonical name of the last path component. Unlike before, the true name is now shown even when the WHERE value uses a different letter case than the on-disk name.
vaintroub
force-pushed
the
10.11-MDEV-37493
branch
from
112c317 to
4104275
Compare
Since MDEV-14432, get_lookup_field_values() lowercases the WHERE value only for lower_case_table_names=1. But mysql.proc.db always stores lowercase when lower_case_table_names is non-zero, so with lower_case_table_names=2 the index lookup in fill_schema_proc() missed.
Fix: lowercase I_S lookup values for any non-zero lower_case_table_names again. To show db and table names in I_S in their on-disk letter case (MDEV-14432), resolve exactly-pinned names in make_db_list() and make_table_name_list() with a new mysys function my_canonical_case_name(). Unlike before, the true name is now shown even when the WHERE value uses a different letter case than the on-disk name.