- developers - lists.mariadb.org

Re: [Maria-developers] Please review MDEV-12001 Split Item_func_round::fix_length_and_dec to virtual methods in Type_handler
by Alexander Barkov 07 Feb '17

07 Feb '17

Hello Alexey, Thanks for your suggestions. An improved version is attached. Please see comments inline: On 02/07/2017 03:51 PM, Alexey Botchkov wrote: > Hi, Alexander. > > Basically i approve this patch. It seems in logic with the previous work > in that area. > Two comments though >> -void Item_func_round::fix_length_and_dec() >> +void Item_func_round::fix_length_and_dec_decimal(uint decimals_to_set) >> { >> - int decimals_to_set; >> - longlong val1; > .... >> + decimals= decimals_to_set; >> + int decimals_delta= args[0]->decimals - decimals_to_set; >> + int precision= args[0]->decimal_precision(); >> + int length_increase= ((decimals_delta <= 0) || truncate) ? 0 : 1; >> + precision-= decimals_delta - length_increase; >> + max_length= my_decimal_precision_to_length_no_truncation(precision, > ..... > > Please move declarations of the 'int decimals_delta' 'int precision' and > 'int length_increase' > to the beginning of the function. It does make reading easier, and our > rules require it. > I'd also replace two lines with one: >> + int precision= args[0]->decimal_precision(); >> + precision-= decimals_delta - length_increase; > to > precision= args[0]->decimal_precision() + length_increase - > decimals_delta; Done. Thanks. It now looks better. > > +void Item_func_round::fix_arg_decimal() > +{ > if (!args[1]->const_item()) > { > > Since we started changing this code we can add some fixes to it. > Firstly i think 'const_item()' should be replaced with the > 'basic_const_item()' there. > The later is faster and safer in case when the 'const item' is a > complicated (yet constant) subquery. As discussed on IRC, it can introduce a behavior change, which is not desirable under terms of this task. > Secondly it'd be nicer if we do the 'if (args[1]->const_item)' the first > choice. (i mean not !args[1]->...) > but if you don't like that you can skip this suggestion :) Done. > > Best regards. > HF. > > > On Mon, Feb 6, 2017 at 11:26 AM, Alexander Barkov <bar(a)mariadb.org > <mailto:bar@mariadb.org>> wrote: > > Hello Alexey, > > Can you please review a patch for MDEV-12001? > > Thanks! > >

1 0

Re: [Maria-developers] [Commits] 90010cb4cbc: MDEV-11974: MariaDB 10.2 encryption does not support spatial indexes
by Marko Mäkelä 07 Feb '17

07 Feb '17

On Tue, Feb 7, 2017 at 3:56 PM, jan <jan.lindstrom(a)mariadb.com> wrote: > revision-id: 90010cb4cbcc538181f043dbeccd776776bdd5a4 (mariadb-10.2.3-171-g90010cb4cbc) > parent(s): 92bbf4ad0477e09bcc86907696cd114ef42e6914 > author: Jan Lindström > committer: Jan Lindström > timestamp: 2017-02-07 15:55:01 +0200 > message: > > MDEV-11974: MariaDB 10.2 encryption does not support spatial indexes Add a short explanation why we cannot support encrypted spatial indexes. > fil0crypt.cc: Do not encrypt page if page type is > FIL_PAGE_RTREE (this is required for background > encryption innodb-encrypt-tables=ON) > > ha_innodb.cc: check_table_options() Do not allow creating > table with ENCRYPTED=YES if table contains spatial index. Instead of file names, please use the function names in the commit comment. > +# > +# Index creation > +# > +CREATE TABLE t1 (pk INT PRIMARY KEY AUTO_INCREMENT, > +c VARCHAR(256), coordinate POINT NOT NULL) ENCRYPTED=YES ENGINE=INNODB; > +--replace_regex /#sql-[0-9a-f_]*`/#sql-temporary`/ > +--error ER_CANT_CREATE_TABLE > +CREATE SPATIAL INDEX b on t1(coordinate); > +--replace_regex /#sql-[0-9a-f_]*`/#sql-temporary`/ > +--error ER_CANT_CREATE_TABLE > +ALTER TABLE t1 ADD SPATIAL INDEX b(coordinate); > +DROP TABLE t1; Test this also with default encryption. > +--echo # Wait max 10 min for key encryption threads to encrypt all space > +let $cnt=600; > +while ($cnt) > +{ > + let $success=`SELECT COUNT(*) = 0 FROM INFORMATION_SCHEMA.INNODB_TABLESPACES_ENCRYPTION WHERE MIN_KEY_VERSION = 0`; Use --let $wait_timeout=600 --let $wait_condition= SELECT COUNT(*) = 0 FROM INFORMATION_SCHEMA.INNODB_TABLESPACES_ENCRYPTION WHERE MIN_KEY_VERSION = 0 --source include/wait_condition.inc > +DROP TABLE t1, t2; > \ No newline at end of file Add the missing newline to the end of the .test file. Please demonstrate in some way that page compression actually works with SPATIAL INDEX. Can you for example run --suite=innodb_gis with some --mysqld=--innodb… option that will cause all tables to be compressed by default? Marko -- DON’T MISS M|17 April 11 - 12, 2017 The Conrad Hotel New York City https://m17.mariadb.com/ Marko Mäkelä, Lead Developer InnoDB MariaDB Corporation

1 0

[Maria-developers] Please review MDEV-12001 Split Item_func_round::fix_length_and_dec to virtual methods in Type_handler
by Alexander Barkov 06 Feb '17

06 Feb '17

Hello Alexey, Can you please review a patch for MDEV-12001? Thanks!

1 0

Re: [Maria-developers] [Commits] 787610a: MDEV-10766: Queries which start with WITH clause do not get inserted into query cache
by Sergey Petrunia 04 Feb '17

04 Feb '17

Hi Varun, On Fri, Feb 03, 2017 at 08:20:51PM +0530, Varun wrote: > revision-id: 787610a1f5cec04311fd16f6c1de87171e5cfdd4 (mariadb-10.2.3-159-g787610a) > parent(s): bc12d993d7bc94a9533028a258afc7e4ceb21e92 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2017-02-03 20:17:16 +0530 > message: > > MDEV-10766: Queries which start with WITH clause do not get inserted into query cache > > Added conditions so that the WITH queries are also added to the query cache > > --- > mysql-test/r/query_cache.result | 30 ++++++++++++++++++++++++++++++ > mysql-test/t/query_cache.test | 14 ++++++++++++++ > sql/sql_cache.cc | 5 ++++- > 3 files changed, 48 insertions(+), 1 deletion(-) > > diff --git a/mysql-test/r/query_cache.result b/mysql-test/r/query_cache.result > index d34c96c..fa169d4 100644 > --- a/mysql-test/r/query_cache.result > +++ b/mysql-test/r/query_cache.result > @@ -2136,6 +2136,36 @@ Qcache_hits 1 > use test; > drop database `foo.bar`; > End of 10.0 tests > +FLUSH USER_STATISTICS; > +FLUSH CLIENT_STATISTICS; I'm wondering what is the reason for the above? Qcache_XXX status variables are flushed with FLUSH STATUS. > +flush status; > +show status like "Qcache_inserts"; > +Variable_name Value > +Qcache_inserts 0 > +create table t1 (i int); > +with cte as (select * from t1) select * from cte; > +i > +show status like "Qcache_queries_in_cache"; > +Variable_name Value > +Qcache_queries_in_cache 1 > +show status like "Qcache_inserts"; > +Variable_name Value > +Qcache_inserts 1 > +show status like "Qcache_hits"; > +Variable_name Value > +Qcache_hits 0 > +with cte as (select * from t1) select * from cte; > +i > +show status like "Qcache_queries_in_cache"; > +Variable_name Value > +Qcache_queries_in_cache 1 > +show status like "Qcache_inserts"; > +Variable_name Value > +Qcache_inserts 1 > +show status like "Qcache_hits"; > +Variable_name Value > +Qcache_hits 1 > +drop table t1; > restore defaults > SET GLOBAL query_cache_type= default; > SET GLOBAL query_cache_size= default; BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 1

[Maria-developers] the patch for mdev-9976 broke some tests
by Alexander Barkov 01 Feb '17

01 Feb '17

Hello Igor, These tests fails in 10.2 due to ROWS now being a reserved word: binlog_encryption.binlog_write_error rpl.rpl_killed_ddl binlog.binlog_write_error Can you please fix this? Thanks!

1 0

[Maria-developers] MDEV-10142 - bb-10.2-compatibility
by jerome brauge 01 Feb '17

01 Feb '17

Hello, To build this branch on windows (Visual Studio Express 2015), I have to add "#pragma warning( disable : 4099 )" in sql_lex.h. But I don't know if it's the right solution. I have some questions: - I read the comment of Michael Widenius on subtask MDEV-10574. I think that it's important to change behavior of string functions whose returns a string (like rtrim, ltrim, substring, concat, ...) to return null instead of ''. Without this, we have to use UDF instead native functions and it's never good from a performance point of view. Have you made a decision ? - On Oracle, we can easily disable/enable triggers. I found an old request for this on jira : MDEV-7579. As you already manage order_action now, perhaps you can simply use negative orders to indicate disabled triggers. - do you have planning to implement operator || to concat strings ? Best regards, J. Brauge

3 16

[Maria-developers] valgrind + SSL
by Alexander Barkov 01 Feb '17

01 Feb '17

Hello Elena, You wrote on IRC: On 01/31/2017 08:10 PM, Slack wrote: > bar: hi! We have this in 10.0 valgrind, what do you think about it? > http://buildbot.askmonty.org/buildbot/builders/work-amd64-valgrind/builds/9… > > i'm asking you because you fixed a similar-looking problem once: > https://jira.mariadb.org/browse/MDEV-4890 I'm not very familiar with this part. But IIUC, two things could happen: - Somebody changed the code in mysqld.cc, so the SSL cleanup routines do not get called correctly any more. - Somebody is wrong inside the library itself. When did this problem first appear? After some commit? Or after the SSQL library update/change on the build box? Is it specific to 10.0? When I was fixing MDEV-4890, I fixed both: - added some cleanup function calls - added the second SSL suppression The problem was reproducible on my box. Now I cannot reproduce the problem on my box. I tried: cmake . -DCMAKE_BUILD_TYPE=Debug -DWITH_VALGRIND=1 ./mtr --valgrind binlog.binlog_max_extension ./mtr --valgrind federated.error_on_close-8313 ./mtr --valgrind rpl.rpl_old_master Note, we have these valgrind suppressions: > { > OpenSSL still reachable. > Memcheck:Leak > fun:*alloc > fun:CRYPTO_malloc > fun:sk_new > obj:*libssl* > fun:SSL_COMP_get_compression_methods > fun:SSL_library_init > } > > > { > OpenSSL still reachable. > Memcheck:Leak > fun:*alloc > fun:CRYPTO_malloc > obj:*libssl* > fun:SSL_COMP_get_compression_methods > fun:SSL_library_init > } Do you know why the problem does not get suppressed? Perhaps we should try to remove "obj:*libssl*" ? But before doing this it would be nice to know when the problem first appeared.

1 0

Re: [Maria-developers] Please review MDEV-9979 Keywords UNBOUNDED, PRECEDING, FOLLOWING, TIES, OTHERS should be non-reserved
by Vicențiu Ciorbaru 31 Jan '17

31 Jan '17

Hi Alexander, OK to push. Please add the version of the standard you used to check these keywords as non-reserved both to the commit message, as well as the keywords themselves. I used 2011 standard draft to check. Vicențiu On Mon, 23 Jan 2017 at 09:28 Alexander Barkov <bar(a)mariadb.com> wrote: > Hello Vicențiu, > > Can you please review a patch for MDEV-9979. > > Thanks! >

2 1

[Maria-developers] Please review MDEV-11692 Comparison data type aggregation for pluggable data types
by Alexander Barkov 31 Jan '17

31 Jan '17

Hello Vicențiu, can you please review a patch for MDEV-11692. The patch is for bb-10.2-ext. Thanks!

2 1

Re: [Maria-developers] [Commits] ef590fa: MDEV-7409 On RBR, extend the PROCESSLIST info to include at least the name of the recently used table
by Daniel Black 31 Jan '17

31 Jan '17

On 24/01/17 04:30, SachinSetiya wrote: > revision-id: ef590faa0e21d25a2dc5153f938135612b17ecbc (mariadb-10.1.20-31-gef590fa) > thd_proc_info(thd, message); > + DBUG_EXECUTE_IF("should_sleep_for_mdev7409",{ > + my_sleep(500000); > + };); Shouldn't these be DEBUG_SYNC_C("do_exec_row"); and then use debug_sync to generate test synchronisation. > int error= write_row(rgi, slave_exec_mode == SLAVE_EXEC_MODE_IDEMPOTENT); > thd_proc_info(thd, tmp); > + thd->db= tmp_db; > > if (error && !thd->is_error()) > { > @@ -12371,32 +12380,45 @@ int Delete_rows_log_event::do_exec_row(rpl_group_info *rgi) > { > int error; > const char *tmp= thd->get_proc_info(); > - const char *message= "Delete_rows_log_event::find_row()"; > + char *tmp_db= thd->db; > + char *message, msg[128]; > + my_snprintf(msg, sizeof(msg),"Delete_rows_log_event::find_row() on table %s", > + m_table->s->table_name.str); > + thd->db= m_table->s->db.str; > + message= msg; Why assign a pointer? Isn't defining message as a char[128] sufficient? Thanks for writing this feature.

3 3