You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
When a Restify repository has searchable BelongsTo relations, the RepositorySearchService adds LEFT JOINs:
SELECT posts.*FROM posts
LEFT JOIN users ONposts.user_id=users.id-- added by searchable BelongsToWHEREUPPER(users.name) LIKE'%john%'ORDER BY id ASC-- AMBIGUOUS! Both posts.id and users.id exist
MySQL throws: Column 'id' in order clause is ambiguous
The Fix: Two changes in Restify
1. SortCollection::qualifyColumns(Model $model) — mirrors the existing SearchablesCollection::qualifyColumns()
Iterates sort filters and prefixes unqualified columns with the model's table name:
id → posts.id
created_at → posts.created_at
users.name → left as-is (already has a dot)
2. RepositorySearchService::prepareOrders() — calls qualifyColumns() before apply()
This is the only place where sorts hit the database. The call happens AFTER collectSorts() (which does normalization, hydration, authorization) and BEFORE apply() (which executes the ORDER BY).
Why NOT other locations?
Why not fix in SortableFilter::filter() at line 61?
$query->orderBy($this->column, $value); // Could qualify here instead
This would avoid mutating $this->column, but:
It's inconsistent with how searchables work (they qualify at the collection level)
Other code paths reading $this->column during apply() would still get unqualified names
The searchables pattern (SearchablesCollection::qualifyColumns()) is already proven and understood in this codebase
Why not fix in InteractWithSearch::collectSorts()?
Repository sort keys are unqualified ('id', 'date'). If we qualify before this step, posts.id !== id and the sort gets filtered out.
prepareOrders() is the correct location — after the full collectSorts() pipeline resolves everything, but before apply() hits the database.
What about Serializer, NaturalSortFilter, closures?
Serializer (sortBy($this->sort->column())) — creates its OWN SortableFilter via sortAsc()/sortDesc(). Completely separate pipeline, never goes through qualifyColumns(). Not affected.
NaturalSortFilter — receives $this->column() (now qualified) in raw SQL:
$query->orderByRaw("CAST({$column} AS UNSIGNED) {$value}, {$column}{$value}");
// CAST(posts.name AS UNSIGNED) — valid SQL in both MySQL and SQLite
Actually beneficial — prevents the same ambiguity in raw SQL.
Closure/invokable sorts — also receive the qualified column. Example:
publicfunction__invoke($request, $query, $value, string$column): void
{
$query->orderBy($column, $order); // $column is now 'posts.name' — better!
}
This is a net improvement — closures that use orderBy directly would have hit the same ambiguity bug without qualification.
All 413 Restify tests pass, confirming no breakages.
Improvement of Data Sorting
A new functionality has been added to the code which improves the way we classify columns in a database using a provided model. This ensures greater accuracy in data classification.
Optimized Data Filtering
The method that prepares orders of data within our repository has been adjusted to use the column qualifying method beforehand. This results in more precise data filtering based on the provided model.
Testing Enhancements for Sortable Filters
To assure the correct functioning of these new changes, we have introduced a new test procedure. This test is designed to ensure that the column sorting function works as intended, particularly when data from different sources are brought together. This minimizes ambiguity and leads to more accurate data handling.
Introduction of Additional Dependents
In order to support these new tests, we've folded in a few new classes into the testing procedure. This should ensure that the tests are comprehensive and truly reflective of the software's capabilities.
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
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.
The Problem
When a Restify repository has searchable BelongsTo relations, the
RepositorySearchServiceadds LEFT JOINs:MySQL throws:
Column 'id' in order clause is ambiguousThe Fix: Two changes in Restify
1.
SortCollection::qualifyColumns(Model $model)— mirrors the existingSearchablesCollection::qualifyColumns()Iterates sort filters and prefixes unqualified columns with the model's table name:
id→posts.idcreated_at→posts.created_atusers.name→ left as-is (already has a dot)2.
RepositorySearchService::prepareOrders()— callsqualifyColumns()beforeapply()This is the only place where sorts hit the database. The call happens AFTER
collectSorts()(which does normalization, hydration, authorization) and BEFOREapply()(which executes the ORDER BY).Why NOT other locations?
Why not fix in
SortableFilter::filter()at line 61?This would avoid mutating
$this->column, but:$this->columnduringapply()would still get unqualified namesSearchablesCollection::qualifyColumns()) is already proven and understood in this codebaseWhy not fix in
InteractWithSearch::collectSorts()?No, because
inRepository()at line 57 compares$filter->columnagainst repository-defined sort keys:Repository sort keys are unqualified (
'id','date'). If we qualify before this step,posts.id !== idand the sort gets filtered out.prepareOrders()is the correct location — after the fullcollectSorts()pipeline resolves everything, but beforeapply()hits the database.What about Serializer, NaturalSortFilter, closures?
Serializer (
sortBy($this->sort->column())) — creates its OWNSortableFilterviasortAsc()/sortDesc(). Completely separate pipeline, never goes throughqualifyColumns(). Not affected.NaturalSortFilter — receives
$this->column()(now qualified) in raw SQL:Actually beneficial — prevents the same ambiguity in raw SQL.
Closure/invokable sorts — also receive the qualified column. Example:
This is a net improvement — closures that use
orderBydirectly would have hit the same ambiguity bug without qualification.All 413 Restify tests pass, confirming no breakages.