Bug #64616

Compare Database does not remove 'NOT NULL'

Added by Philipp Gampe 6 months ago. Updated 6 months ago.

Status:Resolved Start date:2015-01-29
Priority:Should have Due date:
Assigned To:- % Done:

100%
Category:Database API Spent time: -
Target version:-
TYPO3 Version:6.2 Is Regression:No
PHP Version:5.3 Sprint Focus:On Location Sprint
Complexity:hard

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 Core - Bug #60514: Database analyzer does not parse TEXT null / not null fields Closed 2014-07-24
related to Core - Task #64697: Add Tests for NULL fields when comparing SQL field defini... Resolved 2015-01-31
duplicated by Core - Bug #63713: Check fields for NOT NULL by default in getDatabaseExtra() Resolved 2014-12-09
duplicated by Core - Bug #58019: FAL Indexer for broken files: Column 'width' cannot be null Rejected 2014-04-17

Associated revisions

Revision 47bdb4f8
Added by Stephan Großberndt 6 months ago

[BUGFIX] Respect (NOT) NULL when comparing SQL field definitions

When comparing the database or updating extensions the definitions for
NULL / NOT NULL in fields are now respected and updated.

Resolves: #64616
Releases: master, 6.2
Change-Id: I70c63339505b373023f24973313a2e673e8eaf86
Reviewed-on: http://review.typo3.org/36400
Tested-by: Ronny Vorpahl <>
Reviewed-by: Mateusz Wojtuła <>
Reviewed-by: Christian Kuhn <>
Tested-by: Christian Kuhn <>

Revision 2edb4ddd
Added by Stephan Großberndt 6 months ago

[BUGFIX] Respect (NOT) NULL when comparing SQL field definitions

When comparing the database or updating extensions the definitions for
NULL / NOT NULL in fields are now respected and updated.

Resolves: #64616
Releases: master, 6.2
Change-Id: I70c63339505b373023f24973313a2e673e8eaf86
Reviewed-on: http://review.typo3.org/36525
Reviewed-by: Christian Kuhn <>
Tested-by: Christian Kuhn <>

History

#1 Updated by Philipp Gampe 6 months ago

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

#2 Updated by Stephan Großberndt 6 months 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 6 months 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 6 months 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 6 months 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 6 months 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 6 months 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 6 months 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 6 months ago

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

Also available in: Atom PDF