- developers - lists.mariadb.org

Re: [Maria-developers] [Commits] 96403c5e124: Replace some unsigned integers with size_t to avoid type mismatch.
by Sergey Petrunia 31 Mar '17

31 Mar '17

Hi Marko, Please find below the review for patch #1 of the "size_t patches": my comments are marked with 'psergey:' On Thu, Mar 30, 2017 at 02:39:50PM +0300, marko.makela(a)mariadb.com wrote: > commit 96403c5e1240342948cb88600ece14c0c8b72dbc > Author: Marko Mäkelä <marko.makela(a)mariadb.com> > Date: Sat Mar 11 22:03:31 2017 +0200 > > Replace some unsigned integers with size_t to avoid type mismatch. > > diff --git a/sql/key.cc b/sql/key.cc > index bb10e90..9e8693f 100644 > --- a/sql/key.cc > +++ b/sql/key.cc > @@ -1,4 +1,5 @@ > /* Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved. > + Copyright (c) 2017, MariaDB Corporation. > > This program is free software; you can redistribute it and/or modify > it under the terms of the GNU General Public License as published by > @@ -392,7 +393,7 @@ void field_unpack(String *to, Field *field, const uchar *rec, uint max_length, > tmp.length(charpos); > } > if (max_length < field->pack_length()) > - tmp.length(min(tmp.length(),max_length)); > + tmp.length(min(tmp.length(),static_cast<size_t>(max_length))); psergey: This is not necessary as tmp.length() returns a 'size_t', but it's still ok to have I guess. > ErrConvString err(&tmp); > to->append(err.ptr()); > } ... > diff --git a/sql/spatial.cc b/sql/spatial.cc > index 7c9d8bb..399a968 100644 > --- a/sql/spatial.cc > +++ b/sql/spatial.cc > @@ -2278,7 +2278,7 @@ int Gis_multi_line_string::geometry_n(uint32 num, String *result) const > break; > data+= length; > } > - return result->append(data, length, (uint32) 0); > + return result->append(data, length, static_cast<size_t>(0)); psergey: isn't that excessive, can one just write "0" there? > } > > > @@ -2710,8 +2710,9 @@ int Gis_multi_polygon::geometry_n(uint32 num, String *result) const > } while (--num); > if (no_data(data, 0)) // We must check last segment > return 1; > - return result->append(start_of_polygon, (uint32) (data - start_of_polygon), > - (uint32) 0); > + return result->append(start_of_polygon, > + static_cast<size_t>(data - start_of_polygon), > + static_cast<size_t>(0)); psergey: isn't that excessive, can one just write "0" there? > } > > ... > diff --git a/sql/sql_string.h b/sql/sql_string.h > index 18f5f4c..8248f6d 100644 > --- a/sql/sql_string.h > +++ b/sql/sql_string.h > @@ -531,16 +531,19 @@ class String > { > Ptr[str_length++] = c; > } > - void q_append2b(const uint32 n) > + void q_append2b(size_t n) psergey: this just appends the two lower bytes. Is this change really needed? > { > int2store(Ptr + str_length, n); > str_length += 2; > } > - void q_append(const uint32 n) > + void q_append(size_t n) > { psergey: This always appends 4 bytes. It is a little bit scary that this function will optionally discard the upper 4 bytes of the passed values. As far as I understand its usage, this is for writing the data that should always be 4 bytes, regardless of the sizeof(size_t) of the platform we're on. > int4store(Ptr + str_length, n); > str_length += 4; > } > +#if SIZEOF_SIZE_T > 4 > + void q_append(uint32 n) { q_append(static_cast<size_t>(n)); } > +#endif > void q_append(double d) > { > float8store(Ptr + str_length, d); ... > @@ -669,11 +672,11 @@ class String > { > return !sortcmp(this, other, cs); > } > - void q_net_store_length(ulonglong length) > + void q_net_store_length(size_t length) psergey: I dont think this should vary depending on sizeof(size_t). This function is used to construct data to be sent over network, check sql/protocol.cc, net_send_ok(). Apparently network data should have the same number of bits always. > { > DBUG_ASSERT(Alloced_length >= (str_length + net_length_size(length))); > char *pos= (char *) net_store_length((uchar *)(Ptr + str_length), length); > - str_length= uint32(pos - Ptr); > + str_length= static_cast<size_t>(pos - Ptr); > } > void q_net_store_data(const uchar *from, size_t length) > { BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] bb-10.2-compatibility - processlist
by jerome brauge 31 Mar '17

