- developers - lists.mariadb.org

[Maria-developers] Progress Report - Week 3
by Mohammed Hammaad Mateen 25 May '20

25 May '20

Greetings, Hope you are safe and doing great, This post describes the things I've done in my third week [ 18-24 May ] of Community Bonding Period under the mentor-ship of Sergei Golubchik and Oleksandr Byelkin for GSoC-20. The tasks taken up for this week was to study and analyze: *INSERT.... RETURNING:* • *TableList* consists of 1 table instance to use, where each instance is a structure consisting of db argument, table name argument, alias argument and lock type argument. *Referenced this … [View More]

1 0

Re: [Maria-developers] MDEV-22461: JOIN::make_aggr_tables_info(): Assertion `select_options & (1ULL << 17)' failed.
by Sergey Petrunia 24 May '20

24 May '20

Hi Varun, - Please add a testcase to the testsuite - Please address a few cosmetic comments below - Ok to push after addressed. > commit 5e448b77a3812e65623c0a1214049322ace3aacf > Author: Varun Gupta <varun.gupta(a)mariadb.com> > Date: Tue May 5 20:44:43 2020 +0530 > > MDEV-22461: JOIN::make_aggr_tables_info(): Assertion `select_options & (1ULL << 17)' failed. > > A temporary table is needed for window function computation but if only a … [View More]NAMED WINDOW SPEC > is used and there is no window function, then there is no need to create a temporary > table as there is no stage to compute WINDOW FUNCTION > > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index c601946cfa0..0a1d0c2dbcc 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -2014,11 +2014,16 @@ JOIN::optimize_inner() > } > > need_tmp= test_if_need_tmp_table(); > - //TODO this could probably go in test_if_need_tmp_table. > - if (this->select_lex->window_specs.elements > 0) { > - need_tmp= TRUE; > + > + /* > + If window functions are present then we can't have simple_order set to > + TRUE as the window function needs a temp table for compuatation. typo: compuatation. > + ORDER BY is computed after the window function computation is done, so > + the sort would be done on the temp table. > + */ > + if (this->select_lex->have_window_funcs()) Is there a need to have "this->select_lex"? I think not, please remove, as it only confuses the reader. > simple_order= FALSE; > - } > + > > /* > If the hint FORCE INDEX FOR ORDER BY/GROUP BY is used for the table > diff --git a/sql/sql_select.h b/sql/sql_select.h > index 0e011c9267a..7a892c1af89 100644 > --- a/sql/sql_select.h > +++ b/sql/sql_select.h > @@ -1645,7 +1645,8 @@ class JOIN :public Sql_alloc > ((select_distinct || !simple_order || !simple_group) || > (group_list && order) || > MY_TEST(select_options & OPTION_BUFFER_RESULT))) || > - (rollup.state != ROLLUP::STATE_NONE && select_distinct)); > + (rollup.state != ROLLUP::STATE_NONE && select_distinct) || > + select_lex->have_window_funcs()); Please amend the function comment above this code to cover the new condition as well. > } > bool choose_subquery_plan(table_map join_tables); > void get_partial_cost_and_fanout(int end_tab_idx, > BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog [View Less]

1 0

Re: [Maria-developers] b22a28c2295: fixup! 3fe5cd5e1785e3e8de7add9977a1c2ddd403538b
by Michael Widenius 22 May '20

22 May '20

Hi! On Fri, May 22, 2020 at 3:27 PM Andrei Elkin <andrei.elkin(a)mariadb.com> wrote: <cut> CORNER CASES: read-only, pure myisam, binlog-*, @@skip_log_bin, etc > > Aria just makes yet another previously unknown use case of an engine > that produces THD::ha_info but does not support 2pc, which the assert > implied. > > To explain more, the original block > > #ifndef DBUG_OFF > for (ha_info= thd->transaction.all.ha_list; rw_count > 1 &&… [View More]amp; ha_info; > ha_info= ha_info->next()) > DBUG_ASSERT(ha_info->ht() != binlog_hton); > #endif > > claims there most be no binlog hton in a transaction consisting of more > than 1 hton:s, *when* (at this point) this transaction has not been > binlogged yet. > So combination of binlog + Innodb hton would be raise the assert, to > question "why the heck binlogging has not been done yet?!". > > Aria is different from Innodb in this context in that binlogging > was done at the end of the statement, so to miss > `cache_mngr->need_unlog' flagging (which is at xa prepare time logging). Thanks for the explanation, we should have had that in the code. > I think we should fix the assert rather than to remove. This way: > > cat > assert.diff <<. > diff --git a/sql/log.cc b/sql/log.cc > index 792c6bb1a99..aaf1fae1cd6 100644 > --- a/sql/log.cc > +++ b/sql/log.cc > @@ -10124,6 +10124,16 @@ int TC_LOG_BINLOG::unlog_xa_prepare(THD *thd, bool all) > Ha_trx_info *ha_info; > uint rw_count= ha_count_rw_all(thd, &ha_info); > bool rc= false; > +#ifndef DBUG_OFF > + bool no_binlog= true, exist_no_2pc= false; > + for (ha_info= thd->transaction->all.ha_list; rw_count > 1 && ha_info; > + ha_info= ha_info->next()) > + { > + no_binlog= no_binlog && ha_info->ht() != binlog_hton; > + exist_no_2pc= exist_no_2pc || !ha_info->ht()->prepare; > + } > + DBUG_ASSERT(no_binlog || exist_no_2pc); > +#endif > > if (rw_count > 0) > { > . > > I tested it briefly with running XA:s on combination of engines > including. On which tree did you test? bb-10.5-aria? In the end, after discussions on slack, we ended with: #ifndef DBUG_OFF if (rw_count > 1) { /* There must be no binlog_hton used in a transaction consisting of more than 1 engine, *when* (at this point) this transaction has not been binlogged. The one exception is if there is an engine without a prepare method, as in this case the engine doesn't support XA and we have to ignore this check. */ bool binlog= false, exist_hton_without_prepare= false; for (ha_info= thd->transaction->all.ha_list; ha_info; ha_info= ha_info->next()) { if (ha_info->ht() == binlog_hton) binlog= true; if (!ha_info->ht()->prepare) exist_hton_without_prepare= true; } DBUG_ASSERT(!binlog || exist_hton_without_prepare); } #endif > > The reason it was not hit > > is that before Aria was not treated as transactional we could only > > come here in case of errors > > and that code was was apparently not tested, at least with binary logging on. > > > > > With Aria we could come here in case of rollback and we got assert for > > cases that was perfectly ok. > > Well, what 'rollback' do you mean? The function is invoked only > for ha_prepare(). As far as I remember, I come into this code in some edge case when something failed that normally never fails. I think the issue was that Aria doesn't have a prepare handler, which caused issues in ha_prepare(), but not sure. Anyway, we now have a solution for this. Regards, Monty [View Less]

