March 2017 - developers - lists.mariadb.org

[Maria-developers] Is it a big? CURSOR does not see changes made by ALTER TABLE
by Alexander Barkov 02 Mar '17

02 Mar '17

Hello Elena, all. I noticed that CURSORs in combination with ALTER TABLE work strange. Is it a known bug? Thanks. -- I create a table with two columns -- And a procedure fetching two columns from this table SET sql_mode=DEFAULT; DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a INT, b VARCHAR(10)); INSERT INTO t1 VALUES (10,20); DROP PROCEDURE IF EXISTS p1; DELIMITER $$ CREATE PROCEDURE p1() BEGIN DECLARE a,b INT; DECLARE cur CURSOR FOR SELECT * FROM t1; OPEN cur; FETCH cur INTO a,b; CLOSE cur; SELECT a,b; END; $$ DELIMITER ; CALL p1(); -- It returns -- +------+------+ -- | a | b | -- +------+------+ -- | 10 | 20 | -- +------+------+ -- So far so good -- Now I add a new column to the table and call the procedure again ALTER TABLE t1 ADD c INT; CALL p1(); -- Woops. It should have failed, because there are now 3 columns in the table -- But it returns the result, like if the table still had 2 columns.

1 1

Re: [Maria-developers] [svoj@mariadb.org: [Commits] 02d155a: MDEV-11418 - AliSQL: [Feature] Issue#1 KILL IDLE TRANSACTIONS]
by Michael Widenius 01 Mar '17

01 Mar '17

Hi! On Tue, Feb 21, 2017 at 2:37 PM, Sergey Vojtovich <svoj(a)mariadb.org> wrote: > Hi Monty, > > Please review this patch. > > Thanks, > Sergey > > ----- Forwarded message from Sergey Vojtovich <svoj(a)mariadb.org> ----- > > Date: Tue, 21 Feb 2017 16:29:36 +0400 > From: Sergey Vojtovich <svoj(a)mariadb.org> > To: commits(a)mariadb.org > Subject: [Commits] 02d155a: MDEV-11418 - AliSQL: [Feature] Issue#1 KILL IDLE TRANSACTIONS > > revision-id: 02d155aeec87aac8a827a29328d68ecb156094cc (mariadb-10.2.2-162-g02d155a) > parent(s): 1563354570c5634cb2b448b47a138b4ceaff1a72 > committer: Sergey Vojtovich > timestamp: 2017-02-21 16:28:42 +0400 > message: > > MDEV-11418 - AliSQL: [Feature] Issue#1 KILL IDLE TRANSACTIONS > > Terminate idle transactions safely in server layer by setting up socket timeout > parameter. Percona provides another patch to resolve similar problem, but it > calls server layer's callback in InnoDB plugin to close THD, which crashes in > some testcases. See https://bugs.launchpad.net/percona-server/+bug/901060 for > more detailed information. > > 1. export parameter trx_idle_timeout to handle all kinds of transactions, the priority is highest > 2. export parameter trx_readonly_idle_timeout to handle read-only transactions > 3. export parameter trx_changes_idle_timeout to handle read-write transactions <cut> > +BEGIN; > +INSERT INTO t1 VALUES (4),(5),(6); > +SELECT * FROM t1; > +ERROR HY000: MySQL server has gone away It's not optimal to get this kind of error for a timeout as it's make it's very hard for the application to know what is going on. An alternative approach would be to instead of closing the connection do a rollback and for next query send an error "transactions aborted and rolled back because of transaction timeout" What do you think about this approach? To get this done, we should remember in 'net' that we are using a transaction timeout (not default) and when we get the timeout we should do: - Do part of the rollback code + unlock tables in THD::cleanup() (Probably move to a sub function) - Remember that we did a timeout rollback. - When we got next statement and timout_rollback was done, give an error, clear flag and wait for next statement. > +++ b/sql/sql_parse.cc > @@ -1209,7 +1209,23 @@ bool do_command(THD *thd) > number of seconds has passed. > */ > if(!thd->skip_wait_timeout) > - my_net_set_read_timeout(net, thd->variables.net_wait_timeout); > + { > + if (thd->in_active_multi_stmt_transaction()) > + { > + bool is_trx_rw= thd->transaction.all.is_trx_read_write(); > + > + if (thd->variables.trx_idle_timeout > 0) > + my_net_set_read_timeout(net, thd->variables.trx_idle_timeout); > + else if (thd->variables.trx_readonly_idle_timeout > 0 && !is_trx_rw) > + my_net_set_read_timeout(net, thd->variables.trx_readonly_idle_timeout); > + else if (thd->variables.trx_changes_idle_timeout > 0 && is_trx_rw) > + my_net_set_read_timeout(net, thd->variables.trx_changes_idle_timeout); > + else > + my_net_set_read_timeout(net, thd->variables.net_wait_timeout); > + } > + else > + my_net_set_read_timeout(net, thd->variables.net_wait_timeout); > + } Please do it this way instead (less code): if (!thd->skip_wait_timeout) { ulong timeout= thd->variables.net_wait_timeout; if (thd->in_active_multi_stmt_transaction()) { bool is_trx_rw= thd->transaction.all.is_trx_read_write(); if (thd->variables.trx_idle_timeout > 0) timeout= thd->variables.trx_idle_timeout; else if (thd->variables.trx_readonly_idle_timeout > 0 && !is_trx_rw) timeout= thd->variables.trx_readonly_idle_timeout; else if (thd->variables.trx_changes_idle_timeout > 0 && is_trx_rw) timeout= thd->variables.trx_changes_idle_timeout; } my_net_set_read_timeout(net, timeout); } Regards, Monty

1 0

Re: [Maria-developers] [Commits] 00c30e5: MDEV-11645: archive.archive fails in buildbot with valgrind (Use of uninitialised value)
by Sergey Petrunia 01 Mar '17

01 Mar '17

Hi Varun, Please * fix the commit comment * add a comment * fix the coding style as indicated below. (and also please conform to the coding style in the future. If its requirements are not clear, lets sit down once and discuss them) On Wed, Mar 01, 2017 at 07:50:01AM +0530, Varun wrote: > revision-id: 00c30e5a234fbd01484cd069c01e97c075025323 (mariadb-10.2.3-283-g00c30e5) > parent(s): b13cee83f9cfcffa98d227c492411a9acec85f42 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2017-03-01 07:46:26 +0530 > message: > > MDEV-11645: archive.archive fails in buildbot with valgrind (Use of uninitialised value) > > Set null values to record_buffer->buffer for the VARCHAR fields What "NULL values"?? The fix is filling with zeros the space beyond the end of VARCHAR values. VARCHAR NULLs are actually ok already as NULL value has its buffer filled with zeros. Please come up with a comment that describes the ix. > > --- > storage/archive/ha_archive.cc | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/storage/archive/ha_archive.cc b/storage/archive/ha_archive.cc > index 9d6d100..5599b43 100644 > --- a/storage/archive/ha_archive.cc > +++ b/storage/archive/ha_archive.cc > @@ -372,10 +372,25 @@ int Archive_share::write_v1_metafile() > > unsigned int ha_archive::pack_row_v1(uchar *record) > { > + uint offset; > uint *blob, *end; > uchar *pos; > DBUG_ENTER("pack_row_v1"); > memcpy(record_buffer->buffer, record, table->s->reclength); Please add here a comment about what this loop is doing. > + for(Field** field= table->field; (*field) ; field++) space needed in 'for (' > + { > + Field *fld= *field; > + if (fld->type() == MYSQL_TYPE_VARCHAR) > + { > + if (!(fld->is_real_null(record - table->record[0]))) > + { > + ptrdiff_t start= (fld->ptr - table->record[0]); > + Field_varstring *const field_var= (Field_varstring *)fld; > + offset= field_var->data_length() + field_var->length_size(); > + bzero(record_buffer->buffer + start + offset, fld->field_length - offset + 1); The above line is longer than 80 chars. should be broken into: bzero(record_buffer->buffer + start + offset, fld->field_length - offset + 1); > + } > + } > + } > pos= record_buffer->buffer + table->s->reclength; > for (blob= table->s->blob_field, end= blob + table->s->blob_fields; > blob != end; blob++) -- BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0