31 Mar '17

Hello Alexander, Here is a patch to add information provided by JDBC (setClientInfo) in information_schema.processlist (ApplicationName, ClientUser, ClientHostname). Do you think that it can be integrate ? Regards, Jérôme.

2 8

[Maria-developers] Please review: MDEV-12416 OOM in create_virtual_tmp_table() makes the server crash
by Alexander Barkov 31 Mar '17

31 Mar '17

Hello Alexey, can you please review a patch for MDEV-12416? Thanks!

1 0

[Maria-developers] Please review MDEV-12415 Remove sp_name::m_qname
by Alexander Barkov 31 Mar '17

31 Mar '17

Hello Sanja, can you please review a patch for MDEV-12415? Thanks!

1 0

[Maria-developers] bb-10.2-compatibility
by jerome brauge 30 Mar '17

30 Mar '17

Hello Alexander, On oracle when an error occurred inside a stored procedure, the error message contains the stack trace of calling procedures. It's very useful when you have tons of procedures. I wrote this little patch to do something like that by adding the stack trace as "notes". To show the correct lines number (according to result of show create procedure) , I had to correct yylineno in sql_les.cc. But now, some tests failed because the line number of errors have changed ! What is your opinion on this patch ? Regards, Jérôme.

2 3

Re: [Maria-developers] bb-10.2-compatibility
by jerome brauge 29 Mar '17

29 Mar '17

