- developers - lists.mariadb.org

Re: [Maria-developers] a11756e4fba: MDEV-23178: Qualified asterisk not supported in INSERT .. RETURNING
by Sergei Golubchik 16 Jul '21

16 Jul '21

Hi, Rucha! On Jul 15, Rucha Deodhar wrote: > revision-id: a11756e4fba (mariadb-10.5.11-27-ga11756e4fba) > parent(s): f0f47cbca18 > author: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > committer: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > timestamp: 2021-07-05 20:14:19 +0530 > message: > > MDEV-23178: Qualified asterisk not supported in INSERT .. RETURNING > > Analysis: When we have INSERT/REPLACE returning with qualified asterisk in the > RETURNING clause, '*' is not resolved properly because of wrong context. > context->table_list is NULL or has incorrect table because context->table_list > has tables from the FROM clause. > Fix: If filling fields instead of '*' for qualified asterisk in RETURNING, > use first_name_resolution_table for correct resolution of item. please, add a couple of tests with a qualified asterisk, where it's qualified _not_ by the table you're inserting into. Like INSERT INTO t1(id1,val1) VALUES(14,'m') RETURNING t2.* not very interesting case, an obvious error ^^^ INSERT INTO t2 SELECT t1.* FROM t1 WHERE id1=2 RETURNING t1.*; but what will happen here? ^^^ > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > index 1476d708d75..60d14c6dbea 100644 > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -8032,12 +8032,13 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, > else treat natural joins as leaves and do not iterate over their underlying > tables. > */ > - for (TABLE_LIST *tables= (table_name ? context->table_list : > - context->first_name_resolution_table); > + TABLE_LIST *tables= returning_field || !table_name ? context->first_name_resolution_table : > + context->table_list; > + for (; > tables; > - tables= (table_name ? tables->next_local : > - tables->next_name_resolution_table) > + tables= returning_field || !table_name ? tables->next_name_resolution_table : > + tables->next_local > ) > { > Field *field; > TABLE *table= tables->table; 1. please also update the comment to mention RETURNING. 2. Could you apply the following patch as a separate commit before your fix: ============================ --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -8098,12 +8098,14 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name, else treat natural joins as leaves and do not iterate over their underlying tables. */ - for (TABLE_LIST *tables= (table_name ? context->table_list : - context->first_name_resolution_table); - tables; - tables= (table_name ? tables->next_local : - tables->next_name_resolution_table) - ) + TABLE_LIST *first= context->first_name_resolution_table; + TABLE_LIST *TABLE_LIST::* next= &TABLE_LIST::next_name_resolution_table; + if (table_name) + { + first= context->table_list; + next= &TABLE_LIST::next_local; + } + for (TABLE_LIST *tables= first; tables; tables= tables->*next) { Field *field; TABLE *table= tables->table; ============================ it'll remove the condition from inside the loop, will put it in one place only. And will make your change simpler and clearer. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 70252733d41: MDEV-25908: -e does not work for my_print_defaults
by Sergei Golubchik 15 Jul '21

15 Jul '21

Hi, Rucha! On Jul 15, Rucha Deodhar wrote: > revision-id: 70252733d41 (mariadb-10.5.11-35-g70252733d41) > parent(s): f88d130e710 > author: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > committer: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > timestamp: 2021-07-14 22:55:21 +0530 > message: > > MDEV-25908: -e does not work for my_print_defaults > > Analysis: get_defaults_options() rewrites the value for my_defaults_file, > my_defaults_extra_file and my_defaults_group_suffix to 0. So the config file > can't be read. > Fix: Let handle_options() handle --defaults* option and create a new command > line to pass it to load_defaults(). > > --- > extra/my_print_defaults.c | 37 +++++++++++++++++----- > mysql-test/main/my_print_defaults.result | 35 ++++++++++++++++++++- > mysql-test/main/my_print_defaults.test | 54 +++++++++++++++++++++++++++++++- > mysql-test/main/mysqldump.result | 1 + > 4 files changed, 117 insertions(+), 10 deletions(-) > > diff --git a/extra/my_print_defaults.c b/extra/my_print_defaults.c > index 1dec667949f..409ef273b44 100644 > --- a/extra/my_print_defaults.c > +++ b/extra/my_print_defaults.c > @@ -141,28 +141,49 @@ static int get_options(int *argc,char ***argv) > return 0; > } > > +static char *make_args(const char *s1, const char *s2) > +{ > + char *s= malloc(strlen(s1) + strlen(s2) + 1); > + strmov(strmov(s, s1), s2); > + return s; > +} > > int main(int argc, char **argv) > { > - int count, error, args_used; > - char **load_default_groups= 0, *tmp_arguments[6]; > + int count= 0, error, no_defaults= 0; > + char **load_default_groups= 0; > char **argument, **arguments, **org_argv; > int nargs, i= 0; > MY_INIT(argv[0]); > > org_argv= argv; > - args_used= get_defaults_options(argv); > - > - /* Copy defaults-xxx arguments & program name */ > - count=args_used; > - arguments= tmp_arguments; > - memcpy((char*) arguments, (char*) org_argv, count * sizeof(*org_argv)); > + if (*argv && !strcmp(*argv, "--no-defaults")) > + { > + argv++; > + ++count; > + no_defaults= 1; > + } > + /* Copy program name and --no-defaults if present*/ > + memcpy((char*) arguments, (char*) org_argv, (++count)*sizeof(*org_argv)); isn't this wrong? `arguments` is uninitialized at this point. > arguments[count]= 0; > > /* Check out the args */ > if (get_options(&argc,&argv)) > cleanup_and_exit(1); > > +--echo # Testing -e > +--exec $MYSQL_MY_PRINT_DEFAULTS -e $MYSQLTEST_VARDIR/tmp/tmp1.cnf --mysqld mysql.server > +--echo #Testing --defaults-extra-file > +--exec $MYSQL_MY_PRINT_DEFAULTS --defaults-extra-file=$MYSQLTEST_VARDIR/tmp/tmp1.cnf --mysqld mysql.server > + > +--echo # Testing -c option > +--exec $MYSQL_MY_PRINT_DEFAULTS -c $MYSQLTEST_VARDIR/tmp/tmp2.cnf --mysqld mysql.server > +--echo # Testing --defaults-file > +--exec $MYSQL_MY_PRINT_DEFAULTS --defaults-file=$MYSQLTEST_VARDIR/tmp/tmp2.cnf --mysqld mysql.server > + > +--echo # Testing -g option > +--exec $MYSQL_MY_PRINT_DEFAULTS -c $MYSQLTEST_VARDIR/tmp/tmp3.cnf --mysqld mysql.server -g .1 > +--echo # Testing --defaults-group-suffix > +--exec $MYSQL_MY_PRINT_DEFAULTS -c $MYSQLTEST_VARDIR/tmp/tmp3.cnf --mysqld mysql.server --defaults-group-suffix=.1 you don't seem have a --no-defaults test Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] da70f190279: MDEV-25985 Spider handle ">=" as ">" in some cases
by Sergei Golubchik 13 Jul '21

13 Jul '21

Hi, Nayuta! The code change is good, no comments about it at all. But why did you create a new suite spider/bugfix and all those files in it? I know that some later spider version has it. Still, I'd expect it'd be much simpler and a smaller change to drop the test file into the exising spider suite. On Jul 12, Nayuta Yanagisawa wrote: > revision-id: da70f190279 (mariadb-10.2.39-44-gda70f190279) > parent(s): 99f700a820e > author: Nayuta Yanagisawa <nayuta.yanagisawa(a)hey.com> > committer: Nayuta Yanagisawa <nayuta.yanagisawa(a)hey.com> > timestamp: 2021-07-08 14:12:45 +0000 > message: > > MDEV-25985 Spider handle ">=" as ">" in some cases > > The function spider_db_append_key_where_internal() converts > HA_READ_AFTER_KEY to '>'. The conversion seems to be correct for > single-column indexes because HA_READ_AFTER_KEY means "read the > key after the provided value." > > However, how about multi-column indexes? Assume that there is a > multi-column index on c1 and c2 and we search with the condition > 'c1 >= 100 AND c2 > 200'. The key_range.flag corresponds to the > search condition could be HA_READ_AFTER_KEY. In such a case, > we could not simply convert HA_READ_AFTER_KEY to '>'. > > The correct conversion is to convert HA_READ_AFTER_KEY to '>' > only for the last column in key_part_map and to convert > HA_READ_AFTER_KEY to '>=' for the other column. > > The similar discussion also applies to the conversion from > key_range.flag to a sign of inequality. > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] a6e96ce3c96: MDEV-21916: COM_STMT_BULK_EXECUTE with RETURNING insert wrong values
by Sergei Golubchik 12 Jul '21

12 Jul '21

Hi, Oleksandr! ok to push! On Jul 12, Oleksandr Byelkin wrote: > revision-id: a6e96ce3c96 (mariadb-10.5.4-666-ga6e96ce3c96) > parent(s): f4425d3a3db > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2021-06-09 10:43:24 +0200 > message: > > MDEV-21916: COM_STMT_BULK_EXECUTE with RETURNING insert wrong values > > The problem is that array binding uses net buffer to read parameters for each > execution while each execiting with RETURNING write in the same buffer. > > Solution is to allocate new net buffer to avoid changing buffer we are reading > from. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] b1881e07797: MDEV-20131 Assertion `!pk->has_virtual ()' failed in instant_alter_column_possible
by Sergei Golubchik 12 Jul '21

12 Jul '21

Hi, Sachin! On Jul 12, Sachin Kumar wrote: > revision-id: b1881e07797 (mariadb-10.4.20-9-gb1881e07797) > parent(s): 8a2b4d531dc > author: Sachin Kumar <sachin.setiya(a)mariadb.com> > committer: Sachin Kumar <sachin.setiya(a)mariadb.com> > timestamp: 2021-06-11 14:26:19 +0100 > message: > > MDEV-20131 Assertion `!pk->has_virtual ()' failed in instant_alter_column_possible > > Problem:- Primary key on long unique columns is not allowed but when > `CREATE TABLE t2 (a TEXT, PRIMARY KEY(a(1871))) ENGINE=InnoDB;` > is executed with innodb_page_size=8k or 4k, primary key on DB_ROW_HASH_1 column > is created. Reason of this is in mysql_prepare_create_table we check against > max_key_part_length() to see whether key_part length is more then engine supported > key_part length , and if it is true error is thrown for primary key and long > unique index is created for unique key. But in this case max_key_part_length() > returns 3072 which is more the max_key_length() , which later in code triggers > creating of long unique index. > > Solution:- max_supported_key_part_length() should return according to innodb > page size. > > diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc > index ce2e4b6990f..e5e864bffc5 100644 > --- a/storage/innobase/handler/ha_innodb.cc > +++ b/storage/innobase/handler/ha_innodb.cc > @@ -6537,14 +6537,21 @@ ha_innobase::clone( > DBUG_RETURN(new_handler); > } > > - > -uint > -ha_innobase::max_supported_key_part_length() const > -/*==============================================*/ > -{ > - /* A table format specific index column length check will be performed > - at ha_innobase::add_index() and row_create_index_for_mysql() */ > - return(REC_VERSION_56_MAX_INDEX_COL_LEN); > +uint ha_innobase::max_supported_key_part_length() const > +{ > + /* A table format specific index column length check will be performed > + at ha_innobase::add_index() and row_create_index_for_mysql() */ > + /* FIXME: rewrite this as well as ha_innobase::max_supported_key_length() > + using an API that considers the PRIMARY KEY as well as secondary index > + metadata and the ROW_FORMAT and KEY_BLOCK_SIZE */ > + switch (srv_page_size) { > + case 4096: > + return 1173; > + case 8192: > + return 1536; > + default: > + return REC_VERSION_56_MAX_INDEX_COL_LEN; > + } dunno, why would you duplicate the whole switch logic? can you just - return(REC_VERSION_56_MAX_INDEX_COL_LEN); - return MY_MIN(max_supported_key_length(), REC_VERSION_56_MAX_INDEX_COL_LEN); > } > > /******************************************************************//** > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 85186734823: MDEV-24248: my_print_defaults is not taking all the values when using -e
by Sergei Golubchik 12 Jul '21

12 Jul '21

Hi, Rucha! On Jul 12, Rucha Deodhar wrote: > revision-id: 85186734823 (mariadb-10.5.4-665-g85186734823) > parent(s): bf5c050fd28 > author: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > committer: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > timestamp: 2021-06-14 10:27:13 +0530 > message: > > MDEV-24248: my_print_defaults is not taking all the values when using -e > option which is called from mysql.server (extra_args). > > Fix: change mysql.server script to use --defaults-extra-file instead of -e > > diff --git a/mysql-test/main/my_print_defaults.test b/mysql-test/main/my_print_defaults.test > --- /dev/null > +++ b/mysql-test/main/my_print_defaults.test > @@ -0,0 +1,29 @@ > +--echo # > +--echo # Beginning of 10.5 Test > +--echo # > +--echo # > +--echo # MDEV-24248: my_print_defaults is not taking all the values when using > +--echo # -e option which is called from mysql.server (extra_args). > +--echo # > + > +--echo # checking for mysql.server > + > +--write_file $MYSQLTEST_VARDIR/tmp/tmp1.cnf > + > +[mariadb-10.5] don't use the version number in the test - you're forcing some poor soul to update the test on merges for every new version. > +key_buffer_size=20M > +max_allowed_packet=250M > +table_open_cache=1000 > +table_definition_cache=2000 > +read_buffer_size=1M > +thread_cache_size=8 > +max_connections=1024 > +long_query_time=60 > +slow_query_log=1 > +EOF > + > +--exec $MYSQL_MY_PRINT_DEFAULTS --defaults-extra-file=$MYSQLTEST_VARDIR/tmp/tmp1.cnf --mysqld mysql.server > + > +--remove_file $MYSQLTEST_VARDIR/tmp/tmp1.cnf > + > +--echo # End of 10.5 test > diff --git a/support-files/mysql.server.sh b/support-files/mysql.server.sh > index cefb3225e9e..8504e4def4b 100644 > --- a/support-files/mysql.server.sh > +++ b/support-files/mysql.server.sh > @@ -200,11 +200,11 @@ su_kill() { > extra_args="" > if test -r "$basedir/my.cnf" > then > - extra_args="-e $basedir/my.cnf" > + extra_args="--defaults-file-name=$basedir/my.cnf" eh. -e is --defaults-extra-file same below. > else > if test -r "$datadir/my.cnf" > then > - extra_args="-e $datadir/my.cnf" > + extra_args="--defaults-file-name=$datadir/my.cnf" > fi > fi > > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] b29ec61724d: MDEV-25906: SIGSEGV in flush_tables_with_read_lock on FTWRL or FTFE | SIGSEGV in ha_maria::extra
by Sergei Golubchik 01 Jul '21

01 Jul '21

Hi, Oleksandr! On Jul 01, Oleksandr Byelkin wrote: > revision-id: b29ec61724d (mariadb-10.6.2-41-gb29ec61724d) > parent(s): 83684fc9a4d > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2021-07-01 11:20:29 +0200 > message: > > MDEV-25906: SIGSEGV in flush_tables_with_read_lock on FTWRL or FTFE | SIGSEGV in ha_maria::extra > > Fixed check of derived tables of all kinds (view, derived, information schema). > > diff --git a/sql/sql_reload.cc b/sql/sql_reload.cc > index 0fa2fa10df8..e41da9680b0 100644 > --- a/sql/sql_reload.cc > +++ b/sql/sql_reload.cc > @@ -592,10 +592,19 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables) > for (TABLE_LIST *table_list= all_tables; table_list; > table_list= table_list->next_global) > { > - if (!(table_list->is_view() || > - table_list->table->file->ha_table_flags() & HA_CAN_EXPORT)) > + /* > + We allow all deriveds and view here, but prohibit information_schema > + tables (explicitly) and performance_schema tables (implicetly: > + engine do not suport export) > + */ > + if (table_list->is_non_derived() && > + (!table_list->is_base_table() || > + !(table_list->table->file->ha_table_flags() & HA_CAN_EXPORT))) > { > - my_error(ER_ILLEGAL_HA, MYF(0),table_list->table->file->table_type(), > + my_error(ER_ILLEGAL_HA, MYF(0), > + (table_list->is_base_table() ? > + table_list->table->file->table_type(): > + "information_schema"), I'd say if you want to detect I_S tables, you should test for table_list->schema_table. It's not clear what your condition actually does. And there is no such engine "information_schema", so the message is misleading. In fact, this should be access denied. For both. Like: mysqltest: At line 12: query 'flush table information_schema.collations for export' failed: ER_DBACCESS_DENIED_ERROR (1044): Access denied for user 'foo'@'localhost' to database 'information_schema' this is from a test file that only does flush table information_schema.collations for export. > table_list->db.str, table_list->table_name.str); > goto error_reset_bits; > } > @@ -607,7 +616,8 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables) > for (auto table_list= all_tables; table_list; > table_list= table_list->next_global) > { > - if (!table_list->is_view() && > + if (table_list->table && why do you check for table_list->table ? Can it happen that table_list->table == NULL and table_list->is_base_table() == true? > + table_list->is_base_table() && > table_list->table->file->extra(HA_EXTRA_FLUSH)) > goto error_reset_bits; > } Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] MDEV-25325 built-in documentation for performance_schema tables
by Haidong Ji 24 Jun '21

24 Jun '21

Hi, I've grabbed this JIRA story, https://jira.mariadb.org/browse/MDEV-25325 . The coding and test cases creation were all done. I've also submitted a PR here: https://github.com/MariaDB/server/pull/1830 The PR was submitted more than 1 month ago. I'm curious about next steps. Thanks, Haidong

1 0

[Maria-developers] JSON_TABLE: on default values
by Sergey Petrunia 16 Jun '21

16 Jun '21

Hi Alexey, At the moment MariaDB requires that the values in DEFAULT clauses are quoted. Example: select * from json_table( '{"intval": 1000}', '$' columns( col1 int path '$.intval_' default '100' on empty ) ) as T; here, "100" must be quoted, otherwise one gets a parse error. However, the quoted value is interpreted as an SQL literal. This looks puzzling. MySQL-8 also requires that the default value is quoted, but they have a (very odd) reason for it: they interpret the default value as JSON: https://docs.oracle.com/cd/E17952_01/mysql-8.0-en/json-table-functions.html says: DEFAULT json_string ON EMPTY: the provided json_string is parsed as JSON, as long as it is valid, and stored instead of the missing value. Column type rules also apply to the default value. I am not sure why MySQL chose to do this. Looking into the SQL Standard, one can see: <JSON table regular column definition> ::= <column name> <data type> [ PATH <JSON table column path specification> ] [ <JSON table column empty behavior> ON EMPTY ] [ <JSON table column error behavior> ON ERROR ] <JSON table column empty behavior> ::= ERROR | NULL | DEFAULT <value expression> ... This doesn't say whether the <value expression> should be interepreted as JSON or just as a value. But one can find this passage: <quote> Without Feature T826, “General value expression in ON ERROR or ON EMPTY clauses”, the <value expression> contained in <JSON table column empty behavior> or <JSON table column error behavior> contained in a <JSON table regular column definition> JTRCD shall be a <literal> that can be cast to the data type specified by the <data type> contained in JTRCD without raising an exception condition according to the General Rules of Subclause 6.13, “<cast specification>”. </quote> The important part is: ... shall be a <literal> that can be cast to the data type specified ... which means it is not JSON. It is just a literal, and literal can be a string literal (in quotes, 'string') or an integer literal (without quotes) or other kind of literal. Btw, Oracle Database allows non-string literals in the default clause: https://dbfiddle.uk/?rdbms=oracle_18&fiddle=9af7e43ede77ee285e1a65f1f419d3bd What are your thoughts on this? Is MariaDB's behavior intentional? Should we follow the standard and allow all kinds of literals? What was the reason for the limitation that default values are quoted? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

2 4

[Maria-developers] MDEV-25875: RE 02469b: MDEV-17399 JSON_TABLE Accept JSON values for the JSON fields
by Sergey Petrunia 16 Jun '21

16 Jun '21

Hi Alexey, This is review input re https://github.com/MariaDB/server/commit/02469bdead5753eccb5d70c98a158a0702…. First, our workflow dictates that this patch should have its own MDEV entry. I've filed it https://jira.mariadb.org/browse/MDEV-25875. Please use it in commit descriptions. == Updates mixed with another patch == The patch has .result changes like: > @@ -551,10 +551,12 @@ JSON_TABLE('{}', '$' COLUMNS (x INT PATH '$.x' DEFAULT NULL ON ERROR)) jt; > 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 'NULL ON ERROR)) jt' at line 2 > SELECT * FROM > JSON_TABLE('{}', '$' COLUMNS (x INT PATH '$.x' DEFAULT 0 ON EMPTY)) jt; > -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 '0 ON EMPTY)) jt' at line 2 > +x > +0 > SELECT * FROM > JSON_TABLE('{}', '$' COLUMNS (x INT PATH '$.x' DEFAULT 0 ON ERROR)) jt; > -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 '0 ON ERROR)) jt' at line 2 > +x > +NULL > SELECT * FROM > JSON_TABLE('{}', '$' COLUMNS (x DATE > PATH '$.x' These are obviously from another patch (the one about "accepting non-string literals as default"). Please move them to that patch. == Comments == > @@ -377,6 +378,25 @@ static void store_json_in_field(Field *f, const json_engine_t *je) > } > > > +static int store_json_in_json(Field *f, json_engine_t *je) > +{ Please add a comment for the function. Something like "Store the current JSON element pointed by je into field f, as a separate JSON document" The name "store_json_in_json" doesn't look good to me. Do you think store_element_as_json_doc() would be better? > + const uchar *from= je->value_begin; > + const uchar *to; > + > + if (json_value_scalar(je)) > + to= je->value_end; > + else > + { > + int error; > + if ((error= json_skip_level(je))) > + return error; > + to= je->s.c_str; > + } > + f->store((const char *) from, (uint32) (to - from), je->s.cs); > + return 0; > +} > + > + > bool Json_table_nested_path::check_error(const char *str) > { > if (m_engine.s.error) > @@ -541,7 +561,12 @@ int ha_json_table::fill_column_values(THD *thd, uchar * buf, uchar *pos) > } > else > { > - if (!(error= !json_value_scalar(&je))) > + if (jc->m_format_json) > + { > + if (!(error= store_json_in_json(*f, &je))) > + error= er_handler.errors; Can there be a case when this assignment assigns any value other than 0? (My logic - er_handler.errors already had some error status, why did we call store_json_in_json? If there was an error inside store_json_in_json, we do not get to this assignment, as store_json_in_json() returned non-zero error. Is this correct?) > + } > + else if (!(error= !json_value_scalar(&je))) > { > store_json_in_field(*f, &je); > error= er_handler.errors; > @@ -868,6 +893,10 @@ int Json_table_column::set(THD *thd, enum_type ctype, const LEX_CSTRING &path) > anctual content. Not sure though if we should. > */ > m_path.s.c_str= (const uchar *) path.str; > + > + if (ctype == PATH) > + m_format_json= m_field->type_handler() == &type_handler_json_longtext; This will probably cause warnings on some compiler. Please wrap into "MY_TEST(...)" or just brackets, "z= (x == y)". == A discrepancy between this and MySQL == I've found this case: select * from json_table('{"foo": null}', '$' columns( jscol json path '$.foo') ) as T; with the above patch, it produces a JSON null value: +-------+ | jscol | +-------+ | null | +-------+ while in MySQL-8 it produces an SQL NULL: +-------+ | jscol | +-------+ | NULL | +-------+ I'm not yet sure what we should do about this. Any thoughts? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

2 1