Skip to content

Commit

Permalink
Small changes for code readability. Added type binding in JoinedSubcl…
Browse files Browse the repository at this point in the history
…assPersister, which was missing. Fixes DDC-1316.
  • Loading branch information
guilhermeblanco committed Sep 5, 2011
1 parent f29c907 commit 666691f
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 7 deletions.
6 changes: 6 additions & 0 deletions lib/Doctrine/ORM/Persisters/AbstractCollectionPersister.php
Expand Up @@ -65,9 +65,11 @@ public function __construct(EntityManager $em)
public function delete(PersistentCollection $coll)
{
$mapping = $coll->getMapping();

if ( ! $mapping['isOwningSide']) {
return; // ignore inverse side
}

$sql = $this->_getDeleteSQL($coll);
$this->_conn->executeUpdate($sql, $this->_getDeleteSQLParameters($coll));
}
Expand Down Expand Up @@ -96,9 +98,11 @@ abstract protected function _getDeleteSQLParameters(PersistentCollection $coll);
public function update(PersistentCollection $coll)
{
$mapping = $coll->getMapping();

if ( ! $mapping['isOwningSide']) {
return; // ignore inverse side
}

$this->deleteRows($coll);
//$this->updateRows($coll);
$this->insertRows($coll);
Expand All @@ -108,6 +112,7 @@ public function deleteRows(PersistentCollection $coll)
{
$deleteDiff = $coll->getDeleteDiff();
$sql = $this->_getDeleteRowSQL($coll);

foreach ($deleteDiff as $element) {
$this->_conn->executeUpdate($sql, $this->_getDeleteRowSQLParameters($coll, $element));
}
Expand All @@ -120,6 +125,7 @@ public function insertRows(PersistentCollection $coll)
{
$insertDiff = $coll->getInsertDiff();
$sql = $this->_getInsertRowSQL($coll);

foreach ($insertDiff as $element) {
$this->_conn->executeUpdate($sql, $this->_getInsertRowSQLParameters($coll, $element));
}
Expand Down
Expand Up @@ -39,10 +39,12 @@ abstract class AbstractEntityInheritancePersister extends BasicEntityPersister
protected function _prepareInsertData($entity)
{
$data = parent::_prepareInsertData($entity);

// Populate the discriminator column
$discColumn = $this->_class->discriminatorColumn;
$this->_columnTypes[$discColumn['name']] = $discColumn['type'];
$data[$this->_getDiscriminatorColumnTableName()][$discColumn['name']] = $this->_class->discriminatorValue;

return $data;
}

Expand All @@ -63,7 +65,7 @@ protected function _getSelectColumnSQL($field, ClassMetadata $class, $alias = 'r
$columnAlias = $this->_platform->getSQLResultCasing($columnName . $this->_sqlAliasCounter++);
$this->_rsm->addFieldResult($alias, $columnAlias, $field, $class->name);

return "$sql AS $columnAlias";
return $sql . ' AS ' . $columnAlias;
}

protected function getSelectJoinColumnSQL($tableAlias, $joinColumnName, $className)
Expand All @@ -72,6 +74,6 @@ protected function getSelectJoinColumnSQL($tableAlias, $joinColumnName, $classNa
$resultColumnName = $this->_platform->getSQLResultCasing($columnAlias);
$this->_rsm->addMetaResult('r', $resultColumnName, $joinColumnName);

return $tableAlias . ".$joinColumnName AS $columnAlias";
return $tableAlias . '.' . $joinColumnName . ' AS ' . $columnAlias;
}
}
1 change: 1 addition & 0 deletions lib/Doctrine/ORM/Persisters/BasicEntityPersister.php
Expand Up @@ -229,6 +229,7 @@ public function executeInserts()

if (isset($insertData[$tableName])) {
$paramIndex = 1;

foreach ($insertData[$tableName] as $column => $value) {
$stmt->bindValue($paramIndex++, $value, $this->_columnTypes[$column]);
}
Expand Down
16 changes: 13 additions & 3 deletions lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php
Expand Up @@ -22,6 +22,7 @@
use Doctrine\ORM\ORMException,
Doctrine\ORM\Mapping\ClassMetadata,
Doctrine\DBAL\LockMode,
Doctrine\DBAL\Types\Type,
Doctrine\ORM\Query\ResultSetMapping;

/**
Expand Down Expand Up @@ -143,9 +144,11 @@ public function executeInserts()

// Execute insert on root table
$paramIndex = 1;

foreach ($insertData[$rootTableName] as $columnName => $value) {
$rootTableStmt->bindValue($paramIndex++, $value, $this->_columnTypes[$columnName]);
}

$rootTableStmt->execute();

if ($isPostInsertId) {
Expand All @@ -160,12 +163,17 @@ public function executeInserts()
foreach ($subTableStmts as $tableName => $stmt) {
$data = isset($insertData[$tableName]) ? $insertData[$tableName] : array();
$paramIndex = 1;
foreach ((array) $id as $idVal) {
$stmt->bindValue($paramIndex++, $idVal);

foreach ((array) $id as $idName => $idVal) {
$type = isset($this->_columnTypes[$idName]) ? $this->_columnTypes[$idName] : Type::STRING;

This comment has been minimized.

Copy link
@beberlei

beberlei Sep 25, 2011

Member

Why is isset() => true not guaranteed here?

This comment has been minimized.

Copy link
@guilhermeblanco

guilhermeblanco Sep 26, 2011

Author Member

It's not guaranteed because for @id there was never a type definition during Persister's queries.
It was always considering STRING or null, which was falling back to STRING again, which was failing when the user was trying to define an @id with its type defines as \DateTime.

I kept fallback to STRING because it would keep BC. But Maybe I can investigate the issue a little bit more.


$stmt->bindValue($paramIndex++, $idVal, $type);
}

foreach ($data as $columnName => $value) {
$stmt->bindValue($paramIndex++, $value, $this->_columnTypes[$columnName]);
}

$stmt->execute();
}
}
Expand All @@ -191,7 +199,7 @@ public function update($entity)
{
$updateData = $this->_prepareUpdateData($entity);

if ($isVersioned = $this->_class->isVersioned) {
if (($isVersioned = $this->_class->isVersioned) != false) {
$versionedClass = $this->_getVersionedClassMetadata();
$versionedTable = $versionedClass->table['name'];
}
Expand All @@ -200,6 +208,7 @@ public function update($entity)
foreach ($updateData as $tableName => $data) {
$this->_updateTable($entity, $this->_quotedTableMap[$tableName], $data, $isVersioned && $versionedTable == $tableName);
}

// Make sure the table with the version column is updated even if no columns on that
// table were affected.
if ($isVersioned && ! isset($updateData[$versionedTable])) {
Expand Down Expand Up @@ -229,6 +238,7 @@ public function delete($entity)
} else {
// Delete from all tables individually, starting from this class' table up to the root table.
$this->_conn->delete($this->_class->getQuotedTableName($this->_platform), $id);

foreach ($this->_class->parentClasses as $parentClass) {
$this->_conn->delete($this->_em->getClassMetadata($parentClass)->getQuotedTableName($this->_platform), $id);
}
Expand Down
5 changes: 3 additions & 2 deletions lib/Doctrine/ORM/Persisters/ManyToManyPersister.php
Expand Up @@ -39,9 +39,10 @@ class ManyToManyPersister extends AbstractCollectionPersister
*/
protected function _getDeleteRowSQL(PersistentCollection $coll)
{
$mapping = $coll->getMapping();
$mapping = $coll->getMapping();
$joinTable = $mapping['joinTable'];
$columns = $mapping['joinTableColumns'];
$columns = $mapping['joinTableColumns'];

return 'DELETE FROM ' . $joinTable['name'] . ' WHERE ' . implode(' = ? AND ', $columns) . ' = ?';
}

Expand Down

2 comments on commit 666691f

@guilhermeblanco
Copy link
Member Author

Choose a reason for hiding this comment

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

@beberlei please merge to 2.1. This addresses the issue DDC-1316

@beberlei
Copy link
Member

Choose a reason for hiding this comment

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

merged

Please sign in to comment.