January 2022 - developers - lists.mariadb.org

Re: [Maria-developers] e4ea1544097: MDEV-17124: mariadb 10.1.34, views and prepared statements: ERROR 1615 (HY000): Prepared statement needs to be re-prepared
by Sergei Golubchik 20 Jan '22

20 Jan '22

Hi, Oleksandr, On Jan 19, Oleksandr Byelkin wrote: > revision-id: e4ea1544097 (mariadb-10.2.40-198-ge4ea1544097) > parent(s): fd7a553cb80 > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2022-01-19 11:12:57 +0100 > message: > > MDEV-17124: mariadb 10.1.34, views and prepared statements: ERROR 1615 (HY000): Prepared statement needs to be re-prepared > > The problem is that if table definition cache (TDC) is full of real tables > which are in tables cache, view definition can not stay there so will be > removed by its own underlying tables. > In situation above old mechanism of detection matching definition in PS > and current version always require reprepare and so prevent executing > the PS. > > One work around is to increase TDC, other - improve version check for > views/triggers (which is done here). Now in suspicious cases we check: > - MD5 of the view to be sure that version really have changed; > - time of creation of a trigger related to time of statement preparation. > > diff --git a/mysql-test/r/ps_ddl.result b/mysql-test/r/ps_ddl.result > index e69c6e06193..005c78a0319 100644 > --- a/mysql-test/r/ps_ddl.result > +++ b/mysql-test/r/ps_ddl.result > @@ -214,7 +214,7 @@ new trigger: 10 > drop trigger t1_bd; > set @val=11; > execute stmt using @val; > -call p_verify_reprepare_count(1); > +call p_verify_reprepare_count(0); > SUCCESS > > select @message; > @@ -831,8 +831,8 @@ a b c > 20 40 100 > 30 60 150 > call p_verify_reprepare_count(1); > -SUCCESS > - > +ERROR > +Expected: 1, actual: 0 > # Check that we properly handle ALTER VIEW statements. > execute stmt; > a b c > @@ -882,9 +882,9 @@ a b c > 10 50 60 > 20 100 120 > 30 150 180 > -call p_verify_reprepare_count(1); > -SUCCESS > - > +call p_verify_reprepare_count(0); > +ERROR > +Expected: 0, actual: 1 > execute stmt; > a b c you forgot to update the expected number or reprepares in two places. > diff --git a/sql/parse_file.cc b/sql/parse_file.cc > index 999f53bd681..3c750f4df29 100644 > --- a/sql/parse_file.cc > +++ b/sql/parse_file.cc > @@ -145,6 +145,7 @@ write_parameter(IO_CACHE *file, uchar* base, File_option *parameter) > > switch (parameter->type) { > case FILE_OPTIONS_STRING: > + case FILE_OPTIONS_FIXSTRING: > { > LEX_STRING *val_s= (LEX_STRING *)(base + parameter->offset); > if (my_b_write(file, (const uchar *)val_s->str, val_s->length)) > @@ -830,6 +831,22 @@ File_parser::parse(uchar* base, MEM_ROOT *mem_root, > } > ptr= eol+1; > break; > + case FILE_OPTIONS_FIXSTRING: > + { > + /* string have to be allocated already and length set */ > + LEX_STRING *val= (LEX_STRING *)(base + parameter->offset); > + DBUG_ASSERT(val->length != 0); > + if (ptr[val->length] != '\n') > + { > + my_error(ER_FPARSER_ERROR_IN_PARAMETER, MYF(0), > + parameter->name.str, line); > + DBUG_RETURN(TRUE); > + } > + memcpy(val->str, ptr, val->length); > + val->str[val->length]= '\0'; > + ptr+= (val->length + 1); > + break; I'd suggest to use FILE_OPTIONS_STRING here, it's much simpler and it's known to work. This doesn't check that end - ptr < val->length, and it doesn't check that even if ptr[val->length] == '\n', then it's the first '\n' after ptr. It's all possible to fix, but using FILE_OPTIONS_STRING is just easier. > + } > case FILE_OPTIONS_TIMESTAMP: > { > /* string have to be allocated already */ > diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc > index bb1a99d9eef..b9198647f98 100644 > --- a/sql/sql_prepare.cc > +++ b/sql/sql_prepare.cc > @@ -4177,6 +4177,10 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) > Query_arena *old_stmt_arena; > DBUG_ENTER("Prepared_statement::prepare"); > DBUG_ASSERT(m_sql_mode == thd->variables.sql_mode); > + > + // The same format as for triggers to compare > + create_time= make_trigger_time(thd->query_start(), > + thd->query_start_sec_part()); Better use hrtime here, you might compare it with more than triggers in the future. For triggers, you can multiply it by 10000 for comparison. Or you can start storing trigger time with a microsecond precision - that's even better. > /* > If this is an SQLCOM_PREPARE, we also increase Com_prepare_sql. > However, it seems handy if com_stmt_prepare is increased always, > diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc > index 52817ef65ae..38febce4fef 100644 > --- a/sql/sql_trigger.cc > +++ b/sql/sql_trigger.cc > @@ -929,7 +933,21 @@ bool Table_triggers_list::create_trigger(THD *thd, TABLE_LIST *tables, > > if (!sql_create_definition_file(NULL, &file, &triggers_file_type, > (uchar*)this, triggers_file_parameters)) > + { > + // delay if needed to be sure that time of creation is unique > + for (;;) > + { > + my_hrtime_t hrtime= my_hrtime(); > + ulonglong current_time= make_trigger_time(hrtime_to_my_time(hrtime), > + hrtime_sec_part(hrtime)); > + if (trigger->create_time == current_time) > + pthread_yield(); > + else > + break; > + } This shouldn't be needed. Better 1. store statement->create_time with microsecond precision 2. store trigger->create_time with microsecond precision 3. at the end of prepare wait (like above) to make sure statement->create_time is larger than all triggers' create_time. this will be a no-op unless someone created a trigger and prepared in the same microsecond. Safety check will ensure you won't wait longer than one microsecond. > DBUG_RETURN(false); > + } > > err_with_cleanup: > /* Delete .TRN file */ > diff --git a/sql/sql_view.cc b/sql/sql_view.cc > index 020830483c5..830a42b3cf3 100644 > --- a/sql/sql_view.cc > +++ b/sql/sql_view.cc > @@ -1131,7 +1141,31 @@ static int mysql_register_view(THD *thd, TABLE_LIST *view, > DBUG_RETURN(error); > } > > +/** > + Check is TABLE_LEST and SHARE match > + @param[in] view TABLE_LIST of the view > + @param[in] share Share object of view > + > + @return false on error or misspatch > +*/ > > +bool mariadb_view_version_get(TABLE_SHARE *share) > +{ > + DBUG_ASSERT(share->is_view); > + > + if (!(share->tabledef_version.str= > + (uchar*) alloc_root(&share->mem_root, VIEW_MD5_LEN + 1))) > + return TRUE; > + share->tabledef_version.length= VIEW_MD5_LEN; > + > + DBUG_ASSERT(share->view_def != NULL); > + if (share->view_def->parse((uchar *) &share->tabledef_version, NULL, > + view_md5_parameters, 1, > + &file_parser_dummy_hook)) > + return TRUE; > + DBUG_ASSERT(share->tabledef_version.length == VIEW_MD5_LEN); > + return FALSE; > +} I don't like using md5 as a uuid. md5 collisions are very much possible. Better options would be 1. use a timestamp, but store it with microsecond precisions, same logic as with triggers. 2. use a true tabledef uuid, new field in the view frm, same logic as with tables. timestamp is better, because it already exists in frms, and any old view will naturally have a timestamp before now(), so it doesn't matter that it won't have microseconds. uuid is better, because it's more universal, can be used for more than just reprepare checks. But for now I think timestamp benefits outweigh that. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 8f8e7ad5c80: MDEV-27217 DELETE partition selection doesn't work for history partitions
by Sergei Golubchik 13 Jan '22

