[Maria-developers] Review for: MDEV-17399 Add support for JSON_TABLE, part #3

19 Apr 2020

      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@askmonty.org>
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

[Maria-developers] Review for: MDEV-17399 Add support for JSON_TABLE, part #3

Sergey Petrunia