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

support strange joins #24

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

Conversation

MarinPostma
Copy link
Contributor

Sqite has a rather flexible syntax for joins, and the parser should support them. This PR alleviate the restriction on joins, and make them more flexible.

see: https://www.sqlite.org/lang_select.html#strange_join_names

@psarna
Copy link

psarna commented Mar 7, 2023

@MarinPostma #23 (comment) it looks like weird joins are fine on their own, looks like we hit another kind of issue

@MarinPostma
Copy link
Contributor Author

No they aren't, for example, this is not supported by the parser, but should:
SELECT * FROM foo LEFT NATURAL JOIN bar

@gwenn
Copy link
Owner

gwenn commented Mar 8, 2023

I am going to try fixing #23 first.
https://www.sqlite.org/lang_select.html#strange_join_names

Remember: you can use these non-standard join types but you ought not. Stick to using standard JOIN syntax for portability with other SQL database engines.

And I am not sure we want to support these non-standard join types (like Double-Quoted String).

@gwenn
Copy link
Owner

gwenn commented Mar 12, 2023

It seems that the actual rules are here:
https://github.com/sqlite/sqlite/blob/80511f32f7e71062026edd471913ef0455563964/src/select.c#L197-L257

The only restrictions on the join type name are:

  • "INNER" cannot appear together with "OUTER", "LEFT", "RIGHT",
    or "FULL".

  • "CROSS" cannot appear together with "OUTER", "LEFT", "RIGHT,
    or "FULL".

  • If "OUTER" is present then there must also be one of
    "LEFT", "RIGHT", or "FULL"

@MarinPostma
Copy link
Contributor Author

I'll implement that 👍

@gwenn gwenn added the duplicate label May 2, 2023
@gwenn
Copy link
Owner

gwenn commented Dec 3, 2023

See #37

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants