Skip to content

Conversation

@predictor2718
Copy link

Description

This PR fixes incorrect parsing of DESC inside index definitions in
ALTER TABLE statements.

Example:

ALTER TABLE `t`
  ADD UNIQUE KEY `idx` (`a`, `b` DESC);

This change updates AlterOperation::parse() so that ASC and DESC
are not interpreted as new statements within index definitions.

Fixes #592.

…admin#592)

Signed-off-by: Nicolai Ehrhardt <245527909+predictor2718@users.noreply.github.com>
Signed-off-by: Nicolai Ehrhardt <245527909+predictor2718@users.noreply.github.com>
@williamdes williamdes changed the title Fix 592 desc index Fix #592 - desc index Nov 25, 2025
Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

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

Thank you for the PR, I am not too sure about using the in array check
Would DeSC still trigger this ?

Signed-off-by: Nicolai Ehrhardt <245527909+predictor2718@users.noreply.github.com>
Signed-off-by: Nicolai Ehrhardt <245527909+predictor2718@users.noreply.github.com>
@predictor2718
Copy link
Author

Thank you for the review!

Regarding the question about whether DeSC (or other mixed-case variants) would still trigger the check:

The parser normalizes keywords during lexing.
Lexer::parseKeyword() internally calls Context::isKeyword(), which performs a strtoupper() on the token value.
Because of this, any case variation like DESC, Desc, DeSC, etc. is always mapped to the uppercase keyword DESC.

So the condition is case-insensitive and mixed-case inputs won’t cause issues.

To be safe, I also added additional tests covering:

  • ASC
  • DESC
  • lowercase desc
  • two DESC columns

All tests pass with the current implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants