Skip to content
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

fix: parse multiple statements with embedded semicolon #85

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

piotrkira
Copy link

@piotrkira piotrkira commented Apr 28, 2023

Description

Sqlparser is not maintained anymore, as it was used just for this simple thing I've decided to re implement logic of spiting statements into one simple function.

Related Issues

@piotrkira piotrkira force-pushed the fix_parse_multiple_statements branch from a508329 to 29ee493 Compare April 28, 2023 15:35
@haaawk
Copy link
Collaborator

haaawk commented Apr 29, 2023

Thank you for your contribution @piotrkira. Could you please add tests that verify the code works as intended?

@piotrkira
Copy link
Author

piotrkira commented Apr 29, 2023

Thank you for your contribution @piotrkira. Could you please add tests that verify the code works as intended?

Good idea! Btw current tests already cover that functionality, not directly tho. So I'll add some tests.

Copy link

@luisfvieirasilva luisfvieirasilva left a comment

Choose a reason for hiding this comment

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

Thanks @piotrkira for the PR!
Just a few comments

  1. Looks like the code fails when we have a quote inside another one. For instance, something like select "banana'"; select 1
  2. The issue that you tagged is related to the PR but it's not exactly the same problem. It aims to fix function appendStatementPartAndExecuteIfFinished, implementing a similar logic that you used here 😄

if embedded['\''] || embedded['"'] || char != ';' {
continue
}
stmt = statementsString[stmtBegin : i]

Choose a reason for hiding this comment

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

since we're keeping the semicolumn (;) for the last statement, maybe we should keep for other ones as well. What do you think?

Suggested change
stmt = statementsString[stmtBegin : i]
stmt = statementsString[stmtBegin : i+1]

@haaawk
Copy link
Collaborator

haaawk commented May 9, 2023

can we use our antlr parser instead @luisfvieirasilva ?

@piotrkira piotrkira force-pushed the fix_parse_multiple_statements branch from 29ee493 to 593db83 Compare May 12, 2023 19:23
@piotrkira
Copy link
Author

  1. Looks like the code fails when we have a quote inside another one. For instance, something like select "banana'"; select 1

Thanks, such cases are covered now.

@piotrkira piotrkira force-pushed the fix_parse_multiple_statements branch from 593db83 to 0ad5e0a Compare May 12, 2023 19:44
Sqlparser is not maintained anymore, as it was used just for this simple
thing I've decided to re implement logic of spiting statements into one
simple function.

Related-to: tursodatabase#50
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.

Parse multi-line statements correctly
3 participants