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

Minor query builder improvements #1087

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ccakes
Copy link
Contributor

@ccakes ccakes commented Feb 15, 2022

This PR implements the following:

  • Added queries.BuildSubquery() which does the same thing as BuildQuery() except doesn't append a semicolon so query builder results can be embedded in other queries
  • Added `qm.WithSubquery() which allows generating a CTE using a query object built by sqlboiler

The WithSubquery() modifier is interesting imo because if there's interest, there are a number of query mods that could be added to work with subqueries to allow building pretty complex, type-checked queries without horrible code.

@stephenafamo
Copy link
Collaborator

This is quite interesting and it makes complete sense. If everything checks out and it is completely backward compatible, it would make sense to merge now (or wait till v5 if not).

Just so you know, we are thinking of making the query builder more driver-specific, so that each driver could potentially support more specific things. This will likely happen in v5.

The base one will still exist for others to extend if they want so of course, this will still be useful.

@ccakes
Copy link
Contributor Author

ccakes commented Feb 15, 2022

This should be fully backwards compatible. If it's not, I'll make changes so that it is 👍

Just so you know, we are thinking of making the query builder more driver-specific, so that each driver could potentially support more specific things. This will likely happen in v5.

Interesting.. that makes sense. Looking forward to seeing what's coming!

@stephenafamo
Copy link
Collaborator

I just noticed that this will not be backward compatible since the function signature of AppendWith changed.

In its current state, we will only be able to merge it into v5.

@ccakes
Copy link
Contributor Author

ccakes commented Feb 17, 2022

At some point over the next few days I'll see if I can come up with a change here to resolve this, will comment back either way

@sn0rk64
Copy link

sn0rk64 commented Nov 1, 2022

Come on merge it please!

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

Successfully merging this pull request may close these issues.

3 participants