-
Notifications
You must be signed in to change notification settings - Fork 887
Handle convertPatternInExpr. #1088
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
Conversation
This can be handled just by calling convert on the subquery, and it is necessary for some kinds of queries. Like the following, without this we don't pickup the parameter, and thus can't handle the query at all: select * from foo where field in (select substring(?, 1, seq) from seq_1_to_19)
This is used for queries like the following: SELECT a, b from foo where foo.a in (select a from bar where bar.b = ?); SELECT a, b from foo where foo.a in (?, ?); SELECT a, b from foo where foo.a not in (?, ?); SELECT a, b from foo where ? in (foo.a, foo.b); The support is heavily based on that found in the fork found at: https://github.com/xiazemin/sqlc.git However it should be noted that this is not just a straight copy and paste job, there are several pieces from that branch for features that don't exist in the main sqlc repository, the sqlc repository has changed a bit since the fork, and the fork version does not handle the latter case at all.
This is used for queries like the following: SELECT a, b from foo where foo.a in (select a from bar where bar.b = ?); SELECT a, b from foo where foo.a in (?, ?); SELECT a, b from foo where foo.a not in (?, ?); SELECT a, b from foo where ? in (foo.a, foo.b); The support is heavily based on that found in the fork found at: https://github.com/xiazemin/sqlc.git However it should be noted that this is not just a straight copy and paste job, there are several pieces from that branch for features that don't exist in the main sqlc repository, the sqlc repository has changed a bit since the fork, and the fork version does not handle the latter case at all.
This covers all the cases I can think of, though if I missed some, it's quite possible that they are broken in the code as well as absent in the tests.
@asterikx pinging you on this one given the conversation on the previous merge request. |
Well, oops. Please hold off on reviewing this, the code it generates won't compile, as noted in the pipeline checks. I'll take a crack at resolving that sometime in the next week, I hope. |
That is, we want to deduplicate the same column from showing up multiple times, and thus we want to avoid adding a suffix, when we are getting the results from a query, however we most definitely do not want to do this for query parameters. Especially since the logic for handling this in the query parameters is missing the deduplication, but also because the database itself expects the same number of parameters as we have placeholders in the query.
fieldName is after any mangilng to make it fit go variable standards, and since that is the name that we are actually using, that is the name that we should be checking for conflicts. This only matters when we have multiple fields that differ only by things that are changed when we are converting to the go variable standards.
This should now build a little better, now that we have fixed the generator.
Oops, this fixes a bug introduced in 885bd3d.
Needed now that we're merging in master.
Alright, so a bug in the golang code generator has been fixed, I'm not sure how easy it would have been to trigger this previously, but it is far too easy to trigger it with this change set. And after merging the code with master, a logical conflict (lack of support for DefaultSchema in the pre-merge code) has been fixed. At this point I think this should be good to go. |
@zelch This code is now out of date with |
I can definitely get it back up to date with main. As far as Postgres tests, I'm afraid that I only implemented support for dolphin. Adding support for the postgresql engine would definitely be good to do, but I'm not sure that I'm going to have time in the immediate future to tackle it. |
Great, let's get it updated and then we can merge in support as-is. |
Resolved a reasonably trivial merge conflict.
Done, and it is passing all tests. |
This is a second attempt at #976 , but now with support for Expr, List, and Not in addition to Sel.
This is used for queries like the following:
SELECT * from foo where field in (select substring(?, 1, seq) from seq_1_to_19);
SELECT a, b from foo where foo.a in (select a from bar where bar.b = ?);
SELECT a, b from foo where foo.a in (?, ?);
SELECT a, b from foo where foo.a not in (?, ?);
SELECT a, b from foo where ? in (foo.a, foo.b);
The support is heavily based on that found in the fork found at:
https://github.com/xiazemin/sqlc.git
However it should be noted that this is not just a straight copy and
paste job, there are several pieces from that branch for features that
don't exist in the main sqlc repository, the sqlc repository has changed
a bit since the fork, and the fork version does not handle the latter
case at all.