- developers - lists.mariadb.org

Re: [Maria-developers] [Commits] c776cad5f8e: MDEV-22702: Assertion `!field->is_null()' failed in my_decimal::my_decimal
by Sergey Petrunia 06 Aug '20

06 Aug '20

Hi Varun, First: I can still get the assertion failure if I run this: set @@sql_mode='only_full_group_by'; select min(1 mod a1), bit_or(a2) over () from t1; As far as I understand, the query is not valid for only_full_group_by mode. (Window functions as such are ok, but here window function is using a2, which cannot be accessed in the post-grouping context). More input below. On Wed, Jul 08, 2020 at 02:58:31PM +0530, Varun wrote: > revision-id: c776cad5f8ecf2675510deeb55724d7255a52503 (mariadb-10.4.11-267-gc776cad5f8e) > parent(s): cc0dca366357651ddb549e31a12b1ecd39c7380e > author: Varun Gupta > committer: Varun Gupta > timestamp: 2020-07-08 14:58:17 +0530 > message: > > MDEV-22702: Assertion `!field->is_null()' failed in my_decimal::my_decimal > > With implicit grouping with window functions, we need to make sure that all the > fields inside the window functions are nullable as any non-aggregated field can > produce a NULL value. ... > diff --git a/sql/sql_lex.h b/sql/sql_lex.h > index 0983cea44d0..16a0b3b08a6 100644 > --- a/sql/sql_lex.h > +++ b/sql/sql_lex.h > @@ -1510,6 +1510,8 @@ class st_select_lex: public st_select_lex_node > } > > bool have_window_funcs() const { return (window_funcs.elements !=0); } > + uint32 get_number_of_window_funcs() const > + { return (uint32)window_funcs.elements; } Why use uint32 when n_sum_items is uint, and elements is also uint? I suggest just keep using uint. > ORDER *find_common_window_func_partition_fields(THD *thd); > > bool cond_pushdown_is_allowed() const > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 0ca5ab23288..05463efe9a5 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -1201,6 +1201,18 @@ JOIN::prepare(TABLE_LIST *tables_init, > break; > } > } > + /* > + If the query has a window function with an aggregate function, > + then also we have a mix of elements with and without grouping. > + Window function can be in the ORDER BY clause too so the check > + is made separately. Wording could be better. I don't understand what did you mean to say about ORDER BY. can you elaborate? > + Window function is inherited from Item_sum so each window function is > + also registered as a sum item, so need to check that we have an > + explicit aggregate function also in the query. what's "explicit aggregate" ? A regular aggregate function (i.e. not a window function)? > + */ > + if (select_lex->have_window_funcs() && > + select_lex->get_number_of_window_funcs() < select_lex->n_sum_items) > + mixed_implicit_grouping= true; > } > > table_count= select_lex->leaf_tables.elements; BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 1

[Maria-developers] Optimizer call followup: for the parser, JSON_TABLE is not a "table function"
by Sergei Petrunia 05 Aug '20

05 Aug '20

Hello, A followup to the optimizer call discussion: Here is the production from the SQL Standard describing JSON_TABLE: section 7.6 <table reference>: <table primary> ::= <table or query name> [ <query system time period specification> ] [ <correlation or recognition> ] | <derived table> <correlation or recognition> | <lateral derived table> <correlation or recognition> | <collection derived table> <correlation or recognition> | <table function derived table> <correlation or recognition> | <only spec> [ <correlation or recognition> ] | <data change delta table> [ <correlation or recognition> ] | <JSON table> <correlation or recognition> | <JSON table primitive> <correlation name> | <parenthesized joined table> <table function derived table> ::= TABLE <left paren> <collection value expression> <right parent> Section 7.11 <JSON table>: <JSON table> ::= JSON_TABLE <left paren> ....... <right parent> That is, JSON_TABLE and TABLE (...) are two different productions that have nothing in common. So, as far as syntax is concerned, JSON_TABLE is not a "table function". BR -- Sergei P.

1 0

[Maria-developers] Weekly Report
by Kartik Soneji 03 Aug '20

03 Aug '20

