Bug #64616

Compare Database does not remove 'NOT NULL'

Added by Philipp Gampe about 6 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Database API (Doctrine DBAL)
Target version:
-
Start date:
2015-01-29
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
6.2
PHP Version:
5.3
Tags:
Complexity:
hard
Is Regression:
No
Sprint Focus:

Description

The compare database wizard does not detect if fields to not have the NOT NULL clause anymore (if they had such a clause previously).

MariaDB [dummy]> SHOW FIELDS FROM sys_file_processedfile;
+-------------------+--------------+------+-----+---------+----------------+
| Field             | Type         | Null | Key | Default | Extra          |
+-------------------+--------------+------+-----+---------+----------------+
| uid               | int(11)      | NO   | PRI | NULL    | auto_increment |
| pid               | int(11)      | NO   | MUL | 0       |                |
| tstamp            | int(11)      | NO   |     | 0       |                |
| crdate            | int(11)      | NO   |     | 0       |                |
| cruser_id         | int(11)      | NO   |     | 0       |                |
| deleted           | tinyint(4)   | NO   |     | 0       |                |
| storage           | int(11)      | NO   | MUL | 0       |                |
| original          | int(11)      | NO   | MUL | 0       |                |
| identifier        | varchar(512) | YES  |     |         |                |
| name              | tinytext     | YES  |     | NULL    |                |
| configuration     | text         | YES  |     | NULL    |                |
| context           | varchar(200) | NO   |     |         |                |
| checksum          | varchar(255) | NO   |     |         |                |
| is_processed      | varchar(200) | NO   |     |         |                |
| extension         | varchar(255) | NO   |     |         |                |
| mime_type         | varchar(255) | NO   |     |         |                |
| sha1              | tinytext     | YES  |     | NULL    |                |
| size              | int(11)      | NO   |     | 0       |                |
| width             | int(11)      | NO   |     | 0       |                |
| height            | int(11)      | NO   |     | 0       |                |
| originalfilesha1  | char(40)     | YES  |     |         |                |
| task_type         | varchar(200) | NO   |     |         |                |
| configurationsha1 | char(40)     | YES  |     |         |                |
+-------------------+--------------+------+-----+---------+----------------+

Notice the NO for NULL for the fields width and height. The corresponding SQL file (from core sysext):

CREATE TABLE sys_file_processedfile (
    uid int(11) NOT NULL auto_increment,
    tstamp int(11) DEFAULT '0' NOT NULL,
    crdate int(11) DEFAULT '0' NOT NULL,

    storage int(11) DEFAULT '0' NOT NULL,
    original int(11) DEFAULT '0' NOT NULL,
    identifier varchar(512) DEFAULT '' NOT NULL,
    name tinytext,
    configuration text,
    configurationsha1 char(40) DEFAULT '' NOT NULL,
    originalfilesha1 char(40) DEFAULT '' NOT NULL,
    task_type varchar(200) DEFAULT '' NOT NULL,
    checksum varchar(255) DEFAULT '' NOT NULL,
    width int(11) DEFAULT '0',
    height int(11) DEFAULT '0',

    PRIMARY KEY (uid),
    KEY combined_1 (original,task_type,configurationsha1),
    KEY identifier (storage,identifier(199))
);

Notice that it does not have a NOT NULL for the width and height fields.

The database compare does not detect this.


Related issues

Related to TYPO3 Core - Task #64697: Add Tests for NULL fields when comparing SQL field definitions Closed2015-01-31

Actions
Related to TYPO3 Core - Bug #60514: Database analyzer does not parse TEXT null / not null fieldsClosedMorton Jonuschat2014-07-24

Actions
Related to TYPO3 Core - Bug #51090: SQL parser does not analyse NOT NULL in extension SQL filesClosedMorton Jonuschat2013-08-15

Actions
Has duplicate TYPO3 Core - Bug #58019: FAL Indexer for broken files: Column 'width' cannot be nullRejected2014-04-17

Actions
Has duplicate TYPO3 Core - Bug #63713: Check fields for NOT NULL by default in getDatabaseExtra() Closed2014-12-09

Actions
#1

Updated by Philipp Gampe about 6 years ago

  • Subject changed from Compare Database does remove 'NOT NULL' to Compare Database does not remove 'NOT NULL'
#2

Updated by Stephan Großberndt about 6 years ago

I had that issue some time before too. I know its cause, the according parameter is not set when the function to check the fields is called. I did not know if this is on purpose so I did not file a bug report.

#3

Updated by Philipp Gampe about 6 years ago

DBAL needs to xclass anyway?!? Therefore I suggest to improve the behavior for MySQL/MariaDB Databases and fix this for 95% of our users.

If you know the cause, please push a patch. Then we will at least be able to discuss this.

#4

Updated by Stephan Großberndt about 6 years ago

The problem is in TYPO3\CMS\Install\Service\SqlSchemaMigrationService

    /**
     * Compares two arrays with field information and returns information about fields that are MISSING and fields that have CHANGED.
     * FDsrc and FDcomp can be switched if you want the list of stuff to remove rather than update.
     *
     * @param array $FDsrc Field definitions, source (from getFieldDefinitions_fileContent())
     * @param array $FDcomp Field definitions, comparison. (from getFieldDefinitions_database())
     * @param string $onlyTableList Table names (in list) which is the ONLY one observed.
     * @param bool $ignoreNotNullWhenComparing If set, this function ignores NOT NULL statements of the SQL file field definition when comparing current field definition from database with field definition from SQL file. This way, NOT NULL statements will be executed when the field is initially created, but the SQL parser will never complain about missing NOT NULL statements afterwards.
     * @return array Returns an array with 1) all elements from $FDsrc that is not in $FDcomp (in key 'extra') and 2) all elements from $FDsrc that is different from the ones in $FDcomp
     */
    public function getDatabaseExtra($FDsrc, $FDcomp, $onlyTableList = '', $ignoreNotNullWhenComparing = TRUE) {

Why would you have the SQL parser not complain about missing NOT NULL after creating the table?

Either all (?) calls to getDatabaseExtra have to contain $ignoreNotNullWhenComparing = FALSE or the default signature for getDatabaseExtra should be changed to $ignoreNotNullWhenComparing = FALSE.

#5

Updated by Gerrit Code Review about 6 years ago

  • Status changed from Accepted to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/36400

#6

Updated by Gerrit Code Review about 6 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/36400

#7

Updated by Gerrit Code Review about 6 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/36400

#8

Updated by Gerrit Code Review about 6 years ago

Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/36525

#9

Updated by Stephan Großberndt about 6 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
#10

Updated by Anja Leichsenring about 5 years ago

  • Sprint Focus deleted (On Location Sprint)
#11

Updated by Benni Mack over 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF