Skip to content

Commit 6c5f79b

Browse files
Ocramiusmorozov
Ocramius
authored and
morozov
committed
Merge pull request #3494 from morozov/get-locate-expr
Modified AbstractPlatform::getLocateExpression() and ::getSubstringExpression() signatures
2 parents 195a2ad + fab415a commit 6c5f79b

File tree

12 files changed

+111
-81
lines changed

12 files changed

+111
-81
lines changed

‎UPGRADE.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
# Upgrade to 3.0
22

3+
## BC BREAK The type of `$start` in `AbstractPlatform::getLocateExpression()` changed from `string|false` to `?string`
4+
5+
The default value of `$start` is now `null`, not `false`.
6+
7+
## BC BREAK The types of `$start` and `$length` in `AbstractPlatform::getSubstringExpression()` changed from `int` and `?int` to `string` and `?string` respectively
8+
9+
The platform abstraction allows building arbitrary SQL expressions, so even if the arguments represent numeric literals, they should be passed as a string.
10+
311
## BC BREAK The type of `$char` in `AbstractPlatform::getTrimExpression()` changed from `string|false` to `?string`
412

513
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.

‎lib/Doctrine/DBAL/Platforms/AbstractPlatform.php

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -850,17 +850,16 @@ public function getLowerExpression($str)
850850
}
851851

852852
/**
853-
* Returns the SQL snippet to get the position of the first occurrence of substring $substr in string $str.
853+
* Returns the SQL snippet to get the position of the first occurrence of the substring in the string.
854854
*
855-
* @param string $str Literal string.
856-
* @param string $substr Literal string to find.
857-
* @param int|false $startPos Position to start at, beginning of string by default.
858-
*
859-
* @return string
855+
* @param string $string SQL expression producing the string to locate the substring in.
856+
* @param string $substring SQL expression producing the substring to locate.
857+
* @param string|null $start SQL expression producing the position to start at.
858+
* Defaults to the beginning of the string.
860859
*
861860
* @throws DBALException If not supported on this platform.
862861
*/
863-
public function getLocateExpression($str, $substr, $startPos = false)
862+
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
864863
{
865864
throw DBALException::notSupported(__METHOD__);
866865
}
@@ -876,25 +875,22 @@ public function getNowExpression()
876875
}
877876

878877
/**
879-
* Returns a SQL snippet to get a substring inside an SQL statement.
878+
* Returns an SQL snippet to get a substring inside the string.
880879
*
881880
* Note: Not SQL92, but common functionality.
882881
*
883-
* SQLite only supports the 2 parameter variant of this function.
884-
*
885-
* @param string $value An sql string literal or column name/alias.
886-
* @param int $from Where to start the substring portion.
887-
* @param int|null $length The substring portion length.
888-
*
889-
* @return string
882+
* @param string $string SQL expression producing the string from which a substring should be extracted.
883+
* @param string $start SQL expression producing the position to start at,
884+
* @param string|null $length SQL expression producing the length of the substring portion to be returned.
885+
* By default, the entire substring is returned.
890886
*/
891-
public function getSubstringExpression($value, $from, $length = null)
887+
public function getSubstringExpression(string $string, string $start, ?string $length = null) : string
892888
{
893889
if ($length === null) {
894-
return 'SUBSTRING(' . $value . ' FROM ' . $from . ')';
890+
return sprintf('SUBSTRING(%s FROM %s)', $string, $start);
895891
}
896892

897-
return 'SUBSTRING(' . $value . ' FROM ' . $from . ' FOR ' . $length . ')';
893+
return sprintf('SUBSTRING(%s FROM %s FOR %s)', $string, $start, $length);
898894
}
899895