Hi Alexander, Here is the patch that corrects the line number of errors. Only tests from test suite for oracle seems to be affected. Row number are correct when test is recorded with "mysql-test-run.pl -record", but wrong with mysql or mysqltest (I don't understand why !) Regard, Jérôme. > -----Message d'origine----- > De : Alexander Barkov [mailto:bar@mariadb.org] > Envoyé : mardi 28 mars 2017 07:42 > À : jerome brauge > Objet : Re: bb-10.2-compatibility > > Hi Jerome, > > > On 03/28/2017 08:45 AM, jerome brauge wrote: > > Alexander, > > Due to this , when i record test for sp-cursor-rowtype.test, I have: > > > > Warning 1265 Data truncated for column 'b' at row 2 > > > > instead of > > > > Warning 1265 Data truncated for column 'b' at row 1 > > > > for test > > # Fetching a cursor into a cursor%ROWTYPE variable, with truncation > > > > I do a test with an older project to see if it's work better. > > It's Ok to record with a wrong "at row XXX" for now, as this bug is not specific > to bb-10.2-compatibility. > > When this problem is fixed in parent branches (10.0, 10.1, 10.2), we'll merge > into bb-10.2-compatibility and fix wrong "at row XXX" > results. > > > Btw, it's very likely that bb-10.2-compatibility will be merged to 10.3 soon, in > the beginning of April. > > > > > > > Jérôme. > > > >> -----Message d'origine----- > >> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : mardi 28 mars > >> 2017 06:10 À : jerome brauge Objet : Re: bb-10.2-compatibility > >> > >> Hello Jerome, > >> > >> On 03/28/2017 12:03 AM, jerome brauge wrote: > >>> Hello Alexander, > >>> Now that my linux build is ok, I have something strange with > >>> warnings on cursors (detected in sp-cursor-rowtype.test) > >>> > >>> The following code > >>> > >>> set sql_mode=oracle; > >>> use test > >>> CREATE or replace TABLE wjbt1 (a INT, b VARCHAR(10)); CREATE or > >>> replace TABLE wjbt2 (a INT, b INT); INSERT INTO wjbt1 VALUES > >>> (10,'11x'); INSERT INTO wjbt1 VALUES (10,'12x'); delimiter $$ CREATE > >>> or replace PROCEDURE pwjb1() AS CURSOR cur1 IS SELECT b FROM > wjbt1; > >>> BEGIN DECLARE res int; BEGIN OPEN cur1; FETCH cur1 INTO res; FETCH > >>> cur1 INTO res; CLOSE cur1; END; END; $$ delimiter ; call pwjb1(); > >>> show warnings; > >>> > >>> returns: > >>> +---------+------+------------------------------------------+ > >>> | Level | Code | Message | > >>> +---------+------+------------------------------------------+ > >>> | Warning | 1265 | Data truncated for column 'res' at row 3 | > >>> | Warning > >>> | | 1265 | Data truncated for column 'res' at row 3 | > >>> +---------+------+------------------------------------------+ > >>> > >>> I done this test on Linux and Windows with the same behavious. > >>> I don't understand why. Do you have an idea ? > >> > >> > >> Truncation seems to be correct, > >> because the procedure converts '11x' and '12x' to INT. > >> > >> But 'at row 3' seems to be wrong. > >> > >> I reported the problem as: > >> > >> https://jira.mariadb.org/browse/MDEV-12377 > >> > >> Looks like an old bug. MariaDB-10.0 exposes the same behavior. > >> > >> > >>> > >>> Regards, > >>> Jérôme. > >>>

2 1

Re: [Maria-developers] CREATE SEQUENCE is coming
by Daniel Black 28 Mar '17

28 Mar '17

On 28/03/17 07:31, Vladislav Vaintroub wrote: > > > On 3/27/2017 6:15 AM, Daniel Black wrote: >> On 26/03/17 22:11, Michael Widenius wrote: >>> There is still a lot of work to fix edge cases, but in the current >>> implementation most major things seams to work... >>> >>> Comments, suggestions or questions? >> NICE! >> >> How will this value be exposed to getGeneratedKeys in >> mariadb-connector-j? > I do not think it needs to. Statement.getGeneratedKeys returns generated > auto-increment values on insert, the same ulonglong that is in OK > packet, the same thing that LAST_INSERT_ID() is returning. > no sequences involved Ah, ok. I was miss understanding how Postgres does its sequences. I found this quick summary from 2011 (date in http response header) http://www.neilconway.org/docs/sequences/ Oracle ref: http://docs.oracle.com/cd/B28359_01/server.111/b28286/pseudocolumns002.htm#… uses sequence.CURRVAL, sequence.NEXTVAL https://www.postgresql.org/docs/9.6/static/functions-sequence.html As a note itseams PREVIOUS VALUE FOR seems to be a DB2 construct. Postgres uses CURRVAL(sequence) which to me sounds more sound. I guess MDEV-10422 INSERT RETURNING is the alternative. All options work. Thanks for writing this. So it seems they show up as full table where querying them is pretty harmless? I suspect if things like pt-table-sync start trying to modify them ugly things could happen.

1 0

Re: [Maria-developers] CREATE SEQUENCE is coming
by Daniel Black 27 Mar '17

27 Mar '17

On 26/03/17 22:11, Michael Widenius wrote: > There is still a lot of work to fix edge cases, but in the current > implementation most major things seams to work... > > Comments, suggestions or questions? NICE! How will this value be exposed to getGeneratedKeys in mariadb-connector-j?

2 1

[Maria-developers] [GSOC] ColumnStore: Add full support for DECIMAL
by Даниил Медведев 27 Mar '17

27 Mar '17

Hello, I took part in GSoC last year, and closed one of MariaDB tasks. Now I want to continue programming with MariaDB developers and solve a new task. I considered some of proposed tasks, and was interested in "ColumnStore: Add full support for DECIMAL". I think this task is doable in the given time and I might solve it. P.S. My github link: https://github.com/medvdanil Regards, Daniil Medvedev

2 5

Re: [Maria-developers] 99d2448: MDEV-9255 Add generation_expression to information_schema.columns.
by Sergei Golubchik 27 Mar '17

27 Mar '17

Hi, Alexey! On Mar 27, Alexey Botchkov wrote: > revision-id: 99d24484e474ce2f52fe58fc1c053f0aa186f094 (mariadb-10.2.4-84-g99d2448) > parent(s): ad7da60deda2cf97e60118a58f50f648abde6448 > committer: Alexey Botchkov > timestamp: 2017-03-27 17:44:20 +0400 > message: > > MDEV-9255 Add generation_expression to information_schema.columns. > > Added IS_GENERATED and GENERATION_EXPRESSION columns required by > the SQL standard. ok to push! Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0