Project

General

Profile

Actions

Bug #35253

closed

Installer should respect backticks while creating sql queries

Added by Kay Strobach over 12 years ago. Updated almost 9 years ago.

Status:
Rejected
Priority:
Must have
Assignee:
-
Category:
Install Tool
Target version:
-
Start date:
2012-03-26
Due date:
% Done:

80%

Estimated time:
0.50 h
TYPO3 Version:
4.5
PHP Version:
5.3
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

Description

The TYPO3 SQL parser removes backticks from table and field names in SQL statements. This causes errors.

E.g. a valid SQL file entry like this (with backticks):

CREATE TABLE `user_piwikintegration_site` (
  `group` varchar(250) NOT NULL,
) TYPE=InnoDB  AUTO_INCREMENT=2 ;

is converted to this invalid SQL (without backticks):

CREATE TABLE user_piwikintegration_site ( group varchar(250) NOT NULL ) ENGINE=InnoDB;

While the first query is valid, the modified query causes an error:

Error:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'group varchar(250) NOT NULL' at line 2

The Problem can be easily fixed, by editing t3lib_install::getUpdateSuggestions in t3lib/class.t3lib_install.php

Todo:
  • add backticks around table names
  • add backticks around fieldnames

Related issues 1 (0 open1 closed)

Has duplicate TYPO3 Core - Bug #34557: Missing backticks in t3lib_install_Sql->getUpdateSuggestionsClosed2012-03-06

Actions
Actions #1

Updated by Kay Strobach over 12 years ago

file is class.t3lib_install_sql.php -> i'm working on this issue!

Actions #2

