Add MySQL 8 reserved keywords #3128

mlocati · 2018-05-03T14:44:47Z

MySQL 8 added a few reserved keywords.
Comparing https://dev.mysql.com/doc/refman/8.0/en/keywords.html with MySQL57Keywords.php here's the list of the new keywords:

ADMIN
CUBE
CUME_DIST
DENSE_RANK
EMPTY
EXCEPT
FIRST_VALUE
FUNCTION
GROUPING
GROUPS
JSON_TABLE
LAG
LAST_VALUE
LEAD
NTH_VALUE
NTILE
OF
OVER
PERCENT_RANK
PERSIST
PERSIST_ONLY
RANK
RECURSIVE
ROW
ROWS
ROW_NUMBER
SYSTEM
WINDOW

What should I write in the @since phodoc?

PS: it would be nice to add this to the 2.5.x series, for projects using PHP older than 7.1

Majkl578 · 2018-05-03T15:09:53Z

lib/Doctrine/DBAL/Platforms/Keywords/MySQL8Keywords.php

+ * @author Michele Locati <[email protected]>
+ * @link   www.doctrine-project.org
+ */
+class MySQL8Keywords extends MySQL57Keywords


To follow similar naming pattern, this class should be named MySQL80Keywords.
Also if possible, should also only add new keywords (or remove removed ones), like here:

dbal/lib/Doctrine/DBAL/Platforms/Keywords/PostgreSQL94Keywords.php

Lines 49 to 55 in 9f3188a

$parentKeywords = array_diff(parent::getKeywords(), [

'OVER',

]);

return array_merge($parentKeywords, [

'LATERAL',

]);

To follow similar naming pattern, this class should be named MySQL80Keywords.

👍

Also if possible, should also only add new keywords (or remove removed ones), like here:

What about the current MySQL57Keywords class? Should it be updated to have the same behavior vs its underlying MySQLKeywords class

Furthermore: if a keywords will be added to MySQL57Keywords, and it's already present in MySQL80Keywords, with the approach you suggest we'll have duplicated kewords: should be call array_unique (and array_values to keep the array indexes sane)?

lib/Doctrine/DBAL/Platforms/MySQL8Platform.php

lib/Doctrine/DBAL/Platforms/Keywords/MySQL80Keywords.php

morozov · 2018-05-14T23:23:23Z

@mlocati please fix the coding style violations.

mlocati · 2018-05-15T08:46:53Z

@mlocati please fix the coding style violations.

Done.

morozov · 2018-05-15T19:05:25Z

Sorry, I didn't notice you have many commits. Please squash them since the commit history is not really relevant in this case.

mlocati · 2018-05-15T19:12:21Z

Please squash them since the commit history is not really relevant in this case.

Done.

morozov · 2018-06-08T17:16:27Z

Thanks @mlocati!

mlocati · 2018-06-08T17:17:58Z

Will this code be added to the 2.5.x series too? For projects using PHP older than 7.1...

Ocramius · 2018-06-08T17:21:54Z

No, all improvements are only forward-ported. Backporting is only for bug-fixes that are really worth backporting.

iLevye · 2018-06-08T17:38:50Z

From the mysql docs:

ADMIN became nonreserved in 8.0.12

mlocati · 2018-06-08T17:44:26Z

No, all improvements are only forward-ported. Backporting is only for bug-fixes that are really worth backporting.

@Ocramius Consider the case of concrete5 (an open source CMS with tons of users), which uses a table called Groups: without this fix it won't run. So for me it'd be worth backporting it...

morozov · 2018-06-08T17:46:36Z

@iLevye I think it won't hurt if ADMIN stays marked as a reserved keyword. The DBAL doesn't prevent them from being used. On the contrary, it adds additional quotes around them to make the query work. The same quotation syntax can be used for any object name, not only for reserved keywords.

Ocramius · 2018-06-08T17:47:59Z

@Ocramius Consider the case of concrete5 (an open source CMS with tons of users), which uses a table called Groups: without this fix it won't run. So for me it'd be worth backporting it...

Upgrade the DBAL version of concrete5

morozov · 2018-06-08T17:49:25Z

@mlocati In my own experience, the DBAL is extensible enough to let you use custom Connection, Statement and Platform implementations. In the project I'm working on, we use this approach to introduce the needed changes in the DBAL which have not been yet released. Then we upgrade and remove custom implementations.

KorvinSzanto · 2018-06-08T17:59:31Z

Upgrade the DBAL version of concrete5

This is absolutely something we'll do in v9 of concrete5 as long as it ends up with a minimum version higher than PHP 7.1.x, but we can't do that in any v8.x versions since it'd be a backwards incompatible change.

With that in mind, I think adding a layer on our end would be a reasonable solution.

Ocramius · 2018-06-08T18:03:39Z

since it'd be a backwards incompatible change.

https://www.doctrine-project.org/2017/07/25/php-7.1-requirement-and-composer.html

This depends on whether you are pushing a library or a deployable:

in a deployable, any change in composer.lock is a BC break (everything, really)
in a library, you can bump dependencies in every minor release

KorvinSzanto · 2018-06-08T18:26:52Z

You're right that composer based installs will just automatically get the latest version already. The problem is that concrete5 is largely distributed as a prebuilt zip file set at minimum requirements.

mlocati · 2018-06-11T06:53:28Z

@mlocati In my own experience, the DBAL is extensible enough to let you use custom Connection, Statement and Platform implementations.

@morozov Thanks for pointimg me to the right direction: you are correct, extending DBAL is very easy (see concretecms/concretecms#6753 )

morozov · 2018-06-11T17:15:19Z

@mlocati, awesome! Problem solved.

* Fix concretecms#6649 * Call count() only if array in page_list block controller * Fix concretecms#6647 * Add File\Importer::importUploadedFile * Call count() only if array in page_list block controller * update sample content copyright year * Moving back to release candidate 4 * Fix concretecms#6606 * Fix concretecms#6543 * Fix concretecms#6518 * Starting on a fix for concretecms#6623 * Deleting cookie after logout * Make System Email Addresses dashboard page labels more descriptive * Added support for the "media" attribute for CSS resources * Trying to fix tests * Rename $media variable in $assetMedia * Remove unused recordView method * Remove unused getTotalPageViews method * Remove unused getPageStatistics method * Remove unused getTotalPageViews method * Remove unused Statistics page * Remove unused getTotalPageViewsForOthers method * Remove unused getPreviousSessionPageViews method * Fix concretecms#6659 * Avoid accessing undefined $attributePermission in EditUserPropertiesUserAccess::getAccessListItems * Redirect to current page instead of previous frontend page when saving draft on composer * Add missing configuration keus to site config * php-cs-fixer * Use modern code in favicons controller/view dashboard page * Add support for theme-color meta tag * Add support to filter-by-extension filter to FileFolderManager, let filterByExtension accept multiple extensions * php-cs-fixer * Improve PHP doc and simplify code of FileManager service * php-cs-fixer * Consider the case when $tracker property is not set * Fix concretecms#6674 * Add a migration to refresh the image block type * Simplify add/edit form of ExternalForm block type * Don't disclose absolute path in add/edit form of ExternalForm block type * Fixing error when removing cookie by hand * Rework the add content panel lock/pinned functionality + Close the panel when clicking again on the add-content button instead of locking the panel + Lock the panel when double clicking the add-content button * Fix tests * Fixing reinstallation issue in some cases * Fix concretecms#6665 * Use altKey to pine the add-content panel instead of double click * Rename theme_color to browser_toolbar_color configuration key * Choose exact file type & extension in icons dashboard page * Fix bug in FileManager::file * first pass on privacy data notice * finishing up * Removes the previous save-search-preset dialog on click on advanced search button * Fix concretecms#6694 * Fix concretecms#6694 * Add "upscalingEnabled" field to thumbnail types * making exporting of express csv a little more bulletproof * making exporting of express csv a little more bulletproof * Avoid accessing undefined var in page_types/form/base * Avoid accessing undefined vars in page_types/target_types/all * Avoid accessing undefined vars in page_types/target_types/page_type * Avoid accessing undefined var in page_types/target_types/parent_page * php-cs-fixer * Fix setTrustedProxies for Symfony 3.3.0+ * Safer setting of setTrustedProxies * Remove useless "use" statement * Avoid accessing an undefined var in Job::executeJob * Fix concretecms#6710 * Patch the superglobals in the runtime stack * Rename ..._as_ajax to ..._as_xhr * Add on_thumbnail_delete event * Add on_thumbnail_generate event * Fix docblocks; the path is relative, not absolute * Avoid deprecated methods and add doc to system/basics/editor dashboard page * Add PHPDoc to Editor\Plugin * Add PHPDoc to Editor\PluginManager, make select/deselect symmetric, deprecate selectMultiple * php-cs-fixer, simplify registerCkeditorPlugins, allow using EditorInterface in DI * Simplify CkeditorEditor::saveOptionsForm * Add PHPDoc to EditorInterface * CKEditor: php-cs-fixer, add phpdoc * Fix concretecms#4801 * Sort the Editor plugins alphabetically. * Fix loading default plugins * Avoid accessing an undefined var in user_interface/page backend controller * Fix migration of address attribute type settings * Fix migration of address attribute type settings * Allow upscaling of file_manager_listing thumbnail type * Add editor plugin descriptions and configuration preview * Make editor preview sticky * Keep the previously content of the preview editor * Simplify the description of the CKEditor plugins * Throw a UserMessageException in UserInterface backend controller on access denied. * Mark static Pile methods as static * Fix typo in editor plugin description * Make page state that it implements the attribute object interface for csv export purposes * Make page state that it implements the attribute object interface for csv export purposes * adding object interface to event version occurrence class * Avoid using superglobals in Form service * Fix Form tests * Remove typo in PostLoginLocation PHPDoc * Fixing problem with date/time attribute where attributes set to only show a date would also show a time in plain text, including in express email notifications * Adding the ability to unapprove an approved version * Adding the ability to unapprove an approved version * fixing workflow description on unapproval * updating version * updating changelog * updating version number * Avoid accessing undefined array index in desktop_featured_addon controller * Avoid accessing undefined var in desktop_featured_addon view * Avoid accessing undefined array index in desktop_featured_theme controller * Avoid accessing undefined var in desktop_featured_theme view * Avoid accessing undefined array index in permission/detail element * Avoid accessing undefined array indexes in permissions/access_entity tool * Avoid accessing undefined var in Block/Menu constructor * concretecms#6724 PageList start/end times * Add the page selector attribute to the core * php-cs-fixer * Add the newly created node to the loaded child node list * Fix bug in Node::getAllChildNodeIDs * php-cs-fixer * Use 1 less query in FileManager::get() * php-cs-fixer * Add PHPDoc to Filesystem * Let Node::getHierarchicalNodesOfType and Node::populateRecursiveNodes accept a maxDepth parameter * Add the page selector attribute on install * php-cs-fixer * Remove unneeded function call * Added a migration to install the page selector attribute or to unlink it from any package and keep its name consistent * Fix $app in migration * Add FileFolder::getChildFolderByName and FileFolder::getChildFolderByPath * Add PHPDoc to FilesystemTest * Fix query that calculates the number of child nodes * Avoid accessing undefined var in FileFolder::getChildFolderByName * Fix testGetFolderByName test * php-cs-fixer * Backport doctrine/dbal#3128 * Added the setNameSpace() method to modify pagination parameters * Use the setNameSpace() method in the page list block to allow multiple paginations on the same page * 6755: next_previous block skips pages Equivalent date page get skipped as well as pages before/after an inaccessible page. * Fix resetting concrete5 with MySQL8 * Avoid accessing undefined array index if page_list block controller * Avoid accessing undefined array index in multilingual/copy dashboard page controller * Avoid accessing undefined values in ReplaceBlockPageRelationsTask::execute * Avoid accessing undefined var in page/design dialog controller * Add PHPDoc to switch_language view * Fix concretecms#6737 * Fix determination of URL and contents of JavascriptLocalizedAsset * Fix CKEditor configuration preview * Use correct locale ID in Multilingual/Service/Detector::getPreferredSection * Use UI localization context in concrete5 toolbar/account menu * Avoid accessing undefined array indexes in Validation form service * Fix setting the "required" attribute of the privacy agreement on install page * Mark TaskPermission::getByHandle static * Simplify code and check valid pages * php-cs-fixer * Add PHPDoc to Site entity * Use the ::class syntax for class names * Avoid using \Core facade * Add PHPDoc to Site entity * Let Site::getSiteTreeObject return NULL if no locale is defined * Use Category::getAttributeKeyByHandle instead of deprecated Category::getByHandle * Avoid unnecessary function calls * Don't create an attribute value it its key is not valid * Force boolean values in Site::setIsDefault * Pass user to finishAuthentication in AuthenticationTypeController completeAuthentication Ref concretecms#6673 * php-cs-fixer * Fix concretecms#5440 * Fix concretecms#6778 * Fix nasty bug introduced in concretecms#6773

@morozov

Release v2.8.0 [![Build Status](https://travis-ci.org/doctrine/dbal.svg?branch=v2.8.0)](https://travis-ci.org/doctrine/dbal) This is a minor release of Doctrine DBAL that aggregates over 30 fixes and improvements developed over the last 3 months. This release includes all changes of the 2.7.x series, as well as feature additions and improvements that couldn’t land in patch releases. **Backwards Compatibility Breaks** This doesn't contain any intentional Backwards Compatibility (BC) breaks. **Dependency Changes** * The dependency on [doctrine/common](https://github.com/doctrine/common) is removed. DBAL now depends on [doctrine/cache](https://github.com/doctrine/cache) and [doctrine/event-manager](https://github.com/doctrine/event-manager) instead. Please see details in the [UPGRADE.md](UPGRADE.md) documentation. **Deprecations** * The usage of binary fields whose length exceeds the maximum field size on a given platform is deprecated. Please use binary fields of a size which fits all target platforms, or use blob explicitly instead. * The usage of DB-generated UUIDs is deprecated. Their format is inconsistent across supported platforms and therefore the feature is not portable. Use a PHP library (e.g. [ramsey/uuid](https://packagist.org/packages/ramsey/uuid)) to generate UUIDs on the application side. **New features** * Initial support of MySQL 8. * Initial support of PostgreSQL 11. * Ability to evaluate arbitrary SQL expressions via `AbstractPlatform::getDummySelectSQL()`. **Improvements and Fixes** * Improved support of binary fields on Oracle and IBM DB2. * Improved SQL Server configuration capabilities via both `sqlsrv` and `pdo_sqlsrv`. * Improved handling of `AUTOINCREMENT`ed primary keys in SQLite. * Integration tests are run against IBM DB2 on Travis CI. * Code coverage is collected for the Oracle platform on continuousphp. Total issues resolved: **33** **Deprecations:** - [3187: Deprecate usage of binary fields whose length exceeds maximum](doctrine#3187) thanks to @morozov - [3188: Deprecated usage of binary fields whose length exceeds the platform maximum](doctrine#3188) thanks to @morozov - [3192: Added more information to the deprecation notice](doctrine#3192) thanks to @morozov - [3212: Deprecated usage of DB-generated UUIDs](doctrine#3212) thanks to @morozov **New Features:** **Bug Fixes:** - [3149: Introduced binary binding type to support binary parameters on Oracle](doctrine#3149) thanks to @morozov - [3178: Fix incorrect exception thrown from SQLAnywhere16Platform](doctrine#3178) thanks to @Majkl578 - [3044: Functional test for allowing dynamic intervals in date sub/add](doctrine#3044) thanks to @AshleyDawson - [3049: Test failures caused by invalid database connection result in fatal error](doctrine#3049) thanks to @Majkl578 **Improvements:** - [3033: Added support for available DSN parameters for the PDOSqlsrv driver](doctrine#3033) thanks to @aashmelev - [3128: Add MySQL 8 reserved keywords](doctrine#3128) thanks to @mlocati - [3143: initialize sql array into platform files](doctrine#3143) thanks to @AlessandroMinoccheri - [3173: Fix composer branch aliases](doctrine#3173) thanks to @Majkl578 - [3157: When building a limit query, zero offset without a limit should be ignored](doctrine#3157) thanks to @morozov - [3109: Allow to specify arbitrary SQL expression in AbstractPlatform::getDummySelectSQL()](doctrine#3109) thanks to @morozov - [3141: allow creating PRIMARY KEY AUTOINCREMENT fields for sqlite (unit tests)](doctrine#3141) thanks to @TimoBakx - [3180: Import simplified version of Common\Util\Debug for var dumping purposes](doctrine#3180) thanks to @Majkl578 **Documentation Improvements:** - [3117: Added badges for the develop branch in README](doctrine#3117) thanks to @morozov - [3125: Upgrading docs](doctrine#3125) thanks to @jwage - [3144: added improvement type into pull request template](doctrine#3144) thanks to @AlessandroMinoccheri **Code Quality Improvements:** - [3025: Added PHPStan, apply changes for level 3](doctrine#3025) thanks to @Majkl578 - [3200: Php Inspections (EA Ultimate): minor code tweaks](doctrine#3200) thanks to @kalessil - [3204: Fix typo in AbstractPlatform](doctrine#3204) thanks to @Majkl578 - [3205: Ignore OCI-* classes in static analysis (no stubs)](doctrine#3205) thanks to @Majkl578 **Continuous Integration Improvements:** - [3102: Use newer PHPUnit to prevent crashes on failures](doctrine#3102) thanks to @Majkl578 - [3112: Removed hard-coded configuration filenames from the test runner](doctrine#3112) thanks to @morozov - [3133: Travis DB2](doctrine#3133) thanks to @Majkl578, @morozov - [3135: AppVeyor tweaks, retry coverage upload on failure](doctrine#3135) thanks to @Majkl578 - [3137: Workaround for the inability to use a post-PHPUnit script on ContinuousPHP](doctrine#3137) thanks to @morozov - [3151: MSSQL DLL 5.2.0 has been released.](doctrine#3151) thanks to @photodude - [3160: Test against Postgres 11](doctrine#3160) thanks to @Majkl578 **Dependencies** - [3193: DBAL 2.8 needs Common 2.9](doctrine#3193) thanks to @Majkl578 - [3176: Eliminate dependency on doctrine/common](doctrine#3176) thanks to @Majkl578 - [3181: Remove dependency on doctrine/common](doctrine#3181) thanks to @Majkl578 # gpg: Signature made Fri Jul 13 06:02:10 2018 # gpg: using RSA key 374EADAF543AE995 # gpg: Can't check signature: public key not found # Conflicts: # .gitignore # README.md # lib/Doctrine/DBAL/Version.php # tests/appveyor/mssql.sql2008r2sp2.sqlsrv.appveyor.xml # tests/appveyor/mssql.sql2012sp1.sqlsrv.appveyor.xml # tests/appveyor/mssql.sql2017.pdo_sqlsrv.appveyor.xml # tests/appveyor/mssql.sql2017.sqlsrv.appveyor.xml # tests/travis/mariadb.mysqli.travis.xml

mlocati mentioned this pull request May 3, 2018

Groups is a reserved keyword in MySQL 8 concretecms/concretecms#6156

Closed

Majkl578 reviewed May 3, 2018

View reviewed changes

Majkl578 added New Feature MySQL Platforms Platform Detection Reserved Keywords labels May 3, 2018

mlocati mentioned this pull request May 3, 2018

Quote "Groups" in MySQL queries concretecms/concretecms#6622

Merged

morozov reviewed May 3, 2018

View reviewed changes

lib/Doctrine/DBAL/Platforms/Keywords/MySQL80Keywords.php Outdated Show resolved Hide resolved

morozov reviewed May 3, 2018

View reviewed changes

lib/Doctrine/DBAL/Platforms/Keywords/MySQL80Keywords.php Outdated Show resolved Hide resolved

Add MySQL 8 reserved keywords

3805b2b

mlocati force-pushed the mysql8-keywords branch from 6f75fbf to 3805b2b Compare May 15, 2018 19:11

MisatoTremor approved these changes Jun 8, 2018

View reviewed changes

Majkl578 added this to the 2.8.0 milestone Jun 8, 2018

morozov approved these changes Jun 8, 2018

View reviewed changes

morozov merged commit b9fec6b into doctrine:master Jun 8, 2018

mlocati deleted the mysql8-keywords branch June 8, 2018 17:18

mlocati added a commit to mlocati/concrete5 that referenced this pull request Jun 9, 2018

Backport doctrine/dbal#3128

e365808

mlocati mentioned this pull request Jun 9, 2018

Add support of MySQL 8 keywords concretecms/concretecms#6753

Merged

KorvinSzanto pushed a commit to KorvinSzanto/concrete5-core that referenced this pull request Jun 11, 2018

Backport doctrine/dbal#3128

87c2869

KorvinSzanto pushed a commit to concretecms/concretecms-core that referenced this pull request Jun 11, 2018

Backport doctrine/dbal#3128

6a1ba02

KorvinSzanto pushed a commit to KorvinSzanto/concrete5-core that referenced this pull request Jun 11, 2018

Backport doctrine/dbal#3128

d9ef8f7

morozov self-assigned this Jun 12, 2018

KorvinSzanto pushed a commit to KorvinSzanto/concrete5-core that referenced this pull request Jul 13, 2018

Backport doctrine/dbal#3128

a2aa359

m-vo mentioned this pull request Mar 7, 2021

UserCreateCommand SQL error contao/contao#2849

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add MySQL 8 reserved keywords #3128

Add MySQL 8 reserved keywords #3128

mlocati commented May 3, 2018

Majkl578 May 3, 2018

mlocati May 3, 2018

mlocati May 3, 2018

morozov commented May 14, 2018

mlocati commented May 15, 2018

morozov commented May 15, 2018

mlocati commented May 15, 2018

morozov commented Jun 8, 2018

mlocati commented Jun 8, 2018

Ocramius commented Jun 8, 2018

iLevye commented Jun 8, 2018

mlocati commented Jun 8, 2018

morozov commented Jun 8, 2018

Ocramius commented Jun 8, 2018

morozov commented Jun 8, 2018

KorvinSzanto commented Jun 8, 2018

Ocramius commented Jun 8, 2018

KorvinSzanto commented Jun 8, 2018

mlocati commented Jun 11, 2018

morozov commented Jun 11, 2018

	$parentKeywords = array_diff(parent::getKeywords(), [
	'OVER',
	]);

	return array_merge($parentKeywords, [
	'LATERAL',
	]);

Add MySQL 8 reserved keywords #3128

Add MySQL 8 reserved keywords #3128

Conversation

mlocati commented May 3, 2018

Majkl578 May 3, 2018

Choose a reason for hiding this comment

mlocati May 3, 2018

Choose a reason for hiding this comment

mlocati May 3, 2018

Choose a reason for hiding this comment

morozov commented May 14, 2018

mlocati commented May 15, 2018

morozov commented May 15, 2018

mlocati commented May 15, 2018

morozov commented Jun 8, 2018

mlocati commented Jun 8, 2018

Ocramius commented Jun 8, 2018

iLevye commented Jun 8, 2018

mlocati commented Jun 8, 2018

morozov commented Jun 8, 2018

Ocramius commented Jun 8, 2018

morozov commented Jun 8, 2018

KorvinSzanto commented Jun 8, 2018

Ocramius commented Jun 8, 2018

KorvinSzanto commented Jun 8, 2018

mlocati commented Jun 11, 2018

morozov commented Jun 11, 2018