Fix subquery /w order but /wo limit (#1140)

atk4 · Oct 14, 2023 · 609ea70 · 609ea70
1 parent 8e53675
commit 609ea70
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 6 deletions.
diff --git a/src/Persistence/Sql/Expression.php b/src/Persistence/Sql/Expression.php
@@ -193,7 +193,9 @@ protected function consume($expr, string $escapeMode = self::ESCAPE_PARAM)
         $expressionParamBaseBackup = $expr->paramBase;
         try {
             $expr->paramBase = $this->renderParamBase;
-            [$sql, $params] = $expr->render();
+            [$sql, $params] = $expr->wrapInParentheses
+                    ? $expr->renderNested()
+                    : $expr->render();
             foreach ($params as $k => $v) {
                 $this->renderParams[$k] = $v;
             }
@@ -209,11 +211,6 @@ protected function consume($expr, string $escapeMode = self::ESCAPE_PARAM)
             $expr->paramBase = $expressionParamBaseBackup;
         }
 
-        // wrap in parentheses if expression requires so
-        if ($expr->wrapInParentheses) {
-            $sql = '(' . $sql . ')';
-        }
-
         return $sql;
     }
 
@@ -438,6 +435,16 @@ public function render(): array
         }
     }
 
+    /**
+     * @return array{string, array<string, mixed>}
+     */
+    protected function renderNested(): array
+    {
+        [$sql, $params] = $this->render();
+
+        return ['(' . $sql . ')', $params];
+    }
+
     /**
      * Return formatted debug SQL query.
      */

diff --git a/src/Persistence/Sql/Query.php b/src/Persistence/Sql/Query.php
@@ -942,6 +942,25 @@ public function render(): array
         return parent::render();
     }
 
+    protected function renderNested(): array
+    {
+        if (isset($this->args['order']) && !isset($this->args['limit'])) {
+            $orderOrig = $this->args['order'];
+            unset($this->args['order']);
+        } else {
+            $orderOrig = null;
+        }
+        try {
+            [$sql, $params] = parent::renderNested();
+        } finally {
+            if ($orderOrig !== null) {
+                $this->args['order'] = $orderOrig;
+            }
+        }
+
+        return [$sql, $params];
+    }
+
     /**
      * Switch template for this query. Determines what would be done
      * on execute.

diff --git a/tests/Persistence/Sql/WithDb/SelectTest.php b/tests/Persistence/Sql/WithDb/SelectTest.php
@@ -450,4 +450,32 @@ public function testImportAndAutoincrement(): void
             ['id' => 102, 'f1' => 'M'],
         ], $m->export());
     }
+
+    public function testSubqueryWithOrderAndLimit(): void
+    {
+        $subQuery = $this->q('employee');
+        $query = $this->q($subQuery, 't')->field('name')->order('name');
+
+        self::assertSame(
+            [['name' => 'Charlie'], ['name' => 'Harry'], ['name' => 'Jack'], ['name' => 'Oliver']],
+            $query->getRows()
+        );
+
+        // subquery /w limit but /wo order
+        $subQuery->limit(2);
+        self::assertCount(2, $query->getRows());
+
+        $subQuery->order('surname', true);
+        self::assertSame(
+            [['name' => 'Harry'], ['name' => 'Jack']],
+            $query->getRows()
+        );
+
+        // subquery /w order but /wo limit
+        $subQuery->args['limit'] = null;
+        self::assertSame(
+            [['name' => 'Charlie'], ['name' => 'Harry'], ['name' => 'Jack'], ['name' => 'Oliver']],
+            $query->getRows()
+        );
+    }
 }