-
Notifications
You must be signed in to change notification settings - Fork 99
fix orderBy field alias bug #637
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
Merged
Merged
Conversation
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
🦋 Changeset detectedLatest commit: 57007e6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
More templates
@tanstack/angular-db
@tanstack/db
@tanstack/db-ivm
@tanstack/electric-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
Size Change: -60 B (-0.08%) Total Size: 75.2 kB
ℹ️ View Unchanged
|
Size Change: 0 B Total Size: 1.47 kB ℹ️ View Unchanged
|
KyleAMathews
approved these changes
Oct 3, 2025
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.
fixes #610
Description of the fix from Claude:
Fix: orderBy breaks when collection alias matches schema field name
Problem
When a collection alias in the
from
clause had the same name as one of its schema fields,orderBy
would fail to sort correctly. For example:The query would run but produce unsorted results.
Root Cause
The
orderBy
compiler was constructing an evaluation context by spreading the row data and then__select_results
:When
__select_results
contained a field with the same name as a table alias (e.g., both hademail
), the SELECT result would overwrite the table object, breaking the orderBy expression evaluation.Solution
Simplified the context to use the namespaced row directly:
This works because:
row
(e.g.,row.email
is the table object)row.__select_results
(e.g.,row.__select_results.email
is the field value)replaceAggregatesByRefs
function already transforms aggregate references to use__select_results
namespace when neededThis prevents any conflict between table aliases and SELECT field names.
Additional Cleanup
Removed dead code in
replaceAggregatesByRefs
that attempted to support arbitrary SELECT alias references. The actual design pattern is aggregate matching: when you use an aggregate likecount(orders.id)
in HAVING or ORDER BY, it gets matched to the same aggregate in SELECT and replaced with a reference to the already-computed value. SELECT aliases themselves are never directly accessible in HAVING or ORDER BY callbacks.Tests
Added comprehensive test coverage for alias/field name conflicts in
order-by.test.ts
.