Optimize pluck, simplify stuff (#78)

Author: shmax

lib/Connection.php

@@ -391,15 +391,15 @@ public function query_and_fetch_one(string $sql, array &$values = []): int
     /**
      * Execute a raw SQL query and fetch the results.
      *
-     * @param string   $sql     raw SQL string to execute
-     * @param \Closure $handler closure that will be passed the fetched results
+     * @param string       $sql    raw SQL string to execute
+     * @param array<mixed> $values
      */
-    public function query_and_fetch(string $sql, \Closure $handler): void
+    public function query_and_fetch(string $sql, array $values = [], int $method = \PDO::FETCH_ASSOC): \Generator
     {
-        $sth = $this->query($sql);
+        $sth = $this->query($sql, $values);
 
-        while ($row = $sth->fetch(\PDO::FETCH_ASSOC)) {
-            $handler($row);
+        while ($row = $sth->fetch($method)) {
+            yield $row;
         }
     }
 

lib/Model.php

@@ -1599,6 +1599,16 @@ public static function select(): Relation
         return static::Relation()->select(...func_get_args());
     }
 
+    /**
+     * @return Relation<static>
+     *
+     *@see Relation::distinct()
+     */
+    public static function distinct(bool $distinct = true): Relation
+    {
+        return static::Relation()->distinct($distinct);
+    }
+
     /**
      * @return Relation<static>
      *

lib/Relation.php

@@ -107,46 +107,52 @@ public function none(): Relation
     }
 
     /**
-     * Plucks the columns from the table, returning an column_value[] rather than a Model[]
+     * Use pluck as a shortcut to select one or more attributes without
+     * loading an entire record object per row.
      *
-     * $this->where(['name' => 'Bill'])->pluck('id') // returns [1]
-     * $this->where(['age' => 42])->pluck('id', 'name') // returns [[1, "David"], [2, "Fran"], [3, "Jose"]]
-     * $this->where(['age' => 42])->pluck('id, name') // returns [[1, "David"], [2, "Fran"], [3, "Jose"]]
-     * $this->where(['age' => 42])->pluck(['id', 'name']) // returns [[1, "David"], [2, "Fran"], [3, "Jose"]]
+     * $names = Person::pluck('name');
+     *
+     * instead of
+     *
+     * $names = array_map(fn($person) =>$person->name, Person::all()->to_a());
+     *
+     * Pluck returns an Array of attribute values type-casted to match
+     * the plucked column names, if they can be deduced.
+     *
+     * Person::pluck('name')                // SELECT people.name FROM people
+     *  => ['David', 'Jeremy', 'Jose']
+     *
+     * Person::pluck('id', 'name');         // SELECT people.id, people.name FROM people
+     *  => [[1, 'David'], [2, 'Jeremy'], [3, 'Jose']]
+     *
+     * Person::distinct()->pluck('role');   // SELECT DISTINCT role FROM people
+     *  => ['admin', 'member', 'guest']
+     *
+     * Person::where(['age' => 21])         // SELECT people.id FROM people WHERE people.age = 21 LIMIT 5
+     *  ->limit(5).pluck('id')
+     * => [2, 3]
+     *
+     * @see Relation::ids()
      *
      * @return array<mixed>
      */
     public function pluck(): array
     {
-        $columns = $this->possibleListToArray(...func_get_args());
-        if (0 === count($columns)) {
+        $args = func_get_args();
+        if (0 === count($args)) {
             throw new ValidationsArgumentError('pluck requires at least one argument');
         }
 
-        $oldSelect = array_key_exists('select', $this->options) ? $this->options['select'] : null;
-        $this->reselect($columns);
-
-        $models = $this->to_a();
-
-        if (null === $oldSelect) {
-            unset($this->options['select']);
-        } else {
-            $this->options['select'] = $oldSelect;
-        }
-
-        $retValue = [];
-        foreach ($models as $model) {
-            $row = [];
-            foreach ($columns as $column) {
-                array_push($row, $model->$column);
-            }
-            if (1 === count($row)) {
-                $row = $row[0];
-            }
-            array_push($retValue, $row);
-        }
+        $options = array_merge($this->options, ['select' => [static::toSingleArg(...$args)]]);
+        $table = $this->table();
+        $sql = $table->options_to_sql($options);
+        $retValue = iterator_to_array(
+            $table->conn->query_and_fetch($sql->to_s(), $sql->get_where_values(), \PDO::FETCH_NUM)
+        );
 
-        return $retValue;
+        return array_map(static function ($row) {
+            return 1 == count($row) ? $row[0] : $row;
+        }, $retValue);
     }
 
     /**
@@ -192,21 +198,6 @@ public function reselect(): Relation
         return $this;
     }
 
-    /**
-     * Converts "name,id" to ["name", "id"] if necessary
-     *
-     * @return array<string>
-     */
-    private function possibleListToArray(): array
-    {
-        $args = static::toSingleArg(...func_get_args());
-        if (!is_array($args)) {
-            $args = array_map('trim', explode(',', $args));
-        }
-
-        return $args;
-    }
-
     /**
      * Performs JOINs on +args+. The given symbol(s) should match the name of
      * the association(s).
@@ -550,6 +541,24 @@ public function readonly(bool $readonly): Relation
         return $this;
     }
 
+    /**
+     * Specifies whether the records should be unique or not. For example:
+     *
+     * User::select('name') // Might return two records with the same name
+     *
+     * User::select('name')->distinct() // Returns 1 record per distinct name
+     *
+     * User::select('name')->distinct()->distinct(false) // You can also remove the uniqueness
+     *
+     * @return Relation<TModel>
+     */
+    public function distinct(bool $distinct=true): Relation
+    {
+        $this->options['distinct'] = $distinct;
+
+        return $this;
+    }
+
     /**
      * Find by id - This can either be a specific id (1),
      * a list of ids (1, 5, 6), or an array of ids ([5, 6, 10]).

lib/SQLBuilder.php

@@ -18,6 +18,8 @@ class SQLBuilder
 {
     private Connection $connection;
     private string $operation = 'SELECT';
+
+    private bool $distinct = false;
     private string $table;
     private string $select = '*';
 
@@ -175,8 +177,9 @@ public function offset(int $offset): static
         return $this;
     }
 
-    public function select(string $select): static
+    public function select(string $select, bool $distinct = false): static
     {
+        $this->distinct = $distinct;
         $this->operation = 'SELECT';
         $this->select = $select;
 
@@ -344,7 +347,7 @@ private function build_insert(): string
 
     private function build_select(): string
     {
-        $sql = "SELECT $this->select FROM $this->table";
+        $sql = 'SELECT ' . ($this->distinct ? 'DISTINCT ' : '') . "$this->select FROM $this->table";
 
         if (!empty($this->joins)) {
             $sql .= ' ' . $this->joins;

lib/Table.php

@@ -223,7 +223,10 @@ public function options_to_sql(array $options): SQLBuilder
                 $tokens = array_merge($tokens, array_map('trim', explode(',', $select)));
             }
 
-            $sql->select(array_search('*', $tokens) ? '*' : implode(', ', array_unique($tokens)));
+            $sql->select(
+                array_search('*', $tokens) ? '*' : implode(', ', array_unique($tokens)),
+                !empty($options['distinct'])
+            );
         }
 
         $sql->where($options['conditions'] ?? [], $options['mapped_names'] ?? []);

lib/Types.php

@@ -42,6 +42,7 @@
  *  offset?: int,
  *  order?: string,
  *  readonly?: bool,
+ *  distinct?: bool,
  *  select?: string|array<string>,
  * }
  */

test/ActiveRecordCountTest.php

@@ -9,7 +9,7 @@ class ActiveRecordCountTest extends \DatabaseTestCase
 {
     public function testNoArguments()
     {
-        $this->assertEquals(4, Author::count());
+        $this->assertEquals(5, Author::count());
     }
 
     public function testColumnNameAsArgument()

test/ActiveRecordFindTest.php

@@ -234,8 +234,8 @@ public function testFindBySqltakesValuesArray()
     public function testFindLast()
     {
         $author = Author::last();
-        $this->assertEquals(4, $author->author_id);
-        $this->assertEquals('Uncle Bob', $author->name);
+        $this->assertEquals(5, $author->author_id);
+        $this->assertEquals('Tito', $author->name);
     }
 
     public function testFindLastUsingStringCondition()
@@ -316,7 +316,7 @@ public function testFindWithHash()
     {
         $this->assertNotNull(Author::where(['name' => 'Tito'])->first());
         $this->assertNull(Author::where(['name' => 'Mortimer'])->first());
-        $this->assertEquals(1, count(Author::where(['name' => 'Tito'])->to_a()));
+        $this->assertEquals(2, count(Author::where(['name' => 'Tito'])->to_a()));
     }
 
     public function testFindOrCreateByOnExistingRecord()

test/ActiveRecordPluckTest.php

@@ -11,21 +11,39 @@ class ActiveRecordPluckTest extends \DatabaseTestCase
     public function testNoArguments()
     {
         $this->expectException(ValidationsArgumentError::class);
-        $author = Author::pluck();
+        Author::pluck();
     }
 
     public function testSingleArgument()
     {
-        $authors = Author::where(['mixedCaseField' => 'Bill'])->pluck('name');
-        $this->assertEquals(2, count($authors));
-        $this->assertEquals('Bill Clinton', $authors[0]);
-        $this->assertEquals('Uncle Bob', $authors[1]);
+        $authors = Author::pluck('name');
+        $this->assertEquals(5, count($authors));
+        $this->assertEquals('Tito', $authors[0]);
+        $this->assertEquals('George W. Bush', $authors[1]);
+    }
+
+    public function testSingleArgumentWithDistinct()
+    {
+        $authors = Author::distinct()->pluck('name');
+        $this->assertEquals(4, count($authors));
+        $this->assertEquals('Tito', $authors[0]);
+        $this->assertEquals('George W. Bush', $authors[1]);
+    }
+
+    public function testSingleArgumentWithRemoveDistinct()
+    {
+        $authors = Author::distinct()->distinct(false)->pluck('name');
+        $this->assertEquals(5, count($authors));
     }
 
     public function testMultipleArguments()
     {
-        $authors = Author::where(['mixedCaseField' => 'Bill'])->pluck('name', 'author_id');
-        $this->assertMultipleArgumentsResult($authors);
+        $authors = Author::pluck('name', 'author_id');
+        $this->assertEquals(5, count($authors));
+        $this->assertEquals('Tito', $authors[0][0]);
+        $this->assertEquals(1, $authors[0][1]);
+        $this->assertEquals('George W. Bush', $authors[1][0]);
+        $this->assertEquals(2, $authors[1][1]);
     }
 
     public function testCommaDelimitedString()
@@ -69,6 +87,6 @@ public function testIds()
     public function testIdsAll()
     {
         $authors = Author::ids();
-        $this->assertEquals(4, count($authors));
+        $this->assertEquals(5, count($authors));
     }
 }

test/ActiveRecordTest.php

@@ -478,6 +478,6 @@ public function testQuery()
         $this->assertTrue($row['n'] > 1);
 
         $row = Author::query('SELECT COUNT(*) AS n FROM authors WHERE name=?', ['Tito'])->fetch();
-        $this->assertEquals(['n' => 1], $row);
+        $this->assertEquals(['n' => 2], $row);
     }
 }