Skip to content

Loading…

DDC-100: Default values are not escaped #1584

Closed
doctrinebot opened this Issue · 7 comments

2 participants

@doctrinebot

Jira issue originally created by user nicokaiser:

When using default values for strings, they are not escaped properly when the DB schema is created.

@Column(name="name", type="string", length=50, default="Unknown User")

results in

CREATE TABLE users (..., name VARCHAR(50) DEFAULT Unknown User NOT NULL, ...)

which fails because "Unknown User" is not escaped.

@doctrinebot

Comment created by romanb:

The option for database default values might be removed but this is yet unclear.

Use PHP default values instead since that way you have the default value in your object (Doctrine wont go back to the DB just to get the DB-generated default value).

private $name = "Unknown User";
@doctrinebot

Comment created by romanb:

The default option should be removed once the columnDefinition is implemented. Database-level default values can then be set via the columnDefinition.

@doctrinebot

Comment created by @beberlei:

Hm Roman, one problem surfaced when i did a skip try to remove defaults in ClassMetadataInfo:

    public function setVersionMapping(array &$mapping)
    {
        $this->isVersioned = true;
        $this->versionField = $mapping['fieldName'];

        if ( ! isset($mapping['default'])) {
            if ($mapping['type'] == 'integer') {
                $mapping['default'] = 1;
            } else if ($mapping['type'] == 'datetime') {
                $mapping['default'] = 'CURRENT_TIMESTAMP';
            } else {
                throw DoctrineException::unsupportedOptimisticLockingType($this->name, $mapping['fieldName'], $mapping['type']);
            }
        }
    }

Two solutions:

  1. Doctrine knows when a version column is first persisted it could set it in the Code.
  2. Doctrine\DBAL\Schema 'default' won't be removed as a feature, since its a database concept. If we only remove it from all the mapping drivers we could still utilize it at this position there.
@doctrinebot

Comment created by romanb:

@Benjamin: Nr. 1 would not reliably work. In the case of a timestamp, time configuration issues come to mind. So Nr.2 is the way to go. Just remove the support in the ORM layer.

@doctrinebot

Comment created by @beberlei:

will do, it is quite a no-brainer this way.

@doctrinebot

Comment created by @beberlei:

Default has been removed.

@doctrinebot

Issue was closed with resolution "Fixed"

@beberlei beberlei was assigned by doctrinebot
@doctrinebot doctrinebot added this to the 2.0-BETA1 milestone
@doctrinebot doctrinebot closed this
@doctrinebot doctrinebot added the Bug label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.