Skip to content

MDEV-30297 Server crash / assertion failure in Compare_identifiers::operator upon dropping period with empty name#5255

Open
pranavktiwari wants to merge 1 commit into
10.11from
10.11-MDEV-30297
Open

MDEV-30297 Server crash / assertion failure in Compare_identifiers::operator upon dropping period with empty name#5255
pranavktiwari wants to merge 1 commit into
10.11from
10.11-MDEV-30297

Conversation

@pranavktiwari

@pranavktiwari pranavktiwari commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

fixes MDEV-30297

Problem:

`ALTER TABLE ... DROP PERIOD IF EXISTS FOR ``` can trigger an assertion failure in debug builds and crash in non-debug builds.

Cause:

Lex_cstring_with_compare::streq() may invoke Compare_identifiers on default-constructed Lex_cstring objects (str == NULL, length == 0). Compare_identifiers assumes non-null string pointers and asserts or crashes when called with a null pointer.

Fix:

Add null-pointer handling to Lex_cstring_with_compare::streq() and avoid calling Compare_identifiers when either string pointer is null. This prevents invalid comparisons while preserving existing behavior for initialized strings.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request addresses a server crash (MDEV-30297) when dropping a period with an empty name by adding null-pointer and length checks to Lex_cstring_with_compare::streq. A test case is also included to verify the fix. The review feedback suggests optimizing the comparison logic in streq by checking for pointer equality first to provide a fast-path return.

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.

Comment thread sql/vers_string.h

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes a crash/assertion failure when dropping a PERIOD with an empty name by making identifier string equality checks safe for default-constructed (NULL) lex strings, and adds a regression test for the reported case (MDEV-30297).

Changes:

  • Add NULL-pointer handling in Lex_cstring_with_compare::streq() to avoid calling Compare_identifiers (and similar comparators) with NULL str pointers.
  • Add a mysql-test regression case for `ALTER TABLE ... DROP PERIOD IF EXISTS FOR ```.
  • Update the expected test output to include the resulting warning.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
sql/vers_string.h Makes streq() robust against NULL str pointers to prevent comparator crashes/asserts.
mysql-test/suite/period/t/alter.test Adds a regression test for dropping a PERIOD with an empty name.
mysql-test/suite/period/r/alter.result Captures the expected warning output for the new test case.

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

Comment thread sql/vers_string.h

@sanja-byelkin sanja-byelkin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please fix commit comment as we agreed and look on copilot complains. OK to push after fixing

…perator upon dropping period with empty name

Lex_cstring_with_compare::streq() could invoke Compare_identifiers on default-constructed Lex_cstring objects (str == NULL, length == 0). Compare_identifiers assumes non-null strings and asserts in debug builds or crashes in non-debug builds. Guard against null string pointers in streq() before invoking the comparator.
@pranavktiwari pranavktiwari enabled auto-merge (rebase) June 18, 2026 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants