Skip to content

Commit

Permalink
Merge pull request #3494 from morozov/get-locate-expr
Browse files Browse the repository at this point in the history 
Modified AbstractPlatform::getLocateExpression() and ::getSubstringExpression() signatures
  • Loading branch information
@Ocramius @morozov
Ocramius authored and morozov committed Jun 13, 2019
2 parents 8c59135 + e61102e commit 9af7a9f
Show file tree
Hide file tree
Showing 12 changed files with 111 additions and 81 deletions.
8 changes: 8 additions & 0 deletions UPGRADE.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Upgrade to 3.0

## BC BREAK The type of `$start` in `AbstractPlatform::getLocateExpression()` changed from `string|false` to `?string`

The default value of `$start` is now `null`, not `false`.

## BC BREAK The types of `$start` and `$length` in `AbstractPlatform::getSubstringExpression()` changed from `int` and `?int` to `string` and `?string` respectively

The platform abstraction allows building arbitrary SQL expressions, so even if the arguments represent numeric literals, they should be passed as a string.

## BC BREAK The type of `$char` in `AbstractPlatform::getTrimExpression()` changed from `string|false` to `?string`

The default value of `$char` is now `null`, not `false`. Additionally, the method will throw an `InvalidArgumentException` in an invalid value of `$mode` is passed.
Expand Down
32 changes: 14 additions & 18 deletions lib/Doctrine/DBAL/Platforms/AbstractPlatform.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -850,17 +850,16 @@ public function getLowerExpression($str)
}

/**
* Returns the SQL snippet to get the position of the first occurrence of substring $substr in string $str.
* Returns the SQL snippet to get the position of the first occurrence of the substring in the string.
*
* @param string $str Literal string.
* @param string $substr Literal string to find.
* @param int|false $startPos Position to start at, beginning of string by default.
*
* @return string
* @param string $string SQL expression producing the string to locate the substring in.
* @param string $substring SQL expression producing the substring to locate.
* @param string|null $start SQL expression producing the position to start at.
* Defaults to the beginning of the string.
*
* @throws DBALException If not supported on this platform.
*/
public function getLocateExpression($str, $substr, $startPos = false)
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
{
throw DBALException::notSupported(__METHOD__);
}
Expand All @@ -876,25 +875,22 @@ public function getNowExpression()
}

/**
* Returns a SQL snippet to get a substring inside an SQL statement.
* Returns an SQL snippet to get a substring inside the string.
*
* Note: Not SQL92, but common functionality.
*
* SQLite only supports the 2 parameter variant of this function.
*
* @param string $value An sql string literal or column name/alias.
* @param int $from Where to start the substring portion.
* @param int|null $length The substring portion length.
*
* @return string
* @param string $string SQL expression producing the string from which a substring should be extracted.
* @param string $start SQL expression producing the position to start at,
* @param string|null $length SQL expression producing the length of the substring portion to be returned.
* By default, the entire substring is returned.
*/
public function getSubstringExpression($value, $from, $length = null)
public function getSubstringExpression(string $string, string $start, ?string $length = null) : string
{
if ($length === null) {
return 'SUBSTRING(' . $value . ' FROM ' . $from . ')';
return sprintf('SUBSTRING(%s FROM %s)', $string, $start);
}

return 'SUBSTRING(' . $value . ' FROM ' . $from . ' FOR ' . $length . ')';
return sprintf('SUBSTRING(%s FROM %s FOR %s)', $string, $start, $length);
}

