From b9ff877f149dc7c4d3339c773252be4c5d3fcb9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steve=20M=C3=BCller?= Date: Wed, 15 Jan 2014 22:16:21 +0100 Subject: [PATCH] fix SQL generation on table lock hint capable platforms --- .../ORM/Persisters/BasicEntityPersister.php | 29 +++++++------ .../ORM/Persisters/EntityPersister.php | 4 +- .../Persisters/JoinedSubclassPersister.php | 23 +++++----- lib/Doctrine/ORM/Query/SqlWalker.php | 43 +++++++++---------- .../ORM/Query/SelectSqlGenerationTest.php | 4 +- 5 files changed, 51 insertions(+), 52 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 32488aa09..01d1bdd44 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -408,7 +408,7 @@ class BasicEntityPersister implements EntityPersister break; } - + $params[] = $value; $set[] = $column . ' = ' . $placeholder; $types[] = $this->columnTypes[$columnName]; @@ -805,7 +805,7 @@ class BasicEntityPersister implements EntityPersister $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)); } @@ -1052,7 +1052,7 @@ class BasicEntityPersister implements EntityPersister $tableName = $this->quoteStrategy->getTableName($this->class, $this->platform); if ('' !== $filterSql) { - $conditionSql = $conditionSql + $conditionSql = $conditionSql ? $conditionSql . ' AND ' . $filterSql : $filterSql; } @@ -1205,7 +1205,7 @@ class BasicEntityPersister implements EntityPersister 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); @@ -1335,7 +1335,7 @@ class BasicEntityPersister implements EntityPersister 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'])) { @@ -1408,7 +1408,7 @@ class BasicEntityPersister implements EntityPersister $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'])) { @@ -1465,7 +1465,7 @@ class BasicEntityPersister implements EntityPersister break; } - $lock = $this->platform->appendLockHint($this->getLockTablesSql(), $lockMode); + $lock = $this->getLockTablesSql($lockMode); $where = ($conditionSql ? ' WHERE ' . $conditionSql : '') . ' '; $sql = 'SELECT 1 ' . $lock @@ -1480,13 +1480,18 @@ class BasicEntityPersister implements EntityPersister /** * 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 + ); } /** @@ -1796,7 +1801,7 @@ class BasicEntityPersister implements EntityPersister $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)) { diff --git a/lib/Doctrine/ORM/Persisters/EntityPersister.php b/lib/Doctrine/ORM/Persisters/EntityPersister.php index fde9d5540..198f2bae0 100644 --- a/lib/Doctrine/ORM/Persisters/EntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/EntityPersister.php @@ -74,7 +74,7 @@ interface EntityPersister */ public function getSelectSQL($criteria, $assoc = null, $lockMode = 0, $limit = null, $offset = null, array $orderBy = null); - /** + /** * Expands the parameters from the given criteria and use the correct binding types if found. * * @param $criteria @@ -269,7 +269,7 @@ interface EntityPersister * Locks all rows of this entity matching the given criteria with the specified pessimistic lock mode. * * @param array $criteria - * @param int $lockMode + * @param int $lockMode One of the Doctrine\DBAL\LockMode::* constants. * * @return void */ diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index 00d03b51c..a5666bf8d 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -288,7 +288,7 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister foreach ($this->class->parentClasses as $parentClass) { $parentMetadata = $this->em->getClassMetadata($parentClass); $parentTable = $this->quoteStrategy->getTableName($parentMetadata, $this->platform); - + $this->conn->delete($parentTable, $id); } } @@ -342,7 +342,7 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister // 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; } @@ -387,16 +387,13 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister } /** - * 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) { @@ -412,7 +409,7 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $joinSql .= implode(' AND ', $conditions); } - return 'FROM ' . $quotedTableName . ' ' . $baseTableAlias . $joinSql; + return parent::getLockTablesSql($lockMode) . $joinSql; } /** @@ -488,8 +485,8 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister // 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; } @@ -505,17 +502,17 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister } $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(); diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index cdb1bb24f..42706d650 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -457,7 +457,7 @@ class SqlWalker implements TreeWalker } $sqlParts[] = (($this->useSqlTableAliases) ? $this->getSQLTableAlias($class->getTableName(), $dqlAlias) . '.' : '') - . $class->discriminatorColumn['name'] . ' IN (' . implode(', ', $values) . ')'; + . $class->discriminatorColumn['name'] . ' IN (' . implode(', ', $values) . ')'; } $sql = implode(' AND ', $sqlParts); @@ -497,7 +497,7 @@ class SqlWalker implements TreeWalker default: //@todo: throw exception? return ''; - break; + break; } $filterClauses = array(); @@ -576,7 +576,7 @@ class SqlWalker implements TreeWalker $this->rsm->isSelect = false; return $this->walkUpdateClause($AST->updateClause) - . $this->walkWhereClause($AST->whereClause); + . $this->walkWhereClause($AST->whereClause); } /** @@ -588,7 +588,7 @@ class SqlWalker implements TreeWalker $this->rsm->isSelect = false; return $this->walkDeleteClause($AST->deleteClause) - . $this->walkWhereClause($AST->whereClause); + . $this->walkWhereClause($AST->whereClause); } /** @@ -703,10 +703,10 @@ class SqlWalker implements TreeWalker } $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']; @@ -804,10 +804,7 @@ class SqlWalker implements TreeWalker $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); @@ -849,8 +846,11 @@ class SqlWalker implements TreeWalker $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); @@ -902,7 +902,7 @@ class SqlWalker implements TreeWalker 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); @@ -1442,10 +1442,7 @@ class SqlWalker implements TreeWalker $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); @@ -1463,7 +1460,7 @@ class SqlWalker implements TreeWalker public function walkSimpleSelectClause($simpleSelectClause) { return 'SELECT' . ($simpleSelectClause->isDistinct ? ' DISTINCT' : '') - . $this->walkSimpleSelectExpression($simpleSelectClause->simpleSelectExpression); + . $this->walkSimpleSelectExpression($simpleSelectClause->simpleSelectExpression); } /** @@ -1602,7 +1599,7 @@ class SqlWalker implements TreeWalker public function walkAggregateExpression($aggExpression) { return $aggExpression->functionName . '(' . ($aggExpression->isDistinct ? 'DISTINCT ' : '') - . $this->walkSimpleArithmeticExpression($aggExpression->pathExpression) . ')'; + . $this->walkSimpleArithmeticExpression($aggExpression->pathExpression) . ')'; } /** @@ -1907,7 +1904,7 @@ class SqlWalker implements TreeWalker // 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']; @@ -2091,7 +2088,7 @@ class SqlWalker implements TreeWalker } $sql .= ' BETWEEN ' . $this->walkArithmeticExpression($betweenExpr->leftBetweenExpression) - . ' AND ' . $this->walkArithmeticExpression($betweenExpr->rightBetweenExpression); + . ' AND ' . $this->walkArithmeticExpression($betweenExpr->rightBetweenExpression); return $sql; } diff --git a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php index f0afcc798..9ab441627 100644 --- a/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php +++ b/tests/Doctrine/Tests/ORM/Query/SelectSqlGenerationTest.php @@ -2000,11 +2000,11 @@ class SelectSqlGenerationTest extends \Doctrine\Tests\OrmTestCase $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_ WITH (NOLOCK) WHERE (c0_.name + c0_.status + 's') = ?" + "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_ WITH (NOLOCK) WHERE c0_.id = ?" + "SELECT (c0_.id + c0_.name + c0_.status) AS sclr0 FROM cms_users c0_ WHERE c0_.id = ?" ); $connMock->setDatabasePlatform($orgPlatform);