Merge 3.7.x into 4.0.x #5959
Merged
Merge 3.7.x into 4.0.x #5959
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MariaDb aliases columns specified as JSON to LONGTEXT. Since version 10.4.3,
MariaDb adds a CHECK constraint to JSON columns to ensure they contain valid
JSON and sets the column collation to utf8mb4_bin.
Simply setting JSON as the column type in the platform results in introspection
failures as the database reports LONGTEXT (not JSON) with changed collation.
Therefore, inverse the aliasing where the relevant check constraint exists and
ignore collation changes for JSON columns in the relevant database versions.
The main functional changes are in:
- MariaDB1043Platform->getListTableColumnsSQL() and
- MySQLSchemaManager->selectTableColumns()
MariaDb1043Platform extends MariaDb1027Platform rather than MariaDBPlatform
to ensure the platform checks in MySQLSchemaManager (based on
MariaDb1027Platform) continue to work as expected.
The alternative approach of adding the CHECK constraint to the SQL for the
column is not robust as MariaDb syntax seems to require the CHECK constraint to
be the final part of the column declaration and it was not obvious how to
guarantee this (specifically, appending the CHECK constraint to the output of
getJsonTypeDeclarationSQL() did not work).
New public methods:
MariaDBPlatform->getColumnTypeSQLSnippets() generates SQL snippets to
reverse the JSON to LONGTEXT aliasing for JSON-specified columns. It is also
used in MySQLSchemaManager. It still checks that json columns are JSON data
type so that switching the getJsonTypeDeclarationSQL to LONGTEXT is all that
is necessary to revert to old behaviour which is helpful for testing.
Overridden methods in MariaDb1043Platform:
MariaDb1043Platform->getJsonTypeDeclarationSQL(). To return JSON.
MariaDBPlatform->getListTableColumnsSQL(). To reverse the JSON to LONGTEXT
aliasing carried out by MariaDb.
MariaDBPlatform->getColumnDeclarationSQL(). To unset collation and charset
(used by the comparator).
New test cases:
1. Types/JsonTest. Test storage and retrieval of json data.
2. Platforms/MariaDb1043PlatformTest. A clone of MariaDb1027Platform test.
3. Schema/MySQLSchemaManagerTest->testColumnIntrospection(). Ensures
introspected table matches original table. Tests all doctrine types not
just json. Based on failure highlighted in pull request doctrine#5100.
For previous discussion on this topic, see:
doctrine#5100
doctrine#3202
Further background at:
https://mariadb.com/kb/en/json-data-type/
https://mariadb.com/kb/en/information-schema-check_constraints-table/
https://jira.mariadb.org/browse/MDEV-13916
Notes regarding JsonTest.php:
1. Some platforms return json as a stream so convert streams to strings.
2. 'json_table' is a reserved word in MySQL (and a function name in
Oracle SQL). Use json_test_table instead.
3. Some platforms reorder json arrays so the insert order will not
necessarily match the select order. Resort the arrays before testing
for equality.
Select MariaDb1043Platform where MariaDb database is at version 10.4.3 or later.
MariaDb sets character set to utf8mb4 and collation to utf8mb4_bin for columns declared as JSON but still reports them as LONGTEXT with CHARACTER SET and COLLATE so comparator needs to ignore character set and collation for columns declared as JSON. Attempting to set CHARACTER SET or COLLATE for values other than utf8mb4 and utf8mb4_bin causes a database error on MariaDb. Test that character set and collation specifications are not passed to the database and are ignored by the comparator.
MariaDb JSON columns are marked as json with a DC2Type comment hint. The schema manager maps such columns to a json doctrine type which matches the metadata whether the column was declared as LONGTEXT (by the previous MariaDb1027 platform) or JSON. This means the comparator will not detect that a column should be upgraded from LONGTEXT to native JSON and no migration would be generated. Therefore, during column introspection check that the data type of the introspected columns match that which should be set for any doctrine types inferred from DC2Type comments. Set a flag (a platformOption for the relevant column) where it does not. Check whether the flag is set when the comparator diffs the expected and actual tables and mark a column type difference where the flag is set so a migration is generated.
Use Native JSON type instead of LONGTEXT for MariaDB
cgknx
suggested changes
Mar 8, 2023
* 3.7.x: Trigger a runtime deprecation for Connection::executeUpdate() MariaDb1043. Detect the need to migrate JSON columns to native JSON. MariaDb. Test that comparator ignores collation for JSON columns. AbstractMySQLDriver. Use MariaDb1043Platform where applicable. Add MariaDb1043Platform using JSON as json column type.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've reverted parts of #5916, mainly regarding the issue discussed in #5916 (comment), because that code called or overrode methods that had been removed on 4.0.x already.
Can you have a look @cgknx?