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

bug: split sql wrongly like INSERT INTO timestamp VALUES ('1900-1-1 00;00;00'); #68

Closed
discord9 opened this issue Jun 21, 2024 · 6 comments · Fixed by #69
Closed

bug: split sql wrongly like INSERT INTO timestamp VALUES ('1900-1-1 00;00;00'); #68

discord9 opened this issue Jun 21, 2024 · 6 comments · Fixed by #69
Labels
bug Something isn't working

Comments

@discord9
Copy link
Contributor

discord9 commented Jun 21, 2024

Describe this problem

Current version of sqlness parse sql wrongly and will split this sql into three separate ones:

INSERT INTO timestamp VALUES ('1900-1-1 00;00;00');

This is due to this place simply split sql by ; instead of checking it semantic meaning:

for sql in sql.split(QUERY_DELIMITER) {

// An intercetor may generate multiple SQLs, so we need to split them.
        for sql in sql.split(QUERY_DELIMITER) {

Steps to reproduce

CREATE TABLE timestamp(t TIMESTAMP time index);

INSERT INTO timestamp VALUES ('1900-1-1 00;00;00');

will make sqlness think the last sql is three sql:

INSERT INTO timestamp VALUES ('1900-1-1 00;
00;
00');

Expected behavior

not splitting sql

Additional Information

No response

@discord9 discord9 added the bug Something isn't working label Jun 21, 2024
@waynexia
Copy link
Member

Looks like this regression was introduced in #63 cc @jiacai2050

We need to make a new release after this is fixed. Maybe 0.6.1? If we don't class #67 as a breaking change.

@jiacai2050
Copy link
Member

Why you use ; in time string, shouldn't it be :?

@discord9
Copy link
Contributor Author

Why you use ; in time string, shouldn't it be :?

It's a test for parsing incorrect timestamp, so many corner cases for handling timestamp are covered

@jiacai2050
Copy link
Member

Got it.

I already submit a PR fix this, will make a release today.

@jiacai2050
Copy link
Member

Here it is.

@waynexia
Copy link
Member

Thank you @jiacai2050 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants