Merging changes from 2.x

v2.0 Performance Regression with Parameterized Conditionals #129
FaaPz · Jan 22, 2020 · 4bf3d59 · 4bf3d59
1 parent e753cd9
commit 4bf3d59
Show file tree

Hide file tree

Showing 17 changed files with 242 additions and 90 deletions.
diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml
@@ -8,8 +8,8 @@ jobs:
     strategy:
       max-parallel: 15
       matrix:
-        operating-system: [ubuntu-latest, windows-latest, macOS-latest]
-        php-versions: ['7.2', '7.3']
+        operating-system: [ubuntu-latest]
+        php-versions: ['7.2', '7.3', '7.4']
     name: PHP ${{ matrix.php-versions }} Test on ${{ matrix.operating-system }}
     steps:
       - name: Checkout
@@ -18,9 +18,10 @@ jobs:
         uses: shivammathur/setup-php@master
         with:
           php-version: ${{ matrix.php-versions }}
-          extension-csv: ast, mbstring, pdo
-          ini-values-csv: "date.timezone=UTC"
+          extensions: ast, mbstring, pdo
+          ini-values: "date.timezone=UTC"
           coverage: PCOV
+          pecl: true
       - name: Check PHP Version
         run: php -v
       - name: Check PHP Extensions

diff --git a/composer.json b/composer.json
@@ -16,7 +16,7 @@
             "name": "Alexander Barker",
             "email": "[email protected]",
             "homepage": "https://github.com/kwhat/",