Hi, This week - Finished writing common testing foundation that allows other services to be tested easily. For example: --let $test_library = lzma --source test_library.inc --let $test_library = lzo --source test_library.inc ... - Finished the first draft of the Snappy service. Snappy has a C++ api that is used by RocksDB, so we have two options: - Use the mangled C++ function names directly, and load them normally with dlopen. - Re-implement the C++ function from Snappy’s C exports. The mangled function names may vary across platforms, so we are going with the second option for now. - Implemented global status variables for the services. This lays the foundation for integration with the storage engines. - Finished a first draft of the auto logic for the --use-compression= switch. Next week - Need to review my refactoring and decide on next steps. Details of refactor: - Removed the service struct. - Removed the struct from sql/sql_plugin_services.ic. - Define an anonymous struct in sql/compression/<library>.cc. - Initialize the struct with dummy function pointers. - If the library is successfully loaded, replace them with pointers to the real functions. - Define wrapper functions that forward the call to the function pointers in the struct. - Continue replicating the implementation for other libraries. - Decide what to do when dynamic libraries have the versions suffixed to the .so file. For example, liblzma.so.5.2.4 is included with Ubuntu 20, but liblzma.so is not. The liblzma.so -> liblzma.so.5.2.4 symlink is only added after installing the liblzma-dev package. - Sync up with Robert to discuss further actions. Thanks, Kartik

1 0

[Maria-developers] Review for: MDEV-17399 Add support for JSON_TABLE, part #9
by Sergey Petrunia 03 Aug '20

03 Aug '20

Hi Alexey, More input: == Circular dependencies are allowed == This query will cause an assert in the optimizer, for obvious reasons: select * from json_table(JS3.size, '$' columns (size INT PATH '$.size')) as JS1, json_table(JS1.size, '$' columns (size INT PATH '$.size')) as JS2, json_table(JS1.size, '$' columns (size INT PATH '$.size')) as JS3 where 1; == Character set inference is wrong == create table t20 (json varchar(100) character set utf8); insert into t20 values ('{"value":"АБВ"}'); create table tj20 as select T.value from t20, json_table(t20.json, '$' columns (value varchar(32) PATH '$.value')) T; mysql> show create table tj20\G *************************** 1. row *************************** Table: tj20 Create Table: CREATE TABLE `tj20` ( `value` varchar(32) DEFAULT NULL ) ENGINE=InnoDB DEFAULT CHARSET=latin1 The source document uses UTF-8 but the values from it are in latin1? == Comments and code cleanup == There are a lot of comments missing, some duplicate code, and code using out-of-date ways of doing things. Please find attached a patch with obvious fixes for most of it. == Explain == Let's follow MySQL here: let the tabular EXPLAIN show "Table function: json_table" in the Extra column, let EXPLAIN FORMAT=JSON show "table_function": "json_table" in the "table" element. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] Obtaining usernames in MariaDB UDFs
by Denis Scherbakov 02 Aug '20

02 Aug '20

Hello guys, I am building a set of MariaDB UDFs that require to know a name of a user who called them in each and every call. Users should not be able to pretend as someone else, hence usernames as an argument cannot be used. I compile UDFs against MariaDB headers and I was able to obtain the current_thd, but the security_ctx pointer inside is NULL. Could somebody give me a hint what I am doing wrong? Thank you for your answers! Best regards, Denis Scherbakov

2 2

Re: [Maria-developers] 7311586ca25: MDEV-15703 Crash in EXECUTE IMMEDIATE 'CREATE OR REPLACE TABLE t1 (a INT DEFAULT ?)' USING DEFAULT
by Sergei Golubchik 02 Aug '20

02 Aug '20