900896
/**

‎lib/Doctrine/DBAL/Platforms/DB2Platform.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -814,25 +814,25 @@ protected function doModifyLimitQuery(string $query, ?int $limit, int $offset) :
814814
/**
815815
* {@inheritDoc}
816816
*/
817-
public function getLocateExpression($str, $substr, $startPos = false)
817+
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
818818
{
819-
if ($startPos === false) {
820-
return 'LOCATE(' . $substr . ', ' . $str . ')';
819+
if ($start === null) {
820+
return sprintf('LOCATE(%s, %s)', $substring, $string);
821821
}
822822

823-
return 'LOCATE(' . $substr . ', ' . $str . ', ' . $startPos . ')';
823+
return sprintf('LOCATE(%s, %s, %s)', $substring, $string, $start);
824824
}
825825

826826
/**
827827
* {@inheritDoc}
828828
*/
829-
public function getSubstringExpression($value, $from, $length = null)
829+
public function getSubstringExpression(string $string, string $start, ?string $length = null) : string
830830
{
831831
if ($length === null) {
832-
return 'SUBSTR(' . $value . ', ' . $from . ')';
832+
return sprintf('SUBSTR(%s, %s)', $string, $start);
833833
}
834834

835-
return 'SUBSTR(' . $value . ', ' . $from . ', ' . $length . ')';
835+
return sprintf('SUBSTR(%s, %s, %s)', $string, $start, $length);
836836
}
837837

838838
/**

‎lib/Doctrine/DBAL/Platforms/MySqlPlatform.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,13 @@ public function getRegexpExpression()
8181
/**
8282
* {@inheritDoc}
8383
*/
84-
public function getLocateExpression($str, $substr, $startPos = false)
84+
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
8585
{
86-
if ($startPos === false) {
87-
return 'LOCATE(' . $substr . ', ' . $str . ')';
86+
if ($start === null) {
87+
return sprintf('LOCATE(%s, %s)', $substring, $string);
8888
}
8989

90-
return 'LOCATE(' . $substr . ', ' . $str . ', ' . $startPos . ')';
90+
return sprintf('LOCATE(%s, %s, %s)', $substring, $string, $start);
9191
}
9292

9393
/**

‎lib/Doctrine/DBAL/Platforms/OraclePlatform.php

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,13 @@ public static function assertValidIdentifier($identifier)
4747
/**
4848
* {@inheritDoc}
4949
*/
50-
public function getSubstringExpression($value, $position, $length = null)
50+
public function getSubstringExpression(string $string, string $start, ?string $length = null) : string
5151
{
52-
if ($length !== null) {
53-
return sprintf('SUBSTR(%s, %d, %d)', $value, $position, $length);
52+
if ($length === null) {
53+
return sprintf('SUBSTR(%s, %s)', $string, $start);
5454
}
5555

56-
return sprintf('SUBSTR(%s, %d)', $value, $position);
56+
return sprintf('SUBSTR(%s, %s, %s)', $string, $start, $length);
5757
}
5858

5959
/**
@@ -73,13 +73,13 @@ public function getNowExpression($type = 'timestamp')
7373
/**
7474
* {@inheritDoc}
7575
*/
76-
public function getLocateExpression($str, $substr, $startPos = false)
76+
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
7777
{
78-
if ($startPos === false) {
79-
return 'INSTR(' . $str . ', ' . $substr . ')';
78+
if ($start === null) {
79+
return sprintf('INSTR(%s, %s)', $string, $substring);
8080
}
8181

82-
return 'INSTR(' . $str . ', ' . $substr . ', ' . $startPos . ')';
82+
return sprintf('INSTR(%s, %s, %s)', $string, $substring, $start);
8383
}
8484

8585
/**

‎lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -75,18 +75,6 @@ public function setUseBooleanTrueFalseStrings($flag)
7575
$this->useBooleanTrueFalseStrings = (bool) $flag;
7676
}
7777

78-
/**
79-
* {@inheritDoc}
80-
*/
81-
public function getSubstringExpression($value, $from, $length = null)
82-
{
83-
if ($length === null) {
84-
return 'SUBSTRING(' . $value . ' FROM ' . $from . ')';
85-
}
86-
87-
return 'SUBSTRING(' . $value . ' FROM ' . $from . ' FOR ' . $length . ')';
88-
}
89-
9078
/**
9179
* {@inheritDoc}
9280
*/
@@ -106,15 +94,15 @@ public function getRegexpExpression()
10694
/**
10795
* {@inheritDoc}
10896
*/
109-
public function getLocateExpression($str, $substr, $startPos = false)
97+
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
11098
{
111-
if ($startPos !== false) {
112-
$str = $this->getSubstringExpression($str, $startPos);
99+
if ($start !== null) {
100+
$string = $this->getSubstringExpression($string, $start);
113101

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

117-
return 'POSITION(' . $substr . ' IN ' . $str . ')';
105+
return sprintf('POSITION(%s IN %s)', $substring, $string);
118106
}
119107

120108
/**

‎lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -966,13 +966,13 @@ public function getListViewsSQL($database)
966966
/**
967967
* {@inheritdoc}
968968
*/
969-
public function getLocateExpression($str, $substr, $startPos = false)
969+
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
970970
{
971-
if ($startPos === false) {
972-
return 'LOCATE(' . $str . ', ' . $substr . ')';
971+
if ($start === null) {
972+
return sprintf('LOCATE(%s, %s)', $string, $substring);
973973
}
974974

975-
return 'LOCATE(' . $str . ', ' . $substr . ', ' . $startPos . ')';
975+
return sprintf('LOCATE(%s, %s, %s)', $string, $substring, $start);
976976
}
977977

978978
/**
@@ -1089,13 +1089,13 @@ public function getStopDatabaseSQL($database)
10891089
/**
10901090
* {@inheritdoc}
10911091
*/
1092-
public function getSubstringExpression($value, $from, $length = null)
1092+
public function getSubstringExpression(string $string, string $start, ?string $length = null) : string
10931093
{
10941094
if ($length === null) {
1095-
return 'SUBSTRING(' . $value . ', ' . $from . ')';
1095+
return sprintf('SUBSTRING(%s, %s)', $string, $start);
10961096
}
10971097

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

11011101
/**

‎lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,13 +1012,13 @@ public function getDropViewSQL($name)
10121012
/**
10131013
* {@inheritDoc}
10141014
*/
1015-
public function getLocateExpression($str, $substr, $startPos = false)
1015+
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
10161016
{
1017-
if ($startPos === false) {
1018-
return 'CHARINDEX(' . $substr . ', ' . $str . ')';
1017+
if ($start === null) {
1018+
return sprintf('CHARINDEX(%s, %s)', $substring, $string);
10191019
}
10201020

1021-
return 'CHARINDEX(' . $substr . ', ' . $str . ', ' . $startPos . ')';
1021+
return sprintf('CHARINDEX(%s, %s, %s)', $substring, $string, $start);
10221022
}
10231023

10241024
/**
@@ -1104,13 +1104,13 @@ public function getListNamespacesSQL()
11041104
/**
11051105
* {@inheritDoc}
11061106
*/
1107-
public function getSubstringExpression($value, $from, $length = null)
1107+
public function getSubstringExpression(string $string, string $start, ?string $length = null) : string
11081108
{
1109-
if ($length !== null) {
1110-
return 'SUBSTRING(' . $value . ', ' . $from . ', ' . $length . ')';
1109+
if ($length === null) {
1110+
return sprintf('SUBSTRING(%s, %s, LEN(%s) - %s + 1)', $string, $start, $string, $start);
11111111
}
11121112

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

11161116
/**

‎lib/Doctrine/DBAL/Platforms/SqlitePlatform.php

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -96,28 +96,26 @@ public function getTrimExpression(string $str, int $mode = TrimMode::UNSPECIFIED
9696

9797
/**
9898
* {@inheritDoc}
99-
*
100-
* SQLite only supports the 2 parameter variant of this function
10199
*/
102-
public function getSubstringExpression($value, $position, $length = null)
100+
public function getSubstringExpression(string $string, string $start, ?string $length = null) : string
103101
{
104-
if ($length !== null) {
105-
return 'SUBSTR(' . $value . ', ' . $position . ', ' . $length . ')';
102+
if ($length === null) {
103+
return sprintf('SUBSTR(%s, %s)', $string, $start);
106104
}
107105

108-
return 'SUBSTR(' . $value . ', ' . $position . ', LENGTH(' . $value . '))';
106+
return sprintf('SUBSTR(%s, %s, %s)', $string, $start, $length);
109107
}
110108

111109
/**
112110
* {@inheritDoc}
113111
*/
114-
public function getLocateExpression($str, $substr, $startPos = false)
112+
public function getLocateExpression(string $string, string $substring, ?string $start = null) : string
115113
{
116-
if ($startPos === false) {
117-
return 'LOCATE(' . $str . ', ' . $substr . ')';
114+
if ($start === null) {
115+
return sprintf('LOCATE(%s, %s)', $string, $substring);
118116
}
119117

120-
return 'LOCATE(' . $str . ', ' . $substr . ', ' . $startPos . ')';
118+
return sprintf('LOCATE(%s, %s, %s)', $string, $substring, $start);
121119
}
122120

123121
/**

‎tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -674,6 +674,47 @@ public function testLocateExpression()
674674
self::assertEquals(0, $row['locate9']);
675675
}
676676

677+
/**
678+
* @dataProvider substringExpressionProvider
679+
*/
680+
public function testSubstringExpression(string $string, string $start, ?string $length, string $expected) : void
681+
{
682+
$platform = $this->connection->getDatabasePlatform();
683+
684+
$query = $platform->getDummySelectSQL(
685+
$platform->getSubstringExpression($string, $start, $length)
686+
);
687+
688+
$this->assertEquals($expected, $this->connection->fetchColumn($query));
689+
}
690+
691+
/**
692+
* @return mixed[][]
693+
*/
694+
public static function substringExpressionProvider() : iterable
695+
{
696+
return [
697+
'start-no-length' => [
698+
"'abcdef'",
699+
'3',
700+
null,
701+
'cdef',
702+
],
703+
'start-with-length' => [
704+
"'abcdef'",
705+
'2',
706+
'4',
707+
'bcde',
708+
],
709+
'expressions' => [
710+
"'abcdef'",
711+
'1 + 1',
712+
'1 + 1',
713+
'bc',
714+
],
715+
];
716+
}
717+
677718
public function testQuoteSQLInjection()
678719
{
679720
$sql = 'SELECT * FROM fetch_table WHERE test_string = ' . $this->connection->quote("bar' OR '1'='1");

‎tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,6 @@ public function testGeneratesSQLSnippets()
333333
self::assertEquals("'1987/05/02' - 10 YEAR", $this->platform->getDateSubYearsExpression("'1987/05/02'", 10));
334334
self::assertEquals(' WITH RR USE AND KEEP UPDATE LOCKS', $this->platform->getForUpdateSQL());
335335
self::assertEquals('LOCATE(substring_column, string_column)', $this->platform->getLocateExpression('string_column', 'substring_column'));
336-
self::assertEquals('LOCATE(substring_column, string_column)', $this->platform->getLocateExpression('string_column', 'substring_column'));
337336
self::assertEquals('LOCATE(substring_column, string_column, 1)', $this->platform->getLocateExpression('string_column', 'substring_column', 1));
338337
self::assertEquals('SUBSTR(column, 5)', $this->platform->getSubstringExpression('column', 5));
339338
self::assertEquals('SUBSTR(column, 5, 2)', $this->platform->getSubstringExpression('column', 5, 2));

‎tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public function getGenerateTableWithMultiColumnUniqueIndexSql()
3333
public function testGeneratesSqlSnippets()
3434
{
3535
self::assertEquals('REGEXP', $this->platform->getRegexpExpression(), 'Regular expression operator is not correct');
36-
self::assertEquals('SUBSTR(column, 5, LENGTH(column))', $this->platform->getSubstringExpression('column', 5), 'Substring expression without length is not correct');
36+
self::assertEquals('SUBSTR(column, 5)', $this->platform->getSubstringExpression('column', 5), 'Substring expression without length is not correct');
3737
self::assertEquals('SUBSTR(column, 0, 5)', $this->platform->getSubstringExpression('column', 0, 5), 'Substring expression with length is not correct');
3838
}
3939

0 commit comments

Comments
 (0)