join and select now accept both tables and subquery aliases - #19
Merged
Merged
Conversation
royneary
marked this pull request as draft
April 28, 2026 20:13
royneary
force-pushed
the
subquery-aliases
branch
from
May 14, 2026 11:34
baa718b to
077d4f7
Compare
This reverts commit cbb7453.
- add Query.as_ - replace table_name with table_ref in the public API - join and select accept a [< `TABLE | `SUBQUERY] table_ref - all other functions require a [`TABLE] table_ref
royneary
force-pushed
the
subquery-aliases
branch
from
May 14, 2026 11:40
077d4f7 to
60341fa
Compare
Contributor
Author
|
I resolved the FIXME and added tests. I also renamed the test binaries for consistency reasons (that's why suddenly there are 43 changed files). |
royneary
marked this pull request as ready for review
May 14, 2026 11:45
Owner
|
Oh, nice, thanks Christian, this looks great! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi Kiran,
thank you for your reply here. I actually implemented something along those lines before you replied and I'm putting it here to start a discussion.
My main motivation is to fix the issues mentioned in #3, caused by the automatic subquery aliases in the current implementation of
Query.join. I also like the flexibility of joining with a table directly, as proposed in #11. I think joining with a table is a more common use case than joining with a subquery.I implemented
Query.as_to assign an alias to a select query. I think it is similar to thenamed (<sql-query>) ~_as:"..."function that you proposed. It does not return aQuery.tthough, but instead a table reference and a list of expressions, just likeStaticSchema.declare_tabledoes. I then changedQuery.jointo accept a table reference instead of a query.Given the following tables
this allows writing the following queries:
The most common use case (joining with a table) is now possible and it's very concise whereas the "join with a subquery" use case is longer, but less confusing than before because every subquery and the fields it returns are now only accessible through explicit aliases.
This required replacing the
table_nametype in the API with a parameterized typetable_ref. It exists in two variants:[`TABLE] table_refrepresents physical tables[`SUBQUERY`] table_refrepresents subqueriesEither variant can be used as an argument of
joinand the~fromparameter ofselect. All other functions require the first variant.I'm not fixed on names by the way. For example I would be fine with naming the
as_functionnamedlike you proposed, I just saw thatExpr.as_already exists and tried to be consistent.So what do you think? This is definitely a breaking change. Maybe I overlooked a simpler route as I'm just starting to get familiar with the petrol code base.
There is still a FIXME remaining and no tests yet. But that should be the least problem once we know the general direction.Fixes #3.
Requires #18.