Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed the type of $char in AbstractPlatform::getTrimExpression()from string|false to ?string #3491

Merged
merged 1 commit into from
Mar 18, 2019
Merged

Changed the type of $char in AbstractPlatform::getTrimExpression()from string|false to ?string #3491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions UPGRADE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Upgrade to 3.0

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

## BC BREAK `Statement::quote()` only accepts strings.

`Statement::quote()` and `ExpressionBuilder::literal()` no longer accept arguments of an arbitrary type and and don't implement type-specific handling. Only strings can be quoted.
Expand Down
37 changes: 25 additions & 12 deletions lib/Doctrine/DBAL/Platforms/AbstractPlatform.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,37 +753,50 @@ public function getModExpression($expression1, $expression2)
*
* @param string $str The expression to apply the trim to.
* @param int $mode The position of the trim (leading/trailing/both).
* @param string|bool $char The char to trim, has to be quoted already. Defaults to space.
* @param string|null $char The char to trim, has to be quoted already. Defaults to space.
*
* @return string
* @throws InvalidArgumentException
*/
public function getTrimExpression($str, $mode = TrimMode::UNSPECIFIED, $char = false)
public function getTrimExpression(string $str, int $mode = TrimMode::UNSPECIFIED, ?string $char = null) : string
{
$expression = '';
$tokens = [];

switch ($mode) {
case TrimMode::UNSPECIFIED:
break;

case TrimMode::LEADING:
$expression = 'LEADING ';
$tokens[] = 'LEADING';
break;

case TrimMode::TRAILING:
$expression = 'TRAILING ';
$tokens[] = 'TRAILING';
break;

case TrimMode::BOTH:
$expression = 'BOTH ';
$tokens[] = 'BOTH';
break;

default:
throw new InvalidArgumentException(
sprintf(
'The value of $mode is expected to be one of the TrimMode constants, %d given',
$mode
)
);
}

if ($char !== false) {
$expression .= $char . ' ';
if ($char !== null) {
$tokens[] = $char;
}

if ($mode || $char !== false) {
$expression .= 'FROM ';
if (count($tokens) > 0) {
$tokens[] = 'FROM';
}

return 'TRIM(' . $expression . $str . ')';
$tokens[] = $str;

return sprintf('TRIM(%s)', implode(' ', $tokens));
}

/**
Expand Down
15 changes: 11 additions & 4 deletions lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use function func_get_args;
use function get_class;
use function implode;
use function in_array;
use function is_string;
use function preg_match;
use function sprintf;
Expand Down Expand Up @@ -1121,10 +1122,16 @@ public function getTimeTypeDeclarationSQL(array $fieldDeclaration)
/**
* {@inheritdoc}
*/
public function getTrimExpression($str, $pos = TrimMode::UNSPECIFIED, $char = false)
public function getTrimExpression(string $str, int $mode = TrimMode::UNSPECIFIED, ?string $char = null) : string
{
if (! $char) {
switch ($pos) {
if (! in_array($mode, [TrimMode::UNSPECIFIED, TrimMode::LEADING, TrimMode::TRAILING, TrimMode::BOTH], true)) {
throw new InvalidArgumentException(
sprintf('The value of $mode is expected to be one of the TrimMode constants, %d given', $mode)
);
}

if ($char === null) {
switch ($mode) {
case TrimMode::LEADING:
return $this->getLtrimExpression($str);
case TrimMode::TRAILING:
Expand All @@ -1136,7 +1143,7 @@ public function getTrimExpression($str, $pos = TrimMode::UNSPECIFIED, $char = fa

$pattern = "'%[^' + " . $char . " + ']%'";

switch ($pos) {
switch ($mode) {
case TrimMode::LEADING:
return 'SUBSTR(' . $str . ', PATINDEX(' . $pattern . ', ' . $str . '))';
case TrimMode::TRAILING:
Expand Down
24 changes: 14 additions & 10 deletions lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use function explode;
use function func_get_args;
use function implode;
use function in_array;
use function is_array;
use function is_bool;
use function is_numeric;
Expand Down Expand Up @@ -1031,23 +1032,26 @@ public function getModExpression($expression1, $expression2)
/**
* {@inheritDoc}
*/
public function getTrimExpression($str, $pos = TrimMode::UNSPECIFIED, $char = false)
public function getTrimExpression(string $str, int $mode = TrimMode::UNSPECIFIED, ?string $char = null) : string
{
if (! $char) {
switch ($pos) {
if (! in_array($mode, [TrimMode::UNSPECIFIED, TrimMode::LEADING, TrimMode::TRAILING, TrimMode::BOTH], true)) {
throw new InvalidArgumentException(
sprintf('The value of $mode is expected to be one of the TrimMode constants, %d given', $mode)
);
}

if ($char === null) {
switch ($mode) {
case TrimMode::LEADING:
$trimFn = 'LTRIM';
break;
return 'LTRIM(' . $str . ')';

case TrimMode::TRAILING:
$trimFn = 'RTRIM';
return 'RTRIM(' . $str . ')';
break;

default:
return 'LTRIM(RTRIM(' . $str . '))';
}

return $trimFn . '(' . $str . ')';
}

/** Original query used to get those expressions
Expand All @@ -1061,11 +1065,11 @@ public function getTrimExpression($str, $pos = TrimMode::UNSPECIFIED, $char = fa
*/
$pattern = "'%[^' + " . $char . " + ']%'";

if ($pos === TrimMode::LEADING) {
if ($mode === TrimMode::LEADING) {
return 'stuff(' . $str . ', 1, patindex(' . $pattern . ', ' . $str . ') - 1, null)';
}

if ($pos === TrimMode::TRAILING) {
if ($mode === TrimMode::TRAILING) {
return 'reverse(stuff(reverse(' . $str . '), 1, patindex(' . $pattern . ', reverse(' . $str . ')) - 1, null))';
}

Expand Down
25 changes: 20 additions & 5 deletions lib/Doctrine/DBAL/Platforms/SqlitePlatform.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Doctrine\DBAL\Schema\TableDiff;
use Doctrine\DBAL\TransactionIsolationLevel;
use Doctrine\DBAL\Types;
use InvalidArgumentException;
use function array_merge;
use function array_unique;
use function array_values;
Expand Down Expand Up @@ -59,11 +60,14 @@ public function getNowExpression($type = 'timestamp')
/**
* {@inheritDoc}
*/
public function getTrimExpression($str, $pos = TrimMode::UNSPECIFIED, $char = false)
public function getTrimExpression(string $str, int $mode = TrimMode::UNSPECIFIED, ?string $char = null) : string
{
$trimChar = $char !== false ? (', ' . $char) : '';
switch ($mode) {
case TrimMode::UNSPECIFIED:
case TrimMode::BOTH:
$trimFn = 'TRIM';
break;

switch ($pos) {
case TrimMode::LEADING:
$trimFn = 'LTRIM';
break;
Expand All @@ -73,10 +77,21 @@ public function getTrimExpression($str, $pos = TrimMode::UNSPECIFIED, $char = fa
break;

default:
$trimFn = 'TRIM';
throw new InvalidArgumentException(
sprintf(
'The value of $mode is expected to be one of the TrimMode constants, %d given',
$mode
)
);
}

$arguments = [$str];

if ($char !== null) {
$arguments[] = $char;
}

return $trimFn . '(' . $str . $trimChar . ')';
return sprintf('%s(%s)', $trimFn, implode(', ', $arguments));
}

/**
Expand Down
23 changes: 15 additions & 8 deletions tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Doctrine\DBAL\Statement;
use Doctrine\DBAL\Types\Type;
use Doctrine\Tests\DbalFunctionalTestCase;
use InvalidArgumentException;
use PDO;
use const CASE_LOWER;
use const PHP_EOL;
Expand Down Expand Up @@ -508,10 +509,10 @@ public function testTrimExpression($value, $position, $char, $expectedResult)
public function getTrimExpressionData()
{
return [
['test_string', TrimMode::UNSPECIFIED, false, 'foo'],
['test_string', TrimMode::LEADING, false, 'foo'],
['test_string', TrimMode::TRAILING, false, 'foo'],
['test_string', TrimMode::BOTH, false, 'foo'],
['test_string', TrimMode::UNSPECIFIED, null, 'foo'],
['test_string', TrimMode::LEADING, null, 'foo'],
['test_string', TrimMode::TRAILING, null, 'foo'],
['test_string', TrimMode::BOTH, null, 'foo'],
['test_string', TrimMode::UNSPECIFIED, "'f'", 'oo'],
['test_string', TrimMode::UNSPECIFIED, "'o'", 'f'],
['test_string', TrimMode::UNSPECIFIED, "'.'", 'foo'],
Expand All @@ -524,10 +525,10 @@ public function getTrimExpressionData()
['test_string', TrimMode::BOTH, "'f'", 'oo'],
['test_string', TrimMode::BOTH, "'o'", 'f'],
['test_string', TrimMode::BOTH, "'.'", 'foo'],
["' foo '", TrimMode::UNSPECIFIED, false, 'foo'],
["' foo '", TrimMode::LEADING, false, 'foo '],
["' foo '", TrimMode::TRAILING, false, ' foo'],
["' foo '", TrimMode::BOTH, false, 'foo'],
["' foo '", TrimMode::UNSPECIFIED, null, 'foo'],
["' foo '", TrimMode::LEADING, null, 'foo '],
["' foo '", TrimMode::TRAILING, null, ' foo'],
["' foo '", TrimMode::BOTH, null, 'foo'],
["' foo '", TrimMode::UNSPECIFIED, "'f'", ' foo '],
["' foo '", TrimMode::UNSPECIFIED, "'o'", ' foo '],
["' foo '", TrimMode::UNSPECIFIED, "'.'", ' foo '],
Expand All @@ -547,6 +548,12 @@ public function getTrimExpressionData()
];
}

public function testTrimExpressionInvalidMode() : void
{
$this->expectException(InvalidArgumentException::class);
$this->connection->getDatabasePlatform()->getTrimExpression('Trim me!', 0xBEEF);
}

/**
* @group DDC-1014
*/
Expand Down
5 changes: 0 additions & 5 deletions tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,11 +665,6 @@ public function testGeneratesSQLSnippets()
"REVERSE(SUBSTR(REVERSE(column), PATINDEX('%[^' + c + ']%', REVERSE(column))))",
$this->platform->getTrimExpression('column', TrimMode::TRAILING, 'c')
);
self::assertEquals(
"REVERSE(SUBSTR(REVERSE(SUBSTR(column, PATINDEX('%[^' + c + ']%', column))), PATINDEX('%[^' + c + ']%', " .
"REVERSE(SUBSTR(column, PATINDEX('%[^' + c + ']%', column))))))",
$this->platform->getTrimExpression('column', null, 'c')
);
Copy link
Member Author

@morozov morozov Mar 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case abuses the API and covers the same scenario as the following one.

self::assertEquals(
"REVERSE(SUBSTR(REVERSE(SUBSTR(column, PATINDEX('%[^' + c + ']%', column))), PATINDEX('%[^' + c + ']%', " .
"REVERSE(SUBSTR(column, PATINDEX('%[^' + c + ']%', column))))))",
Expand Down