Merge pull request #1782 from mringler/reintroduce_late_select_for_ba…

…ckward_compatibility

Reintroduce late select for backward compatibility

src/Propel/Generator/Behavior/QueryCache/QueryCacheBehavior.php

@@ -230,6 +230,7 @@ public function doSelect(ConnectionInterface \$con = null)
     if (!\$this->hasSelectClause() && !\$this->getPrimaryCriteria()) {
         \$this->addSelfSelectColumns();
     }
+    \$this->configureSelectColumns();
 
     \$dbMap = Propel::getServiceContainer()->getDatabaseMap(" . $this->tableClassName . "::DATABASE_NAME);
     \$db = Propel::getServiceContainer()->getAdapter(" . $this->tableClassName . "::DATABASE_NAME);
@@ -284,6 +285,8 @@ public function doCount(ConnectionInterface \$con = null)
             \$this->addSelfSelectColumns();
         }
 
+        \$this->configureSelectColumns();
+
         \$needsComplexCount = \$this->getGroupByColumns()
             || \$this->getOffset()
             || \$this->getLimit() >= 0

src/Propel/Generator/Builder/Om/QueryBuilder.php

@@ -654,7 +654,7 @@ public function findPk(\$key, ConnectionInterface \$con = null)
         \$this->basePreSelect(\$con);
 
         if (
-            \$this->formatter || \$this->modelAlias || \$this->with
+            \$this->formatter || \$this->modelAlias || \$this->with || \$this->select
             || \$this->selectColumns || \$this->asColumns || \$this->selectModifiers
             || \$this->map || \$this->having || \$this->joins
         ) {

src/Propel/Runtime/ActiveQuery/ModelCriteria.php

@@ -82,6 +82,12 @@ class ModelCriteria extends BaseModelCriteria
      */
     protected $isKeepQuery = true;
 
+    // this is for the select method
+    /**
+     * @var string|array|null
+     */
+    protected $select;
+
     /**
      * Used to memorize whether we added self-select columns before.
      *
@@ -466,50 +472,42 @@ public function select($columnArray)
         if (empty($columnArray)) {
             throw new PropelException('You must ask for at least one column');
         }
-        $this->isSelfSelected = true;
-        if ($this->formatter === null) {
-            $this->setFormatter(SimpleArrayFormatter::class);
-        }
 
         if ($columnArray === '*') {
-            $columnArray = [];
-            foreach ($this->getTableMap()->getColumns() as $columnMap) {
-                $columnArray[] = $this->modelName . '.' . $columnMap->getPhpName();
-            }
+            $columnArray = $this->resolveSelectAll();
         }
         if (!is_array($columnArray)) {
             $columnArray = [$columnArray];
         }
+        $this->select = $columnArray;
+        $this->isSelfSelected = true;
 
-        $this->selectColumns = [];
+        return $this;
+    }
 
-        foreach ($columnArray as $columnName) {
-            if (array_key_exists($columnName, $this->asColumns)) {
-                continue;
-            }
-            [$columnMap, $realColumnName] = $this->getColumnFromName($columnName);
-            if ($realColumnName === null) {
-                throw new PropelException("Cannot find selected column '$columnName'");
-            }
-            // always put quotes around the columnName to be safe, we strip them in the formatter
-            $this->addAsColumn('"' . $columnName . '"', $realColumnName);
+    /**
+     * @return string[]
+     */
+    protected function resolveSelectAll(): array
+    {
+        $columnArray = [];
+        foreach ($this->getTableMap()->getColumns() as $columnMap) {
+            $columnArray[] = $this->modelName . '.' . $columnMap->getPhpName();
         }
 
-        return $this;
+        return $columnArray;
     }
 
     /**
      * Retrieves the columns defined by a previous call to select().
      *
-     * @deprecated Not needed anymore, selected columns are part of {@link Criteria::$asColumns}
-     *
      * @see select()
      *
      * @return array|string A list of column names (e.g. array('Title', 'Category.Name', 'c.Content')) or a single column name (e.g. 'Name')
      */
     public function getSelect()
     {
-        return array_values($this->asColumns);
+        return $this->select;
     }
 
     /**
@@ -976,6 +974,7 @@ public function clear()
         $this->with = [];
         $this->primaryCriteria = null;
         $this->formatter = null;
+        $this->select = null;
 
         return $this;
     }
@@ -1637,6 +1636,8 @@ public function count(?ConnectionInterface $con = null)
      */
     public function doCount(?ConnectionInterface $con = null)
     {
+        $this->configureSelectColumns();
+
         // check that the columns of the main class are already added (if this is the primary ModelCriteria)
         if (!$this->hasSelectClause() && !$this->getPrimaryCriteria()) {
             $this->addSelfSelectColumns();
@@ -2203,19 +2204,56 @@ public function getModelJoinByTableName($tableName)
      */
     public function doSelect(?ConnectionInterface $con = null)
     {
+        $this->configureSelectColumns();
+
         $this->addSelfSelectColumns();
 
         return parent::doSelect($con);
     }
 
     /**
-     * @deprecated This method was used to add columns from {@link select()} during query generation, but that is handled
-     * right away now.
+     * {@inheritDoc}
+     *
+     * @see \Propel\Runtime\ActiveQuery\Criteria::createSelectSql()
+     *
+     * @param array $params Parameters that are to be replaced in prepared statement.
+     *
+     * @return string
+     */
+    public function createSelectSql(&$params)
+    {
+        $this->configureSelectColumns();
+
+        return parent::createSelectSql($params);
+    }
+
+    /**
+     * @throws \Propel\Runtime\Exception\PropelException
      *
      * @return void
      */
     public function configureSelectColumns()
     {
+        if (!$this->select) {
+            return;
+        }
+
+        if ($this->formatter === null) {
+            $this->setFormatter(SimpleArrayFormatter::class);
+        }
+        $this->selectColumns = [];
+
+        foreach ($this->select as $columnName) {
+            if (array_key_exists($columnName, $this->asColumns)) {
+                continue;
+            }
+            [$columnMap, $realColumnName] = $this->getColumnFromName($columnName);
+            if ($realColumnName === null) {
+                throw new PropelException("Cannot find selected column '$columnName'");
+            }
+            // always put quotes around the columnName to be safe, we strip them in the formatter
+            $this->addAsColumn('"' . $columnName . '"', $realColumnName);
+        }
     }
 
     /**

tests/Propel/Tests/Runtime/ActiveQuery/ModelCriteriaGroupByArrayTest.php

@@ -86,7 +86,7 @@ public function testGroupByArray()
             ->orderByLastName()
             ->find();
 
-        $expectedSql = 'SELECT author.first_name AS "FirstName", author.last_name AS "LastName", COUNT(book.id) AS nbBooks FROM author LEFT JOIN book ON (author.id=book.author_id) GROUP BY author.first_name,author.last_name ORDER BY author.last_name ASC';
+        $expectedSql = 'SELECT COUNT(book.id) AS nbBooks, author.first_name AS "FirstName", author.last_name AS "LastName" FROM author LEFT JOIN book ON (author.id=book.author_id) GROUP BY author.first_name,author.last_name ORDER BY author.last_name ASC';
 
         $this->assertEquals($expectedSql, $this->con->getLastExecutedQuery());
 

tests/Propel/Tests/Runtime/ActiveQuery/ModelCriteriaSelectTest.php

@@ -452,35 +452,40 @@ public function testSelectArrayPaginate()
     /**
      * @return void
      */
-    public function testGetSelectAddsToAsColumns()
+    public function testGetSelectReturnsNullByDefault()
     {
         $c = new ModelCriteria('bookstore', 'Propel\Tests\Bookstore\Book');
-        $c->select('Title');
+        $this->assertNull($c->getSelect());
+    }
 
-        $this->assertIsArray($c->getAsColumns());
-        $this->assertCount(1, $c->getAsColumns());
-        $this->assertArrayHasKey('"Title"', $c->getAsColumns());
-        $clause = $c->getAsColumns()['"Title"'];
-        $this->assertEquals('book.title', $clause);
+    /**
+     * @return void
+     */
+    public function testGetSelectReturnsArrayWhenSelectingASingleColumn()
+    {
+        $c = new ModelCriteria('bookstore', 'Propel\Tests\Bookstore\Book');
+        $c->select('Title');
+        $this->assertEquals(['Title'], $c->getSelect());
+    }
+
+    /**
+     * @return void
+     */
+    public function testGetSelectReturnsArrayWhenSelectingSeveralColumns()
+    {
+        $c = new ModelCriteria('bookstore', 'Propel\Tests\Bookstore\Book');
+        $c->select(['Id', 'Title']);
+        $this->assertEquals(['Id', 'Title'], $c->getSelect());
     }
 
     /**
      * @return void
      */
-    public function testGetSelectAddsArrayToAsColumns()
+    public function testGetSelectReturnsArrayWhenSelectingASingleColumnAsArray()
     {
         $c = new ModelCriteria('bookstore', 'Propel\Tests\Bookstore\Book');
-        $columns = ['Id', 'Title'];
-        $c->select($columns);
-
-        $this->assertIsArray($c->getAsColumns());
-        $this->assertCount(2, $c->getAsColumns());
-        foreach ($columns as $columnName) {
-            $quotedName = "\"$columnName\"";
-            $this->assertArrayHasKey($quotedName, $c->getAsColumns());
-            $clause = $c->getAsColumns()[$quotedName];
-            $this->assertEquals('book.' . strtolower($columnName), $clause);
-        }
+        $c->select(['Title']);
+        $this->assertEquals(['Title'], $c->getSelect());
     }
 
     /**
@@ -490,17 +495,14 @@ public function testGetSelectReturnsArrayWhenSelectingAllColumns()
     {
         $c = new ModelCriteria('bookstore', 'Propel\Tests\Bookstore\Book');
         $c->select('*');
-        $tableColumns = BookTableMap::getTableMap()->getColumns();
-
-        $this->assertIsArray($c->getAsColumns());
-        $this->assertCount(count($tableColumns), $c->getAsColumns());
-        foreach ($tableColumns as $columnMap) {
-            $columnName = $c->getModelName() . '.' . $columnMap->getPhpName();
-            $quotedName = "\"$columnName\"";
-            $this->assertArrayHasKey($quotedName, $c->getAsColumns());
-            $clause = $c->getAsColumns()[$quotedName];
-            $this->assertEquals($columnMap->getFullyQualifiedName(), $clause);
-        }
+        $this->assertEquals([
+            'Propel\Tests\Bookstore\Book.Id',
+            'Propel\Tests\Bookstore\Book.Title',
+            'Propel\Tests\Bookstore\Book.ISBN',
+            'Propel\Tests\Bookstore\Book.Price',
+            'Propel\Tests\Bookstore\Book.PublisherId',
+            'Propel\Tests\Bookstore\Book.AuthorId',
+        ], $c->getSelect());
     }
 
     /**
@@ -525,5 +527,6 @@ public function testSelectNonexistentColumnThrowsException()
         $this->expectException(PropelException::class);
         $c = new ModelCriteria('bookstore', 'Propel\Tests\Bookstore\Book');
         $c->select(['Id', 'LeUnknonwColumn']);
+        $c->configureSelectColumns();
     }
 }

tests/Propel/Tests/Runtime/ActiveQuery/ModelCriteriaTest.php

@@ -3110,15 +3110,13 @@ public function testCloneCopiesFormatter()
     /**
      * @return void
      */
-    public function testCloneCopiesAsColumns()
+    public function testCloneCopiesSelect()
     {
         $bookQuery1 = BookQuery::create();
         $bookQuery1->select(['Id', 'Title']);
         $bookQuery2 = clone $bookQuery1;
         $bookQuery2->select(['ISBN', 'Price']);
-        $this->assertCount(2, $bookQuery1->getAsColumns());
-        $this->assertArrayHasKey('"Id"', $bookQuery1->getAsColumns());
-        $this->assertArrayHasKey('"Title"', $bookQuery1->getAsColumns());
+        $this->assertEquals(['Id', 'Title'], $bookQuery1->getSelect());
     }
 
     /**

tests/Propel/Tests/Runtime/ActiveQuery/SubQueryTest.php

@@ -283,6 +283,7 @@ public function testSubQueryWithSelectColumns()
         $c = new BookQuery();
         $c->addSelectQuery($subCriteria, 'alias1', false);
         $c->select(['alias1.Id']);
+        $c->configureSelectColumns();
 
         $sql = $this->getSql('SELECT alias1.id AS "alias1.Id" FROM (SELECT book.id, book.title, book.isbn, book.price, book.publisher_id, book.author_id FROM book) AS alias1');