MDEV-13594 (was MDEV-18530): Implement -> and ->> JSON path operators#4711
MDEV-13594 (was MDEV-18530): Implement -> and ->> JSON path operators#4711kjarir wants to merge 5 commits intoMariaDB:mainfrom
Conversation
Summary: Adds support for MySQL-compatible JSON path operators '->' and '->>' as aliases for JSON_EXTRACT and JSON_UNQUOTE(JSON_EXTRACT). Changes: - Lexer: Modified MINUS_OR_COMMENT to detect '->' and '->>'. - Parser: Restructured primary_expr to support operator chaining. - Symbols: Added JSON_ARROW_SYM and JSON_UNQUOTED_ARROW_SYM. - Tests: Added comprehensive verification for literal/column extraction, chaining, and usage in VIRTUAL columns with INDEX. Labels: gsoc26
|
Fixed a syntax error in the newly added test file |
|
I've reviewed the CI logs and found that the Buildbots and AppVeyor were failing due to a mismatch in the |
grooverdan
left a comment
There was a problem hiding this comment.
Well done on implementation with a good start to tests.
Do you know where these operators show up in terms of https://mariadb.com/docs/server/reference/sql-structure/operators/operator-precedence ? Can there be test cases around validate they stay in that order (if possible)?
Do the results of these show up the same results as the MySQL implementation? (mysql-test/suite/json/inc/json_functions.inc). If you include these into MariaDB, adapting to its form (so maybe not an include if not needed), in their own commit. Include authorship and credit to the original Oracle authors and include what changes you made also in commit message.
mysql-test/main/json_operators.test
Outdated
| @@ -0,0 +1,64 @@ | |||
| --source include/have_innodb.inc | |||
There was a problem hiding this comment.
This doesn't use or implement any specific innodb features so this can be removed.
The MDEV header below should be echo statements.
MDEV-18530 is a MDEV closed as a duplicate therefore using the MDEV-13594 MDEV is recommented.
After adding a test case like this, mtr --record and then commit the json_operators.result file in the same commit.
|
|
||
| --echo # 5. Edge Cases: Invalid JSON and Paths | ||
| --echo # Native functions return NULL or error; operators should match | ||
| SELECT '{"a": 1'->'$.a'; |
There was a problem hiding this comment.
as these may be ERROR conditions there will be a --error before the failing SQL statements with the error they genenrate.
mysql-test/main/json_operators.test
Outdated
| SELECT '{"a": 1}'->'invalid_path'; | ||
|
|
||
| --echo # 6. Integration with Generated Columns | ||
| --echo # This is a common use case for these operators in MySQL |
There was a problem hiding this comment.
Don't really need to say "in MySQL"
mysql-test/main/json_operators.test
Outdated
| --echo # If the string from ->> is valid JSON, another -> can follow. | ||
| SELECT '{"outer": "{\\"inner\\": 1}"}'->>'$.outer'->'$.inner' AS complex_chain; | ||
|
|
||
| # End of MDEV-18530 tests |
There was a problem hiding this comment.
The final statement in a test case is"
--echo End of 13.0 tests
For the purpose of ease of merging. The end of a particular MDEV's tests becomes obvious with the beginning of the next MDEV.
| if (unlikely(list == NULL) || | ||
| unlikely(list->push_back($1, thd->mem_root)) || | ||
| unlikely(list->push_back($3, thd->mem_root))) | ||
| MYSQL_YYABORT; |
There was a problem hiding this comment.
There is a list memory allocation to both a local variable, that if one of the two push_backs fail, becomes a leak. The other allocations have this assigned to a lex structure of $$ at the time MYSQL_YYABORT is called.
7800009 to
ad067f1
Compare
Based on direct feedback from grooverdan:
- sql/sql_yacc.yy:
* Restructured JSON arrow rule to assign 16246 (the resulting item)
before performing potentially failing push_back operations on the
argument list. This prevents memory-root allocations from being
orphaned if push_back or subsequent allocation fails (YYABORT).
* Added JSON_ARROW_SYM and JSON_UNQUOTED_ARROW_SYM to the grammar
precedence table (%left) to confirm binding tighter than other
SQL operators.
* Updated MDEV reference from duplicate MDEV-18530 to MDEV-13594.
- mysql-test/main/json_operators.test:
* Removed unnecessary InnoDB dependency (--source include/have_innodb.inc)
as no engine-specific features are used.
* Used standardized --echo tags in test header and added precedence
tests (Section 4) confirming arrows bind tighter than + and *.
* Annotated invalid JSON and path scenarios with --echo error ER_*
markers, matching standard MariaDB JSON warning reporting patterns.
* Standardized end-of-test marker to: --echo End of 13.0 tests.
- mysql-test/main/json_operators.result:
* Generated the official result file using mtr --record after building
the server locally. This captures the exact MariaDB warning messages
(e.g., ER_JSON_UNEXPECTED_END) and formatting requirements.
This commit addresses all feedback from @grooverdan: - Fixed a potential memory leak in sql_yacc.yy by ensuring List allocation or Item construction is assigned to 47449 (or a local captured by the parser) before potentially failing push_back calls. - Ported comprehensive JSON arrow operator tests from MySQL 8.0 to ensure compatibility and coverage of prepared statements. - Added explicit attribution to Oracle Corporation for the ported tests (mysql-test/suite/json/inc/json_functions.inc and json_prep_stmts.test). - Removed unnecessary InnoDB dependency in tests. - Switched # comments to --echo and standardized the end-of-test marker. - Included official .result file generated via 'mtr --record'.
Summary:
Adds support for MySQL-compatible JSON path operators '->' and '->>' as aliases for JSON_EXTRACT and JSON_UNQUOTE(JSON_EXTRACT).
Key Changes:
Labels: gsoc26