Optimize parameter conversion for Oracle. Handle quotes inside literals.

lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php

@@ -121,27 +121,68 @@ class OCI8Statement implements \IteratorAggregate, Statement
      * @param string $statement The SQL statement to convert.
      *
      * @return string
+     * @throws \Doctrine\DBAL\Driver\OCI8\OCI8Exception
      */
     static public function convertPositionalToNamedPlaceholders($statement)
     {
-        $count = 1;
-        $inLiteral = false; // a valid query never starts with quotes
-        $stmtLen = strlen($statement);
         $paramMap = array();
-        for ($i = 0; $i < $stmtLen; $i++) {
-            if ($statement[$i] == '?' && !$inLiteral) {
-                // real positional parameter detected
-                $paramMap[$count] = ":param$count";
-                $len = strlen($paramMap[$count]);
-                $statement = substr_replace($statement, ":param$count", $i, 1);
-                $i += $len-1; // jump ahead
-                $stmtLen = strlen($statement); // adjust statement length
-                ++$count;
-            } elseif ($statement[$i] == "'" || $statement[$i] == '"') {
-                $inLiteral = ! $inLiteral; // switch state!
+        $count = 1;
+        $start = $offset = 0;
+        $fragments = array();
+        $quote = false;
+        $capture = function ($regex, $callback) use ($statement, $start, &$offset) {
+            if (preg_match($regex, $statement, $matches, PREG_OFFSET_CAPTURE, $offset)) {
+                $offset = $matches[0][1];
+                $callback($matches[0][0]);
+                return true;
+            }
+            return false;
+        };
+
+        do {
+            if (!$quote) {
+                $result = $capture('/[?\'"]/', function ($token) use (
+                    $statement,
+                    &$fragments,
+                    &$start,
+                    &$offset,
+                    &$quote,
+                    &$count,
+                    &$paramMap
+                ) {
+                    if ($token == '?') {
+                        $param = ':param' . $count;
+                        $fragments[] = substr($statement, $start, $offset - $start);
+                        $fragments[] = $param;
+                        $paramMap[$count] = $param;
+                        $start = ++$offset;
+                        ++$count;
+                    } else {
+                        $quote = $token;
+                        ++$offset;
+                    }
+                });
+            } else {
+                $result = $capture('/(?<!\\\\)' . $quote . '/', function () use (&$offset, &$quote) {
+                    $quote = false;
+                    ++$offset;
+                });
             }
+        } while ($result);
+
+        if ($quote) {
+            throw new OCI8Exception(sprintf(
+                'The statement contains non-terminated string literal starting at offset %d',
+                $offset - 1
+            ));
+        }
+
+        if ($start < strlen($statement)) {
+            $fragments[] = substr($statement, $start);
         }
 
+        $statement = implode('', $fragments);
+
         return array($statement, $paramMap);
     }
 

tests/Doctrine/Tests/DBAL/Driver/OCI8/OCI8StatementTest.php

@@ -2,7 +2,11 @@
 
 namespace Doctrine\Tests\DBAL;
 
-class OCI8StatementTest extends \Doctrine\Tests\DbalTestCase
+use Doctrine\DBAL\Driver\OCI8\OCI8Exception;
+use Doctrine\DBAL\Driver\OCI8\OCI8Statement;
+use Doctrine\Tests\DbalTestCase;
+
+class OCI8StatementTest extends DbalTestCase
 {
     protected function setUp()
     {
@@ -81,4 +85,52 @@ class OCI8StatementTest extends \Doctrine\Tests\DbalTestCase
         );
     }
 
+    /**
+     * @dataProvider convertSuccessProvider
+     */
+    public function testConvertSuccess($statement, $expectedStatement, $expectedParamCount)
+    {
+        list($converted, $paramMap) = OCI8Statement::convertPositionalToNamedPlaceholders($statement);
+        $this->assertEquals($expectedStatement, $converted);
+        $this->assertCount($expectedParamCount, $paramMap);
+    }
+
+    public static function convertSuccessProvider()
+    {
+        return array(
+            'simple' => array(
+                'INSERT INTO table (column) VALUES(?)',
+                'INSERT INTO table (column) VALUES(:param1)',
+                1,
+            ),
+            'literal-with-placeholder' => array(
+                "INSERT INTO table (col1, col2, col3) VALUES('?', ?, \"?\", ?)",
+                "INSERT INTO table (col1, col2, col3) VALUES('?', :param1, \"?\", :param2)",
+                2,
+            ),
+            'literal-with-quotes' => array(
+                "INSERT INTO table (col1, col2, col3, col4) VALUES('?\"?\\'?', ?, \"?'?\\\"?\", ?)",
+                "INSERT INTO table (col1, col2, col3, col4) VALUES('?\"?\'?', :param1, \"?'?\\\"?\", :param2)",
+                2,
+            ),
+        );
+    }
+
+    /**
+     * @dataProvider convertFailureProvider
+     */
+    public function testConvertFailure($statement)
+    {
+        $this->expectException(OCI8Exception::class);
+        OCI8Statement::convertPositionalToNamedPlaceholders($statement);
+    }
+
+    public static function convertFailureProvider()
+    {
+        return array(
+            'non-terminated-literal' => array(
+                'SELECT "literal',
+            ),
+        );
+    }
 }