Fixed construction of condition builder values (#17)

elvanto · Feb 7, 2022 · b5fc6aa · b5fc6aa
1 parent f39603c
commit b5fc6aa
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 57 deletions.
diff --git a/lib/PicoDb/Builder/AggregatedConditionBuilder.php b/lib/PicoDb/Builder/AggregatedConditionBuilder.php
@@ -3,7 +3,6 @@
 namespace PicoDb\Builder;
 
 use PicoDb\Database;
-use PicoDb\Table;
 
 /**
  * Class AggregatedConditionBuilder

diff --git a/lib/PicoDb/Builder/BaseConditionBuilder.php b/lib/PicoDb/Builder/BaseConditionBuilder.php
@@ -75,17 +75,6 @@ public function getValues()
         return $this->values;
     }
 
-    /**
-     * Adds condition values
-     *
-     * @access public
-     * @param  array $values
-     */
-    public function addValues($values)
-    {
-        $this->values = array_merge($this->values, $values);
-    }
-
     /**
      * Returns true if there is some conditions
      *
@@ -222,7 +211,7 @@ public function in($column, array $values)
     public function inSubquery($column, Table $subquery)
     {
         $this->addCondition($this->db->escapeIdentifier($column).' IN ('.$subquery->buildSelectQuery().')');
-        $this->values = array_merge($this->values, $subquery->getConditionBuilder()->getValues());
+        $this->values = array_merge($this->values, $subquery->getValues());
     }
 
     /**
@@ -250,7 +239,7 @@ public function notIn($column, array $values)
     public function notInSubquery($column, Table $subquery)
     {
         $this->addCondition($this->db->escapeIdentifier($column).' NOT IN ('.$subquery->buildSelectQuery().')');
-        $this->values = array_merge($this->values, $subquery->getConditionBuilder()->getValues());
+        $this->values = array_merge($this->values, $subquery->getValues());
     }
 
     /**
@@ -314,7 +303,7 @@ public function gt($column, $value)
     public function gtSubquery($column, Table $subquery)
     {
         $this->addCondition($this->db->escapeIdentifier($column).' > ('.$subquery->buildSelectQuery().')');
-        $this->values = array_merge($this->values, $subquery->getConditionBuilder()->getValues());
+        $this->values = array_merge($this->values, $subquery->getValues());
     }
 
     /**
@@ -340,7 +329,7 @@ public function lt($column, $value)
     public function ltSubquery($column, Table $subquery)
     {
         $this->addCondition($this->db->escapeIdentifier($column).' < ('.$subquery->buildSelectQuery().')');
-        $this->values = array_merge($this->values, $subquery->getConditionBuilder()->getValues());
+        $this->values = array_merge($this->values, $subquery->getValues());
     }
 
     /**
@@ -366,7 +355,7 @@ public function gte($column, $value)
     public function gteSubquery($column, Table $subquery)
     {
         $this->addCondition($this->db->escapeIdentifier($column).' >= ('.$subquery->buildSelectQuery().')');
-        $this->values = array_merge($this->values, $subquery->getConditionBuilder()->getValues());
+        $this->values = array_merge($this->values, $subquery->getValues());
     }
 
     /**
@@ -392,7 +381,7 @@ public function lte($column, $value)
     public function lteSubquery($column, Table $subquery)
     {
         $this->addCondition($this->db->escapeIdentifier($column).' <= ('.$subquery->buildSelectQuery().')');
-        $this->values = array_merge($this->values, $subquery->getConditionBuilder()->getValues());
+        $this->values = array_merge($this->values, $subquery->getValues());
     }
 
     /**

diff --git a/lib/PicoDb/Table.php b/lib/PicoDb/Table.php
@@ -835,7 +835,7 @@ public function buildSelectQuery()
 
     /**
      * Sets the conditionalBuilder flag to use AggregateConditionBuilder (HAVING)
-     * 
+     *
      * @return $this
      */
     public function having()
@@ -870,37 +870,25 @@ public function __call($name, array $arguments)
         } else {
             call_user_func_array(array($this->conditionBuilder, $name), $arguments);
         }
-        if (count($arguments) == 2 && end($arguments) instanceof Table) {
-            $this->addSubqueryValues(end($arguments));
-        }
+
         return $this;
     }
 
     /**
      * Clone function ensures that cloned objects are really clones
      */
-     public function __clone()
-     {
-         $this->conditionBuilder = clone $this->conditionBuilder;
-         $this->aggregatedConditionBuilder = clone $this->aggregatedConditionBuilder;
-     }
-
-     /**
-      * Adds values to builder when subquery is added to the other
-      *
-      * @access private
-      * @param  Table $subquery
-      */
-     private function addSubqueryValues(Table $subquery)
-     {
-         if ($this->conditionalBuilder === 'HAVING') {
-             $this->conditionBuilder->addValues($subquery->getConditionBuilder()->getValues());
-         } else {
-             $this->aggregatedConditionBuilder->addValues($subquery->getAggregatedConditionBuilder()->getValues());
-         }
-     }
-
-     private function getValues()
+    public function __clone()
+    {
+        $this->conditionBuilder = clone $this->conditionBuilder;
+        $this->aggregatedConditionBuilder = clone $this->aggregatedConditionBuilder;
+    }
+
+    /**
+     * Values used to construct a select query
+     *
+     * @return array
+     */
+    public function getValues()
     {
         return array_merge(
             $this->joinValues,

diff --git a/tests/MysqlTableTest.php b/tests/MysqlTableTest.php
@@ -241,7 +241,7 @@ public function testNotBetween()
         $this->assertEquals('SELECT * FROM `test`   WHERE `a` NOT BETWEEN ? AND ?', $query->buildSelectQuery());
         $this->assertEquals([1,5], $query->getConditionBuilder()->getValues());
     }
-    
+
     public function testConditionIsNull()
     {
         $table = $this->db->table('test');
@@ -311,7 +311,7 @@ public function testCountSubQueryHaving()
         $query = $this->db->table('foo')
             ->inSubquery('foo', $subQuery);
 
-        $this->assertEquals([100], $query->getAggregatedConditionBuilder()->getValues());
+        $this->assertEquals([2, 100], $query->getConditionBuilder()->getValues());
         $this->assertEquals(2, $query->count());
     }
 
@@ -346,8 +346,7 @@ public function testHavingSubquery()
         $table->inSubquery('a', $subquery);
 
         $this->assertEquals('SELECT * FROM `test`   WHERE `a` IS NOT NULL AND (`b` = ? OR `c` >= ?) AND `a` IN (SELECT `a` FROM `test`   GROUP BY `a`  HAVING SUM( d ) >= ?)', $table->buildSelectQuery());
-        $this->assertEquals(array(2, 5), $table->getConditionBuilder()->getValues());
-        $this->assertEquals(array(10), $table->getAggregatedConditionBuilder()->getValues());
+        $this->assertEquals(array(2, 5, 10), $table->getConditionBuilder()->getValues());
     }
 
     public function testPersist()
@@ -447,7 +446,7 @@ public function testSumSubqueryHaving()
             ->inSubquery('foo', $subQuery);
 
         $this->assertEquals('SELECT * FROM `foobar`   WHERE `status` = ? AND `foo` IN (SELECT foo FROM `foopoints`   GROUP BY `foo`  HAVING SUM(foopoints.points) > ?)', $query->buildSelectQuery());
-        $this->assertEquals([15], $query->getAggregatedConditionBuilder()->getValues());
+        $this->assertEquals([0, 15], $query->getConditionBuilder()->getValues());
         $this->assertEquals(6, $query->sum('foo'));
 
         $this->db->execute('DROP TABLE IF EXISTS foopoints');

