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.

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

Introduce BinaryType #452

Merged
merged 2 commits into from Dec 22, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/en/reference/types.rst
Expand Up @@ -20,6 +20,7 @@ vendors:
- BigInt
- String (string with maximum length, for example 255)
- Text (strings without maximum length)
- Binary (binary string with maximum length, for example 255)
- Decimal (restricted floats, *NOTE* Only works with a setlocale()
configuration that uses decimal points!)
- Boolean
Expand Down
57 changes: 57 additions & 0 deletions lib/Doctrine/DBAL/Platforms/AbstractPlatform.php
Expand Up @@ -231,6 +231,28 @@ public function getVarcharTypeDeclarationSQL(array $field)
return $this->getVarcharTypeDeclarationSQLSnippet($field['length'], $fixed);
}

/**
* Returns the SQL snippet used to declare a BINARY/VARBINARY column type.
*
* @param array $field The column definition.
*
* @return string
*/
public function getBinaryTypeDeclarationSQL(array $field)
{
if ( ! isset($field['length'])) {
$field['length'] = $this->getBinaryDefaultLength();
}

$fixed = isset($field['fixed']) ? $field['fixed'] : false;

if ($field['length'] > $this->getBinaryMaxLength()) {
return $this->getBlobTypeDeclarationSQL($field);
}

return $this->getBinaryTypeDeclarationSQLSnippet($field['length'], $fixed);
}

/**
* Returns the SQL snippet to declare a GUID/UUID field.
*
Expand Down Expand Up @@ -259,6 +281,21 @@ protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed)
throw DBALException::notSupported('VARCHARs not supported by Platform.');
}

/**
* Returns the SQL snippet used to declare a BINARY/VARBINARY column type.
*
* @param integer $length The length of the column.
* @param boolean $fixed Whether the column length is fixed.
*
* @return string
*
* @throws \Doctrine\DBAL\DBALException If not supported on this platform.
*/
protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed)
{
throw DBALException::notSupported('BINARY/VARBINARY column types are not supported by this platform.');
}

/**
* Returns the SQL snippet used to declare a CLOB column type.
*
Expand Down Expand Up @@ -478,6 +515,26 @@ public function getVarcharDefaultLength()
return 255;
}

/**
* Gets the maximum length of a binary field.
*
* @return integer
*/
public function getBinaryMaxLength()
{
return 4000;
Copy link
Member

Choose a reason for hiding this comment

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

@deeky666 can we add a comment on why this is 4000?
random

Copy link
Member Author

Choose a reason for hiding this comment

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

lol awesome picture ;). I don't know why this is the default in AbstractPlatform guess it's legacy. It's wrongly used throughout the platforms (adopted it from varchar implementation). 4000 is the default for Oracle maybe that's the reason ^^

}

/**
* Gets the default length of a binary field.
*
* @return integer
*/
public function getBinaryDefaultLength()
{
return 255;
}

/**
* Gets all SQL wildcard characters of the platform.
*
Expand Down
26 changes: 26 additions & 0 deletions lib/Doctrine/DBAL/Platforms/DB2Platform.php
Expand Up @@ -25,6 +25,22 @@

class DB2Platform extends AbstractPlatform
{
/**
* {@inheritdoc}
*/
public function getBinaryMaxLength()
{
return 32704;
}

/**
* {@inheritdoc}
*/
public function getBinaryDefaultLength()
{
return 1;
}

/**
* {@inheritDoc}
*/
Expand All @@ -47,6 +63,8 @@ public function initializeDoctrineTypeMappings()
'date' => 'date',
'varchar' => 'string',
'character' => 'string',
'varbinary' => 'binary',
'binary' => 'binary',
'clob' => 'text',
'blob' => 'blob',
'decimal' => 'decimal',
Expand All @@ -65,6 +83,14 @@ protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed)
: ($length ? 'VARCHAR(' . $length . ')' : 'VARCHAR(255)');
}

/**
* {@inheritdoc}
*/
protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed)
{
return $fixed ? 'BINARY(' . ($length ?: 255) . ')' : 'VARBINARY(' . ($length ?: 255) . ')';
Copy link
Member

Choose a reason for hiding this comment

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

Should you use the defaults instead of hardcoding 255?

Copy link
Member

Choose a reason for hiding this comment

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

reasoning is discussed in PR description.

Copy link
Member Author

Choose a reason for hiding this comment

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

See the description. I wanted to do that but dropped the "right" implementation in favour of behaving as similar as possible with varchar implementation of the platform. Sounds weird I know but didn't know how to do it otherwise.

}