-            "role": "Developer"
+            "role": "Collaborator"
         }
     ],
     "support": {

diff --git a/src/AbstractStatement.php b/src/AbstractStatement.php
@@ -35,9 +35,9 @@ public function execute()
         try {
             $success = $stmt->execute($this->getValues());
             if (!$success) {
-                $info = $stmt->errorInfo();
+                list($state, $code, $message) = $stmt->errorInfo();
 
-                throw new DatabaseException($info[2], $info[0]);
+                throw new DatabaseException($message, $state);
             }
         } catch (PDOException $e) {
             throw new DatabaseException($e->getMessage(), $e->getCode(), $e);

diff --git a/src/Clause/Conditional.php b/src/Clause/Conditional.php
@@ -43,38 +43,70 @@ public function getValues(): array
             $values = [$values];
         }
 
+        $count = count($values);
+        for ($i = 0; $i < $count; $i++) {
+            if ($values[$i] instanceof QueryInterface) {
+                $value = $values[$i]->getValues();
+                array_splice($values, $i, 1, $value);
+                $i += count($value);
+            }
+        }
+
         return $values;
     }
 
     /**
-     * @throws DatabaseException
+     * @param mixed $value
      *
      * @return string
+     * @throws DatabaseException
+     */
+    protected function getPlaceholder($value): string
+    {
+        $placeholder = '?';
+        if ($value instanceof QueryInterface) {
+            $placeholder = "{$value}";
+        }
+
+        return $placeholder;
+    }
+
+    /**
+     * @return string
+     * @throws DatabaseException
      */
     public function __toString(): string
     {
-        $sql = "{$this->column} {$this->operator}";
+        $sql = "{$this->column} {$this->operator} ";
         switch ($this->operator) {
             case 'BETWEEN':
             case 'NOT BETWEEN':
-                if (count($this->getValues()) != 2) {
+                if (count($this->value) != 2) {
                     throw new DatabaseException('Conditional operator "BETWEEN" requires two arguments');
                 }
 
-                $sql .= ' (? AND ?)';
+                $sql .= "({$this->getPlaceholder($this->value[0])} AND {$this->getPlaceholder($this->value[0])})";
                 break;
 
             case 'IN':
             case 'NOT IN':
-                if (count($this->getValues()) < 1) {
+                if (count($this->value) < 1) {
                     throw new DatabaseException('Conditional operator "IN" requires at least one argument');
                 }
 
-                $sql .= ' (' . substr(str_repeat('?, ', count($this->getValues())), 0, -2) . ')';
+                $placeholders = '';
+                foreach ($this->value as $value) {
+                    if (!empty($placeholders)) {
+                        $placeholders .= ', ';
+                    }
+
+                    $placeholders .= $this->getPlaceholder($value);
+                }
+                $sql .= "({$placeholders})";
                 break;
 
             default:
-                $sql .= ' ?';
+                $sql .= $this->getPlaceholder($this->value);
         }
 
         return $sql;

diff --git a/src/Clause/Grouping.php b/src/Clause/Grouping.php
@@ -14,11 +14,13 @@ class Grouping extends Conditional
 
     /**
      * @param string      $rule
+     * @param Conditional $clause
      * @param Conditional ...$clauses
      */
-    public function __construct(string $rule, Conditional ...$clauses)
+    public function __construct(string $rule, Conditional $clause, Conditional ...$clauses)
     {
-        parent::__construct('', strtoupper(trim($rule)), $clauses);
+        array_unshift($clauses, $clause);
+        parent::__construct('', $rule, $clauses);
     }
 
     /**
@@ -41,13 +43,17 @@ public function __toString(): string
     {
         $sql = '';
         foreach ($this->value as $clause) {
-            if ($clause instanceof self) {
-                $clause = "({$clause})";
+            if (!empty($sql)) {
+                $sql .= " {$this->operator} ";
             }
 
-            $sql .= "{$clause} {$this->operator} ";
+            if ($clause instanceof self) {
+                $sql .= "({$clause})";
+            } else {
+                $sql .= "{$clause}";
+            }
         }
 
-        return preg_replace('/' . preg_quote(" $this->operator ", '/') . '$/', '', $sql);
+        return $sql;
     }
 }
diff --git a/src/Clause/Join.php b/src/Clause/Join.php
@@ -43,9 +43,8 @@ public function getValues(): array
     }
 
     /**
-     * @throws DatabaseException
-     *
      * @return string
+     * @throws DatabaseException
      */
     public function __toString(): string
     {

diff --git a/src/Clause/Limit.php b/src/Clause/Limit.php
@@ -21,7 +21,7 @@ class Limit implements QueryInterface
      * @param int      $size
      * @param int|null $offset
      */
-    public function __construct(int $size, int $offset = null)
+    public function __construct(int $size, ?int $offset = null)
     {
         $this->size = $size;
         $this->offset = $offset;
@@ -32,11 +32,11 @@ public function __construct(int $size, int $offset = null)
      */
     public function getValues(): array
     {
-        $values = [];
-        if (isset($this->offset)) {
-            $values[] = $this->offset;
+        if ($this->offset !== null) {
+            $values = [$this->offset, $this->size];
+        } else {
+            $values = [$this->size];
         }
-        $values[] = $this->size;
 
         return $values;
     }
@@ -46,8 +46,8 @@ public function getValues(): array
      */
     public function __toString(): string
     {
-        $sql = '?';
-        if (isset($this->offset)) {
+        $sql = 'LIMIT ?';
+        if ($this->offset !== null) {
             $sql .= ', ?';
         }
 

diff --git a/src/Clause/Method.php b/src/Clause/Method.php
@@ -34,7 +34,9 @@ public function getValues(): array
     {
         $values = [];
         foreach ($this->values as $value) {
-            if (!$value instanceof QueryInterface) {
+            if ($value instanceof QueryInterface) {
+                $values = array_merge($values, $value->getValues());
+            } else {
                 $values[] = $value;
             }
         }
@@ -49,11 +51,13 @@ public function __toString(): string
     {
         $placeholders = '';
         foreach ($this->values as $value) {
-            if (!$value instanceof QueryInterface) {
-                if (!empty($placeholders)) {
-                    $placeholders .= ', ';
-                }
+            if (!empty($placeholders)) {
+                $placeholders .= ', ';
+            }
 
+            if ($value instanceof QueryInterface) {
+                $placeholders .= "{$value}";
+            } else {
                 $placeholders .= '?';
             }
         }

diff --git a/src/Statement/Call.php b/src/Statement/Call.php
@@ -55,18 +55,15 @@ public function getValues(): array
     }
 
     /**
-     * @throws DatabaseException
-     *
      * @return string
+     * @throws DatabaseException
      */
     public function __toString(): string
     {
         if (!$this->method instanceof Clause\Method) {
-            throw new DatabaseException('No method is set for stored procedure call');
+            throw new DatabaseException('No method set for call statement');
         }
 
-        $sql = "CALL {$this->method}";
-
-        return $sql;
+        return "CALL {$this->method}";
     }
 }
diff --git a/src/Statement/Delete.php b/src/Statement/Delete.php
@@ -67,9 +67,8 @@ public function getValues(): array
     }
 
     /**
-     * @throws DatabaseException
-     *
      * @return string
+     * @throws DatabaseException
      */
     public function __toString(): string
     {
@@ -95,7 +94,7 @@ public function __toString(): string
             $sql .= ' ' . implode(' ', $this->join);
         }
 
-        if ($this->where != null) {
+        if ($this->where !== null) {
             $sql .= " WHERE {$this->where}";
         }
 
@@ -107,8 +106,8 @@ public function __toString(): string
             $sql = substr($sql, 0, -2);
         }
 
-        if ($this->limit != null) {
-            $sql .= " LIMIT {$this->limit}";
+        if ($this->limit !== null) {
+            $sql .= " {$this->limit}";
         }
 
         return $sql;

diff --git a/src/Statement/Insert.php b/src/Statement/Insert.php
@@ -97,45 +97,56 @@ public function ignore(): self
     }
 
     /**
-     * @throws DatabaseException
-     *
      * @return string
+     * @throws DatabaseException
      */
     public function __toString(): string
     {
         if (empty($this->table)) {
             throw new DatabaseException('No table is set for insertion');
         }
 
-        if (empty($this->columns)) {
+        $size = count($this->values);
+        if ($size < 1) {
             throw new DatabaseException('Missing columns for insertion');
         }
 
-        if (empty($this->values) || count($this->columns) != count($this->values)) {
+        if (count($this->columns) > 0 && count($this->columns) != count($this->values)) {
             throw new DatabaseException('Missing values for insertion');
         }
 
-        $placeholders = '';
-        foreach ($this->values as $value) {
-            if (!empty($placeholders)) {
-                $placeholders .= ', ';
+        if ($this->values[0] instanceof Select) {
+            if (count($this->values) > 1) {
+                throw new DatabaseException('Ignoring additional values after select for insert statement');
             }
 
-            if ($value instanceof QueryInterface) {
-                $placeholders .= "{$value}";
-            } else {
-                $placeholders .= '?';
+            $placeholders = " {$this->values[0]}";
+        } else {
+            $plug = '';
+            foreach ($this->values as $value) {
+                if (!empty($plug)) {
+                    $plug .= ', ';
+                }
+
+                if ($value instanceof QueryInterface) {
+                    $plug .= "{$value}";
+                } else {
+                    $plug .= '?';
+                }
             }
-        }
 
-        $columns = implode(', ', $this->columns);
+            $placeholders = " VALUES ({$plug})";
+        }
 
         $sql = 'INSERT';
         if ($this->ignore) {
             $sql .= ' IGNORE';
         }
-        $sql .= " INTO {$this->table} ({$columns})";
-        $sql .= " VALUES ({$placeholders})";
+        $sql .= " INTO {$this->table}";
+        if (!empty($this->columns)) {
+            $sql .= ' (' . implode(', ', $this->columns) . ')';
+        }
+        $sql .= "{$placeholders}";
 
         return $sql;
     }
@@ -158,9 +169,8 @@ public function getValues(): array
     }
 
     /**
-     * @throws DatabaseException
-     *
      * @return int|string
+     * @throws DatabaseException
      */
     public function execute()
     {