Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0-BETA1
    • Component/s: ORM
    • Security Level: All
    • Labels:
      None

      Description

      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.

        Activity

        Hide
        Roman S. Borschel added a comment -

        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";
        
        Show
        Roman S. Borschel added a comment - 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" ;
        Hide
        Roman S. Borschel added a comment -

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

        Show
        Roman S. Borschel added a comment - The default option should be removed once the columnDefinition is implemented. Database-level default values can then be set via the columnDefinition.
        Hide
        Benjamin Eberlei added a comment -

        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.

        Show
        Benjamin Eberlei added a comment - 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.
        Hide
        Roman S. Borschel added a comment -

        @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.

        Show
        Roman S. Borschel added a comment - @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.
        Hide
        Benjamin Eberlei added a comment -

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

        Show
        Benjamin Eberlei added a comment - will do, it is quite a no-brainer this way.
        Hide
        Benjamin Eberlei added a comment -

        Default has been removed.

        Show
        Benjamin Eberlei added a comment - Default has been removed.

          People

          • Assignee:
            Benjamin Eberlei
            Reporter:
            Nico Kaiser
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: