-
-
Notifications
You must be signed in to change notification settings - Fork 110
Fix #592 - desc index #636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
...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
left a comment
There was a problem hiding this 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
commented
Nov 26, 2025
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:
ASCDESC- lowercase
desc - two
DESCcolumns
All tests pass with the current implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is test files missing here
Also, maybe add an col1 ASC, col2 DESC test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your reply and Happy New Year!
I have linked the files and added an Asc-Desc and Desc-Asc test.
Best regards
Description
This PR fixes incorrect parsing of
DESCinside index definitions inALTER TABLEstatements.Example:
This change updates AlterOperation::parse() so that ASC and DESC
are not interpreted as new statements within index definitions.
Fixes #592.