/**
* {@inheritDoc}
*/
Expand Down
23 changes: 22 additions & 1 deletion lib/Doctrine/DBAL/Platforms/DrizzlePlatform.php
Expand Up @@ -23,6 +23,7 @@
use Doctrine\DBAL\Schema\TableDiff;
use Doctrine\DBAL\Schema\Index;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Types\BinaryType;

/**
* Drizzle platform
Expand Down Expand Up @@ -165,6 +166,14 @@ protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed)
return $length ? 'VARCHAR(' . $length . ')' : 'VARCHAR(255)';
}

/**
* {@inheritdoc}
*/
protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed)
{
return 'VARBINARY(' . ($length ?: 255) . ')';
}

/**
* {@inheritDoc}
*/
Expand All @@ -173,8 +182,9 @@ protected function initializeDoctrineTypeMappings()
$this->doctrineTypeMapping = array(
'boolean' => 'boolean',
'varchar' => 'string',
'varbinary' => 'binary',
'integer' => 'integer',
'blob' => 'text',
'blob' => 'blob',
'decimal' => 'decimal',
'datetime' => 'datetime',
'date' => 'date',
Expand Down Expand Up @@ -425,6 +435,17 @@ public function getAlterTableSQL(TableDiff $diff)
/* @var $columnDiff \Doctrine\DBAL\Schema\ColumnDiff */
$column = $columnDiff->column;
$columnArray = $column->toArray();

// Do not generate column alteration clause if type is binary and only fixed property has changed.
// Drizzle only supports binary type columns with variable length.
// Avoids unnecessary table alteration statements.
if ($column['type'] instanceof BinaryType &&
$columnDiff->hasChanged('fixed') &&
count($columnDiff->changedProperties) === 1
) {
continue;
}

$columnArray['comment'] = $this->getColumnComment($column);
$queryParts[] = 'CHANGE ' . ($columnDiff->getOldColumnName()->getQuotedName($this)) . ' '
. $this->getColumnDeclarationSQL($column->getQuotedName($this), $columnArray);
Expand Down
20 changes: 18 additions & 2 deletions lib/Doctrine/DBAL/Platforms/MySqlPlatform.php
Expand Up @@ -252,6 +252,14 @@ protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed)
: ($length ? 'VARCHAR(' . $length . ')' : 'VARCHAR(255)');
}

/**
* {@inheritdoc}
*/
protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed)
{
return $fixed ? 'BINARY(' . ($length ?: 255) . ')' : 'VARBINARY(' . ($length ?: 255) . ')';
}

/**
* Gets the SQL snippet used to declare a CLOB column type.
* TINYTEXT : 2 ^ 8 - 1 = 255
Expand Down Expand Up @@ -809,8 +817,8 @@ protected function initializeDoctrineTypeMappings()
'blob' => 'blob',
'mediumblob' => 'blob',
'tinyblob' => 'blob',
'binary' => 'blob',
'varbinary' => 'blob',
'binary' => 'binary',
'varbinary' => 'binary',
'set' => 'simple_array',
);
}
Expand All @@ -823,6 +831,14 @@ public function getVarcharMaxLength()
return 65535;
}

/**
* {@inheritdoc}
*/
public function getBinaryMaxLength()
{
return 65535;
}

/**
* {@inheritDoc}
*/
Expand Down
33 changes: 31 additions & 2 deletions lib/Doctrine/DBAL/Platforms/OraclePlatform.php
Expand Up @@ -25,6 +25,7 @@
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Schema\TableDiff;
use Doctrine\DBAL\DBALException;
use Doctrine\DBAL\Types\BinaryType;

/**
* OraclePlatform.
Expand Down Expand Up @@ -335,6 +336,22 @@ protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed)
: ($length ? 'VARCHAR2(' . $length . ')' : 'VARCHAR2(4000)');
}

/**
* {@inheritdoc}
*/
protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed)
{
return 'RAW(' . ($length ?: $this->getBinaryMaxLength()) . ')';
}

