Skip to content

Conversation

yoavcloud
Copy link
Contributor

Some needed adjustments to the lookahead logic when considering the LIMIT keyword as an implicit table alias.

Keyword::LIMIT
if matches!(
parser.peek_token().token,
Token::Number(_, _) | Token::Placeholder(_)
Copy link
Contributor

Choose a reason for hiding this comment

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

from their docs they seem to mention that null and empty string, $ values are possible, could we also detect for those here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the great feedback! The latest commit introduces a more accurate lookahead logic.

@yoavcloud yoavcloud force-pushed the sf_limit_lookahead branch from 6e08364 to 7240269 Compare July 16, 2025 04:56
@yoavcloud yoavcloud requested a review from iffyio July 16, 2025 04:57
Copy link
Contributor

@iffyio iffyio left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @yoavcloud!
cc @alamb

@iffyio iffyio merged commit 92db206 into apache:main Jul 17, 2025
10 checks passed
@alamb
Copy link
Contributor

alamb commented Jul 17, 2025

It is pretty epic that this code keeps rolling along

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.

3 participants