Updated by Kay Strobach over 12 years ago

  • % Done changed from 0 to 80
        public function getUpdateSuggestions($diffArr, $keyList = 'extra,diff') {
        $statements = array();
        $deletedPrefixKey = $this->deletedPrefixKey;
        $remove = 0;
        if ($keyList == 'remove') {
            $remove = 1;
            $keyList = 'extra';
        }
        $keyList = explode(',', $keyList);
        foreach ($keyList as $theKey) {
            if (is_array($diffArr[$theKey])) {
                foreach ($diffArr[$theKey] as $table => $info) {
                    $whole_table = array();
                    if (is_array($info['fields'])) {
                        foreach ($info['fields'] as $fN => $fV) {
                            if ($info['whole_table']) {
                                $whole_table[] = '`' . $fN . '` ' . $fV;
                            } else {
                                    // Special case to work around MySQL problems when adding auto_increment fields:
                                if (stristr($fV, 'auto_increment')) {
                                        // The field can only be set "auto_increment" if there exists a PRIMARY key of that field already.
                                        // The check does not look up which field is primary but just assumes it must be the field with the auto_increment value...
                                    if (isset($diffArr['extra'][$table]['keys']['PRIMARY'])) {
                                            // Remove "auto_increment" from the statement - it will be suggested in a 2nd step after the primary key was created
                                        $fV = str_replace(' auto_increment', '', $fV);
                                    } else {
                                            // In the next step, attempt to clear the table once again (2 = force)
                                        $info['extra']['CLEAR'] = 2;
                                    }
                                }
                                if ($theKey == 'extra') {
                                    if ($remove) {
                                        if (substr($fN, 0, strlen($deletedPrefixKey)) != $deletedPrefixKey) {
                                            $statement = 'ALTER TABLE `' . $table . '` CHANGE `' . $fN . '` `' . $deletedPrefixKey . $fN . '` ' . $fV . ';';
                                            $statements['change'][md5($statement)] = $statement;
                                        } else {
                                            $statement = 'ALTER TABLE `' . $table . '` DROP `' . $fN . '`;';
                                            $statements['drop'][md5($statement)] = $statement;
                                        }
                                    } else {
                                        $statement = 'ALTER TABLE `' . $table . '` ADD `' . $fN . '` ' . $fV . ';';
                                        $statements['add'][md5($statement)] = $statement;
                                    }
                                } elseif ($theKey == 'diff') {
                                    $statement = 'ALTER TABLE `' . $table . '` CHANGE `' . $fN . '` `' . $fN . '` ' . $fV . ';';
                                    $statements['change'][md5($statement)] = $statement;
                                    $statements['change_currentValue'][md5($statement)] = $diffArr['diff_currentValues'][$table]['fields'][$fN];
                                }
                            }
                        }
                    }
                    if (is_array($info['keys'])) {
                        foreach ($info['keys'] as $fN => $fV) {
                            if ($info['whole_table']) {
                                $whole_table[] = $fV;
                            } else {
                                if ($theKey == 'extra') {
                                    if ($remove) {
                                        $statement = 'ALTER TABLE `' . $table . '`' . ($fN == 'PRIMARY' ? ' DROP PRIMARY KEY' : ' DROP KEY `' . $fN . '`') . ';';
                                        $statements['drop'][md5($statement)] = $statement;
                                    } else {
                                        $statement = 'ALTER TABLE `' . $table . '` ADD ' . $fV . ';';
                                        $statements['add'][md5($statement)] = $statement;
                                    }
                                } elseif ($theKey == 'diff') {
                                    $statement = 'ALTER TABLE `' . $table . '`' . ($fN == 'PRIMARY' ? ' DROP PRIMARY KEY' : ' DROP KEY `' . $fN . '`') . ';';
                                    $statements['change'][md5($statement)] = $statement;
                                    $statement = 'ALTER TABLE ' . $table . ' ADD ' . $fV . ';';
                                    $statements['change'][md5($statement)] = $statement;
                                }
                            }
                        }
                    }
                    if (is_array($info['extra'])) {
                        $extras = array();
                        $extras_currentValue = array();
                        $clear_table = FALSE;

                        foreach ($info['extra'] as $fN => $fV) {

                                // Only consider statements which are missing in the database but don't remove existing properties
                            if (!$remove) {
                                if (!$info['whole_table']) { // If the whole table is created at once, we take care of this later by imploding all elements of $info['extra']
                                    if ($fN == 'CLEAR') {
                                            // Truncate table must happen later, not now
                                            // Valid values for CLEAR: 1=only clear if keys are missing, 2=clear anyway (force)
                                        if (count($info['keys']) || $fV == 2) {
                                            $clear_table = TRUE;
                                        }
                                        continue;
                                    } else {
                                        $extras[] = $fN . '=' . $fV;
                                        $extras_currentValue[] = $fN . '=' . $diffArr['diff_currentValues'][$table]['extra'][$fN];
                                    }
                                }
                            }
                        }
                        if ($clear_table) {
                            $statement = 'TRUNCATE TABLE `' . $table . '`;';
                            $statements['clear_table'][md5($statement)] = $statement;
                        }
                        if (count($extras)) {
                            $statement = 'ALTER TABLE `' . $table . '` ' . implode(' ', $extras) . ';';
                            $statements['change'][md5($statement)] = $statement;
                            $statements['change_currentValue'][md5($statement)] = implode(' ', $extras_currentValue);
                        }
                    }
                    if ($info['whole_table']) {
                        if ($remove) {
                            if (substr($table, 0, strlen($deletedPrefixKey)) != $deletedPrefixKey) {
                                $statement = 'ALTER TABLE `' . $table . '` RENAME `' . $deletedPrefixKey . $table . '`;';
                                $statements['change_table'][md5($statement)] = $statement;
                            } else {
                                $statement = 'DROP TABLE `' . $table . '`;';
                                $statements['drop_table'][md5($statement)] = $statement;
                            }
                                // count:
                            $count = $GLOBALS['TYPO3_DB']->exec_SELECTcountRows('*', $table);
                            $statements['tables_count'][md5($statement)] = $count ? 'Records in table: ' . $count : '';
                        } else {
                            $statement = 'CREATE TABLE `' . $table . "` (\n" . implode(",\n", $whole_table) . "\n)";
                            if ($info['extra']) {
                                foreach ($info['extra'] as $k => $v) {
                                    if ($k == 'COLLATE' || $k == 'CLEAR') {
                                        continue; // Skip these special statements. TODO: collation support is currently disabled (needs more testing)
                                    }
                                    $statement .= ' ' . $k . '=' . $v; // Add extra attributes like ENGINE, CHARSET, etc.
                                }
                            }
                            $statement .= ';';
                            $statements['create_table'][md5($statement)] = $statement;
                        }
                    }
                }
            }
        }

        return $statements;
    }

This function works in 4.7 and 4.5, the basement is 4.7 This function is located in
  • 4.5: t3lib_install::getUpdateSuggestions in t3lib/class.t3lib_install.php
  • 4.7: t3lib_install_sql::getUpdateSuggestions in t3lib/class.t3lib_install_sql.php
Actions #3

Updated by Gerrit Code Review over 12 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9894

Actions #4

Updated by Gerrit Code Review over 12 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9894

Actions #5

Updated by Ernesto Baschny over 12 years ago

  • Target version deleted (4.5.14)
Actions #6

Updated by Gerrit Code Review about 12 years ago

Patch set 3 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9894

Actions #7

Updated by Gerrit Code Review over 11 years ago

Patch set 4 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/9894

Actions #8

Updated by Gerrit Code Review over 11 years ago

Patch set 5 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/9894

Actions #9

Updated by Mathias Schreiber almost 10 years ago

  • Status changed from Under Review to Accepted
  • Is Regression set to No
Actions #10

Updated by Morton Jonuschat almost 9 years ago

  • Status changed from Accepted to Rejected

Escaping SQL identifiers with backticks is a MySQL specific syntax that breaks DBAL/SqlParser and won't be supported at this time. It is advised not to use reserved SQL keywords as table/field/index names.

Actions

Also available in: Atom PDF