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

[8.x] Fix undefined array key in SqlServerGrammar when using orderByRaw #37859

Merged
merged 3 commits into from
Jun 29, 2021
Merged

[8.x] Fix undefined array key in SqlServerGrammar when using orderByRaw #37859

merged 3 commits into from
Jun 29, 2021

Conversation

Namoshek
Copy link
Contributor

This PR fixes the exception

ErrorException: Undefined array key "column" in /var/www/vendor/laravel/framework/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php:220

which is triggered when using orderByRaw after the latest changes to the SqlServerGrammar.


A bit of background (and please correct me if I'm wrong):

When using Builder::orderBy($column, $direction), an order is added with keys column and direction (line 1986+):

public function orderBy($column, $direction = 'asc')
{
if ($this->isQueryable($column)) {
[$query, $bindings] = $this->createSub($column);
$column = new Expression('('.$query.')');
$this->addBinding($bindings, $this->unions ? 'unionOrder' : 'order');
}
$direction = strtolower($direction);
if (! in_array($direction, ['asc', 'desc'], true)) {
throw new InvalidArgumentException('Order direction must be "asc" or "desc".');
}
$this->{$this->unions ? 'unionOrders' : 'orders'}[] = [
'column' => $column,
'direction' => $direction,
];
return $this;
}

When using Builder::orderByRaw($sql), an order is added with keys type and sql (line 2051):

public function orderByRaw($sql, $bindings = [])
{
$type = 'Raw';
$this->{$this->unions ? 'unionOrders' : 'orders'}[] = compact('type', 'sql');
$this->addBinding($bindings, $this->unions ? 'unionOrder' : 'order');
return $this;
}

Because of this, the patched code currently fails with an exception.

Fixes usage of `orderByRaw`:
> ErrorException: Undefined array key \"column\" in /var/www/vendor/laravel/framework/src/Illuminate/Database/Query/Grammars/SqlServerGrammar.php:220
@driesvints
Copy link
Member

Can you provide a test?

@driesvints driesvints changed the title Fix undefined array key in SqlServerGrammar when using orderByRaw [8.x] Fix undefined array key in SqlServerGrammar when using orderByRaw Jun 29, 2021
The tests are similar to the existing ones used by the generic query builder and grammar.
@Namoshek
Copy link
Contributor Author

I added a test case based on the generic one above the new one. The interesting part is the last query which uses limit/offset and orderByRaw. The other queries also work without the patch, but I guess having more test cases is no harm.

The issue is pretty easy to trigger when having a table with a slightly complex column design (e.g. sorting by enums).

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

Successfully merging this pull request may close these issues.

3 participants