/**
* {@inheritdoc}
*/
public function getBinaryMaxLength()
{
return 2000;
}

/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -644,7 +661,19 @@ public function getAlterTableSQL(TableDiff $diff)
continue;
}

/* @var $columnDiff \Doctrine\DBAL\Schema\ColumnDiff */
$column = $columnDiff->column;

// Do not generate column alteration clause if type is binary and only fixed property has changed.
// Oracle only supports binary type columns with variable length.
// Avoids unnecessary table alteration statements.
if ($column->getType() instanceof BinaryType &&
$columnDiff->hasChanged('fixed') &&
count($columnDiff->changedProperties) === 1
) {
continue;
}

$columnHasChangedComment = $columnDiff->hasChanged('comment');

/**
Expand Down Expand Up @@ -903,8 +932,8 @@ protected function initializeDoctrineTypeMappings()
'long' => 'string',
'clob' => 'text',
'nclob' => 'text',
'raw' => 'text',
'long raw' => 'text',
'raw' => 'binary',
'long raw' => 'blob',
'rowid' => 'string',
'urowid' => 'string',
'blob' => 'blob',
Expand Down
73 changes: 73 additions & 0 deletions lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php
Expand Up @@ -19,7 +19,11 @@

namespace Doctrine\DBAL\Platforms;

use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Schema\ColumnDiff;
use Doctrine\DBAL\Schema\TableDiff;
use Doctrine\DBAL\Types\BinaryType;
use Doctrine\DBAL\Types\BlobType;

/**
* PostgreSqlPlatform.
Expand Down Expand Up @@ -458,6 +462,10 @@ public function getAlterTableSQL(TableDiff $diff)
continue;
}

if ($this->isUnchangedBinaryColumn($columnDiff)) {
continue;
}

$oldColumnName = $columnDiff->getOldColumnName()->getQuotedName($this);
$column = $columnDiff->column;

Expand Down Expand Up @@ -533,6 +541,47 @@ public function getAlterTableSQL(TableDiff $diff)
return array_merge($sql, $tableSql, $columnSql);
}

/**
* Checks whether a given column diff is a logically unchanged binary type column.
*
* Used to determine whether a column alteration for a binary type column can be skipped.
* Doctrine's {@link \Doctrine\DBAL\Types\BinaryType} and {@link \Doctrine\DBAL\Types\BlobType}
* are mapped to the same database column type on this platform as this platform
* does not have a native VARBINARY/BINARY column type. Therefore the {@link \Doctrine\DBAL\Schema\Comparator}
* might detect differences for binary type columns which do not have to be propagated
* to database as there actually is no difference at database level.
*
* @param ColumnDiff $columnDiff The column diff to check against.
*
* @return boolean True if the given column diff is an unchanged binary type column, false otherwise.
*/
private function isUnchangedBinaryColumn(ColumnDiff $columnDiff)
{
$columnType = $columnDiff->column->getType();

if ( ! $columnType instanceof BinaryType && ! $columnType instanceof BlobType) {
return false;
}

$fromColumn = $columnDiff->fromColumn instanceof Column ? $columnDiff->fromColumn : null;

if ($fromColumn) {
$fromColumnType = $fromColumn->getType();

if ( ! $fromColumnType instanceof BinaryType && ! $fromColumnType instanceof BlobType) {
return false;
}

return count(array_diff($columnDiff->changedProperties, array('type', 'length', 'fixed'))) === 0;
}

if ($columnDiff->hasChanged('type')) {
return false;
}

return count(array_diff($columnDiff->changedProperties, array('length', 'fixed'))) === 0;
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -792,6 +841,14 @@ protected function getVarcharTypeDeclarationSQLSnippet($length, $fixed)
: ($length ? 'VARCHAR(' . $length . ')' : 'VARCHAR(255)');
}

/**
* {@inheritdoc}
*/
protected function getBinaryTypeDeclarationSQLSnippet($length, $fixed)
{
return 'BYTEA';
}

/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -905,6 +962,22 @@ public function getVarcharMaxLength()
return 65535;
}

/**
* {@inheritdoc}
*/
public function getBinaryMaxLength()
{
return 0;
}

/**
* {@inheritdoc}
*/
public function getBinaryDefaultLength()
{
return 0;
}

/**
* {@inheritDoc}
*/
Expand Down