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

IN doesn't work with parentheses #207

Closed
asdine opened this issue Sep 27, 2020 · 2 comments · Fixed by #223
Closed

IN doesn't work with parentheses #207

asdine opened this issue Sep 27, 2020 · 2 comments · Fixed by #223
Labels
bug Something isn't working
Milestone

Comments

@asdine
Copy link
Collaborator

asdine commented Sep 27, 2020

genji> select 1 IN (1, 2, 3);
found ,, expected ) at line 1, char 15
genji>
sqlite> select 1 IN (1, 2, 3);
1
sqlite>

Other example:

SELECT * FROM foo WHERE a IN (1, 2, 3)

This is a valid SQL expression, but currently, we only support this:

SELECT * FROM foo WHERE a IN [1, 2, 3]

This might be more complex than just parsing parentheses when we are in an IN operator.

I think we need to parse parentheses in two different ways:

Single expression: (expr)

Example:

SELECT (1);
-> 1
SELECT (1 + 1);
-> 2

This is already supported, the parser returns an expr.Parentheses expression.

List of expressions: (expr, expr, ...)

Example:

SELECT (1, 2);
-> [1, 2]
SELECT (1 + 1, 3 > 5);
-> [2, false]

If we have more than one element in that list of parentheses, this must be parsed as an expr.LiteralExprList.
There is already a function that can parse this kind of expression: https://github.com/genjidb/genji/blob/9da693665fe93d3fcfdffe3312a48712fc40023f/sql/parser/expr.go#L418:18

@asdine asdine added the bug Something isn't working label Sep 27, 2020
@asdine asdine added this to the v0.9.0 milestone Sep 27, 2020
@asdine
Copy link
Collaborator Author

asdine commented Oct 6, 2020

@tdakkota I have updated the issue above, let me know if you have any question

@tdakkota
Copy link
Contributor

tdakkota commented Oct 6, 2020

I updated PR.

Just for record:
Current fix uses look ahead parsing, it determines type of expression by next token after first expression, so I added parseListUntil function.

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.

2 participants