MDEV-38740 Implement JSON as a pluggable data type - #5263
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a pluggable JSON data type plugin (type_json) for MariaDB, migrating the built-in JSON type handling to a modular plugin structure. The changes include adding the plugin implementation (plugin.cc, sql_type_json.cc, sql_type_json.h), updating parser rules in sql_yacc.yy, and adding comprehensive test suites. Feedback on the implementation highlights several critical issues: a potential null pointer dereference in Field_json::report_wrong_value when table is null, an incorrect charset mapping in make_conversion_table_field which should strictly use utf8mb4_bin, and a regression in aggregate_for_comparison that prevents comparisons between JSON and numeric or temporal types. Additionally, minor cleanups are recommended, such as removing commented-out method declarations and fixing a comment typo in plugin.cc.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| Field *Type_handler_json::make_conversion_table_field( | ||
| MEM_ROOT *root, TABLE *table, uint metadata, const Field *target) const | ||
| { | ||
| /* A JSON field uses 4 bytes for length as json file is a long blob*/ | ||
| uint pack_length= metadata & 0x00ff; | ||
| if (pack_length != 4) | ||
| return NULL; | ||
|
|
||
| return new (root) Field_json(NULL, (uchar *) "", 1, Field::NONE, | ||
| &empty_clex_str, table->s, target->charset()); | ||
| } |
There was a problem hiding this comment.
In Type_handler_json::make_conversion_table_field, the charset of the conversion field is set to target->charset(). However, a Field_json must always use the utf8mb4_bin character set to ensure correct JSON representation and comparison behavior. If the target field has a different character set (e.g., latin1), creating a Field_json with that character set violates this invariant and can lead to encoding issues or assertion failures.
Field *Type_handler_json::make_conversion_table_field(
MEM_ROOT *root, TABLE *table, uint metadata, const Field *target) const
{
/* A JSON field uses 4 bytes for length as json file is a long blob*/
uint pack_length= metadata & 0x00ff;
if (pack_length != 4)
return NULL;
return new (root) Field_json(NULL, (uchar *) "", 1, Field::NONE,
&empty_clex_str, table->s, &my_charset_utf8mb4_bin);
}| // const Type_handler *type_handler_for_comparison() const override; | ||
| // const Type_handler *type_handler_for_tmp_table(const Item *item) const | ||
| // override; | ||
| // bool Item_hybrid_func_fix_attributes(THD *thd, const LEX_CSTRING &func_name, | ||
| // Type_handler_hybrid_field_type *handler, Type_all_attributes *func, | ||
| // Item **items, uint nitems) const override; | ||
|
|
||
| // virtual Item *create_typecast_item(THD *thd, Item *item, | ||
| // const Type_cast_attributes &attr) const override; | ||
|
|
||
| // Item *make_constructor_item(THD *thd, List<Item> *args) const override; |
There was a problem hiding this comment.
There are several commented-out method declarations in the Type_handler_json class. To maintain code cleanliness and readability, these unused, commented-out lines should be removed.
References
- Keep the API of custom wrapper classes minimal by avoiding or removing unused methods.
| const Type_handler * | ||
| Type_collection_json::aggregate_for_comparison(const Type_handler *a, | ||
| const Type_handler *b) const | ||
| { | ||
| if (a->type_collection() == this) | ||
| swap_variables(const Type_handler *, a, b); | ||
| if (a == &type_handler_json || a == &type_handler_hex_hybrid || | ||
| a == &type_handler_tiny_blob || a == &type_handler_blob || | ||
| a == &type_handler_medium_blob || a == &type_handler_long_blob || | ||
| a == &type_handler_varchar || a == &type_handler_string || | ||
| a == &type_handler_null) | ||
| return b; | ||
| return NULL; | ||
| } |
There was a problem hiding this comment.
In Type_collection_json::aggregate_for_comparison, the aggregation only allows comparison between JSON and other string/blob/null types. Any comparison between JSON and numeric types (like INT, DOUBLE, DECIMAL) or temporal types (like DATETIME, DATE) will return NULL. This prevents implicit comparison between JSON columns and numbers/dates, which is a regression compared to the previous LONGTEXT-based JSON implementation. Consider allowing aggregation with other basic types or delegating to the standard string type collection.
gkodinov
added
the
External Contribution
grooverdan
left a comment
grooverdan
left a comment
There was a problem hiding this comment.
fantastic work. I tested with #5278 and it shows good matches in making the most of JSON types.
- Section 11: use LONGTEXT instead of JSON column to avoid dependency on check_constraint_checks which will be disabled in PR MariaDB#5263 - Remove remaining SHOW WARNINGS from section 13.7 (MTR outputs warnings automatically) - Remove '(mentor)' annotations from echo labels in sections 15, 19, 20 - NULL candidate semantics: remove args[0]->null_value check, allowing NULL MEMBER OF ('[null]') = 1 (SQL NULL treated as JSON null via json_quote_item returning 'null' per MDEV-13645). Intentional divergence from MySQL documented behavior per grooverdan's request. - Simplify stored function tests: single-line CREATE FUNCTION without delimiters, positive VARCHAR test (f() RETURNS VARCHAR), JSON-returning function test (j_row_result() RETURNS JSON) - Add optimizer trace test (section 22) showing MEMBER OF participates correctly in equality_propagation and condition_processing - Add set_maybe_null() in fix_length_and_dec() for cursor protocol compatibility per grooverdan
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your contribution. This is a preliminary review.
Can you please consider squashing the commits to (ideally) a single one? Unless there's a valid reason stated to keep more than one commit that is.
Also, Please rebase your change to the latest main and make sure the embedded test failure is fixed?
|
|
||
| static struct st_mariadb_data_type plugin_descriptor_json= | ||
| { | ||
| MariaDB_DATA_TYPE_INTERFACE_VERSION, |
There was a problem hiding this comment.
different offset from the one below. please be consistent.
| @@ -148,6 +148,8 @@ bool Type_handler_json_common::make_json_valid_expr_if_needed(THD *thd, | |||
| } | |||
|
|
|||
|
|
|||
| namespace { | |||
There was a problem hiding this comment.
Since there are two different definitions of the Type_collection_json class, one in the plugin and another in sql/sql_type_json.cc, the compiler/linker gets confused because they share the same name. This caused a multiple-definition conflict in the Windows CI build.
To solve this problem, I considered a few options:
- Merge the two classes and make the JSON implementation in /sql depend on the plugin implementation. However, this might break the current flow and affect compatibility.
- Wrap the code inside an anonymous namespace. This gives the class internal linkage, making it private to the file it's defined in, which safely avoids any naming collisions.
I went with the second option, as it is the safest approach without breaking the existing architecture.
There was a problem hiding this comment.
How about option 3: call the plugin class somehow otherwise. It's passed by pointer, so you can call it Fred for example ;)
Or even hide the plugin class. It's passed by pointer. No need to pollute the ld namespace.
There was a problem hiding this comment.
Yes, makes sense. I will go with renaming the plugin class.
|
One a personal note: I'd appreciate documenting better why you need to make the changes outside of the plugin. |
There was a problem hiding this comment.
Pull request overview
This PR implements JSON as a first-class pluggable data type using the MariaDB_DATA_TYPE_PLUGIN framework, shifting JSON from being a hardcoded LONGTEXT-based type to a dedicated type handler/field implementation while updating parser resolution and tests accordingly.
Changes:
- Updates the SQL parser to resolve
JSONvia UDT/type plugin infrastructure (and enables nativeJSONinJSON_TABLEcolumn definitions). - Adds a mandatory
type_jsonplugin implementingType_handler_json,Field_json, type aggregation rules, JSON validation on store, and JSON type casting/constructor support. - Refreshes many MTR expectations to reflect
SHOW CREATE TABLE/metadata displayingjsonand the new error/warning behavior.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sql/sql_yacc.yy | Moves JSON to UDT keyword handling and adds JSON support in JSON_TABLE column type grammar. |
| sql/sql_type_json.h | Extends JSON-type-handler detection to recognize plugin JSON by handler name. |
| sql/sql_type_json.cc | Wraps internal JSON type-collection in an anonymous namespace to avoid duplicate symbols. |
| sql/json_table.cc | Detects JSON type via Type_handler_json_common::is_json_type_handler() rather than a single built-in handler pointer. |
| plugin/type_json/sql_type_json.h | Introduces plugin-side JSON type handler, type collection, JSON field, and JSON CAST item class definitions. |
| plugin/type_json/sql_type_json.cc | Implements plugin-side JSON type behavior (aggregation, CAST/constructor, validation on store, replication conversion rules). |
| plugin/type_json/plugin.cc | Registers the JSON data type plugin (MariaDB_DATA_TYPE_PLUGIN). |
| plugin/type_json/CMakeLists.txt | Adds the new type_json plugin build definition (MANDATORY). |
| plugin/type_inet/mysql-test/type_inet/type_inet6_mix_json.test | Updates INET6+JSON mixing tests to expect errors (JSON no longer behaves like LONGTEXT aliasing). |
| plugin/type_inet/mysql-test/type_inet/type_inet6_mix_json.result | Updates results for revised INET6+JSON error behavior and metadata. |
| mysql-test/suite/plugins/r/feedback_plugin_load.result | Updates feedback plugin load expectations (collation usage set changes). |
| mysql-test/suite/json/t/json_table_mysql.test | Adjusts JSON_TABLE tests to reflect new JSON cast usability (removes parse-error expectation). |
| mysql-test/suite/json/r/json_table_mysql.result | Updates JSON_TABLE results to successful output plus truncation warning behavior. |
| mysql-test/suite/funcs_1/r/is_columns_mysql.result | Updates Information Schema column/type expectations to show json type with NULL charset/collation. |
| mysql-test/suite/compat/oracle/r/column_compression.result | Updates SHOW CREATE TABLE expectations to emit json instead of LONGTEXT+CHECK. |
| mysql-test/main/type_json.test | Adds/updates tests for JSON CAST/constructor behavior, implicit validation on insert/select, and numeric op rejection. |
| mysql-test/main/type_json.result | Updates expected output/errors/metadata for the revised JSON type behavior. |
| mysql-test/main/system_mysql_db.result | Updates system table DDL expectations to show json columns for privilege/options JSON fields. |
| mysql-test/main/system_mysql_db_fix50568.result | Same as above for the fix50568 variant expectations. |
| mysql-test/main/system_mysql_db_fix50117.result | Same as above for the fix50117 variant expectations. |
| mysql-test/main/system_mysql_db_fix50030.result | Same as above for the fix50030 variant expectations. |
| mysql-test/main/system_mysql_db_fix40123.result | Same as above for the fix40123 variant expectations. |
| mysql-test/main/mysql-metadata.result | Updates metadata output to reflect LONG_BLOB (type=json) rather than BLOB (format=json). |
| mysql-test/main/mysql_json_table_recreate.test | Updates expectations so JSON rejects explicit charset/collation attributes with ER_UNSUPPORTED_DATA_TYPE_ATTRIBUTE. |
| mysql-test/main/mysql_json_table_recreate.result | Updates results accordingly for the JSON charset/collation attribute rejection. |
| mysql-test/main/json_normalize.result | Updates SHOW CREATE TABLE output to show json type. |
| mysql-test/main/information_schema.result | Updates Information Schema expectations impacted by system-table type changes. |
| mysql-test/main/func_json.test | Updates JSON invalid-value expectations from constraint failures to truncated-wrong-value-for-field errors. |
| mysql-test/main/func_json.result | Updates results accordingly for new JSON validation error behavior. |
| mysql-test/main/ctype_utf32.result | Updates SHOW CREATE TABLE output to show json type. |
| mysql-test/main/ctype_utf32_uca.result | Updates SHOW CREATE TABLE output to show json type. |
| mysql-test/main/ctype_utf16_uca.result | Updates SHOW CREATE TABLE output to show json type. |
| mysql-test/main/ctype_binary.result | Updates SHOW CREATE TABLE output to show json type. |
| mysql-test/main/column_compression.result | Updates SHOW CREATE TABLE output to emit json in compression-related expectations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const Type_handler * | ||
| Type_collection_json::aggregate_for_num_op(const Type_handler *a, | ||
| const Type_handler *b) const | ||
| { | ||
| return NULL; | ||
| } |
There was a problem hiding this comment.
add set_maybe_null() to the Item_json_typecast::fix_length_and_dec()
sql_type_json.h
Item_append_extended_type_info()
we call set_format_name()
in make_send_field()
we call set_data_type_name()
i guess we have to call the set_data_type_name() in both spots.
Type_handler_json::Column_definition_prepare_stage1
could be removed i think since it just calls the parent's function.
mysql_json_table_recreate.result
@@ -200,17 +200,11 @@ ERROR HY000: 'MYSQL_JSON' is not allowed in this context
create table `testjson` (
`t` json /* JSON from MySQL 5.7*/ CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL
) ENGINE=InnoDB DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci;
-ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL
-) ENGINE=InnoDB DEFAULT CH...' at line 2
+ERROR HY000: Data type 'json' doesn't support CHARACTER SET attribute.
here we should remove the CHARSET statement in the test, not launch this error!
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
funcs_1.is_columns_mysql_embedded needs re-recording, please attend to that.
Implement JSON as a pluggable data type using the MariaDB_DATA_TYPE_PLUGIN1 API. - Add JSON type parsing, type recognition, and JSON_TABLE support. - Add implicit validation, JSON casts, and the JSON constructor. - Add replication support for older string/blob types. - Enforce utf8mb4_bin and reject explicit charset attributes. - Reject unsupported numeric operations on JSON values. - Update client and extended metadata for JSON columns. - Display JSON columns as `json` in SHOW CREATE TABLE and DESCRIBE. - Refactor and expand JSON tests for the new native JSON behavior. - Keep JSON tests in the main suite to ensure they are always run. - Align the plugin structure with the XMLTYPE plugin. - Resolve plugin integration and cross-platform build issues.
This PR implements JSON as a distinct pluggable data type in MariaDB using the MariaDB_DATA_TYPE_PLUGIN framework.
Related issue: MDEV-38740
This work is part of GSoC '26.
What This PR Adds:
Type_handler_json,Field_json, andType_collection_json, following theType_handler_xmltypearchitectural pattern while preserving existing JSON type behavior.Type_collection_jsonwith aggregation rules (aggregate_for_result,aggregate_for_comparison,aggregate_for_min_max,aggregate_for_num_op) to handle type mixing, JSON is compatible with scalar types but rejects numeric operations and incompatible types like INET6.SHOW CREATE TABLEandDESCRIBEnow explicitly display the column type asjsoninstead oflongtext.Field_jsonconstructor to remove the redundant charset parameter, internally enforcing the requiredutf8mb4_bincharset for all JSON fields.Field_jsonnow overrideshas_charset()to returnfalse, and rejects anyCHARACTER SETorCOLLATEattributes withER_UNSUPPORTED_DATA_TYPE_ATTRIBUTE.CHECK(json_valid())constraint and provided implicit validation throughField_json::store(), which validates JSON on insert and raisesER_TRUNCATED_WRONG_VALUE_FOR_FIELDfor invalid values.CAST(expr AS JSON)) viaItem_json_typecast, including strict JSON validation that returnsNULLwithER_TRUNCATED_WRONG_VALUEwarning for invalid input.JSON()constructor function viamake_constructor_itemsoJSON('{"a":1}')works as a shorthand forCAST('{"a":1}' AS JSON).ER_ILLEGAL_PARAMETER_DATA_TYPE_FOR_OPERATION.sql_yacc.yy) to moveJSON_SYMfromfield_type_lobtoreserved_keyword_udt_not_param_type, so JSON is resolved as a UDT through the plugin rather than being hardcoded aslongtext.JSON_SYMhandling injson_table_field_typegrammar rule soJSON_TABLEcolumns can use the native JSON type properly.Json_table_column::set()to useType_handler_json_common::is_json_type_handler()instead of comparing directly withtype_handler_long_blob_json, so it recognizes both the built-in and plugin JSON type handlers.Type_handler_json_common::is_json_type_handler()to also match by handler name ("json"), so the plugin-based JSON type is correctly identified.Type_collection_jsoninsql_type_json.ccin an anonymous namespace to avoid duplicate symbol errors on Windows.Field_json::rpl_conv_type_fromto allow replicating from older string/blob types (blob, text, VARCHAR, etc.) into JSON columns, fixing row-based replication test failures.Added Tests
CREATE TABLE:
INSERT:
CAST:
Numeric / Aggregate / Arithmetic Operations:
Updated Tests
Type Mixing (INET6 + JSON):
SHOW CREATE TABLE / Metadata: