From 597a0af1460549cc2b32c9fdbf128ee82d54e629 Mon Sep 17 00:00:00 2001 From: Joseph Silber Date: Sat, 14 Nov 2015 21:53:34 -0500 Subject: [PATCH 1/4] Allow table-qualified columns in pluck operations --- src/Illuminate/Database/Query/Builder.php | 25 +++++-------------- .../DatabaseEloquentIntegrationTest.php | 14 +++++++++++ 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index 893b465b8f35..7293ea045fbd 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -7,7 +7,6 @@ use Illuminate\Support\Arr; use Illuminate\Support\Str; use InvalidArgumentException; -use Illuminate\Support\Collection; use Illuminate\Pagination\Paginator; use Illuminate\Support\Traits\Macroable; use Illuminate\Contracts\Support\Arrayable; @@ -1543,11 +1542,9 @@ public function chunk($count, callable $callback) */ public function pluck($column, $key = null) { - $columns = $this->getPluckSelect($column, $key); + $results = $this->get(func_get_args()); - $results = new Collection($this->get($columns)); - - return $results->pluck($columns[0], Arr::get($columns, 1))->all(); + return Arr::pluck($results, $this->stripTable($column), $this->stripTable($key)); } /** @@ -1565,24 +1562,14 @@ public function lists($column, $key = null) } /** - * Get the columns that should be used in a pluck select. + * Strip off the table name from a column identifier. * * @param string $column - * @param string $key - * @return array + * @return string */ - protected function getPluckSelect($column, $key) + protected function stripTable($column) { - $select = is_null($key) ? [$column] : [$column, $key]; - - // If the selected columns contain "dots", we will remove it so that the pluck - // operation can run normally. Specifying the table is not needed, since we - // really want the names of the columns as it is in this resulting array. - return array_map(function ($column) { - $dot = strpos($column, '.'); - - return $dot === false ? $column : substr($column, $dot + 1); - }, $select); + return is_null($column) ? $column : last(explode('.', $column)); } /** diff --git a/tests/Database/DatabaseEloquentIntegrationTest.php b/tests/Database/DatabaseEloquentIntegrationTest.php index 4bad51b6ff1a..80de3709ba05 100644 --- a/tests/Database/DatabaseEloquentIntegrationTest.php +++ b/tests/Database/DatabaseEloquentIntegrationTest.php @@ -179,6 +179,20 @@ public function testPluck() $this->assertEquals([1 => 'taylorotwell@gmail.com', 2 => 'abigailotwell@gmail.com'], $keyed); } + public function testPluckWithJoin() + { + $user1 = EloquentTestUser::create(['id' => 1, 'email' => 'taylorotwell@gmail.com']); + $user2 = EloquentTestUser::create(['id' => 2, 'email' => 'abigailotwell@gmail.com']); + + $user2->posts()->create(['id' => 1, 'name' => 'First post']); + $user1->posts()->create(['id' => 2, 'name' => 'Second post']); + + $query = EloquentTestUser::join('posts', 'users.id', '=', 'posts.user_id'); + + $this->assertEquals([1 => 'First post', 2 => 'Second post'], $query->pluck('name', 'posts.id')->all()); + $this->assertEquals([2 => 'First post', 1 => 'Second post'], $query->pluck('name', 'users.id')->all()); + } + public function testFindOrFail() { EloquentTestUser::create(['id' => 1, 'email' => 'taylorotwell@gmail.com']); From 4fd6db18d1586eeb8f12424394f143e465015bbc Mon Sep 17 00:00:00 2001 From: Joseph Silber Date: Sat, 14 Nov 2015 22:20:29 -0500 Subject: [PATCH 2/4] Allow column aliases in query builder pluck --- src/Illuminate/Database/Query/Builder.php | 4 ++-- tests/Database/DatabaseEloquentIntegrationTest.php | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index 7293ea045fbd..bb3ceb55ae29 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -1562,14 +1562,14 @@ public function lists($column, $key = null) } /** - * Strip off the table name from a column identifier. + * Strip off the table name or alias from a column identifier. * * @param string $column * @return string */ protected function stripTable($column) { - return is_null($column) ? $column : last(explode('.', $column)); + return is_null($column) ? $column : last(preg_split('~\.| ~', $column)); } /** diff --git a/tests/Database/DatabaseEloquentIntegrationTest.php b/tests/Database/DatabaseEloquentIntegrationTest.php index 80de3709ba05..11ce324a4274 100644 --- a/tests/Database/DatabaseEloquentIntegrationTest.php +++ b/tests/Database/DatabaseEloquentIntegrationTest.php @@ -41,6 +41,7 @@ public function setUp() { $this->schema()->create('users', function ($table) { $table->increments('id'); + $table->string('name')->nullable(); $table->string('email')->unique(); $table->string('role')->default('standard'); $table->timestamps(); @@ -181,16 +182,17 @@ public function testPluck() public function testPluckWithJoin() { - $user1 = EloquentTestUser::create(['id' => 1, 'email' => 'taylorotwell@gmail.com']); - $user2 = EloquentTestUser::create(['id' => 2, 'email' => 'abigailotwell@gmail.com']); + $user1 = EloquentTestUser::create(['id' => 1, 'name' => 'Taylor', 'email' => 'taylorotwell@gmail.com']); + $user2 = EloquentTestUser::create(['id' => 2, 'name' => 'Abigail', 'email' => 'abigailotwell@gmail.com']); $user2->posts()->create(['id' => 1, 'name' => 'First post']); $user1->posts()->create(['id' => 2, 'name' => 'Second post']); $query = EloquentTestUser::join('posts', 'users.id', '=', 'posts.user_id'); - $this->assertEquals([1 => 'First post', 2 => 'Second post'], $query->pluck('name', 'posts.id')->all()); - $this->assertEquals([2 => 'First post', 1 => 'Second post'], $query->pluck('name', 'users.id')->all()); + $this->assertEquals([1 => 'First post', 2 => 'Second post'], $query->pluck('posts.name', 'posts.id')->all()); + $this->assertEquals([2 => 'First post', 1 => 'Second post'], $query->pluck('posts.name', 'users.id')->all()); + $this->assertEquals(['Abigail' => 'First post', 'Taylor' => 'Second post'], $query->pluck('posts.name', 'users.name as user_name')->all()); } public function testFindOrFail() From 2a52916d47908413c517e86f30e8822ce0d9bd48 Mon Sep 17 00:00:00 2001 From: Joseph Silber Date: Sat, 14 Nov 2015 23:08:44 -0500 Subject: [PATCH 3/4] Improve docblocks --- src/Illuminate/Database/Query/Builder.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index bb3ceb55ae29..a800023ae458 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -1537,7 +1537,7 @@ public function chunk($count, callable $callback) * Get an array with the values of a given column. * * @param string $column - * @param string $key + * @param string|null $key * @return array */ public function pluck($column, $key = null) @@ -1551,7 +1551,7 @@ public function pluck($column, $key = null) * Alias for the "pluck" method. * * @param string $column - * @param string $key + * @param string|null $key * @return array * * @deprecated since version 5.2. Use the "pluck" method directly. @@ -1565,7 +1565,7 @@ public function lists($column, $key = null) * Strip off the table name or alias from a column identifier. * * @param string $column - * @return string + * @return string|null */ protected function stripTable($column) { From 354d11e663c2ddefc8d79b2d32e8a3f785cbf237 Mon Sep 17 00:00:00 2001 From: Joseph Silber Date: Sun, 15 Nov 2015 19:25:38 -0500 Subject: [PATCH 4/4] Add a comment about stripping the table name from the columns --- src/Illuminate/Database/Query/Builder.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Illuminate/Database/Query/Builder.php b/src/Illuminate/Database/Query/Builder.php index a800023ae458..2f964c06a26e 100755 --- a/src/Illuminate/Database/Query/Builder.php +++ b/src/Illuminate/Database/Query/Builder.php @@ -1544,6 +1544,9 @@ public function pluck($column, $key = null) { $results = $this->get(func_get_args()); + // If the columns are qualified with a table or have an alias, we cannot use + // those directly in the "pluck" operation, since the results from the DB + // are only keyed by the column itself. We will strip everything else. return Arr::pluck($results, $this->stripTable($column), $this->stripTable($key)); }