Conversation
6c971f3 to
3194b8e
Compare
|
@javiercbk I would like to help out on getting at least a little bit better sqlite support for sqlc. I cloned your branch and am running it locally and it does work, at least for simple things. For anything that I find that I can fix, would you like me to make a PR to your fork of the sqlc repo? |
|
@javiercbk this PR seems pretty valuable even if it doesn't support omitting the column names. From what I can tell the tests are failing because of a version mismatch in the testdata file headers. Want to fix that and see if we can get Kyle to merge this? @DBarney I'm about to push up a small PR with some improvements to SQlite + SELECT. I'm just working through things in the order I run into use-cases, but maybe its worth collaborating to make sure we don't duplicate work? |
|
@russellhaering that sounds good to me. And that’s basically what I’m doing too. I have a set of Postgresql queries that I am switching to sqlite, and as I run into issues I was going to fix them add them to the PR. |
|
@javiercbk @russellhaering I just noticed I already did duplicate work in #1687 I checked quickly and found my PR covers both this PR ( What do you think? |
3194b8e to
1623935
Compare
|
I'm going to fix that failing test right now. I think I've commited something I was working on by accident (insert with select and some values) |
|
@hakobera dude, I've just seen your comment...if your PR covers all that, then by all means lets go with your solution. |
|
@hakobera I'll checkout your PR and will do some testing. Using your work I would try to implement some missing operations |
I'm adding BASIC insert support for sqlite.
New feature for sqlite:
Previously
sqlcis unable to generate inserts for sqlite because it was simply returningast.TODO.I'm implementing a basic support for inserts although "some gymnastics" are required to work with the parser.
The main problem is that the parser gives a list of expressions so, take the following example:
In this case the parser would give you a slice of 4 bind expression, the ONLY way to know how to generate the
ast.Listrequired is to have the columns explicit in the query.Currently, it seems impossible to generate the correct instruction for the following sql
In this case, we are screwed. Values are not "grouped" in any meaningful way. This can be solved by a quick "hack" but I really didn't dive deep in the parser as well to go with the hack approach. This is the reason why we are forced to explicitly put the columns on the insert. You would get a parser error if you want to do the query above although sqlite has no problem with this type of update.
I want to support INSERT with SELECT but that would be another kind of beast to tame. In the mean time, basic INSERT support is all that I needed in my project.
I'm open to comments, suggestions, questions and concerns.