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

[5.6] Optimize query builder's pluck() method #23482

Merged
merged 4 commits into from
May 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 97 additions & 16 deletions src/Illuminate/Database/Query/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -1830,17 +1830,11 @@ public function value($column)
*/
public function get($columns = ['*'])
{
$original = $this->columns;

if (is_null($original)) {
$this->columns = $columns;
}

$results = $this->processor->processSelect($this, $this->runSelect());

$this->columns = $original;

return collect($results);
return collect(
$this->onceWithColumns($columns, function () {
return $this->processor->processSelect($this, $this->runSelect());
})
);
}

/**
Expand Down Expand Up @@ -2036,15 +2030,52 @@ protected function enforceOrderBy()
*/
public function pluck($column, $key = null)
{
$results = $this->get(is_null($key) ? [$column] : [$column, $key]);
$queryResult = $this->onceWithColumns(
is_null($key) ? [$column] : [$column, $key],
function () {
return $this->processor->processSelect($this, $this->runSelect());
}
);

if (empty($queryResult)) {
return collect();
}

// If the columns are qualified with a table or have an alias, we cannot use
// those directly in the "pluck" operations since the results from the DB
// are only keyed by the column itself. We'll strip the table out here.
return $results->pluck(
$this->stripTableForPluck($column),
$this->stripTableForPluck($key)
);
$column = $this->stripTableForPluck($column);
$key = $this->stripTableForPluck($key);

if (is_array($queryResult[0])) {
return $this->pluckFromArrayColumn($queryResult, $column, $key);
} else {
return $this->pluckFromObjectColumn($queryResult, $column, $key);
}
}

/**
* Execute the given callback while selecting the given columns.
*
* After running the callback, the columns are reset to the original value.
*
* @param array $columns
* @param callable $callback
* @return mixed
*/
protected function onceWithColumns($columns, $callback)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the correct location for the helper method? Or should it be placed below get()?

{
$original = $this->columns;

if (is_null($original)) {
$this->columns = $columns;
}

$result = $callback();

$this->columns = $original;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be wrapped in a try/finally block? We want to make sure we always reset them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't before my refactoring, so I don't want to change this here...


return $result;
}

/**
Expand All @@ -2058,6 +2089,56 @@ protected function stripTableForPluck($column)
return is_null($column) ? $column : last(preg_split('~\.| ~', $column));
}

/**
* Retrieve column values from rows represented as objects.
*
* @param array $queryResult
* @param string $column
* @param string $key
* @return \Illuminate\Support\Collection
*/
protected function pluckFromObjectColumn($queryResult, $column, $key)
{
$results = [];

if (is_null($key)) {
foreach ($queryResult as $row) {
$results[] = $row->$column;
}
} else {
foreach ($queryResult as $row) {
$results[$row->$key] = $row->$column;
}
}

return collect($results);
}

/**
* Retrieve column values from rows represented as arrays.
*
* @param array $queryResult
* @param string $column
* @param string $key
* @return \Illuminate\Support\Collection
*/
protected function pluckFromArrayColumn($queryResult, $column, $key)
{
$results = [];

if (is_null($key)) {
foreach ($queryResult as $row) {
$results[] = $row[$column];
}
} else {
foreach ($queryResult as $row) {
$results[$row[$key]] = $row[$column];
}
}

return collect($results);
}

/**
* Concatenate values of a given column as a string.
*
Expand Down
2 changes: 1 addition & 1 deletion tests/Database/DatabaseQueryBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1315,7 +1315,7 @@ public function testFirstMethodReturnsFirstResult()
$this->assertEquals(['foo' => 'bar'], $results);
}

public function testListMethodsGetsArrayOfColumnValues()
public function testPluckMethodGetsCollectionOfColumnValues()
{
$builder = $this->getBuilder();
$builder->getConnection()->shouldReceive('select')->once()->andReturn([['foo' => 'bar'], ['foo' => 'baz']]);
Expand Down