← Back to team overview

maria-developers team mailing list archive

Review for: MDEV-17399 Add support for JSON_TABLE, part #3

 

Hi Alexey,

Please find the next batch of input below. (I haven't finished the review but
I'm sending it now so you can start addressing it)

> commit 68ca18518337443090bdeddf6d612129b6843c21
> Author: Alexey Botchkov <holyfoot@xxxxxxxxxxxx>
> Date:   Fri Apr 17 14:42:40 2020 +0400
>
>     MDEV-17399 Add support for JSON_TABLE.
>
>     Syntax for JSON_TABLE added.
>     The ha_json_table handler added. Executing the JSON_TABLE we
>     create the temporary table of the ha_json_table, add dependencies
>     of other tables and sometimes conditions to WHERE.
>



...
> +public:
> +  ha_json_table(TABLE_SHARE *share_arg, Table_function_json_table *jt):
> +    handler(&table_function_hton.m_hton, share_arg), m_jt(jt)
> +  {
> +    mark_trx_read_write_done= 1;

This definitely needs a comment.

> +  ~ha_json_table() {}
> +  handler *clone(const char *name, MEM_ROOT *mem_root) { return NULL; }
> +  const char *index_type(uint inx) { return "HASH"; }

Why is ha_json_table still trying to pretend it supports indexes?  Is this a
left-over from the old way of interfacing with the optimizer that can now be
removed?

> +int Table_function_json_table::setup(THD *thd, TABLE_LIST *sql_table,
> +                                     COND **cond)
> +{
> +  if (m_json->fix_fields(thd, &m_json))
> +    return TRUE;
> +
> +  m_dep_tables= m_json->used_tables();

Why does this function have a 'cond' argument? Please remove it if it's not
necessary.

== Duplicate column names are allowed ==

select * from 
  JSON_TABLE('{"color": "black", "price": 1000}',
              "$" COLUMNS (rowSeq FOR ORDINALITY, 
                           color VARCHAR(100) PATH '$.color',
                           color VARCHAR(100) PATH '$.price')
            ) as T;

+--------+-------+-------+
| rowSeq | color | color |
+--------+-------+-------+
|      0 | black | 1000  |
+--------+-------+-------+

Is it really okay that duplicate column names are allowed?

== RIGHT JOIN is allowed ==

create table t1 (item_name varchar(32), item_props varchar(1024));
insert into t1 values ('Laptop', '{"color": "black", "price": 1000}');
insert into t1 values ('Jeans',  '{"color": "blue", "price": 50}');

This query should not be allowed, because JSON_TABLE has a dependency on an
inner side of an outer join:

select * 
from 
  t1 right join
  json_table(t1.item_props,
             '$' columns(color varchar(100) path '$.color', 
                          f2 for ordinality)
            ) as T on 1;


=== NATURAL JOIN crashes ==

create table t10 (color varchar(100), f2 int);
select * 
from 
  t10 natural join
  json_table(t1.item_props,
             '$' columns(color varchar(100) path '$.color', 
                          f2 for ordinality)
            ) as JT ;

  Thread 31 "mysqld" received signal SIGSEGV, Segmentation fault.
  0x0000555555d8ac12 in Select_limit_counters::get_select_limit (this=0xa5a5a5a5a5a5ad2d) at /home/psergey/dev-git/10.5-json/sql/sql_limit.h:67
(gdb) wher
  #0  0x0000555555d8ac12 in Select_limit_counters::get_select_limit (this=0xa5a5a5a5a5a5ad2d) at /home/psergey/dev-git/10.5-json/sql/sql_limit.h:67
  #1  0x0000555555e2a4a0 in JOIN::optimize_inner (this=0x7fffac018520) at /home/psergey/dev-git/10.5-json/sql/sql_select.cc:1805
  #2  0x0000555555e29b12 in JOIN::optimize (this=0x7fffac018520) at /home/psergey/dev-git/10.5-json/sql/sql_select.cc:1612

BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog




Follow ups