13 Jan '22

Hi, Aleksey! ok to push. see one comment below. On Jan 13, Aleksey Midenkov wrote: > revision-id: 8f8e7ad5c80 (mariadb-10.3.31-80-g8f8e7ad5c80) > parent(s): 88f8aa20bba > author: Aleksey Midenkov > committer: Aleksey Midenkov > timestamp: 2022-01-12 21:47:03 +0300 > message: > > MDEV-27217 DELETE partition selection doesn't work for history partitions > > LIMIT history switching requires the number of history partitions to > be marked for read: from first to last non-empty plus one empty. The > least we can do is to fail with error message if the needed partition > was not marked for read. As this is handler interface we require new > handler error code to display user-friendly error message. > > diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc > index bd73642cd0d..2f945f29ed1 100644 > --- a/sql/ha_partition.cc > +++ b/sql/ha_partition.cc > @@ -9872,6 +9874,11 @@ void ha_partition::print_error(int error, myf errflag) > m_part_info->print_no_partition_found(table, errflag); > DBUG_VOID_RETURN; > } > + else if (error == HA_ERR_PARTITION_LIST) > + { > + handler::print_error(error, errflag); > + DBUG_VOID_RETURN; > + } this seems to be redundant, the else branch at the end covers it. > else if (error == HA_ERR_ROW_IN_WRONG_PARTITION) > { Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] ef70e809a40: MDEV-27217 DELETE partition selection doesn't work for history partitions
by Sergei Golubchik 12 Jan '22

