Skip to content

Commit

Permalink
[DBAL-2495] Optimize and improve parameter conversion in OCI8Statement
Browse files Browse the repository at this point in the history
1. Reworked the parser syntax to use proper Oracle escaping syntax as '' instead of \'.
2. Moved valid tests to the Functional section to ensure correct SQL syntax and logic.
  • Loading branch information
morozov committed May 24, 2017
1 parent 9c3afca commit 8cbf648
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 38 deletions.
2 changes: 1 addition & 1 deletion lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ private static function findClosingQuote(
$token = self::findToken(
$statement,
$tokenOffset,
'/(?<!\\\\)' . preg_quote($currentLiteralDelimiter, '/') . '/'
'/' . preg_quote($currentLiteralDelimiter, '/') . '/'
);

if (!$token) {
Expand Down
52 changes: 15 additions & 37 deletions tests/Doctrine/Tests/DBAL/Driver/OCI8/OCI8StatementTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,52 +86,30 @@ public static function executeDataProvider()
}

/**
* @dataProvider convertSuccessProvider
* @dataProvider nonTerminatedLiteralProvider
*/
public function testConvertSuccess($statement, $expectedStatement, $expectedParamCount)
public function testConvertNonTerminatedLiteral($sql, $message)
{
list($converted, $paramMap) = OCI8Statement::convertPositionalToNamedPlaceholders($statement);
$this->assertEquals($expectedStatement, $converted);
$this->assertCount($expectedParamCount, $paramMap);
$this->expectException(OCI8Exception::class);
$this->expectExceptionMessageRegExp($message);
OCI8Statement::convertPositionalToNamedPlaceholders($sql);
}

public static function convertSuccessProvider()
public static function nonTerminatedLiteralProvider()
{
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,
'no-matching-quote' => array(
"SELECT 'literal FROM DUAL",
'/offset 7/',
),
'literal-with-quotes' => array(
"INSERT INTO table (col1, col2, col3, col4) VALUES('?\"?\\'?', ?, \"?'?\\\"?\", ?)",
"INSERT INTO table (col1, col2, col3, col4) VALUES('?\"?\'?', :param1, \"?'?\\\"?\", :param2)",
2,
'no-matching-double-quote' => array(
'SELECT 1 "COL1 FROM DUAL',
'/offset 9/',
),
'placeholder-at-the-end' => array(
"SELECT id FROM table WHERE col = ?",
"SELECT id FROM table WHERE col = :param1",
1,
),
'multi-line-literal' => array(
"SELECT id FROM table WHERE col1 = ? AND col2 = 'Hello,
World!' AND col3 = ?",
"SELECT id FROM table WHERE col1 = :param1 AND col2 = 'Hello,
World!' AND col3 = :param2",
2,
'incorrect-escaping-syntax' => array(
"SELECT 'quoted \\'string' FROM DUAL",
'/offset 23/',
),
);
}

public function testConvertNonTerminatedLiteral()
{
$this->expectException(OCI8Exception::class);
$this->expectExceptionMessageRegExp('/offset 7/');
OCI8Statement::convertPositionalToNamedPlaceholders('SELECT "literal');
}
}
85 changes: 85 additions & 0 deletions tests/Doctrine/Tests/DBAL/Functional/Driver/OCI8/StatementTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?php

namespace Doctrine\Tests\DBAL\Functional\Driver\OCI8;

use Doctrine\DBAL\Driver\OCI8\Driver;
use Doctrine\Tests\DbalFunctionalTestCase;

class StatementTest extends DbalFunctionalTestCase
{
protected function setUp()
{
if (! extension_loaded('oci8')) {
$this->markTestSkipped('oci8 is not installed.');
}

parent::setUp();

if (! $this->_conn->getDriver() instanceof Driver) {
$this->markTestSkipped('oci8 only test.');
}
}

/**
* @dataProvider queryConversionProvider
*/
public function testQueryConversion($query, array $params, array $expected)
{
$this->assertEquals(
$expected,
$this->_conn->executeQuery($query, $params)->fetch()
);
}

public static function queryConversionProvider()
{
return array(
'simple' => array(
'SELECT ? COL1 FROM DUAL',
array(1),
array(
'COL1' => 1,
),
),
'literal-with-placeholder' => array(
"SELECT '?' COL1, ? COL2 FROM DUAL",
array(2),
array(
'COL1' => '?',
'COL2' => 2,
),
),
'literal-with-quotes' => array(
"SELECT ? COL1, '?\"?''?' \"COL?\" FROM DUAL",
array(3),
array(
'COL1' => 3,
'COL?' => '?"?\'?',
),
),
'placeholder-at-the-end' => array(
'SELECT ? COL1 FROM DUAL WHERE 1 = ?',
array(4, 1),
array(
'COL1' => 4,
),
),
'multi-line-literal' => array(
"SELECT 'Hello,
World?!' COL1 FROM DUAL WHERE 1 = ?",
array(1),
array(
'COL1' => 'Hello,
World?!',
),
),
'empty-literal' => array(
"SELECT '' COL1 FROM DUAL",
array(),
array(
'COL1' => '',
),
),
);
}
}

0 comments on commit 8cbf648

Please sign in to comment.