From f91b279ef0fd7e8ff26de971352fcb8731528d37 Mon Sep 17 00:00:00 2001
From: Maxime Rainville <maxime.rainville@archipro.co.nz>
Date: Thu, 25 Jul 2024 13:17:37 +1200
Subject: [PATCH] FIX Don't generate table alias for "from" statement that are
 not column names.

---
 src/ORM/Queries/SQLConditionalExpression.php | 13 ++-
 tests/php/ORM/SQLSelectTest.php              | 97 ++++++++++++++++++++
 2 files changed, 109 insertions(+), 1 deletion(-)

diff --git a/src/ORM/Queries/SQLConditionalExpression.php b/src/ORM/Queries/SQLConditionalExpression.php
index 20c16c61a..ba6afda0f 100644
--- a/src/ORM/Queries/SQLConditionalExpression.php
+++ b/src/ORM/Queries/SQLConditionalExpression.php
@@ -78,7 +78,18 @@ abstract class SQLConditionalExpression extends SQLExpression
         if (is_array($from)) {
             $this->from = array_merge($this->from, $from);
         } elseif (!empty($from)) {
-            $this->from[str_replace(['"','`'], '', $from)] = $from;
+            // Check if the from clause looks like a regular table name
+            // Table name most be an uninterrupted string, with no spaces.
+            // It may be padded with spaces. e.g. ` TableName ` will be
+            // treated as Table Name, but not ` Table Name `.
+            if (preg_match('/^\s*[^\s]+\s*$/', $from)) {
+                // Add an alias for the table name, stripping any quotes
+                $this->from[str_replace(['"','`'], '', $from)] = $from;
+            } else {
+                // Add from clause without an alias - this is probably a full
+                // sub-select with its own explicit alias.
+                $this->from[] = $from;
+            }
         }
 
         return $this;
diff --git a/tests/php/ORM/SQLSelectTest.php b/tests/php/ORM/SQLSelectTest.php
index 1d4b2f8f5..73f3a586f 100755
--- a/tests/php/ORM/SQLSelectTest.php
+++ b/tests/php/ORM/SQLSelectTest.php
@@ -1327,4 +1327,101 @@ class SQLSelectTest extends SapphireTest
 
         $select->addWith('cte', new SQLSelect());
     }
+
+    public function subqueryProvider()
+    {
+        return [
+            'no-explicit-alias-string' => ['( SELECT DISTINCT "SQLSelectTest_DO"."ClassName" FROM "SQLSelectTest_DO") AS "FINAL"'],
+            'no-alias-array' => [['( SELECT DISTINCT "SQLSelectTest_DO"."ClassName" FROM "SQLSelectTest_DO") AS "FINAL"']],
+            'no-alias-array-numeric-key' => [[0 => '( SELECT DISTINCT "SQLSelectTest_DO"."ClassName" FROM "SQLSelectTest_DO") AS "FINAL"']],
+            'explicit-alias-string' => [['FINAL' => '( SELECT DISTINCT "SQLSelectTest_DO"."ClassName" FROM "SQLSelectTest_DO")']],
+        ];
+    }
+
+    /**
+     * @dataProvider subqueryProvider
+     */
+    public function testSubqueries($subquery)
+    {
+        $query = new SQLSelect('*', $subquery);
+
+        $actualSQL = $query->sql();
+
+        $this->assertSQLEquals(
+            'SELECT * FROM ( SELECT DISTINCT "SQLSelectTest_DO"."ClassName" FROM "SQLSelectTest_DO") AS "FINAL"',
+            $actualSQL
+        );
+    }
+
+    public function addFromProvider()
+    {
+        return [
+            'string' => [
+                'MyTable', ['MyTable' => 'MyTable'],
+                'Plain table name get alias automatic alias'
+            ],
+            'string padded with spaces' => [
+                ' MyTable  ', [' MyTable  ' => ' MyTable  '],
+                'Plain table name get alias automatic alias'
+            ],
+            'quoted string' => [
+                '"MyTable"', ['MyTable' => '"MyTable"'],
+                'Quoted table name get alias without the quotes'
+            ],
+            'underscore in table name string' => [
+                '"My_Table_123"', ['My_Table_123' => '"My_Table_123"'],
+                'Numbers and underscores are allowed in table names'
+            ],
+            'backtick string' => [
+                '`MyTable`', ['MyTable' => '`MyTable`'],
+                'Backtick quoted table name get alias without the quotes'
+            ],
+            'subquery string' => [
+                ' (SELECT * from "FooBar") as FooBar ', [' (SELECT * from "FooBar") as FooBar '],
+                'String that don\'t look like table name don\'t get alias'
+            ],
+            'array' => [
+                ['MyTable'], ['MyTable'],
+                'Arrays are passed through as is'
+            ],
+            'array-associative-key' => [
+                ['MyTableAlias' => 'MyTable'], ['MyTableAlias' => 'MyTable'],
+                'Associative arrays are passed through as is and aliases are preserved'
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider addFromProvider
+     */
+    public function testAddFrom($input, $out, $message = ""): void
+    {
+        $query = new SQLSelect();
+        $query->addFrom($input);
+        $this->assertEquals($out, $query->getFrom(), $message);
+    }
+
+    public function testAddFromRetainPreviousData()
+    {
+        // Initial setup
+        $query = new SQLSelect();
+        $query->addFrom('MyTable');
+        $query->addFrom('"MyOtherTable"');
+
+        // This will override some value and add a new one
+        $query->addFrom([
+            'MyTable' => '(SELECT * FROM "MyTable" where "Foo" = "Bar")',
+            'ThirdTable',
+        ]);
+
+        $this->assertEquals(
+            [
+                'MyTable' => '(SELECT * FROM "MyTable" where "Foo" = "Bar")',
+                'MyOtherTable' => '"MyOtherTable"',
+                'ThirdTable',
+            ],
+            $query->getFrom(),
+            'MyTable entry got merge over, MyOtherTable was retained, ThirdTable was added'
+        );
+    }
 }