diff --git a/tests/PostgresTableTest.php b/tests/PostgresTableTest.php
@@ -310,7 +310,7 @@ public function testCountSubQueryHaving()
         $query = $this->db->table('foo')
             ->inSubquery('foo', $subQuery);
 
-        $this->assertEquals([100], $query->getAggregatedConditionBuilder()->getValues());
+        $this->assertEquals([2, 100], $query->getConditionBuilder()->getValues());
         $this->assertEquals(2, $query->count());
     }
 
@@ -345,8 +345,7 @@ public function testHavingSubquery()
         $table->inSubquery('a', $subquery);
 
         $this->assertEquals('SELECT * FROM "test"   WHERE "a" IS NOT NULL AND ("b" = ? OR "c" >= ?) AND "a" IN (SELECT "a" FROM "test"   GROUP BY "a"  HAVING SUM( d ) >= ?)', $table->buildSelectQuery());
-        $this->assertEquals(array(2, 5), $table->getConditionBuilder()->getValues());
-        $this->assertEquals(array(10), $table->getAggregatedConditionBuilder()->getValues());
+        $this->assertEquals(array(2, 5, 10), $table->getConditionBuilder()->getValues());
     }
 
     public function testPersist()
@@ -446,7 +445,7 @@ public function testSumSubqueryHaving()
             ->inSubquery('foo', $subQuery);
 
         $this->assertEquals('SELECT * FROM "foobar"   WHERE "status" = ? AND "foo" IN (SELECT foo FROM "foopoints"   GROUP BY "foo"  HAVING SUM(foopoints.points) > ?)', $query->buildSelectQuery());
-        $this->assertEquals([15], $query->getAggregatedConditionBuilder()->getValues());
+        $this->assertEquals([0, 15], $query->getConditionBuilder()->getValues());
         $this->assertEquals(6, $query->sum('foo'));
 
         $this->db->execute('DROP TABLE IF EXISTS foopoints');

diff --git a/tests/SqliteTableTest.php b/tests/SqliteTableTest.php
@@ -356,7 +356,7 @@ public function testCountSubQueryHaving()
         $query = $this->db->table('foo')
             ->inSubquery('foo', $subQuery);
 
-        $this->assertEquals([100], $query->getAggregatedConditionBuilder()->getValues());
+        $this->assertEquals([2, 100], $query->getConditionBuilder()->getValues());
         $this->assertEquals(2, $query->count());
     }
 
@@ -391,8 +391,7 @@ public function testHavingSubquery()
         $table->inSubquery('a', $subquery);
 
         $this->assertEquals('SELECT * FROM "test"   WHERE "a" IS NOT NULL AND ("b" = ? OR "c" >= ?) AND "a" IN (SELECT "a" FROM "test"   GROUP BY "a"  HAVING SUM( d ) >= ?)', $table->buildSelectQuery());
-        $this->assertEquals(array(2, 5), $table->getConditionBuilder()->getValues());
-        $this->assertEquals(array(10), $table->getAggregatedConditionBuilder()->getValues());
+        $this->assertEquals(array(2, 5, 10), $table->getConditionBuilder()->getValues());
     }
 
     public function testMultipleOrConditions()
@@ -534,7 +533,7 @@ public function testSumSubqueryHaving()
             ->inSubquery('foo', $subQuery);
 
         $this->assertEquals('SELECT * FROM "foobar"   WHERE "status" = ? AND "foo" IN (SELECT foo FROM "foopoints"   GROUP BY "foo"  HAVING "points" > ?)', $query->buildSelectQuery());
-        $this->assertEquals([10], $query->getAggregatedConditionBuilder()->getValues());
+        $this->assertEquals([0, 10], $query->getConditionBuilder()->getValues());
         $this->assertEquals(6, $query->sum('foo'));
 
         $this->db->execute('DROP TABLE IF EXISTS foopoints');