Repository navigation
feat(join): read a join without an alias under its table ID - #1038
Merged
Merged
Conversation
0.7.2 reads a join given no alias under the collection it joins and exposes isAliasDefaulted(), which the join validator needs to explain why a table ID cannot serve as an alias. It also refuses a NaN or infinite distance when compiling a distance filter, so the Postgres builder test now expects that refusal instead of a compiled exact check for those three inputs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A join given no alias is now read under the ID of the table it joins, so rows come back keyed "<tableId>.<column>". Query's constructor applies the same default as the query library's factories and parser, so joins built directly read the same way. Self-joins need no alias: the main collection is always read under the reserved alias, never under its own ID. When the table ID can't be an alias, the query is refused with the reason, the ID, and a request for an alias. That covers an ID that isn't an identifier (Appwrite IDs may hold '-' and '.', and '.' would clash with alias.column), the reserved main alias, a relationship key, and an ID another join already uses (joining a table twice). An explicit alias keeps today's messages. The defaulted state comes from Query::isAliasDefaulted(), which is set when the query is built and kept through clone + setAttribute(). Query hooks rewrite the joined table (Appwrite swaps its public ID for the internal collection) before validation runs, so the alias and the error wording both stay on the ID the caller gave, never the internal name. An e2e test with a rewriting hook pins this. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 45 functions across 10 files. (1 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the primary change: joins without explicit aliases are read under their table ID. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 45 functions across 10 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
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
1 participant
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.
What
A join given no alias is now read under the ID of the table it joins. Rows come back keyed
"<tableId>.<column>". When that ID can't serve as an alias, the query is refused with a message that names the ID, gives the reason, and asks for an alias:my-orders,orders.v2: Appwrite IDs may hold-and., and.clashes withalias.column)Table ID "my-orders" cannot be used as the join alias: an alias must start with a letter or an underscore and contain only letters, digits and underscores. Pass an alias for the jointable_main, any case)Table ID "table_main" cannot be used as the join alias: it is reserved for the main collection. Pass an alias for the joinTable ID "customers" cannot be used as the join alias: it is the key of the relationship attribute "customers". Pass an alias for the joinTable ID "customers" cannot be used as the join alias: another join of the query already uses it as its alias. Pass an alias for the joinAn explicit alias keeps today's messages exactly (
Join alias must start with…,Join alias "x" is reserved…,…give the join another alias,Join alias "x" is declared more than once). The duplicate check stays inSQL::assertJoinAliases(), where it already lived, so its wording and status are unchanged.A self-join needs no alias. The main collection is always read under the reserved alias, never under its own ID, so the defaulted alias names only the joined copy.
isValidLeftColumn()keeps refusing a left column qualified by that ID. Both the validator and e2e tests pin this.How
query/0.7.2) defaults an empty alias in its factories and parser and addsQuery::isAliasDefaulted(). The flag is derived when the query is built (the alias equals the collection), so it survivestoArray()/parse()and a laterclone+setAttribute(). No database-specific rule lives there.Utopia\Database\Query::__construct()applies the same default, so joins built directly (new Query(Method::Join, 'orders', [...])) read the same as the factories.Validator\Query\Join::describeInvalidAlias(string $alias, bool $defaulted = false)and the newdescribeDuplicateAlias()choose the wording fromisAliasDefaulted(). The relationship-key refusal inisValidQuery()does the same.SQL::assertJoinAliases()passes the flag for the skip-validation path.Database::rewriteQueries()runs the query hooks before validation. Appwrite'sJoinshook swaps the public table ID fordatabase_<n>_collection_<seq>viaclone+setAttribute(), and the alias and the flag are fixed before that. So rows stay keyed by the public ID, and a refusal names the public ID, never the internal one.testJoinWithoutAliasKeepsTheTableIdAHookRewritespins this with a rewriting hook (tests/e2e/Adapter/Support/JoinTables.php).PostgresDistanceTestnow expects that refusal for those three inputs.Tests: fail before, pass after
"main + 0.7.1" is this PR's tests against
main'ssrc/with utopia-php/query 0.7.1. "main + 0.7.2" swaps in 0.7.2 only, which shows the database side is needed beyond the bump.E2E (
tests/e2e/Adapter/Scopes/JoinAliasTests.php, 9 tests):testJoinWithoutAliasIsReadUnderItsTableId(rows andjda_customers.namekeys;count(); parsed join)Join alias is required)testLeftJoinWithoutAliasIsReadUnderItsTableIdtestCrossJoinWithoutAliasIsReadUnderItsTableIdtestSelfJoinWithoutAliasIsReadUnderItsTableIdtestJoinWithoutAliasRefusesATableIdThatIsNotAnAlias(-and.)testJoinWithoutAliasRefusesTheReservedMainAliastestJoinWithoutAliasRefusesARelationshipKeytestJoiningATableTwiceWithoutAliasesIsRefusedtestJoinWithoutAliasKeepsTheTableIdAHookRewritesBy adapter, for those 9 tests:
Capability::Joins)Unit:
Validator\Query\JoinAliasTest(defaulted accepts, 8 refused table IDs, relationship key, self-join left/right columns, wording helpers)Join alias is required; missing methods)JoinAliasTest::testAJoinWithoutAnAliasIsReadUnderItsCollection(factory, constructor, parsed; with and without validation)JoinAliasTest::testJoiningACollectionTwiceWithoutAliasesNamesTheCollection(validator and skip-validation paths)JoinAliasTest::testCollidingAliasIsRejected*(2 new defaulted cases)QuerySerializationTestjoin-without-alias (5 parse layouts, round trips, built and renamed joins)Full local runs with this branch: unit
OK (13095 tests). E2E for MySQL, Postgres, MariaDB, SQLite, MongoDB, Memory, Pool and Redis pass in full. Pint and both PHPStan configs are clean.🤖 Generated with Claude Code
Summary by CodeRabbit