Hi, Oleksandr! On Jul 29, Oleksandr Byelkin wrote: > revision-id: 7311586ca25 (mariadb-10.2.31-317-g7311586ca25) > parent(s): a1e52e7f32c > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2020-07-17 13:47:26 +0200 > message: > > MDEV-15703 Crash in EXECUTE IMMEDIATE 'CREATE OR REPLACE TABLE t1 (a INT DEFAULT ?)' USING DEFAULT > > Part 1: make better asserts. > > diff --git a/sql/field.cc b/sql/field.cc > index 65bd9d22857..cf0cc655888 100644 > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -11138,6 +11138,16 @@ bool Field::save_in_field_default_value(bool view_error_processing) > { > THD *thd= table->in_use; > > + if ( // table is not opened properly > + table == NULL || table->pos_in_table_list == NULL || > + table->pos_in_table_list->select_lex == NULL || > + // we are in subquery/derived/CTE > + !table->pos_in_table_list->top_table()->select_lex->is_top_level_select()) > + { > + DBUG_ASSERT(0); // shoud not happened > + my_error(ER_INVALID_DEFAULT_PARAM, MYF(0)); > + return false; > + } that's kind of questionable. if this condition can never be true, why do you have my_error() there? Just do asserts, like DBUG_ASSERT(table); DBUG_ASSERT(table->pos_in_table_list); DBUG_ASSERT(table->pos_in_table_list->select_lex); DBUG_ASSERT(table->pos_in_table_list->top_table()->select_lex->is_top_level_select()); > if (flags & NO_DEFAULT_VALUE_FLAG && > real_type() != MYSQL_TYPE_ENUM) > { > @@ -11177,12 +11187,29 @@ bool Field::save_in_field_default_value(bool view_error_processing) > bool Field::save_in_field_ignore_value(bool view_error_processing) > { > enum_sql_command com= table->in_use->lex->sql_command; > + > + if ( // table is not opened properly > + table == NULL || table->pos_in_table_list == NULL || > + table->pos_in_table_list->select_lex == NULL || > + // we are in subquery/derived/CTE > + !table->pos_in_table_list->top_table()->select_lex->is_top_level_select()) > + { > + DBUG_ASSERT(0); // shoud not happened > + my_error(ER_INVALID_DEFAULT_PARAM, MYF(0)); > + return false; > + } ditto > // All insert-like commands > if (com == SQLCOM_INSERT || com == SQLCOM_REPLACE || > com == SQLCOM_INSERT_SELECT || com == SQLCOM_REPLACE_SELECT || > com == SQLCOM_LOAD) > return save_in_field_default_value(view_error_processing); > - return 0; // ignore > + if (com == SQLCOM_UPDATE || com == SQLCOM_UPDATE_MULTI) > + return 0; // ignore > + > + // unexpected command > + DBUG_ASSERT(0); // shoud not happened ditto but why it should not happen? What if one does SELECT DEFAULT(f) FROM t? > + my_error(ER_INVALID_DEFAULT_PARAM, MYF(0)); > + return false; > } > > > diff --git a/sql/item.cc b/sql/item.cc > index 9451d4203ca..382eb3ac47a 100644 > --- a/sql/item.cc > +++ b/sql/item.cc > @@ -3930,13 +3930,19 @@ int Item_param::save_in_field(Field *field, bool no_conversions) > case NULL_VALUE: > return set_field_to_null_with_conversions(field, no_conversions); > case DEFAULT_VALUE: > - return field->save_in_field_default_value(field->table->pos_in_table_list-> > + return field->save_in_field_default_value((field->table && > + field->table->pos_in_table_list)? in what case can field->table be NULL here? > + field->table->pos_in_table_list-> > top_table() != > - field->table->pos_in_table_list); > + field->table->pos_in_table_list: > + FALSE); > case IGNORE_VALUE: > - return field->save_in_field_ignore_value(field->table->pos_in_table_list-> > + return field->save_in_field_ignore_value((field->table && > + field->table->pos_in_table_list)? > + field->table->pos_in_table_list-> > top_table() != > - field->table->pos_in_table_list); > + field->table->pos_in_table_list: > + FALSE); > case NO_VALUE: > DBUG_ASSERT(0); // Should not be possible > return true; > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

Re: [Maria-developers] f854ac4d9e0: MDEV-21916: COM_STMT_BULK_EXECUTE with RETURNING insert wrong values
by Sergei Golubchik 02 Aug '20

02 Aug '20

Hi, Oleksandr! On Jul 29, Oleksandr Byelkin wrote: > revision-id: f854ac4d9e0 (mariadb-10.5.4-20-gf854ac4d9e0) > parent(s): 6cee9b1953b > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2020-07-03 13:39:49 +0200 > message: > > MDEV-21916: COM_STMT_BULK_EXECUTE with RETURNING insert wrong values > > To allocate new net buffer to avoid changing bufer we are reading. please add a more descriptive comment. I'm still not sure I understood correctly what the bug was. > diff --git a/sql/net_serv.cc b/sql/net_serv.cc > --- a/sql/net_serv.cc > +++ b/sql/net_serv.cc > @@ -178,6 +178,14 @@ my_bool my_net_init(NET *net, Vio *vio, void *thd, uint my_flags) > DBUG_RETURN(0); > } > > + > +inline void net_reset_new_packet(NET *net) static > +{ > + net->buff_end= net->buff + net->max_packet; > + net->write_pos= net->read_pos= net->buff; > +} > + > + > my_bool net_allocate_new_packet(NET *net, void *thd, uint my_flags) > { > DBUG_ENTER("net_allocate_new_packet"); > @@ -186,11 +194,26 @@ my_bool net_allocate_new_packet(NET *net, void *thd, uint my_flags) > NET_HEADER_SIZE + COMP_HEADER_SIZE + 1, > MYF(MY_WME | my_flags)))) > DBUG_RETURN(1); > - net->buff_end=net->buff+net->max_packet; > - net->write_pos=net->read_pos = net->buff; > + net_reset_new_packet(net); > DBUG_RETURN(0); > } > > +unsigned char *net_try_allocate_new_packet(NET *net, void *thd, uint my_flags) > +{ > + unsigned char * old_buff= net->buff; > + if (net_allocate_new_packet(net, thd, my_flags)) > + { > + /* > + We failed to allocate a new buffer => return the old buffer and > + return an error > + */ > + net->buff= old_buff; > + net_reset_new_packet(net); > + old_buff= NULL; wouldn't it be better to make net_allocate_new_packet not to change net->buff on failure? because currently you have exactly the same problem in sql_parse.cc (in 10.4) and your patch doesn't fix it. simply if (!(buff= (uchar*) my_malloc(...))) DBUG_RETURN(1); net->buff= buff; net->buff_end= net->buff + net->max_packet; net->write_pos= net->read_pos= net->buff; DBUG_RETURN(0); > + } > + return old_buff; > +} > + > > void net_end(NET *net) > { > diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc > --- a/sql/sql_delete.cc > +++ b/sql/sql_delete.cc > @@ -685,7 +685,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds, > !table->prepare_triggers_for_delete_stmt_or_event()) > will_batch= !table->file->start_bulk_delete(); > > - if (returning) > + if (returning && !thd->get_stmt_da()->is_set()) how are result sets for bulk delete returning sent? > { > if (result->send_result_set_metadata(returning->item_list, > Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF)) > diff --git a/sql/sql_error.cc b/sql/sql_error.cc > --- a/sql/sql_error.cc > +++ b/sql/sql_error.cc > @@ -380,16 +380,33 @@ Diagnostics_area::set_eof_status(THD *thd) > if (unlikely(is_error() || is_disabled())) > return; > > - /* > - If inside a stored procedure, do not return the total > - number of warnings, since they are not available to the client > - anyway. > - */ > - m_statement_warn_count= (thd->spcont ? > - 0 : > - current_statement_warn_count()); > + if (m_status == DA_EOF_BULK) > + { > + /* > + If inside a stored procedure, do not return the total > + number of warnings, since they are not available to the client > + anyway. > + */ > + if (!thd->spcont) > + m_statement_warn_count+= current_statement_warn_count(); > + } > + else > + { > + /* > + If inside a stored procedure, do not return the total > + number of warnings, since they are not available to the client > + anyway. > + */ did you have to repeat the same comment twice? > + if (thd->spcont) > + { > + m_statement_warn_count= 0; > + m_affected_rows= 0; affected rows too? old code didn't do that > + } > + else > + m_statement_warn_count= current_statement_warn_count(); > + m_status= (is_bulk_op() ? DA_EOF_BULK : DA_EOF); > + } > > - m_status= DA_EOF; > DBUG_VOID_RETURN; > } > > diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc > --- a/sql/sql_prepare.cc > +++ b/sql/sql_prepare.cc > @@ -4364,24 +4368,37 @@ Prepared_statement::execute_bulk_loop(String *expanded_query, > if (state == Query_arena::STMT_ERROR) > { > my_message(last_errno, last_error, MYF(0)); > - thd->set_bulk_execution(0); > - return TRUE; > + goto err; > } > /* Check for non zero parameter count*/ > if (param_count == 0) > { > DBUG_PRINT("error", ("Statement with no parameters for bulk execution.")); > my_error(ER_UNSUPPORTED_PS, MYF(0)); > - thd->set_bulk_execution(0); > - return TRUE; > + goto err; > } > > if (!(sql_command_flags[lex->sql_command] & CF_SP_BULK_SAFE)) > { > DBUG_PRINT("error", ("Command is not supported in bulk execution.")); > my_error(ER_UNSUPPORTED_PS, MYF(0)); > - thd->set_bulk_execution(0); > - return TRUE; > + goto err; > + } > + /* > + Here second buffer for not optimized commands, > + optimized commands do it inside thier internal loop. > + */ > + if (!(sql_command_flags[lex->sql_command] & CF_SP_BULK_OPTIMIZED) && > + this->lex->has_returning()) > + { > + // Above check can be true for SELECT in future > + DBUG_ASSERT(lex->sql_command != SQLCOM_SELECT); > + if ((readbuff= > + net_try_allocate_new_packet(&thd->net, thd, > + MYF(MY_THREAD_SPECIFIC))) == NULL) sorry, I'm confused. You allocate a buffer here, in Prepared_statement::execute_bulk_loop, and then again in mysql_insert() ? > + { > + goto err; > + } > } Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

Re: [Maria-developers] ca7e6f8d572: MDEV-19632 Replication aborts with ER_SLAVE_CONVERSION_FAILED upon CREATE ... SELECT in ORACLE mode
by Sergei Golubchik 31 Jul '20

31 Jul '20

Hi, Alexander! On Jul 28, Alexander Barkov wrote: > revision-id: ca7e6f8d572 (mariadb-10.3.21-178-gca7e6f8d572) > parent(s): f3f23b5c4bd > author: Alexander Barkov <bar(a)mariadb.com> > committer: Alexander Barkov <bar(a)mariadb.com> > timestamp: 2020-07-08 12:37:17 +0400 > message: > > MDEV-19632 Replication aborts with ER_SLAVE_CONVERSION_FAILED upon CREATE ... SELECT in ORACLE mode > > diff --git a/mysql-test/main/lowercase_fs_off.result b/mysql-test/main/lowercase_fs_off.result > index f2a8ec14641..3f0b08a78c4 100644 > --- a/mysql-test/main/lowercase_fs_off.result > +++ b/mysql-test/main/lowercase_fs_off.result > @@ -158,3 +158,13 @@ show triggers like '%T1%'; > Trigger Event Table Statement Timing Created sql_mode Definer character_set_client collation_connection Database Collation > drop table t1; > set GLOBAL sql_mode=default; > +# > +# MDEV-19632 Replication aborts with ER_SLAVE_CONVERSION_FAILED upon CREATE ... SELECT in ORACLE mode > +# > +# Compatibility schema names respect the filesystem case sensitivity I agree, it should. But this is not consistent with information_schema. So I'd suggest a comment in eq_name method, explaining why compatibility schema names respect the filesystem case sensitivity. (because, e.g., we eventually turn them into real schemas, on disk). > +CREATE TABLE t1 (a MARIADB_SCHEMA.date); > +ERROR HY000: Unknown data type: 'MARIADB_SCHEMA.date' > +CREATE TABLE t1 (a Mariadb_schema.date); > +ERROR HY000: Unknown data type: 'Mariadb_schema.date' > +CREATE TABLE t1 (a mariadb_schema.date); > +DROP TABLE t1; > diff --git a/mysql-test/suite/compat/oracle/t/type_date.test b/mysql-test/suite/compat/oracle/t/type_date.test > index 61f7aa53944..34adc5889b4 100644 > --- a/mysql-test/suite/compat/oracle/t/type_date.test > +++ b/mysql-test/suite/compat/oracle/t/type_date.test > @@ -2,3 +2,101 @@ SET sql_mode=ORACLE; ... > +--echo # > +--echo # Qualified syntax is not supported yet in SP > +--echo # an MDEV for that? > diff --git a/sql/field.cc b/sql/field.cc > index 8accfb35b0b..4ed38f2c06f 100644 > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -10407,6 +10407,19 @@ void Column_definition::set_attributes(const Lex_field_type_st &type, > set_handler(type.type_handler()); > charset= cs; > > +#if MYSQL_VERSION_ID > 100500 > +#error When merging to 10.5, please move the code below to > +#error Type_handler_timestamp_common::Column_definition_set_attributes() good > +#else > + /* > + Unlike other types TIMESTAMP fields are NOT NULL by default. > + Unless --explicit-defaults-for-timestamp is given. > + */ > + if (!opt_explicit_defaults_for_timestamp && > + type.type_handler()->field_type() == MYSQL_TYPE_TIMESTAMP) > + flags|= NOT_NULL_FLAG; > +#endif > + > if (type.length()) > { > int err; > diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc > index 0b423355170..3fb07601f99 100644 > --- a/sql/sql_lex.cc > +++ b/sql/sql_lex.cc > @@ -2080,6 +2081,13 @@ int Lex_input_stream::scan_ident_start(THD *thd, Lex_ident_cli_st *str) > > uint length= yyLength(); > yyUnget(); // ptr points now after last token char > + > + if (thd->lex->parsing_options.lookup_keywords_after_qualifier) > + { > + if (int tokval= find_keyword(str, length, 0)) > + return tokval; > + } why is that? > + > str->set_ident(m_tok_start, length, is_8bit); > m_cpp_text_start= m_cpp_tok_start; > m_cpp_text_end= m_cpp_text_start + length; > @@ -8284,3 +8292,32 @@ bool LEX::tvc_finalize_derived() > current_select->linkage= DERIVED_TABLE_TYPE; > return tvc_finalize(); > } > + > + > +bool LEX::map_data_type(const Lex_ident_sys_st &schema_name, > + Lex_field_type_st *type) const > +{ > + const Schema *schema= schema_name.str ? > + Schema::find_by_name(schema_name) : > + Schema::find_implied(thd); > + if (!schema) > + { > + char buf[128]; > + const Name type_name= type->type_handler()->name(); > + my_snprintf(buf, sizeof(buf), "%.*s.%.*s", > + (int) schema_name.length, schema_name.str, > + (int) type_name.length(), type_name.ptr()); > +#if MYSQL_VERSION_ID > 100500 > +#error Please remove the old code > + my_error(ER_UNKNOWN_DATA_TYPE, MYF(0), buf); > +#else > + my_printf_error(ER_UNKNOWN_ERROR, "Unknown data type: '%-.64s'", > + MYF(0), buf); > +#endif > + return true; > + } > + const Type_handler *mapped= schema->map_data_type(thd, type->type_handler()); > + if (mapped != type->type_handler()) > + type->set_handler(mapped); why do you need an if() here? type->set_handler(mapped) is not that expensive. > + return false; > +} > diff --git a/sql/sql_show.cc b/sql/sql_show.cc > index 3dbc7724928..fcc2d44fdd6 100644 > --- a/sql/sql_show.cc > +++ b/sql/sql_show.cc > @@ -2224,6 +2224,13 @@ int show_create_table(THD *thd, TABLE_LIST *table_list, String *packet, > append_identifier(thd, packet, &field->field_name); > packet->append(' '); > > + const Type_handler *th= field->type_handler(); > + const Schema *implied_schema= Schema::find_implied(thd); > + if (th != implied_schema->map_data_type(thd, th)) > + { > + packet->append(th->schema()->name(), system_charset_info); sorry, I don't see it. How can th->schema() return not mariadb_schema? > + packet->append(STRING_WITH_LEN("."), system_charset_info); > + } > type.set(tmp, sizeof(tmp), system_charset_info); > field->sql_type(type); > packet->append(type.ptr(), type.length(), system_charset_info); > diff --git a/storage/test_sql_discovery/mysql-test/sql_discovery/simple.result b/storage/test_sql_discovery/mysql-test/sql_discovery/simple.result > index 23b7804638f..1feea5e47ee 100644 > --- a/storage/test_sql_discovery/mysql-test/sql_discovery/simple.result > +++ b/storage/test_sql_discovery/mysql-test/sql_discovery/simple.result > @@ -82,7 +82,7 @@ select * from t1; > ERROR HY000: Engine TEST_SQL_DISCOVERY failed to discover table `test`.`t1` with 'create table t1 (a uint)' > show warnings; > Level Code Message > -Error 1064 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 'uint)' at line 1 > +Error 1064 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 ')' at line 1 this is not ideal :( can you try to keep the old error message? Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

[Maria-developers] Review for: MDEV-17399 Add support for JSON_TABLE, part #7
by Sergey Petrunia 30 Jul '20

30 Jul '20

Hi Alexey, On Thu, Jul 16, 2020 at 04:34:46PM +0400, Alexey Botchkov wrote: > > In the new patch i think i addressed most of your comments. > https://github.com/MariaDB/server/commit/d3a311a8e9508ef94dddb6b7f4d366337a… > ... > > The above doesn't do proper quoting, which is wrong. Testcase: > > > > select * > > from > > json_table('[{"co\\\\lor": "blue", "price": 50}]', > > '$[*]' columns( color varchar(100) path '$.co\\\\lor') > > ) as T; > > +-------+ > > | color | > > +-------+ > > | blue | > > +-------+ > > > > create view v2 as > > select * > > from > > json_table('[{"co\\\\lor": "blue", "price": 50}]', > > '$[*]' columns( color varchar(100) path '$.co\\\\lor') > > ) as T; > > select * from v2; Quoting is still an issue apparently: create view v1 as select * from json_table('[{"color": "blue", "price": 50}]', '$[*]' columns(color text path '$.nonexistent', seq for ordinality ) ) as `ALIAS NOT QUOTED`; select * from v1; ERROR 1064 (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 'NOT QUOTED' at line 1 create view v2 as select * from json_table('[{"color": "blue", "price": 50}, {"color": "red", "price": 100}]', '$[*]' columns(color text path '$.QUOTES " HERE "', seq for ordinality ) ) as T; select * from v2; ERROR 1064 (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 'HERE "", `seq` FOR ORDINALITY)) T' at line 1 Could you please go through the printout functions and make sure all printed literals are properly escaped/quoted? Printout code for other functions can be used as an example. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 1

Re: [Maria-developers] 2b527a4e858: MDEV-17729: Assertion `! is_set() || m_can_overwrite_status' failed in
by Sergei Golubchik 30 Jul '20

30 Jul '20

Hi, Rucha! On Jul 30, Rucha Deodhar wrote: > > > > > > - if ((level == Sql_condition::WARN_LEVEL_WARN) && > > > - really_abort_on_warning()) > > > + if ((killed == NOT_KILLED) && (level == Sql_condition::WARN_LEVEL_WARN) && > > > + really_abort_on_warning()) > > > > I don't understand this. I presume the assert that fails is in > > da->set_error_status(), called below as > > > > Yes, the assertion fails in set_error_status(). > > > if (!da->is_error()) > > { > > set_row_count_func(-1); > > da->set_error_status(sql_errno, msg, sqlstate, cond); > > } > > > > but if(!da->is_error()) should prevent set_error_status() from being > > called when it would need to overwrite the status. So how comes it is > > still called and triggers the assert? > > The status does not change to DA_ERROR yet, it changes in > set_error_status(), so !thd->is_error() becomes true and does not prevent > set_error_status() from being called. The reason we get assertion failure > in set_error_status() is because !is_set() is false (because status is > DA_OK) and m_can_overwrite_status is false too. I see. I also was confused by how can THD::raise_condition() be called after the status is already DA_OK? An error should not happen after DA_OK. But it turns out it was not just any error, it was push_warning() with the message "Query execution was interrupted. The query examined at least 4 rows, which exceeds LIMIT ROWS EXAMINED (2). The query result may be incomplete". This warning should not be converted to an error, obviously. It is converted, because thd->abort_on_warning is true. It's set to true in select_insert::prepare(). And the warning is pushed in handle_select(). Perhaps this warning should never have thd->abort_on_warning ? Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0