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

Jump to bottom

[DDC-2310] [DDC-2675] [2.4] Fix SQL generation on table lock hint capable platforms #925

Merged
merged 1 commit into from
Nov 11, 2014
Merged

[DDC-2310] [DDC-2675] [2.4] Fix SQL generation on table lock hint capable platforms #925

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
29 changes: 17 additions & 12 deletions lib/Doctrine/ORM/Persisters/BasicEntityPersister.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ protected final function updateTable($entity, $quotedTableName, array $updateDat

break;
}

$params[] = $value;
$set[] = $column . ' = ' . $placeholder;
$types[] = $this->columnTypes[$columnName];
Expand Down Expand Up @@ -850,7 +850,7 @@ public function refresh(array $id, $entity, $lockMode = 0)
$sql = $this->getSelectSQL($id, null, $lockMode);
list($params, $types) = $this->expandParameters($id);
$stmt = $this->conn->executeQuery($sql, $params, $types);

$hydrator = $this->em->newHydrator(Query::HYDRATE_OBJECT);
$hydrator->hydrateAll($stmt, $this->rsm, array(Query::HINT_REFRESH => true));
}
Expand Down Expand Up @@ -1130,7 +1130,7 @@ protected function getSelectSQL($criteria, $assoc = null, $lockMode = 0, $limit
$tableName = $this->quoteStrategy->getTableName($this->class, $this->platform);

if ('' !== $filterSql) {
$conditionSql = $conditionSql
$conditionSql = $conditionSql
? $conditionSql . ' AND ' . $filterSql
: $filterSql;
}
Expand Down Expand Up @@ -1283,7 +1283,7 @@ protected function getSelectColumnsSQL()
if ($assoc['isOwningSide']) {
$tableAlias = $this->getSQLTableAlias($association['targetEntity'], $assocAlias);
$this->selectJoinSql .= ' ' . $this->getJoinSQLForJoinColumns($association['joinColumns']);

foreach ($association['joinColumns'] as $joinColumn) {
$sourceCol = $this->quoteStrategy->getJoinColumnName($joinColumn, $this->class, $this->platform);
$targetCol = $this->quoteStrategy->getReferencedJoinColumnName($joinColumn, $this->class, $this->platform);
Expand Down Expand Up @@ -1415,7 +1415,7 @@ protected function getInsertSQL()
foreach ($columns as $column) {
$placeholder = '?';

if (isset($this->class->fieldNames[$column])
if (isset($this->class->fieldNames[$column])
&& isset($this->columnTypes[$this->class->fieldNames[$column]])
&& isset($this->class->fieldMappings[$this->class->fieldNames[$column]]['requireSQLConversion'])) {

Expand Down Expand Up @@ -1488,7 +1488,7 @@ protected function getSelectColumnSQL($field, ClassMetadata $class, $alias = 'r'
$columnName = $this->quoteStrategy->getColumnName($field, $class, $this->platform);
$sql = $tableAlias . '.' . $columnName;
$columnAlias = $this->getSQLColumnAlias($class->columnNames[$field]);

$this->rsm->addFieldResult($alias, $columnAlias, $field);

if (isset($class->fieldMappings[$field]['requireSQLConversion'])) {
Expand Down Expand Up @@ -1550,7 +1550,7 @@ public function lock(array $criteria, $lockMode)
break;
}

$lock = $this->platform->appendLockHint($this->getLockTablesSql(), $lockMode);
$lock = $this->getLockTablesSql($lockMode);
$where = ($conditionSql ? ' WHERE ' . $conditionSql : '') . ' ';
$sql = 'SELECT 1 '
. $lock
Expand All @@ -1565,13 +1565,18 @@ public function lock(array $criteria, $lockMode)
/**
* Gets the FROM and optionally JOIN conditions to lock the entity managed by this persister.
*
* @param integer $lockMode One of the Doctrine\DBAL\LockMode::* constants.
*
* @return string
*/
protected function getLockTablesSql()
protected function getLockTablesSql($lockMode)
{
return 'FROM '
. $this->quoteStrategy->getTableName($this->class, $this->platform) . ' '
. $this->getSQLTableAlias($this->class->name);
return $this->platform->appendLockHint(
'FROM '
. $this->quoteStrategy->getTableName($this->class, $this->platform) . ' '
. $this->getSQLTableAlias($this->class->name),
$lockMode
);
}

/**
Expand Down Expand Up @@ -1910,7 +1915,7 @@ public function exists($entity, array $extraConditions = array())
$alias = $this->getSQLTableAlias($this->class->name);

$sql = 'SELECT 1 '
. $this->getLockTablesSql()
. $this->getLockTablesSql(null)
. ' WHERE ' . $this->getSelectConditionSQL($criteria);

if ($filterSql = $this->generateFilterConditionSQL($this->class, $alias)) {
Expand Down
23 changes: 10 additions & 13 deletions lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ public function delete($entity)
foreach ($this->class->parentClasses as $parentClass) {
$parentMetadata = $this->em->getClassMetadata($parentClass);
$parentTable = $this->quoteStrategy->getTableName($parentMetadata, $this->platform);

$this->conn->delete($parentTable, $id);
}
}
Expand Down Expand Up @@ -342,7 +342,7 @@ protected function getSelectSQL($criteria, $assoc = null, $lockMode = 0, $limit

// If the current class in the root entity, add the filters
if ($filterSql = $this->generateFilterConditionSQL($this->em->getClassMetadata($this->class->rootEntityName), $this->getSQLTableAlias($this->class->rootEntityName))) {
$conditionSql .= $conditionSql
$conditionSql .= $conditionSql
? ' AND ' . $filterSql
: $filterSql;
}
Expand Down Expand Up @@ -387,16 +387,13 @@ protected function getSelectSQL($criteria, $assoc = null, $lockMode = 0, $limit
}

/**
* Get the FROM and optionally JOIN conditions to lock the entity managed by this persister.
*
* @return string
* {@inheritdoc}
*/
public function getLockTablesSql()
protected function getLockTablesSql($lockMode)
{
$joinSql = '';
$identifierColumns = $this->class->getIdentifierColumnNames();
$baseTableAlias = $this->getSQLTableAlias($this->class->name);
$quotedTableName = $this->quoteStrategy->getTableName($this->class, $this->platform);

// INNER JOIN parent tables
foreach ($this->class->parentClasses as $parentClassName) {
Expand All @@ -412,7 +409,7 @@ public function getLockTablesSql()
$joinSql .= implode(' AND ', $conditions);
}

return 'FROM ' . $quotedTableName . ' ' . $baseTableAlias . $joinSql;
return parent::getLockTablesSql($lockMode) . $joinSql;
}

/**
Expand Down Expand Up @@ -488,8 +485,8 @@ protected function getSelectColumnsSQL()

// Add join columns (foreign keys)
foreach ($subClass->associationMappings as $mapping) {
if ( ! $mapping['isOwningSide']
|| ! ($mapping['type'] & ClassMetadata::TO_ONE)
if ( ! $mapping['isOwningSide']
|| ! ($mapping['type'] & ClassMetadata::TO_ONE)
|| isset($mapping['inherited'])) {
continue;
}
Expand All @@ -505,17 +502,17 @@ protected function getSelectColumnsSQL()
}

$this->selectColumnListSql = implode(', ', $columnList);

return $this->selectColumnListSql;
}

/**
* {@inheritdoc}
* {@inheritdoc}
*/
protected function getInsertColumnList()
{
// Identifier columns must always come first in the column list of subclasses.
$columns = $this->class->parentClasses
$columns = $this->class->parentClasses
? $this->class->getIdentifierColumnNames()
: array();

Expand Down
43 changes: 20 additions & 23 deletions lib/Doctrine/ORM/Query/SqlWalker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ private function _generateDiscriminatorColumnConditionSQL(array $dqlAliases)
}

$sqlParts[] = (($this->useSqlTableAliases) ? $this->getSQLTableAlias($class->getTableName(), $dqlAlias) . '.' : '')
. $class->discriminatorColumn['name'] . ' IN (' . implode(', ', $values) . ')';
. $class->discriminatorColumn['name'] . ' IN (' . implode(', ', $values) . ')';
}

$sql = implode(' AND ', $sqlParts);
Expand Down Expand Up @@ -496,7 +496,7 @@ private function generateFilterConditionSQL(ClassMetadata $targetEntity, $target
default:
//@todo: throw exception?
return '';
break;
break;
}

$filterClauses = array();
Expand Down Expand Up @@ -574,7 +574,7 @@ public function walkUpdateStatement(AST\UpdateStatement $AST)
$this->useSqlTableAliases = false;

return $this->walkUpdateClause($AST->updateClause)
. $this->walkWhereClause($AST->whereClause);
. $this->walkWhereClause($AST->whereClause);
}

/**
Expand All @@ -585,7 +585,7 @@ public function walkDeleteStatement(AST\DeleteStatement $AST)
$this->useSqlTableAliases = false;

return $this->walkDeleteClause($AST->deleteClause)
. $this->walkWhereClause($AST->whereClause);
. $this->walkWhereClause($AST->whereClause);
}

/**
Expand Down Expand Up @@ -700,10 +700,10 @@ public function walkSelectClause($selectClause)
}

$addMetaColumns = ! $this->query->getHint(Query::HINT_FORCE_PARTIAL_LOAD) &&
$this->query->getHydrationMode() == Query::HYDRATE_OBJECT
||
$this->query->getHydrationMode() != Query::HYDRATE_OBJECT &&
$this->query->getHint(Query::HINT_INCLUDE_META_COLUMNS);
$this->query->getHydrationMode() == Query::HYDRATE_OBJECT
||
$this->query->getHydrationMode() != Query::HYDRATE_OBJECT &&
$this->query->getHint(Query::HINT_INCLUDE_META_COLUMNS);

foreach ($this->selectedClasses as $selectedClass) {
$class = $selectedClass['class'];
Expand Down Expand Up @@ -801,10 +801,7 @@ public function walkFromClause($fromClause)
$sqlParts = array();

foreach ($identificationVarDecls as $identificationVariableDecl) {
$sql = $this->platform->appendLockHint(
$this->walkRangeVariableDeclaration($identificationVariableDecl->rangeVariableDeclaration),
$this->query->getHint(Query::HINT_LOCK_MODE)
);
$sql = $this->walkRangeVariableDeclaration($identificationVariableDecl->rangeVariableDeclaration);

foreach ($identificationVariableDecl->joins as $join) {
$sql .= $this->walkJoin($join);
Expand Down Expand Up @@ -846,8 +843,11 @@ public function walkRangeVariableDeclaration($rangeVariableDeclaration)
$this->rootAliases[] = $dqlAlias;
}

$sql = $this->quoteStrategy->getTableName($class,$this->platform) . ' '
. $this->getSQLTableAlias($class->getTableName(), $dqlAlias);
$sql = $this->platform->appendLockHint(
$this->quoteStrategy->getTableName($class, $this->platform) . ' ' .
$this->getSQLTableAlias($class->getTableName(), $dqlAlias),
$this->query->getHint(Query::HINT_LOCK_MODE)
);

if ($class->isInheritanceTypeJoined()) {
$sql .= $this->_generateClassTableInheritanceJoins($class, $dqlAlias);
Expand Down Expand Up @@ -899,7 +899,7 @@ public function walkJoinAssociationDeclaration($joinAssociationDeclaration, $joi
case ($assoc['type'] & ClassMetadata::TO_ONE):
$conditions = array();

foreach ($assoc['joinColumns'] as $joinColumn) {
foreach ($assoc['joinColumns'] as $joinColumn) {
$quotedSourceColumn = $this->quoteStrategy->getJoinColumnName($joinColumn, $targetClass, $this->platform);
$quotedTargetColumn = $this->quoteStrategy->getReferencedJoinColumnName($joinColumn, $targetClass, $this->platform);

Expand Down Expand Up @@ -1439,10 +1439,7 @@ public function walkSubselectFromClause($subselectFromClause)
$sqlParts = array ();

foreach ($identificationVarDecls as $subselectIdVarDecl) {
$sql = $this->platform->appendLockHint(
$this->walkRangeVariableDeclaration($subselectIdVarDecl->rangeVariableDeclaration),
$this->query->getHint(Query::HINT_LOCK_MODE)
);
$sql = $this->walkRangeVariableDeclaration($subselectIdVarDecl->rangeVariableDeclaration);

foreach ($subselectIdVarDecl->joins as $join) {
$sql .= $this->walkJoin($join);
Expand All @@ -1460,7 +1457,7 @@ public function walkSubselectFromClause($subselectFromClause)
public function walkSimpleSelectClause($simpleSelectClause)
{
return 'SELECT' . ($simpleSelectClause->isDistinct ? ' DISTINCT' : '')
. $this->walkSimpleSelectExpression($simpleSelectClause->simpleSelectExpression);
. $this->walkSimpleSelectExpression($simpleSelectClause->simpleSelectExpression);
}

/**
Expand Down Expand Up @@ -1592,7 +1589,7 @@ public function walkSimpleSelectExpression($simpleSelectExpression)
public function walkAggregateExpression($aggExpression)
{
return $aggExpression->functionName . '(' . ($aggExpression->isDistinct ? 'DISTINCT ' : '')
. $this->walkSimpleArithmeticExpression($aggExpression->pathExpression) . ')';
. $this->walkSimpleArithmeticExpression($aggExpression->pathExpression) . ')';
}

/**
Expand Down Expand Up @@ -1887,7 +1884,7 @@ public function walkCollectionMemberExpression($collMemberExpr)

// join to target table
$sql .= $this->quoteStrategy->getJoinTableName($owningAssoc, $targetClass, $this->platform) . ' ' . $joinTableAlias
. ' INNER JOIN ' . $this->quoteStrategy->getTableName($targetClass, $this->platform) . ' ' . $targetTableAlias . ' ON ';
. ' INNER JOIN ' . $this->quoteStrategy->getTableName($targetClass, $this->platform) . ' ' . $targetTableAlias . ' ON ';

// join conditions
$joinColumns = $assoc['isOwningSide'] ? $joinTable['inverseJoinColumns'] : $joinTable['joinColumns'];
Expand Down Expand Up @@ -2067,7 +2064,7 @@ public function walkBetweenExpression($betweenExpr)
if ($betweenExpr->not) $sql .= ' NOT';

$sql .= ' BETWEEN ' . $this->walkArithmeticExpression($betweenExpr->leftBetweenExpression)
. ' AND ' . $this->walkArithmeticExpression($betweenExpr->rightBetweenExpression);
. ' AND ' . $this->walkArithmeticExpression($betweenExpr->rightBetweenExpression);

return $sql;
}
Expand Down
4 changes: 2 additions & 2 deletions tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1990,15 +1990,15 @@ public function testSupportsMoreThanTwoParametersInConcatFunction()
"SELECT c0_.id || c0_.name || c0_.status AS sclr0 FROM cms_users c0_ WHERE c0_.id = ?"
);

/*$connMock->setDatabasePlatform(new \Doctrine\DBAL\Platforms\SQLServerPlatform());
$connMock->setDatabasePlatform(new \Doctrine\DBAL\Platforms\SQLServerPlatform());
$this->assertSqlGeneration(
"SELECT u.id FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE CONCAT(u.name, u.status, 's') = ?1",
"SELECT c0_.id AS id0 FROM cms_users c0_ WHERE (c0_.name + c0_.status + 's') = ?"
);
$this->assertSqlGeneration(
"SELECT CONCAT(u.id, u.name, u.status) FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.id = ?1",
"SELECT (c0_.id + c0_.name + c0_.status) AS sclr0 FROM cms_users c0_ WHERE c0_.id = ?"
);*/
);

$connMock->setDatabasePlatform($orgPlatform);
}
Expand Down