12 Jan '22

Hi, Aleksey! On Jan 12, Aleksey Midenkov wrote: > revision-id: ef70e809a40 (mariadb-10.3.31-80-gef70e809a40) > parent(s): 88f8aa20bba > author: Aleksey Midenkov > committer: Aleksey Midenkov > timestamp: 2021-12-23 04:15:39 +0300 > message: > > MDEV-27217 DELETE partition selection doesn't work for history partitions > > LIMIT history switching requires the number of history partitions to > be marked for read: from first to last non-empty plus one empty. The > least we can do is to fail with error message if the needed partition > was not marked for read. As this is handler interface we require new > handler error code to display user-friendly error message. > > Switching by INTERVAL works out-of-the-box with > ER_ROW_DOES_NOT_MATCH_GIVEN_PARTITION_SET error. Code-wise it's pretty much fine. Two problems related to error messages: We cannot have part of the error message a hard-coded English phrase. It doesn't work with localized error messages: "Невозможно выбрать раздел: modifying system versioned table" Also we cannot add new error messages in 10.3. Or any old GA. Currently you can add new error messages to 10.6 or any later version. You can reuse ER_UNUSED_1 for your new error message, that'll work. And, because you cannot have English phrases in it, I'd suggest something more generic, like Not allowed for system-versioned table %`s.%`s In the context of delete from t1 partition (p1); ERROR HY000: Not allowed for system-versioned table `test`.`t1` it seems to be quite clear. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 7a746072a5e: MDEV-26878: Query failing with syntax error sets ROW_NUMBER to non-zero
by Sergei Golubchik 12 Jan '22

12 Jan '22