1 0

Re: [Maria-developers] acbe14b122c: Aria will now register it's transactions
by Sergei Golubchik 22 May '20

22 May '20

Hi, Michael! See below On May 20, Michael Widenius wrote: > revision-id: acbe14b122c (mariadb-10.5.2-255-gacbe14b122c) > parent(s): 48a758696bf > author: Michael Widenius <monty(a)mariadb.com> > committer: Michael Widenius <monty(a)mariadb.com> > timestamp: 2020-05-19 17:52:17 +0300 > message: > > Aria will now register it's transactions > > MDEV-22531 Remove maria::implicit_commit() > diff --git a/sql/sql_class.h b/sql/sql_class.h > index … [View More]d4a95fa3fd8..a7a071f7cdb 100644 > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -5149,6 +5151,40 @@ class THD: public THD_count, /* this must be first */ > > }; > > + > +/* > + Start a new independent transaction for the THD. > + The old one is stored in this object and restored when calling > + restore_old_transaction() or when the object is freed > +*/ > + > +class start_new_trans > +{ > + /* container for handler's private per-connection data */ > + Ha_data old_ha_data[MAX_HA]; > + struct THD::st_transactions *old_transaction, new_transaction; > + Open_tables_backup open_tables_state_backup; > + MDL_savepoint mdl_savepoint; > + PSI_transaction_locker *m_transaction_psi; > + THD *org_thd; > + uint in_sub_stmt; > + uint server_status; > + > +public: > + start_new_trans(THD *thd); > + ~start_new_trans() > + { > + destroy(); > + } > + void destroy() > + { > + if (org_thd) // Safety > + restore_old_transaction(); > + new_transaction.free(); > + } > + void restore_old_transaction(); interesting. You made restore_old_transaction() public and you use it in many places. Why not to use destroy() instead? When one would want to use restore_old_transaction() instead of destroy()? > +}; > + > /** A short cut for thd->get_stmt_da()->set_ok_status(). */ > > inline void > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index dda8e00f6bf..51d7380f622 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -5742,6 +5744,90 @@ void THD::mark_transaction_to_rollback(bool all) > } > > > +/** > + Commit the whole transaction (both statment and all) > + > + This is used mainly to commit an independent transaction, > + like reading system tables. > + > + @return 0 0k > + @return <>0 error code. my_error() has been called() > +*/ > + > +int THD::commit_whole_transaction_and_close_tables() > +{ > + int error, error2; > + DBUG_ENTER("THD::commit_whole_transaction_and_close_tables"); > + > + /* > + This can only happened if we failed to open any table in the > + new transaction > + */ > + if (!open_tables) > + DBUG_RETURN(0); Generally, I think, it should still end the transaction here. Or add an assert that there is no active transaction at the moment. > + > + /* > + Ensure table was locked (opened with open_and_lock_tables()). If not > + the THD can't be part of any transactions and doesn't have to call > + this function. > + */ > + DBUG_ASSERT(lock); > + > + error= ha_commit_trans(this, FALSE); > + /* This will call external_lock to unlock all tables */ > + if ((error2= mysql_unlock_tables(this, lock))) > + { > + my_error(ER_ERROR_DURING_COMMIT, MYF(0), error2); > + error= error2; > + } > + lock= 0; > + if ((error2= ha_commit_trans(this, TRUE))) > + error= error2; > + close_thread_tables(this); I wonder why you're doing it in that specific order. commit(stmt)-unlock-commit(all)-close > + DBUG_RETURN(error); > +} > + > +/** > + Start a new independent transaction > +*/ > + > +start_new_trans::start_new_trans(THD *thd) > +{ > + org_thd= thd; > + mdl_savepoint= thd->mdl_context.mdl_savepoint(); > + memcpy(old_ha_data, thd->ha_data, sizeof(old_ha_data)); > + thd->reset_n_backup_open_tables_state(&open_tables_state_backup); > + bzero(thd->ha_data, sizeof(thd->ha_data)); > + old_transaction= thd->transaction; > + thd->transaction= &new_transaction; > + new_transaction.on= 1; > + in_sub_stmt= thd->in_sub_stmt; > + thd->in_sub_stmt= 0; > + server_status= thd->server_status; > + m_transaction_psi= thd->m_transaction_psi; > + thd->m_transaction_psi= 0; > + thd->server_status&= ~(SERVER_STATUS_IN_TRANS | > + SERVER_STATUS_IN_TRANS_READONLY); > + thd->server_status|= SERVER_STATUS_AUTOCOMMIT; > +} Few thoughts: 1. If you need to save and restore _all that_ then, perhaps, all that should be inside st_transactions ? 2. strictly speaking, ha_data is _per connection_. If you just bzero it, the engine will think it's a new connection, and you cannot just overwrite it on restore without hton->close_connection. A strictly "proper" solution would be to introduce ha_data per transaction in addition to what we have now. But it looks like an overkill. So I'd just add close_system_tables() now into restore_old_transaction() A strictly "proper" solution would be to introduce ha_data per transaction in addition to what we have now. But it looks like an overkill. So I'd just add ha_close_connection() now into restore_old_transaction() and that's all. I see that you've added free_transaction() call, but isn't it redundant? ha_close_connection() does that now. Otherwise very good, thanks, I cannot wait to start using it more for other features. > + > + > +void start_new_trans::restore_old_transaction() > +{ > + org_thd->transaction= old_transaction; > + org_thd->restore_backup_open_tables_state(&open_tables_state_backup); > + ha_free_transactions(org_thd); > + memcpy(org_thd->ha_data, old_ha_data, sizeof(old_ha_data)); > + org_thd->mdl_context.rollback_to_savepoint(mdl_savepoint); > + org_thd->in_sub_stmt= in_sub_stmt; > + org_thd->server_status= server_status; > + if (org_thd->m_transaction_psi) > + MYSQL_COMMIT_TRANSACTION(org_thd->m_transaction_psi); > + org_thd->m_transaction_psi= m_transaction_psi; > + org_thd= 0; > +} > + > + > /** > Decide on logging format to use for the statement and issue errors > or warnings as needed. The decision depends on the following > diff --git a/sql/event_db_repository.cc b/sql/event_db_repository.cc > index af43d92dea7..82b3968de85 100644 > --- a/sql/event_db_repository.cc > +++ b/sql/event_db_repository.cc > @@ -742,7 +748,8 @@ Event_db_repository::create_event(THD *thd, Event_parse_data *parse_data, > ret= 0; > > end: > - close_thread_tables(thd); > + if (table) > + thd->commit_whole_transaction_and_close_tables(); You're checking twice. Here in the caller, and the first thing inside commit_whole_transaction_and_close_tables(). Here and in many places. If you want to check in the caller, there's no need to check inside? You can add an assert instead, can't you? > thd->mdl_context.rollback_to_savepoint(mdl_savepoint); > > thd->variables.sql_mode= saved_mode; > @@ -1117,22 +1124,20 @@ update_timing_fields_for_event(THD *thd, > TABLE *table= NULL; > Field **fields; > int ret= 1; > - enum_binlog_format save_binlog_format; > MYSQL_TIME time; > DBUG_ENTER("Event_db_repository::update_timing_fields_for_event"); > > - /* > - Turn off row binlogging of event timing updates. These are not used > - for RBR of events replicated to the slave. > - */ > - save_binlog_format= thd->set_current_stmt_binlog_format_stmt(); > - > DBUG_ASSERT(thd->security_ctx->master_access & PRIV_IGNORE_READ_ONLY); > > if (open_event_table(thd, TL_WRITE, &table)) > goto end; Why do you reuse a current transaction, instead of creating a new one as everywhere above? > > fields= table->field; > + /* > + Turn off row binlogging of event timing updates. These are not used > + for RBR of events replicated to the slave. > + */ > + table->file->row_logging= 0; > > if (find_named_event(event_db_name, event_name, table)) > goto end; > diff --git a/sql/handler.h b/sql/handler.h > index e4903172c33..ebe23b97062 100644 > --- a/sql/handler.h > +++ b/sql/handler.h > @@ -1765,6 +1766,12 @@ handlerton *ha_default_tmp_handlerton(THD *thd); > */ > #define HTON_TABLE_MAY_NOT_EXIST_ON_SLAVE (1 << 15) > > +/* > + True if handler cannot roolback transactions. If not true, the transaction > + will be put in the transactional binlog cache. > +*/ > +#define HTON_NO_ROLLBACK (1 << 16) How is it different from HA_PERSISTENT_TABLE? > + > class Ha_trx_info; > > struct THD_TRANS > diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc > index 582c9bb110b..0c53bdc5bbe 100644 > --- a/sql/ha_partition.cc > +++ b/sql/ha_partition.cc > @@ -11016,7 +11016,7 @@ int ha_partition::check_misplaced_rows(uint read_part_id, bool do_repair) > If the engine supports transactions, the failure will be > rollbacked. if you're changing it anyway, can you also change rollbacked -> rolled back ? thanks. > */ > - if (!m_file[correct_part_id]->has_transactions()) > + if (!m_file[correct_part_id]->has_transactions_and_rollback()) > { > /* Log this error, so the DBA can notice it and fix it! */ > sql_print_error("Table '%-192s' failed to move/insert a row" > diff --git a/sql/handler.cc b/sql/handler.cc > index 39841cc28d7..e3f5773717d 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -4514,7 +4515,6 @@ void handler::mark_trx_read_write_internal() > */ > if (ha_info->is_started()) > { > - DBUG_ASSERT(has_transaction_manager()); there should be *some* assert here, I think > /* > table_share can be NULL in ha_delete_table(). See implementation > of standalone function ha_delete_table() in sql_base.cc. > diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt > index 250da9948a0..1f3bf212d3f 100644 > --- a/sql/share/errmsg-utf8.txt > +++ b/sql/share/errmsg-utf8.txt > @@ -7961,3 +7961,5 @@ ER_KEY_CONTAINS_PERIOD_FIELDS > eng "Key %`s cannot explicitly include column %`s" > ER_KEY_CANT_HAVE_WITHOUT_OVERLAPS > eng "Key %`s cannot have WITHOUT OVERLAPS" > +ER_DATA_WAS_COMMITED_UNDER_ROLLBACK > + eng "Engine %s does not support rollback. Changes where commited during rollback call" A confusing error message. I'd just say "table is not transactional". This is the user point of view difference between a "transactional engine" like InnoDB and "crash safe engine" like Aria. It's very confusing to invent a new category of "transactional engines that cannot roll back" even if internally Aria is treated like one. > diff --git a/sql/sp.cc b/sql/sp.cc > index 51bbeeef368..1629290eb73 100644 > --- a/sql/sp.cc > +++ b/sql/sp.cc > @@ -470,27 +471,29 @@ static Proc_table_intact proc_table_intact; > currently open tables will be saved, and from which will be > restored when we will end work with mysql.proc. > > + NOTES > + On must have a start_new_trans object active when calling this function I think this: DBUG_ASSERT(thd->transcation != &thd->default_transaction); could be a bit safer than a NOTE :) > + > @retval > 0 Error > @retval > \# Pointer to TABLE object of mysql.proc > */ > > -TABLE *open_proc_table_for_read(THD *thd, Open_tables_backup *backup) > +TABLE *open_proc_table_for_read(THD *thd) > { > TABLE_LIST table; > - > DBUG_ENTER("open_proc_table_for_read"); > > table.init_one_table(&MYSQL_SCHEMA_NAME, &MYSQL_PROC_NAME, NULL, TL_READ); > > - if (open_system_tables_for_read(thd, &table, backup)) > + if (open_system_tables_for_read(thd, &table)) > DBUG_RETURN(NULL); > > if (!proc_table_intact.check(table.table, &proc_table_def)) > DBUG_RETURN(table.table); > > - close_system_tables(thd, backup); > + thd->commit_whole_transaction_and_close_tables(); > > DBUG_RETURN(NULL); > } > @@ -517,7 +520,8 @@ static TABLE *open_proc_table_for_update(THD *thd) > MDL_savepoint mdl_savepoint= thd->mdl_context.mdl_savepoint(); > DBUG_ENTER("open_proc_table_for_update"); > same? also need start_new_trans? > > - table_list.init_one_table(&MYSQL_SCHEMA_NAME, &MYSQL_PROC_NAME, NULL, TL_WRITE); > + table_list.init_one_table(&MYSQL_SCHEMA_NAME, &MYSQL_PROC_NAME, NULL, > + TL_WRITE); > > if (!(table= open_system_table_for_update(thd, &table_list))) > DBUG_RETURN(NULL); > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > index 6c938670fdc..325a0f1d41d 100644 > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -4259,7 +4259,7 @@ bool open_tables(THD *thd, const DDL_options_st &options, > list, we still need to call open_and_process_routine() to take > MDL locks on the routines. > */ > - if (thd->locked_tables_mode <= LTM_LOCK_TABLES) > + if (thd->locked_tables_mode <= LTM_LOCK_TABLES && *sroutine_to_open) why? > { > /* > Process elements of the prelocking set which are present there > @@ -8887,17 +8887,16 @@ bool is_equal(const LEX_CSTRING *a, const LEX_CSTRING *b) > open_system_tables_for_read() > thd Thread context. > table_list List of tables to open. > - backup Pointer to Open_tables_state instance where > - information about currently open tables will be > - saved, and from which will be restored when we will > - end work with system tables. > > NOTES > + Caller should have used start_new_trans object to start a new > + transcation when reading system tables. assert, may be? > + > Thanks to restrictions which we put on opening and locking of > system tables for writing, we can open and lock them for reading > - even when we already have some other tables open and locked. One > - must call close_system_tables() to close systems tables opened > - with this call. > + even when we already have some other tables open and locked. > + One should call thd->commit_whole_transaction_and_close_tables() > + to close systems tables opened with this call. > > NOTES > In some situations we use this function to open system tables for > diff --git a/sql/sql_help.cc b/sql/sql_help.cc > index c9307b578fc..3ccab553bfe 100644 > --- a/sql/sql_help.cc > +++ b/sql/sql_help.cc > @@ -709,8 +709,9 @@ static bool mysqld_help_internal(THD *thd, const char *mask) > Reset and backup the current open tables state to > make it possible. > */ > - Open_tables_backup open_tables_state_backup; > - if (open_system_tables_for_read(thd, tables, &open_tables_state_backup)) > + start_new_trans new_trans(thd); I'm starting to think that this (and in some other places) is rather redundant. Transactions can be expensive, the overhead being quite big. It seems like a overkill to wrap read-only system table accesses in a separate transaction, they could as well be performed as a part of the parent transaction. Write accesses has to be wrapped in a dedicated transaction, of course. > + > + if (open_system_tables_for_read(thd, tables)) > goto error2; > > /* > diff --git a/sql/sql_show.cc b/sql/sql_show.cc > index 2528134f4ee..db5b4d1c5fd 100644 > --- a/sql/sql_show.cc > +++ b/sql/sql_show.cc > @@ -6105,7 +6105,7 @@ static my_bool iter_schema_engines(THD *thd, plugin_ref plugin, > table->field[1]->store(option_name, strlen(option_name), scs); > table->field[2]->store(plugin_decl(plugin)->descr, > strlen(plugin_decl(plugin)->descr), scs); > - tmp= &yesno[MY_TEST(hton->commit)]; > + tmp= &yesno[MY_TEST(hton->commit && !(hton->flags & HTON_NO_ROLLBACK))]; Why not to say that hton->rollback==NULL means no rollback? Then you won't need a new flag for that. And you'll remove redundancy, what would it mean if HTON_NO_ROLLBACK is present, but hton->rollback!=NULL? Or the other way around, HTON_NO_ROLLBACK is not present, but hton->rollback==NULL? > table->field[3]->store(tmp->str, tmp->length, scs); > table->field[3]->set_notnull(); > tmp= &yesno[MY_TEST(hton->prepare)]; > diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc > index 55e8e52c052..7a817aea97e 100644 > --- a/sql/sql_statistics.cc > +++ b/sql/sql_statistics.cc > @@ -230,17 +230,17 @@ index_stat_def= {INDEX_STAT_N_FIELDS, index_stat_fields, 4, index_stat_pk_col}; > Open all statistical tables and lock them > */ > > -static int open_stat_tables(THD *thd, TABLE_LIST *tables, > - Open_tables_backup *backup, bool for_write) > +static int open_stat_tables(THD *thd, TABLE_LIST *tables, bool for_write) > { > int rc; > - > Dummy_error_handler deh; // suppress errors > + DBUG_ASSERT(thd->transaction != &thd->default_transaction); I see, you already use such an assert :) > + > thd->push_internal_handler(&deh); > init_table_list_for_stat_tables(tables, for_write); > init_mdl_requests(tables); > thd->in_sub_stmt|= SUB_STMT_STAT_TABLES; > - rc= open_system_tables_for_read(thd, tables, backup); > + rc= open_system_tables_for_read(thd, tables); > thd->in_sub_stmt&= ~SUB_STMT_STAT_TABLES; > thd->pop_internal_handler(); > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org [View Less]

2 1

Re: [Maria-developers] b22a28c2295: fixup! 3fe5cd5e1785e3e8de7add9977a1c2ddd403538b
by Sergei Golubchik 22 May '20

22 May '20

Hi, Michael! On May 20, Michael Widenius wrote: > revision-id: b22a28c2295 (mariadb-10.5.2-256-gb22a28c2295) > parent(s): acbe14b122c > author: Michael Widenius <monty(a)mariadb.com> > committer: Michael Widenius <monty(a)mariadb.com> > timestamp: 2020-05-19 17:52:18 +0300 > message: > > fixup! 3fe5cd5e1785e3e8de7add9977a1c2ddd403538b > > MDEV-22607 Assertion `ha_info->ht() != binlog_hton' failed in > MYSQL_BIN_LOG::unlog_xa_prepare … [View More]

2 1

Re: [Maria-developers] 20d54b09830: Update galera to work with independent sub transactions
by Sergei Golubchik 22 May '20

22 May '20

Hi, Michael! I see, you basically disable wsrep for these out-of-band transactions. I'm sure it works fine now, with Aria. But will it work with InnoDB? On May 20, Michael Widenius wrote: > revision-id: 20d54b09830 (mariadb-10.5.2-257-g20d54b09830) > parent(s): b22a28c2295 > author: Michael Widenius <monty(a)mariadb.com> > committer: Michael Widenius <monty(a)mariadb.com> > timestamp: 2020-05-19 17:55:00 +0300 > message: > > Update galera to work with … [View More]

2 1

Re: [Maria-developers] c5238d7b3b7: MDEV-22545: my_vsnprintf behaves not as in C standard
by Sergei Golubchik 20 May '20

20 May '20

Hi, Oleksandr! On May 20, Oleksandr Byelkin wrote: > revision-id: c5238d7b3b7 (mariadb-10.5.3-200-gc5238d7b3b7) > parent(s): 8f9c53586bf > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2020-05-20 08:35:12 +0200 > message: > > MDEV-22545: my_vsnprintf behaves not as in C standard > > Added parameter %T for string which should be visibly truncated. > > diff --git a/include/mysql/… [View More]service_my_snprintf.h b/include/mysql/service_my_snprintf.h > index bd1f069c527..94a9e07dcbb 100644 > --- a/include/mysql/service_my_snprintf.h > +++ b/include/mysql/service_my_snprintf.h > @@ -69,6 +70,9 @@ > Format 'M': takes one integer, prints this integer, space, double quote > error message, double quote. In other words > printf("%M", n) === printf("%d \"%s\"", n, strerror(n)) > + > + Format 'T': takes string and print it like s but if the strints shoud be should > + truncated puts "..." at the end. > */ > > #ifdef __cplusplus > diff --git a/mysql-test/main/frm-debug.result b/mysql-test/main/frm-debug.result > index caf344e241d..332d7e00a8f 100644 > --- a/mysql-test/main/frm-debug.result > +++ b/mysql-test/main/frm-debug.result > @@ -13,8 +13,8 @@ CREATE TABLE t1 (c01 INT, c02 CHAR(20), c03 TEXT, c04 DOUBLE); > Warnings: > Note 1105 build_frm_image: Field data type info length: 14 > Note 1105 DBUG: [0] name='c01' type_info='' > -Note 1105 DBUG: [1] name='c02' type_info='xc...' > -Note 1105 DBUG: [2] name='c03' type_info='xb...' > +Note 1105 DBUG: [1] name='c02' type_info='xchar' > +Note 1105 DBUG: [2] name='c03' type_info='xblob' good > Note 1105 DBUG: [3] name='c04' type_info='' > SET SESSION debug_dbug="-d,frm_data_type_info_emulate"; > SET SESSION debug_dbug="-d,frm_data_type_info"; > diff --git a/mysql-test/main/xml.result b/mysql-test/main/xml.result > index efaca961b4a..84dbd68438f 100644 > --- a/mysql-test/main/xml.result > +++ b/mysql-test/main/xml.result > @@ -1007,7 +1007,7 @@ Warning 1292 Truncated incorrect INTEGER value: 'string ' > Warning 1292 Truncated incorrect INTEGER value: 'string ' > DROP PROCEDURE spxml; > select UpdateXML('<a>a</a>',repeat('a b ',1000),''); > -ERROR HY000: XPATH syntax error: 'b a b a b a b a b a b a b a b...' > +ERROR HY000: XPATH syntax error: 'b a b a b a b a b a b a b a b a ' this doesn't look right, the string is truncated, isn't it? > select ExtractValue('<a>a</a>', '/a[@x=@y0123456789_0123456789_0123456789_0123456789]'); > ERROR HY000: XPATH error: comparison of two nodesets is not supported: '=@y0123456789_0123456789_0123...' > select ExtractValue('<a>a</a>', '/a[@x=$y0123456789_0123456789_0123456789_0123456789]'); > @@ -1110,9 +1110,9 @@ NULL > # Bug#57279 updatexml dies with: Assertion failed: str_arg[length] == 0 > # > SELECT UPDATEXML(NULL, (LPAD(0.1111E-15, '2011', 1)), 1); > -ERROR 22007: Illegal double '111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111...' value found during parsing > +ERROR 22007: Illegal double '111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111' value found during parsing same here, lpad appends 2011 characters, so this "double" is clearly truncated. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org [View Less]

1 0

Re: [Maria-developers] b97e45651d1: MDEV-16937 Strict SQL with system versioned tables causes issues
by Sergei Golubchik 19 May '20

19 May '20

Hi, Aleksey! First - is commit b97e45651d1 the one I was supposed to review? On May 17, Aleksey Midenkov wrote: > revision-id: b97e45651d1 (mariadb-10.4.7-33-gb97e45651d1) > parent(s): 7587975bf06 > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Aleksey Midenkov <midenok(a)gmail.com> > timestamp: 2019-08-19 11:58:07 +0300 > message: > > MDEV-16937 Strict SQL with system versioned tables causes issues > > Vers SQL: respect system fields … [View More]in NO_ZERO_DATE mode. > > This is the subject for refactoring in tempesta-tech/mariadb#379 > > diff --git a/mysql-test/suite/versioning/engines.combinations b/mysql-test/suite/versioning/engines.combinations > index 26b5bab23f1..57e2af6cd06 100644 > --- a/mysql-test/suite/versioning/engines.combinations > +++ b/mysql-test/suite/versioning/engines.combinations > @@ -7,5 +7,10 @@ default-storage-engine=innodb > [myisam] > default-storage-engine=myisam > > +[traditional] > +default-storage-engine=myisam > +sql-mode=traditional I'm not sure about it. It's, of course, safe to run all tests with every possible settings, but it really blows up testing times. May be it'd make sense to have a more targeted test here? > + > [heap] > default-storage-engine=memory > + > diff --git a/mysql-test/suite/versioning/r/select.result b/mysql-test/suite/versioning/r/select.result > index 3569268ce1d..68df246af6b 100644 > --- a/mysql-test/suite/versioning/r/select.result > +++ b/mysql-test/suite/versioning/r/select.result > @@ -45,7 +45,7 @@ ASOF_x y > 7 107 > 8 108 > 9 109 > -select x as FROMTO_x, y from t1 for system_time from timestamp '0-0-0 0:0:0' to timestamp @t1; > +select x as FROMTO_x, y from t1 for system_time from timestamp '1970-01-01 00:00:00' to timestamp @t1; Does this test use a fixed time zone? If not, is 1970-01-01 00:00:00 a valid timestamp in all time zones? If unsure, better use 1971, that will definitely be fine everywhere. > FROMTO_x y > 0 100 > 1 101 > diff --git a/sql/field.cc b/sql/field.cc > index 969c32a5180..f5580239a9f 100644 > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -11139,6 +11139,13 @@ bool Field::save_in_field_default_value(bool view_error_processing) > { > THD *thd= table->in_use; > > + /* > + TODO: refactor setting the system fields via default_value mechanism. > + This condition will go away as well as other conditions with VERS_SYSTEM_FIELD. > + */ MDEV? There's a pull request, but it is marked as obsolete > + if (flags & VERS_SYSTEM_FIELD) > + return false; > + > if (unlikely(flags & NO_DEFAULT_VALUE_FLAG && > real_type() != MYSQL_TYPE_ENUM)) > { > diff --git a/sql/unireg.cc b/sql/unireg.cc > index d019b5f8a75..484ad0334e4 100644 > --- a/sql/unireg.cc > +++ b/sql/unireg.cc > @@ -1060,7 +1060,8 @@ static bool make_empty_rec(THD *thd, uchar *buff, uint table_options, > !f_bit_as_char(field->pack_flag)) > null_count+= field->length & 7; > > - error= make_empty_rec_store_default(thd, regfield, field->default_value); > + if (!(field->flags & VERS_SYSTEM_FIELD)) > + error= make_empty_rec_store_default(thd, regfield, field->default_value); will the field be initialized at all? It should have something, even bzero will do, but we shouldn't dump uninited memory to frm. > delete regfield; // Avoid memory leaks > if (error) > goto err; > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org [View Less]

2 3

[Maria-developers] Progress report - Week 2
by Mohammed Hammaad Mateen 18 May '20

18 May '20

Greetings, Hope you are safe and doing great, This post describes the things I've done in my second week [ 11-17 May ] of Community Bonding Period under the mentor-ship of Sergei Golubchik and Oleksandr Byelkin for GSoC-20. Tasks done this week were mostly the continuation of the leftover tasks from the previous week and some related study. Things I've done include: 1:* I was caught up in analysing how was DELETE .... RETURNING implemented, which led me to read from:* • *sp_head.cc* : this … [View More]

1 0

Re: [Maria-developers] 82b6a5cb85f: MDEV-22556: Incorrect result for window function when using encrypt-tmp-files=ON
by Sergei Golubchik 17 May '20

17 May '20

Hi, Varun! Ok to push. One comment below On May 17, Varun Gupta wrote: > revision-id: 82b6a5cb85f (mariadb-10.4.11-192-g82b6a5cb85f) > parent(s): 3bfe305c5cd > author: Varun Gupta <varun.gupta(a)mariadb.com> > committer: Varun Gupta <varun.gupta(a)mariadb.com> > timestamp: 2020-05-15 02:37:16 +0530 > message: > > MDEV-22556: Incorrect result for window function when using encrypt-tmp-files=ON > > diff --git a/mysql-test/suite/encryption/t/… [View More]

1 0