Error when updating 3.15 -> 3.18 under MSSQL

[gsmaclean]

Support Request Template
What version of phpBB are you using? phpBB 3.1.5
What is your board's URL? http://goldwingdocs.com/forum
Who do you host your board with? Self
How did you install your board? I used the download package from phpBB.com
What is the most recent action performed on your board? Update from a previous version of phpBB3
Is registration required to reproduce this issue? No
Do you have any MODs installed? No
Do you have any extensions installed? No
What version of phpBB3 did you update from? phpBB 3.1.5
What styles do you currently have installed? Prosilver
What language(s) is your board currently using? English
Which database type/version are you using? MS SQL Server
What is your level of experience? Experienced with PHP and phpBB
What actions did you take (updating your board; installing a MOD, style or extension; etc.) prior to this problem becoming noticeable? Updating to 3.1.7
Please describe your problem.

When the database update script is run, it fails when running this update:

\phpbb\db\migration\data\v31x\increase_size_of_dateformat

The error it gives is:

Code: Select all

SQL ERROR [ mssqlnative ]

SQLSTATE: 42000 code: 102 message: [Microsoft][SQL Server Native Client 11.0][SQL Server]Incorrect syntax near 'd'. [102]

Looking at the generated SQL, it is obviously wrong:

Code: Select all

ALTER TABLE [phpbb_users] ADD CONSTRAINT [DF_phpbb_users_user_dateformat_1] DEFAULT (''d M Y H:i'') FOR [user_dateformat]

Note the double apostrophes enclosing the date format string.

The correct SQL should be:

Code: Select all

ALTER TABLE [phpbb_users] ADD CONSTRAINT [DF_phpbb_users_user_dateformat_1] DEFAULT ('d M Y H:i') FOR [user_dateformat]

Does anyone actually test this code on SQL Server before it is released? This is about the fourth time I have discovered blatant MSSQL bugs in production PHPBB code that completely prevent it from updating/running.

Generated by SRT Generator

[3Di]

This bug has been dealt with in phpBB 3.1.9, that's in the pipeline.

[gsmaclean]

OK, the problem is line 2342 of \phpbb\db\tools.php:

Code: Select all

$statements[] = 'ALTER TABLE [' . $table_name . '] ADD CONSTRAINT [DF_' . $table_name . '_' . $column_name . '_1] ' . $this->db->sql_escape($column_data['default']) . ' FOR [' . $column_name . ']';

Beginning at line 1462, we can see the column data being created:

Code: Select all

			case 'mssqlnative':
				$sql .= " {$column_type} ";
				$sql_default = " {$column_type} ";

				// For adding columns we need the default definition
				if (!is_null($column_data[1]))
				{
					// For hexadecimal values do not use single quotes
					if (strpos($column_data[1], '0x') === 0)
					{
						$return_array['default'] = 'DEFAULT (' . $column_data[1] . ') ';
						$sql_default .= $return_array['default'];
					}
					else
					{
						$return_array['default'] = 'DEFAULT (' . ((is_numeric($column_data[1])) ? $column_data[1] : "'{$column_data[1]}'") . ') ';
						$sql_default .= $return_array['default'];
					}
				}

Note this line:

Code: Select all

$return_array['default'] = 'DEFAULT (' . ((is_numeric($column_data[1])) ? $column_data[1] : "'{$column_data[1]}'") . ') ';

Notice where the apostrophe is being added, specifically around:

Code: Select all

'{$column_data[1]}'

We then escape this already-escaped value at line 2342:

Code: Select all

$statements[] = 'ALTER TABLE [' . $table_name . '] ADD CONSTRAINT [DF_' . $table_name . '_' . $column_name . '_1] ' . $this->db->sql_escape($column_data['default']) . ' FOR [' . $column_name . ']';

Which double-escapes it, writing two apostrophes, making the SQL invalid, and causing the upgrade to fail.

Changing line 2342 to:

Code: Select all

$statements[] = 'ALTER TABLE [' . $table_name . '] ADD CONSTRAINT [DF_' . $table_name . '_' . $column_name . '_1] ' . $column_data['default'] . ' FOR [' . $column_name . ']';

...fixes the problem and allows the upgrade to complete.

Sure do love having to debug PHP every time I try to upgrade my board.

[3Di]

https://github.com/phpbb/phpbb/commit/4 ... 212d47d008

[gsmaclean]

https://github.com/phpbb/phpbb/commit/4 ... 212d47d008

Thanks. Glad my fix was correct. Sure would be nice to have this mentioned somewhere for those of us trying to upgrade, and ending up with a half-converted database and a non-functional site. Glad I found this out on my test server before pushing live to production.