Hi, Rucha! On Jan 12, Rucha Deodhar wrote: > On Tue, Jan 11, 2022 at 11:49 PM Sergei Golubchik <serg(a)mariadb.org> wrote: > > > +--error ER_PARSE_ERROR > > > +INSERT INTO t1 VALUES XXX (1),(2); > > > > > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy > > > index 8ebefbb3d82..47ed3ded4a7 100644 > > > --- a/sql/sql_yacc.yy > > > +++ b/sql/sql_yacc.yy > > > @@ -8426,6 +8426,8 @@ query_specification_start: > > > { > > > SELECT_LEX *sel; > > > LEX *lex= Lex; > > > + if (lex->sql_command == SQLCOM_INSERT) > > > + thd->get_stmt_da()->inc_current_row_for_warning(); > > > if (!(sel= lex->alloc_select(TRUE)) || lex->push_select(sel)) > > > MYSQL_YYABORT; > > > > better not to have an if() in this rule that's used almost for > > everything. > > When we have INSERT...SELECT counter has to be 1 for errors like > ER_ILLEGAL_VALUE_FOR_TYPE which are caught during parsing. > Example: > INSERT INTO t1 SELECT a, b, 1.79769313486232e+308 FROM t2; > > So I added if() to increment the counter if we are parsing INSERT so > that ROW_NUMBER for statements like the one above gives ROW_NUMBER=1. I see. But you put a condition on every query's execution path to fix the case that, I believe, literally nobody cares about. I don't think any user ever will check the ROW_NUMBER after the syntax error :) And if someone will, we can say that in INSERT INTO t1 VALUES XXX (1),(2); the first row starts after VALUES, so ROW_NUMBER=1 is expected. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 9ea85a70a75: MDEV-24654 GTID event falsely marked transactional
by Sergei Golubchik 11 Jan '22

11 Jan '22

Hi, Andrei! On Jan 11, Andrei Elkin wrote: > Howdy, Sergei! > > To the question of the usage of trX cache by non-trX let me answer > broadly to mention @@binlog_direct_non_transactional_update = false > leads to aggregation of mixed, say innodb + myisam, events in trx > cache. That's different. The bug summary is "GTID event falsely marked transactional", meaning, the group of events is not transactional and it's falsely marked as transactional. If you have an innodb/myisam mix, it is correctly marked transactional. You need to have only non-transactional events in the transactional cache. And I'm trying to understand how can that happen. > I may need to process deeper the referred comments though. > > Next to the commit's idea, it actually covers your concern. I wonder > on the 'Wrong' conclusion. There is no intent to xidify anything > extra. Th commit assumes xid eVent is created, before Gtid one, > accordingly. And that fact is propagated to Gtid ctor. Your commit, if I'm not mistaken, assumes that "last event is Xid if and only if the cache contains a transaction". Which is incorrect, a transaction does not necesarily have to end with a Xid event. /Sergei > On Mon, 10 Jan 2022, 20:56 Sergei Golubchik, <serg(a)mariadb.org> wrote: > > > Hi, Andrei! > > > > On Jan 10, Andrei Elkin wrote: > > > revision-id: 9ea85a70a75 (mariadb-10.2.40-4-g9ea85a70a75) > > > parent(s): 160d97a4aaa > > > author: Andrei Elkin > > > committer: Andrei Elkin > > > timestamp: 2021-08-08 14:20:57 +0300 > > > message: > > > > > > MDEV-24654 GTID event falsely marked transactional > > > > > > GTID event can be falsely marked transactional in few cases including > > > binary-logging on the slave side upon execution. > > > In some execution branches bin-logging of non-transactional > > > events can be done through transactional cache. E.g see comments > > > in THD::binlog_write_table_map(). > > > > I saw that and still couldn't understand why bin-logging of > > non-transactional events can be done thr-ough transactional cache. > > > > > In order to not create the false 'trans' tag the fact of Xid event > > > logging is checked to compute correct argument to Gtid_log_event > > > ctor. > > > > This is wrong. Transactions only end with a Xid event if all > > participating engines support XA. Try to create an InnoDB+FederatedX > > transaction (I just did) and it'll end with Query_log_event("COMMIT") > > > > > diff --git a/sql/log.cc b/sql/log.cc > > > index 1d9b4645421..7fef2e0d739 100644 > > > --- a/sql/log.cc > > > +++ b/sql/log.cc > > > @@ -8118,9 +8118,11 @@ > > MYSQL_BIN_LOG::write_transaction_or_stmt(group_commit_entry *entry, > > > uint64 commit_id) > > > { > > > binlog_cache_mngr *mngr= entry->cache_mngr; > > > + bool has_xid= entry->end_event->get_type_code() == XID_EVENT; > > > DBUG_ENTER("MYSQL_BIN_LOG::write_transaction_or_stmt"); > > > > > > - if (write_gtid_event(entry->thd, false, entry->using_trx_cache, > > commit_id)) > > > + if (write_gtid_event(entry->thd, false, > > > + entry->using_trx_cache && has_xid, commit_id)) > > > DBUG_RETURN(ER_ERROR_ON_WRITE); > > > > > > if (entry->using_stmt_cache && !mngr->stmt_cache.empty() && > > > > > Regards, > > Sergei > > VP of MariaDB Server Engineering > > and security(a)mariadb.org > > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

Re: [Maria-developers] 9ea85a70a75: MDEV-24654 GTID event falsely marked transactional
by Sergei Golubchik 11 Jan '22

11 Jan '22

Hi, Andrei! On Jan 11, Andrei Elkin wrote: > On Tue, Jan 11, 2022 at 1:06 PM Sergei Golubchik <serg(a)mariadb.org> wrote: > > > Hi, Andrei! > > > > On Jan 11, Andrei Elkin wrote: > > > Howdy, Sergei! > > > > > > To the question of the usage of trX cache by non-trX let me answer > > > broadly to mention @@binlog_direct_non_transactional_update = false > > > leads to aggregation of mixed, say innodb + myisam, events in trx > > > cache. > > > > That's different. The bug summary is "GTID event falsely marked > > transactional", meaning, the group of events is not transactional and > > it's falsely marked as transactional. If you have an innodb/myisam > > mix, it is correctly marked transactional. > > > > You need to have only non-transactional events in the transactional > > cache. And I'm trying to understand how can that happen. > > Got your very specific point is about a pure myisam can be trx-cached. > This is OPTION_BINLOG_GTID > commit 5426facdcbfba2d78dab3c709cbf278073383b7c 's decision > >> - All tables are regarded as transactional tables in the binary log > (to ensure things are executed exactly as on the master) Okay, thanks, that answers my question Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 7a746072a5e: MDEV-26878: Query failing with syntax error sets ROW_NUMBER to non-zero
by Sergei Golubchik 11 Jan '22

11 Jan '22

Hi, Rucha! On Jan 11, Rucha Deodhar wrote: > revision-id: 7a746072a5e (mariadb-10.7.1-26-g7a746072a5e) > parent(s): 7dfaded9625 > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2022-01-05 12:05:22 +0530 > message: > > MDEV-26878: Query failing with syntax error sets ROW_NUMBER to non-zero > > Analysis: m_current_row_for_warning counter is set to 0 at the beginning > of parsing. > Fix: Reset it to 1 and increment it before starting to parse insert values, > during SET and SELECT for INSERT...SET statement and INSERT...SELECT > respectively. > > diff --git a/mysql-test/main/get_diagnostics.result b/mysql-test/main/get_diagnostics.result > diff --git a/mysql-test/main/get_diagnostics.test b/mysql-test/main/get_diagnostics.test > index e8d81dca1e6..467f624c49c 100644 > --- a/mysql-test/main/get_diagnostics.test > +++ b/mysql-test/main/get_diagnostics.test > @@ -1687,3 +1687,18 @@ GET DIAGNOSTICS CONDITION 3 @n = ROW_NUMBER; > SELECT @n; > > DROP TABLE t; > + > +--echo # > +--echo # MDEV-26878: Query failing with syntax error sets ROW_NUMBER to > +--echo # non-zero > +--echo # > + > +CREATE TABLE t1 (id1 INT); > + > +--error ER_PARSE_ERROR > +INSERT INTO t1 VALUES XXX (1),(2); please add a test, like, INTO t1 VALUES (1), (ALL); what ROW_NUMBER will it show? > + > +GET DIAGNOSTICS CONDITION 1 @n= ROW_NUMBER; > +SELECT @n; > + > +DROP TABLE t1; > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy > index 8ebefbb3d82..47ed3ded4a7 100644 > --- a/sql/sql_yacc.yy > +++ b/sql/sql_yacc.yy > @@ -8426,6 +8426,8 @@ query_specification_start: > { > SELECT_LEX *sel; > LEX *lex= Lex; > + if (lex->sql_command == SQLCOM_INSERT) > + thd->get_stmt_da()->inc_current_row_for_warning(); > if (!(sel= lex->alloc_select(TRUE)) || lex->push_select(sel)) > MYSQL_YYABORT; better not to have an if() in this rule that's used almost for everything. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 8e21b91fd31: MDEV-26875: Wrong user in SET DEFAULT ROLE error
by Sergei Golubchik 11 Jan '22

11 Jan '22

Hi, Anel! On Jan 11, Anel Husakovic wrote: > revision-id: 8e21b91fd31 (mariadb-10.5.12-71-g8e21b91fd31) > parent(s): e5a9dcfda20 > author: Anel Husakovic > committer: Anel Husakovic > timestamp: 2021-10-25 14:43:38 +0200 > message: > > MDEV-26875: Wrong user in SET DEFAULT ROLE error > > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index 65f053aa09b..0a59f5fce37 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -3273,7 +3273,7 @@ static int check_user_can_set_role(THD *thd, const char *user, > { > /* Role is not granted but current user can see the role */ > my_printf_error(ER_INVALID_ROLE, "User %`s@%`s has not been granted role %`s", > - MYF(0), thd->security_ctx->priv_user, > + MYF(0), user, > thd->security_ctx->priv_host, rolename); That's strange. you'll use `user` and `thd->security_ctx->priv_host` ? They generally correspond to two different accounts. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] b14b6f31fa2: MDEV-26843: Inconsistent behavior of ROW_NUMBER upon resignalling from function Analysis: ROW_NUMBER should be 0 because the error produced is not related to statement that modifies table data. It is just wrong number of arguments. However, ROW_NUMBER is 1 because m_current_row_for_warning is not set to 0 before executing function. Fix: Set m_current_row_for_warning to 0.
by Sergei Golubchik 11 Jan '22

11 Jan '22

Hi, Rucha! What if you have INSERT t1 VALUES (f1(1)), (f1(2)), (f1()); ? On Jan 11, Rucha Deodhar wrote: > revision-id: b14b6f31fa2 (mariadb-10.6.1-143-gb14b6f31fa2) > parent(s): d555ae38381 > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2021-10-21 17:13:13 +0530 > message: > > MDEV-26843: Inconsistent behavior of ROW_NUMBER upon resignalling from > function > Analysis: ROW_NUMBER should be 0 because the error produced is not related to > statement that modifies table data. It is just wrong number of arguments. > However, ROW_NUMBER is 1 because m_current_row_for_warning is not set to 0 > before executing function. > Fix: Set m_current_row_for_warning to 0. > > diff --git a/sql/sp_head.cc b/sql/sp_head.cc > index d1cf5c2d9bd..ebae2f356bd 100644 > --- a/sql/sp_head.cc > +++ b/sql/sp_head.cc > @@ -2033,6 +2033,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, > */ > if (argcount != m_pcont->context_var_count()) > { > + thd->get_stmt_da()->reset_current_row_for_warning(0); > /* > Need to use my_error here, or it will not terminate the > invoking query properly. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 394bb653b0b: MDEV-26695: Number of an invalid row is not calculated for table value constructor
by Sergei Golubchik 11 Jan '22

11 Jan '22

Hi, Rucha! please, see below On Jan 11, Rucha Deodhar wrote: > revision-id: 394bb653b0b (mariadb-10.6.1-140-g394bb653b0b) > parent(s): 3c857a07d92 > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2021-10-20 01:30:04 +0530 > message: > > MDEV-26695: Number of an invalid row is not calculated for table value > constructor > > Analysis: counter does not increment while sending rows for table value > constructor and so row_number assumes the default value (0 in this case). > Fix: Increment the counter to avoid counter using default value. > > diff --git a/sql/sql_tvc.cc b/sql/sql_tvc.cc > index 49f319b3856..a191ec9c328 100644 > --- a/sql/sql_tvc.cc > +++ b/sql/sql_tvc.cc > @@ -422,7 +422,9 @@ bool table_value_constr::exec(SELECT_LEX *sl) > DBUG_ENTER("table_value_constr::exec"); > List_iterator_fast<List_item> li(lists_of_values); > List_item *elem; > + THD *cur_thd= sl->parent_lex->thd; > ha_rows send_records= 0; > + int rc; > > if (select_options & SELECT_DESCRIBE) > DBUG_RETURN(false); > @@ -438,16 +440,21 @@ bool table_value_constr::exec(SELECT_LEX *sl) > > while ((elem= li++)) > { > + cur_thd->get_stmt_da()->inc_current_row_for_warning(); > if (send_records >= sl->master_unit()->lim.get_select_limit()) > - break; > + goto reset_counter_and_exit; Why goto, break worked here just fine, didn't it? Also, note that if you jump from here out of the loop, you'll do if (rc>0) where rc wasn't initialized yet. > - int rc= > - result->send_data_with_check(*elem, sl->master_unit(), send_records); > + rc= result->send_data_with_check(*elem, sl->master_unit(), send_records); > if (!rc) > send_records++; > else if (rc > 0) > - DBUG_RETURN(true); > + goto reset_counter_and_exit; break here too? > } > > +reset_counter_and_exit: > + cur_thd->get_stmt_da()->reset_current_row_for_warning(0); why do you need to reset here? > + if (rc>0) > + DBUG_RETURN(true); > + > if (result->send_eof()) > DBUG_RETURN(true); Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0