Skip to content
This repository was archived by the owner on Jan 29, 2020. It is now read-only.

Adding a fix for #267. #268

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Adding a fix for #267. #268

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
81f398d
Adding a fix for #267.
rarog Sep 27, 2017
2b3a1ca
Merge branch 'master' of https://github.com/zendframework/zend-db.git
rarog Apr 28, 2018
8397dc0
Reverting BC
rarog Apr 28, 2018
b5f8a27
Repairing tests
rarog Apr 28, 2018
402d5ec
Correcting errors done by reverting BC changes and adding tests.
rarog Apr 28, 2018
fb331fb
Fixing formatting
rarog Apr 28, 2018
e0426c0
Adding linebreaks to strings.
rarog Apr 28, 2018
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
2 changes: 1 addition & 1 deletion src/Adapter/Platform/AbstractPlatform.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ abstract class AbstractPlatform implements PlatformInterface
/**
* @var string
*/
protected $quoteIdentifierTo = '\'';
protected $quoteIdentifierTo = '""';
Copy link
Contributor

@mwillbanks mwillbanks Oct 20, 2017

Choose a reason for hiding this comment

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

Why would you change the default, this is absolutely a MASSIVE BC BREAK. I advise you to please no do that especially considering this could have been extended.

Copy link
Author

@rarog rarog Oct 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I could leave it as it is, but in that case I'd comment it, that it's only for BC reasons and contradicts the ANSI SQL standard.
I can't name any SQL implementation that uses single quotes for escaping if identifiers. The single quote is for strings.

I attach a wall of text aka TL;DR:

In chapter 5.2 <token> and <separator> of
Information Technology - Database Language SQL it's defined as

         <delimited identifier> ::=
              <double quote> <delimited identifier body> <double quote>

         <delimited identifier body> ::= <delimited identifier part>...

         <delimited identifier part> ::=
                <nondoublequote character>
              | <doublequote symbol>

         <nondoublequote character> ::= !! See the Syntax Rules

         <doublequote symbol> ::= <double quote><double quote>

         <delimiter token> ::=
                <character string literal>
              | <date string>
              | <time string>
              | <timestamp string>
              | <interval string>
              | <delimited identifier>

and same is recommended by Apache docs:

Ordinary identifiers are identifiers not surrounded by double quotation marks. Delimited identifiers are identifiers surrounded by double quotation marks.

By standard DB/2, Oracle, PostgreSQL and SQLite use double quote for identifier escaping. Sybase and MS SQLServer default to quoting via brackets [] but for example SQLServer does support the double quote when SET QUOTED_IDENTIFIER is ON, and it is on, in ANSI mode:

When SET ANSI_DEFAULTS is ON, SET QUOTED_IDENTIFIER is enabled.

MySQL/MariaDB understand by default the backtick, but also do support double quote in ANSI mode:
SET sql_mode = 'ANSI'; enables it for the running session.


/**
* @var bool
Expand Down
7 changes: 0 additions & 7 deletions src/Adapter/Platform/Postgresql.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@

class Postgresql extends AbstractPlatform
{
/**
* Overrides value from AbstractPlatform to use proper escaping for Postgres
*
* @var string
*/
protected $quoteIdentifierTo = '""';

/**
* @var resource|\PDO
*/
Expand Down
10 changes: 0 additions & 10 deletions src/Adapter/Platform/Sqlite.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,6 @@

class Sqlite extends AbstractPlatform
{
/**
* {@inheritDoc}
*/
protected $quoteIdentifier = ['"','"'];

/**
* {@inheritDoc}
*/
protected $quoteIdentifierTo = '\'';

/**
* @var \PDO
*/
Expand Down
2 changes: 2 additions & 0 deletions src/Sql/AbstractSql.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ protected function processExpression(
);
} elseif ($type == ExpressionInterface::TYPE_IDENTIFIER) {
$values[$vIndex] = $platform->quoteIdentifierInFragment($value);
} elseif ($type == ExpressionInterface::TYPE_IDENTIFIER_ATOMIC) {
$values[$vIndex] = $platform->quoteIdentifier($value);
} elseif ($type == ExpressionInterface::TYPE_VALUE) {
// if prepareType is set, it means that this particular value must be
// passed back to the statement in a way it can be used as a placeholder value
Expand Down
2 changes: 1 addition & 1 deletion src/Sql/Ddl/Column/AbstractTimestampColumn.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public function getExpressionData()
$params[] = $this->name;
$params[] = $this->type;

$types = [self::TYPE_IDENTIFIER, self::TYPE_LITERAL];
$types = [self::TYPE_IDENTIFIER_ATOMIC, self::TYPE_LITERAL];

if (!$this->isNullable) {
$spec .= ' NOT NULL';
Expand Down
2 changes: 1 addition & 1 deletion src/Sql/Ddl/Column/Column.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public function getExpressionData()
$params[] = $this->name;
$params[] = $this->type;

$types = [self::TYPE_IDENTIFIER, self::TYPE_LITERAL];
$types = [self::TYPE_IDENTIFIER_ATOMIC, self::TYPE_LITERAL];

if (!$this->isNullable) {
$spec .= ' NOT NULL';
Expand Down
4 changes: 2 additions & 2 deletions src/Sql/Ddl/Constraint/AbstractConstraint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,15 +109,15 @@ public function getExpressionData()
if ($this->name) {
$newSpec .= $this->namedSpecification;
$values[] = $this->name;
$newSpecTypes[] = self::TYPE_IDENTIFIER;
$newSpecTypes[] = self::TYPE_IDENTIFIER_ATOMIC;
}

$newSpec .= $this->specification;

if ($colCount) {
$values = array_merge($values, $this->columns);
$newSpecParts = array_fill(0, $colCount, '%s');
$newSpecTypes = array_merge($newSpecTypes, array_fill(0, $colCount, self::TYPE_IDENTIFIER));
$newSpecTypes = array_merge($newSpecTypes, array_fill(0, $colCount, self::TYPE_IDENTIFIER_ATOMIC));
$newSpec .= sprintf($this->columnSpecification, implode(', ', $newSpecParts));
}

Expand Down
4 changes: 2 additions & 2 deletions src/Sql/Ddl/Index/Index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public function getExpressionData()
$colCount = count($this->columns);
$values = [];
$values[] = $this->name ?: '';
$newSpecTypes = [self::TYPE_IDENTIFIER];
$newSpecTypes = [self::TYPE_IDENTIFIER_ATOMIC];
$newSpecParts = [];

for ($i = 0; $i < $colCount; $i++) {
Expand All @@ -66,7 +66,7 @@ public function getExpressionData()
}

$newSpecParts[] = $specPart;
$newSpecTypes[] = self::TYPE_IDENTIFIER;
$newSpecTypes[] = self::TYPE_IDENTIFIER_ATOMIC;
}

$newSpec = str_replace('...', implode(', ', $newSpecParts), $this->specification);
Expand Down
1 change: 1 addition & 0 deletions src/Sql/ExpressionInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
interface ExpressionInterface
{
const TYPE_IDENTIFIER = 'identifier';
const TYPE_IDENTIFIER_ATOMIC = 'identifier_atomic';
const TYPE_VALUE = 'value';
const TYPE_LITERAL = 'literal';
const TYPE_SELECT = 'select';
Expand Down