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

Insert and InsertLegacy Incompatibility #1063

Closed
johnedquinn opened this issue May 4, 2023 · 0 comments · Fixed by #1065
Closed

Insert and InsertLegacy Incompatibility #1063

johnedquinn opened this issue May 4, 2023 · 0 comments · Fixed by #1065
Labels
bug Something isn't working

Comments

@johnedquinn
Copy link
Member

johnedquinn commented May 4, 2023

Original Comment

@dlurton -- regarding the query for reproducing the issue. It seems that this actually has to do with the mixing-and-matching of the "legacy" INSERT command with the "current" ON CONFLICT clause. The grammar says:

insertCommand
    : INSERT INTO pathSimple VALUE value=expr ( AT pos=expr )? onConflictClause?  # InsertLegacy
    | INSERT INTO symbolPrimitive asIdent? value=expr onConflictClause?           # Insert
    ;

onConflictClause
    : ON CONFLICT WHERE expr DO NOTHING                                           # OnConflictLegacy
    | ON CONFLICT conflictTarget? conflictAction                                  # OnConflict
    ;

While the grammar says we can mix-and-match the "legacy" and "current" commands/clauses, we actually don't allow it in the visitor. Ideally, this should be expressed more explicitly within our grammar. I spoke to @am357 , and it seems that we are maintaining the "legacy" insert and "legacy" on-conflict clauses for backward-compatibility.

Anyways, this issue is present in main, and I'll leave it out of the scope of this PR.

Originally posted by @johnedquinn in #1061 (comment)

Further Clarification

The ANTLR grammar currently supports mixing and matching of the insert, insertLegacy, onConflict, and onConflictLegacy rules. We should only be supporting:

  • insert with onConflict
  • insertLegacy with onConflictLegacy

Proposed Solution

  • We need to modify this in the grammar
  • We potentially need to update the AST, but I haven't looked into this very much.

Definition of Done

  • The inability to parse the mix-and-match.
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.

1 participant