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

add support for sqlite RETURNING clause #1312

Closed
wants to merge 1 commit into from

Conversation

elv-gilles
Copy link

This PR adds support for sqlite RETURNING clause.

Requests involving the returning clause must include a boolean field before the SQL statement (see ParseRequest in http/request_parser.go).

@otoolep
Copy link
Member

otoolep commented Jun 20, 2023

Thanks for your contribution, however this is not the way to solve this. Instead the SQL parser that rqlite uses should be updated to detect when a SQL statement contains RETURNING and then rqlite should automatically do the right thing. That is my plan anyway.

The SQL parser is here: https://github.com/rqlite/sql

It would that code that needs support added for detecting RETURNING.

@otoolep
Copy link
Member

otoolep commented Jun 20, 2023

Unless I am missing something that will prevent RETURNING support being added to the SQL parser, I do not plan to merge this PR.

@otoolep
Copy link
Member

otoolep commented Jun 20, 2023

Thanks again for your contribution. If you'd instead like to add support to the SQL parser Go code, I would be interested in a such a PR. Basically some way that the parser could tell you RETURNING was in a given statement, allowing new logic to kick in in rqlite.

@otoolep otoolep closed this Jun 20, 2023
@otoolep
Copy link
Member

otoolep commented Jun 20, 2023

I would suggest you propose some of your ideas first though (usually as a GitHub issue), as I wouldn't like folks to spend a long time coding something, only to find out later that it didn't make sense when created as a PR.

@elv-gilles
Copy link
Author

OK, no problem: I missed the SQL parser.

I agree this is a better solution and am looking into the parser.

@elv-gilles elv-gilles deleted the support-sqlite-returning branch June 20, 2023 16:27
@elv-gilles
Copy link
Author

I implemented support in the sql parser (rqlite/sql#9), which unfortunately seems to have an issue (rqlite/sql#10).

For the time being, I'll use what I proposed in this PR (from fork)

@elv-gilles
Copy link
Author

Notes:

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.

2 participants