/**
Expand Down
14 changes: 7 additions & 7 deletions lib/Doctrine/DBAL/Platforms/DB2Platform.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -814,25 +814,25 @@ protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) :
/**
* {@inheritDoc}
*/
public function getLocateExpression($str, $substr, $startPos = false)
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
{
if ($startPos === false) {
return 'LOCATE(' . $substr . ', ' . $str . ')';
if ($start === null) {
return sprintf('LOCATE(%s, %s)', $substring, $string);
}

return 'LOCATE(' . $substr . ', ' . $str . ', ' . $startPos . ')';
return sprintf('LOCATE(%s, %s, %s)', $substring, $string, $start);
}

/**
* {@inheritDoc}
*/
public function getSubstringExpression($value, $from, $length = null)
public function getSubstringExpression(string $string, string $start, ?string $length = null) : string
{
if ($length === null) {
return 'SUBSTR(' . $value . ', ' . $from . ')';
return sprintf('SUBSTR(%s, %s)', $string, $start);
}

return 'SUBSTR(' . $value . ', ' . $from . ', ' . $length . ')';
return sprintf('SUBSTR(%s, %s, %s)', $string, $start, $length);
}

/**
Expand Down
8 changes: 4 additions & 4 deletions lib/Doctrine/DBAL/Platforms/MySqlPlatform.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@ public function getRegexpExpression()
/**
* {@inheritDoc}
*/
public function getLocateExpression($str, $substr, $startPos = false)
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
{
if ($startPos === false) {
return 'LOCATE(' . $substr . ', ' . $str . ')';
if ($start === null) {
return sprintf('LOCATE(%s, %s)', $substring, $string);
}

return 'LOCATE(' . $substr . ', ' . $str . ', ' . $startPos . ')';
return sprintf('LOCATE(%s, %s, %s)', $substring, $string, $start);
}

/**
Expand Down
16 changes: 8 additions & 8 deletions lib/Doctrine/DBAL/Platforms/OraclePlatform.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ public static function assertValidIdentifier($identifier)
/**
* {@inheritDoc}
*/
public function getSubstringExpression($value, $position, $length = null)
public function getSubstringExpression(string $string, string $start, ?string $length = null) : string
{
if ($length !== null) {
return sprintf('SUBSTR(%s, %d, %d)', $value, $position, $length);
if ($length === null) {
return sprintf('SUBSTR(%s, %s)', $string, $start);
}

return sprintf('SUBSTR(%s, %d)', $value, $position);
return sprintf('SUBSTR(%s, %s, %s)', $string, $start, $length);
}

/**
Expand All @@ -73,13 +73,13 @@ public function getNowExpression($type = 'timestamp')
/**
* {@inheritDoc}
*/
public function getLocateExpression($str, $substr, $startPos = false)
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
{
if ($startPos === false) {
return 'INSTR(' . $str . ', ' . $substr . ')';
if ($start === null) {
return sprintf('INSTR(%s, %s)', $string, $substring);
}

return 'INSTR(' . $str . ', ' . $substr . ', ' . $startPos . ')';
return sprintf('INSTR(%s, %s, %s)', $string, $substring, $start);
}

/**
Expand Down
22 changes: 5 additions & 17 deletions lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,6 @@ public function setUseBooleanTrueFalseStrings($flag)
$this->useBooleanTrueFalseStrings = (bool) $flag;
}

/**
* {@inheritDoc}
*/
public function getSubstringExpression($value, $from, $length = null)
{
if ($length === null) {
return 'SUBSTRING(' . $value . ' FROM ' . $from . ')';
}

return 'SUBSTRING(' . $value . ' FROM ' . $from . ' FOR ' . $length . ')';
}

/**
* {@inheritDoc}
*/
Expand All @@ -106,15 +94,15 @@ public function getRegexpExpression()
/**
* {@inheritDoc}
*/
public function getLocateExpression($str, $substr, $startPos = false)
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
{
if ($startPos !== false) {
$str = $this->getSubstringExpression($str, $startPos);
if ($start !== null) {
$string = $this->getSubstringExpression($string, $start);

return 'CASE WHEN (POSITION(' . $substr . ' IN ' . $str . ') = 0) THEN 0 ELSE (POSITION(' . $substr . ' IN ' . $str . ') + ' . ($startPos-1) . ') END';
return 'CASE WHEN (POSITION(' . $substring . ' IN ' . $string . ') = 0) THEN 0 ELSE (POSITION(' . $substring . ' IN ' . $string . ') + ' . $start . ' - 1) END';
}

return 'POSITION(' . $substr . ' IN ' . $str . ')';
return sprintf('POSITION(%s IN %s)', $substring, $string);
}

/**
Expand Down
14 changes: 7 additions & 7 deletions lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -966,13 +966,13 @@ public function getListViewsSQL($database)
/**
* {@inheritdoc}
*/
public function getLocateExpression($str, $substr, $startPos = false)
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
{
if ($startPos === false) {
return 'LOCATE(' . $str . ', ' . $substr . ')';
if ($start === null) {
return sprintf('LOCATE(%s, %s)', $string, $substring);
}

return 'LOCATE(' . $str . ', ' . $substr . ', ' . $startPos . ')';
return sprintf('LOCATE(%s, %s, %s)', $string, $substring, $start);
}

/**
Expand Down Expand Up @@ -1089,13 +1089,13 @@ public function getStopDatabaseSQL($database)
/**
* {@inheritdoc}
*/
public function getSubstringExpression($value, $from, $length = null)
public function getSubstringExpression(string $string, string $start, ?string $length = null) : string
{
if ($length === null) {
return 'SUBSTRING(' . $value . ', ' . $from . ')';
return sprintf('SUBSTRING(%s, %s)', $string, $start);
}

return 'SUBSTRING(' . $value . ', ' . $from . ', ' . $length . ')';
return sprintf('SUBSTRING(%s, %s, %s)', $string, $start, $length);
}

/**
Expand Down
16 changes: 8 additions & 8 deletions lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1011,13 +1011,13 @@ public function getDropViewSQL($name)
/**
* {@inheritDoc}
*/
public function getLocateExpression($str, $substr, $startPos = false)
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
{
if ($startPos === false) {
return 'CHARINDEX(' . $substr . ', ' . $str . ')';
if ($start === null) {
return sprintf('CHARINDEX(%s, %s)', $substring, $string);
}

return 'CHARINDEX(' . $substr . ', ' . $str . ', ' . $startPos . ')';
return sprintf('CHARINDEX(%s, %s, %s)', $substring, $string, $start);
}

/**
Expand Down Expand Up @@ -1103,13 +1103,13 @@ public function getListNamespacesSQL()
/**
* {@inheritDoc}
*/
public function getSubstringExpression($value, $from, $length = null)
public function getSubstringExpression(string $string, string $start, ?string $length = null) : string
{
if ($length !== null) {
return 'SUBSTRING(' . $value . ', ' . $from . ', ' . $length . ')';
if ($length === null) {
return sprintf('SUBSTRING(%s, %s, LEN(%s) - %s + 1)', $string, $start, $string, $start);
}

return 'SUBSTRING(' . $value . ', ' . $from . ', LEN(' . $value . ') - ' . $from . ' + 1)';
return sprintf('SUBSTRING(%s, %s, %s)', $string, $start, $length);
}

/**
Expand Down
18 changes: 8 additions & 10 deletions lib/Doctrine/DBAL/Platforms/SqlitePlatform.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,28 +96,26 @@ public function getTrimExpression(string $str, int $mode = TrimMode::UNSPECIFIED

/**
* {@inheritDoc}
*
* SQLite only supports the 2 parameter variant of this function
*/
public function getSubstringExpression($value, $position, $length = null)
public function getSubstringExpression(string $string, string $start, ?string $length = null) : string
{
if ($length !== null) {
return 'SUBSTR(' . $value . ', ' . $position . ', ' . $length . ')';
if ($length === null) {
return sprintf('SUBSTR(%s, %s)', $string, $start);
}

return 'SUBSTR(' . $value . ', ' . $position . ', LENGTH(' . $value . '))';
return sprintf('SUBSTR(%s, %s, %s)', $string, $start, $length);
}

/**
* {@inheritDoc}
*/
public function getLocateExpression($str, $substr, $startPos = false)
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
{
if ($startPos === false) {
return 'LOCATE(' . $str . ', ' . $substr . ')';
if ($start === null) {
return sprintf('LOCATE(%s, %s)', $string, $substring);
}

return 'LOCATE(' . $str . ', ' . $substr . ', ' . $startPos . ')';
return sprintf('LOCATE(%s, %s, %s)', $string, $substring, $start);
}

/**
Expand Down
41 changes: 41 additions & 0 deletions tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,47 @@ public function testLocateExpression() : void
self::assertEquals(0, $row['locate9']);
}

/**
* @dataProvider substringExpressionProvider
*/
public function testSubstringExpression(string $string, string $start, ?string $length, string $expected) : void
{
$platform = $this->connection->getDatabasePlatform();

$query = $platform->getDummySelectSQL(
$platform->getSubstringExpression($string, $start, $length)
);

$this->assertEquals($expected, $this->connection->fetchColumn($query));
}

/**
* @return mixed[][]
*/
public static function substringExpressionProvider() : iterable
{
return [
'start-no-length' => [
"'abcdef'",
'3',
null,
'cdef',
],
'start-with-length' => [
"'abcdef'",
'2',
'4',
'bcde',
],
'expressions' => [
"'abcdef'",
'1 + 1',
'1 + 1',
'bc',
],
];
}

public function testQuoteSQLInjection() : void
{
$sql = 'SELECT * FROM fetch_table WHERE test_string = ' . $this->connection->quote("bar' OR '1'='1");
Expand Down
1 change: 0 additions & 1 deletion tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,6 @@ public function testGeneratesSQLSnippets() : void
self::assertEquals("'1987/05/02' - 10 YEAR", $this->platform->getDateSubYearsExpression("'1987/05/02'", 10));
self::assertEquals(' WITH RR USE AND KEEP UPDATE LOCKS', $this->platform->getForUpdateSQL());
self::assertEquals('LOCATE(substring_column, string_column)', $this->platform->getLocateExpression('string_column', 'substring_column'));
self::assertEquals('LOCATE(substring_column, string_column)', $this->platform->getLocateExpression('string_column', 'substring_column'));
self::assertEquals('LOCATE(substring_column, string_column, 1)', $this->platform->getLocateExpression('string_column', 'substring_column', 1));
self::assertEquals('SUBSTR(column, 5)', $this->platform->getSubstringExpression('column', 5));
self::assertEquals('SUBSTR(column, 5, 2)', $this->platform->getSubstringExpression('column', 5, 2));
Expand Down
2 changes: 1 addition & 1 deletion tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public function getGenerateTableWithMultiColumnUniqueIndexSql() : array
public function testGeneratesSqlSnippets() : void
{
self::assertEquals('REGEXP', $this->platform->getRegexpExpression(), 'Regular expression operator is not correct');
self::assertEquals('SUBSTR(column, 5, LENGTH(column))', $this->platform->getSubstringExpression('column', 5), 'Substring expression without length is not correct');
self::assertEquals('SUBSTR(column, 5)', $this->platform->getSubstringExpression('column', 5), 'Substring expression without length is not correct');
self::assertEquals('SUBSTR(column, 0, 5)', $this->platform->getSubstringExpression('column', 0, 5), 'Substring expression with length is not correct');
}

Expand Down

0 comments on commit 9af7a9f

Please sign in to comment.