Fixed SQL Server Platform NULL declaration #230

Merged
merged 3 commits into from Nov 25, 2012

Conversation

Projects
None yet
4 participants
Contributor

Lusitanian commented Nov 21, 2012

Per previous pull request but based on master. SQL server does not use 'DEFAULT NULL' for marking columns as nullable, only 'NULL'.

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DBAL-387

@stof stof commented on an outdated diff Nov 21, 2012

lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php
@@ -896,4 +896,31 @@ public function getBlobTypeDeclarationSQL(array $field)
{
return 'VARBINARY(MAX)';
}
+
+ /**
+ * Obtain DBMS specific SQL code portion needed to set a default value
+ * declaration to be used in statements like CREATE TABLE.
+ *
+ * @param array $field field definition array
+ *
+ * @return string DBMS specific SQL code portion needed to set a default value
+ */
@stof

stof Nov 21, 2012

Member

Please use {@inheritdoc} instead of duplicating the phpdoc

@stof stof commented on an outdated diff Nov 21, 2012

lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php
@@ -896,4 +896,31 @@ public function getBlobTypeDeclarationSQL(array $field)
{
return 'VARBINARY(MAX)';
}
+
@stof

stof Nov 21, 2012

Member

please remove trailing whitespaces. Empty lines should be really empty

@stof stof commented on an outdated diff Nov 21, 2012

lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php
+ $default = empty($field['notnull']) ? ' NULL' : '';
+
+ if (isset($field['default'])) {
+ $default = " DEFAULT '".$field['default']."'";
+ if (isset($field['type'])) {
+ if (in_array((string)$field['type'], array("Integer", "BigInteger", "SmallInteger"))) {
+ $default = " DEFAULT ".$field['default'];
+ } else if ((string)$field['type'] == 'DateTime' && $field['default'] == $this->getCurrentTimestampSQL()) {
+ $default = " DEFAULT ".$this->getCurrentTimestampSQL();
+ } else if ((string) $field['type'] == 'Boolean') {
+ $default = " DEFAULT '" . $this->convertBooleans($field['default']) . "'";
+ }
+ }
+ }
+ return $default;
+ }
@stof

stof Nov 21, 2012

Member

To remove some indentation levels, I would format it this way:

if ( ! isset($field['default'])) {
    return empty($field['notnull']) ? ' NULL' : '';
}

if ( ! isset($field['type'])) {
    return " DEFAULT '" . $field['default'] . "'";
}

if (in_array((string) $field['type'], array('Integer', 'BigInteger', 'SmallInteger'))) {
    return " DEFAULT " . $field['default'];
}

if ((string) $field['type'] == 'DateTime' && $field['default'] == $this->getCurrentTimestampSQL()) {
    return " DEFAULT " . $this->getCurrentTimestampSQL();
}

if ((string) $field['type'] == 'Boolean') {
    return " DEFAULT '" . $this->convertBooleans($field['default']) . "'";
}

return " DEFAULT '" . $field['default'] . "'";
Owner

beberlei commented Nov 25, 2012

@Lusitanian Can you apply Stofs suggestions? It is mergeable then.

Contributor

Lusitanian commented Nov 25, 2012

@beberlei Done.

@beberlei beberlei added a commit that referenced this pull request Nov 25, 2012

@beberlei beberlei Merge pull request #230 from Lusitanian/master
Fixed SQL Server Platform NULL declaration
d1e83ae

@beberlei beberlei merged commit d1e83ae into doctrine:master Nov 25, 2012

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment