[MDEV-27036] Json_writer allows one to create duplicate object members Created: 2021-11-12  Updated: 2021-12-20  Resolved: 2021-11-26

Status: Closed
Project: MariaDB Server
Component/s: Optimizer
Affects Version/s: 10.4
Fix Version/s: 10.8.0

Type: Bug Priority: Major
Reporter: Sergei Petrunia Assignee: Sergei Krivonos (Inactive)
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Problem/Incident
causes MDEV-27204 [ERROR] Json_writer: a member name wa... Closed
causes MDEV-27206 [ERROR] Duplicated key: cause, Assert... Closed
Relates
relates to MDEV-23766 Optimizer tracing code is prone to pr... Closed
Epic Link: Tracer JSON consistency

 Description   

Json_writer code allows one to write multiple members with the same name in a JSON object.

This is likely to prevent proper processing of the created document.

The most striking example is EXPLAIN FORMAT=JSON:

{
 
  "query_block": {
 
    "select_id": 1,
 
    "table": {
 
      "table_name": "table",
 
      ...
 
    },
 
     "table": {
 
      ...

The solution has two parts:

Part #1: Follow the MDEV-23766 approach and make debug build generate assertion failures.

Part #2: Fix EXPLAIN FORMAT=JSON code to not produce such invalid (or valid but not processable) JSON output.

As for which version to fix this in: The problem with EXPLAIN FORMAT=JSON output is present from the very early versions (10.1). However, the fix will cause a lot of changes in EXPLAIN FORMAT=JSON output (the changes will be dummy but they will be there). This may not be desirable for older versions. Because of that, for now I'm setting fixVersion to 10.8.

A question to be resolved: how to modify EXPLAIN/ANALYZE to make them well-formed JSON? Should we wrap them into an array, like MySQL does or do something else?)

 Comments   
Comment by Sergei Krivonos (Inactive) [ 2021-11-12 ]

https://buildbot.mariadb.org/#/grid?branch=bb-10.4-MDEV-27036

Comment by Sergei Petrunia [ 2021-11-14 ]

MySQL 8's EXPLAIN FORMAT=JSON looks like this:

"query_block" : { 
    ...
 
    "nested_loop": [
 
      {
 
        "table": { ... }
 
      },
 
      {
 
        "table": { ... }
 
      }
 
    ]

Comment by Sergei Petrunia [ 2021-11-14 ]

sergei.krivonos, the patch looks like a move in the right direction.

Some things that don't look right:

  • the checking is done in release builds, as well, right?
  • there is no unittest?
  • Diagnostics is printed to std::cerr ... I think, sql_print_error is a safer option.
  • What is the reason for having

    named_items.push({});

    call in the Json_writer::start_array() ?

  • How the above call is different from named_items.emplace(); call a few lines above?
  • When we are comparing JSON names, is it correct to use basically a binary comparison? This has two parts:
    • Part#1: Are JSON names case-sensitive?
    • Part#2: if they are, then for two UTF8MB4 strings, does 

      std::string(X)!=std::string(Y)

      mean that X!=Y for any X and Y?

Comment by Sergei Petrunia [ 2021-11-16 ]

More review input:

https://lists.launchpad.net/maria-developers/msg12988.html
https://lists.launchpad.net/maria-developers/msg12987.html

Comment by Sergei Krivonos (Inactive) [ 2021-11-18 ]

https://buildbot.mariadb.org/#/grid?branch=bb-10.8-MDEV-27036

Comment by Sergei Krivonos (Inactive) [ 2021-11-19 ]

10.4:
https://buildbot.mariadb.org/#/grid?branch=bb-10.4-MDEV-27036

Comment by Sergei Krivonos (Inactive) [ 2021-11-22 ]

tests to fix in bb-10.8-MDEV-27036:
./mysql-test-run main.innodb_ext_key main.rowid_filter_innodb main.cte_recursive main.order_by json.json_table main.explain_json_format_partitions main.derived_cond_pushdown main.opt_trace main.win main.intersect_all main.except main.except_all main.explain_json main.subselect_cache main.rowid_filter main.analyze_format_json main.having_cond_pushdown main.set_operation main.subselect_no_semijoin main.intersect json.json_table_mysql main.analyze_stmt_orderby main.explain_json_innodb main.join_cache main.range main.range_mrr_icp

Comment by Sergei Krivonos (Inactive) [ 2021-11-23 ]

https://buildbot.mariadb.org/#/grid?branch=bb-10.8-MDEV-27036

Comment by Marko Mäkelä [ 2021-11-24 ]

Is there a 10.5 version of this? The 10.4 version that was already pushed causes a merge conflict in 10.5 for some constructor parameters.

Comment by Sergei Krivonos (Inactive) [ 2021-11-24 ]

marko,
merged 10.4 into 10.5 https://buildbot.mariadb.org/#/grid?branch=bb-10.5-MDEV-23766
psergei,
do you think we could just rebase this on 10.5 as well?
Thanks

Comment by Sergei Petrunia [ 2021-11-26 ]

Ok to push the latest contents of bb-10.8-MDEV-27036, after one thing is fixed: do not disable the unit test.

Generated at Thu Feb 08 09:49:50 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.