- developers - lists.mariadb.org

Re: [Maria-developers] [Commits] cb918231a25: MDEV-11582 InnoDB: Failing assertion: !((field)->vcol_info && !(field)->stored_in_db())
by Alexander Barkov 09 Feb '17

09 Feb '17

Hello Sergei, The patch looks fine. I suggest one improvement. Currently SERIAL can have various redundant, ignored, non-applicable attributes, e.g.: CREATE TABLE t1 (a SERIAL NULL); CREATE TABLE t1 (a SERIAL AUTO_INCREMENT); CREATE TABLE t1 (a SERIAL SERIAL DEFAULT VALUE); CREATE OR REPLACE TABLE t1 (a SERIAL COLLATE `binary`); Can we split the "attribute:" rule to disallow these for SERIAL? Thanks! On 02/09/2017 03:06 PM, serg(a)mariadb.org wrote: > revision-id: cb918231a25ee792749d7e64ebc97e19c0faaf09 (mariadb-10.2.3-192-gcb918231a25) > parent(s): d1cc58f55ad516453aed42b65d71810e1392f745 > author: Sergei Golubchik > committer: Sergei Golubchik > timestamp: 2017-02-09 12:06:15 +0100 > message: > > MDEV-11582 InnoDB: Failing assertion: !((field)->vcol_info && !(field)->stored_in_db()) > > change the parser not to allow SERIAL as a normal data type. > make a special rule for it, where it could be used for define > fields, but not generated fields, not return type of a stored function, etc. > > --- > mysql-test/r/parser.result | 12 ++++++++++++ > mysql-test/t/parser.test | 11 +++++++++++ > sql/sql_yacc.yy | 22 ++++++++++++++-------- > 3 files changed, 37 insertions(+), 8 deletions(-) > > diff --git a/mysql-test/r/parser.result b/mysql-test/r/parser.result > index 7a0a8667350..1292b0bd2e5 100644 > --- a/mysql-test/r/parser.result > +++ b/mysql-test/r/parser.result > @@ -1289,3 +1289,15 @@ SELECT 1; > ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'UNION > SELECT 1' at line 2 > DROP TABLE t1; > +create table t1 (i int, vc serial as (i)); > +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'as (i))' at line 1 > +create function fs() returns serial return 1; > +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'serial return 1' at line 1 > +create table t1 ( id serial ); > +show create table t1; > +Table Create Table > +t1 CREATE TABLE `t1` ( > + `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT, > + UNIQUE KEY `id` (`id`) > +) ENGINE=MyISAM DEFAULT CHARSET=latin1 > +drop table t1; > diff --git a/mysql-test/t/parser.test b/mysql-test/t/parser.test > index 1fa7df7fc3d..dc538db6073 100644 > --- a/mysql-test/t/parser.test > +++ b/mysql-test/t/parser.test > @@ -1321,3 +1321,14 @@ UNION > SELECT 1; > > DROP TABLE t1; > + > +# > +# MDEV-11582 InnoDB: Failing assertion: !((field)->vcol_info && !(field)->stored_in_db()) > +# > +--error ER_PARSE_ERROR > +create table t1 (i int, vc serial as (i)); > +--error ER_PARSE_ERROR > +create function fs() returns serial return 1; > +create table t1 ( id serial ); > +show create table t1; > +drop table t1; > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy > index 12423e71f1f..db22708db43 100644 > --- a/sql/sql_yacc.yy > +++ b/sql/sql_yacc.yy > @@ -6145,8 +6145,7 @@ field_spec: > lex->init_last_field(f, $1.str, NULL); > $<create_field>$= f; > } > - field_type { Lex->set_last_field_type($3); } > - field_def > + field_type_or_serial > { > LEX *lex=Lex; > $$= $<create_field>2; > @@ -6164,6 +6163,19 @@ field_spec: > } > ; > > +field_type_or_serial: > + field_type { Lex->set_last_field_type($1); } field_def > + | SERIAL_SYM > + { > + Lex_field_type_st type; > + type.set(MYSQL_TYPE_LONGLONG); > + Lex->set_last_field_type(type); > + Lex->last_field->flags|= AUTO_INCREMENT_FLAG | NOT_NULL_FLAG > + | UNSIGNED_FLAG | UNIQUE_KEY_FLAG; > + } > + opt_attribute > + ; > + > field_def: > opt_attribute > | opt_generated_always AS virtual_column_func > @@ -6441,12 +6453,6 @@ field_type: > { $$.set(MYSQL_TYPE_SET); } > | LONG_SYM opt_binary > { $$.set(MYSQL_TYPE_MEDIUM_BLOB); } > - | SERIAL_SYM > - { > - $$.set(MYSQL_TYPE_LONGLONG); > - Lex->last_field->flags|= (AUTO_INCREMENT_FLAG | NOT_NULL_FLAG | UNSIGNED_FLAG | > - UNIQUE_KEY_FLAG); > - } > ; > > spatial_type: > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits >

1 0

[Maria-developers] Archive Storage Engine
by Varun Gupta 08 Feb '17

08 Feb '17

Hi, Here is the issue https://jira.mariadb.org/browse/MDEV-11645 on which I need suggestions The observations made are: - in 10.1, varchar endspace was bzero-ed archive depended on it to not pack garbage - in 10.2, varchar endspace is not bzero-ed which breaks the archive SE. Was this change intended? If yes, then we should update ha_archive to fill the garbage parts of the buffer with zeroes that will cause it to be (slightly?) slower but there's no other way. BR, Varun

3 4

Re: [Maria-developers] [Commits] e84a5064674: After review fixes for MDEV-11759.
by Marko Mäkelä 08 Feb '17

08 Feb '17

On Tue, Feb 7, 2017 at 8:12 PM, jan <jan.lindstrom(a)mariadb.com> wrote: > # Write file to make mysql-test-run.pl expect the "crash", but don't > # start it until it's told to > -# We give 30 seconds to do a clean shutdown because we do not want > -# to redo apply the pages of t1.ibd at the time of recovery. > -# We want SQL to initiate the first access to t1.ibd. > # Wait until disconnected. Remove all these messages. > diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc > index fed1cf94d84..4299600709a 100644 > --- a/storage/innobase/fil/fil0crypt.cc > +++ b/storage/innobase/fil/fil0crypt.cc > @@ -982,7 +982,16 @@ fil_space_verify_crypt_checksum( > /* Declare empty pages non-corrupted */ > if (checksum == 0 > && *reinterpret_cast<const ib_uint64_t*>(page + FIL_PAGE_LSN) == 0) { > - return (true); > + /* make sure that the page is really empty */ > + for (ulint i = 0; i < UNIV_PAGE_SIZE; i++) { > + if (page[i] != 0) { > + ib_logf(IB_LOG_LEVEL_INFO, > + "Checksum fields zero but page is not empty."); > + > + return(false); > + } > + } > + return(true); > } Please use buf_page_is_zeroes() instead of duplicating the code. Do not spam the error log if the checksum happens to be 0. I guess we could also have FIL_PAGE_LSN at 0 when importing a file. > @@ -1016,16 +1025,28 @@ fil_space_verify_crypt_checksum( > bool encrypted = (checksum == cchecksum1 || checksum == cchecksum2 > || checksum == BUF_NO_CHECKSUM_MAGIC); > > - /* Old InnoDB versions did not initialize > - FIL_PAGE_FILE_FLUSH_LSN field so there could be garbage > - and above checksum check could produce false positive. > - Thus we also check does the traditional stored > - checksum fields match the calculated one. Both of these > - could naturally produce false positive but then > - we just decrypt the page and after that corrupted > - pages very probable stay corrupted and valid > - pages very probable stay valid. > + /* MySQL 5.6 and MariaDB 10.0 and 10.1 will write an LSN to the > + first page of each system tablespace file at > + FIL_PAGE_FILE_FLUSH_LSN offset. On other pages and in other files, > + the field might have been uninitialized until MySQL 5.5. In MySQL 5.7 > + (and MariaDB Server 10.2.2) WL#7990 stopped writing the field for other > + than page 0 of the system tablespace. I think that we should conduct some research to make the comments more useful and reliable. Looking at the MySQL 3.23.49 source code, I see that we indeed did not initialize FIL_PAGE_FILE_FLUSH_LSN (or FIL_PAGE_ARCH_LOG_NO for that matter) in the system tablespace. The latter field was repurposed as SPACE_ID later. This means that the system tablespace (which was the only option before MySQL 4.1) can contain garbage in the FIL_PAGE_FILE_FLUSH_LSN field, except if we can infer from some other attributes of the system tablespace that it was created by MySQL 5.5 or later. The next interesting MySQL version to check is 4.1.2 where innodb_file_per_table was introduced. The oldest version in the 4.1 series that I was able to find is 4.1.21. It indeed does not make any effort to initialize FIL_PAGE_FILE_FLUSH_LSN on other pages than the first one. Because the buffer pool can be used for general-purpose memory allocations (BUF_BLOCK_MEMORY), it is possible that the FLUSH_LSN field will be written as nonzero garbage, even if we were able to assume that the initially allocated memory was zero-initialized by the operating system. So, the above comment is correct. > + Starting from MariaDB 10.1 the fiedl has been repurposed for > + encryption key_version. Correct the typo "fiedl". > + > + Starting with MySQL 5.7 (and MariaDB Server 10.2), the > + field has been repurposed for SPATIAL INDEX pages. Refer to the field name FIL_RTREE_SPLIT_SEQ_NUM in the comment, so that it can be found easily by grep. > + > + Note that FIL_PAGE_FILE_FLUSH_LSN is not included in the InnoDB page > + checksum. > + > + Thus,FIL_PAGE_FILE_FLUSH_LSN could contain uninitialized value, 0, > + correct key_version or spatial index information, thus we also > + verify page using traditional checksum check to detect if page > + would be valid but not encrypted. > */ Well, 10.1 cannot handle spatial index information. I would simply say: "Thus, FIL_PAGE_FILE_FLUSH_LSN could contain any value. While the field would usually be 0 for pages that are not encrypted, we cannot assume that a nonzero value means that the page is encrypted. Therefore we must validate the page both as encrypted and unencrypted when FIL_PAGE_FILE_FLUSH_LSN does not contain 0." OK to push with these changes. Note: I should still go through the whole logic, to be able to say if we really fixed the issues. I currently believe that more work is needed, but that should be tracked as a separate issue. 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

Re: [Maria-developers] [Commits] 3a8f101: MDEV-11379 - AliSQL: [Feature] Issue#8: SELECT FOR UPDATE WAIT
by Michael Widenius 08 Feb '17

08 Feb '17

Hi! >>>>> "Sergey" == Sergey Vojtovich <svoj(a)mariadb.org> writes: Sergey> revision-id: 3a8f101a173b987b314ba5fcc5c74d80ab56802b (mariadb-10.2.2-160-g3a8f101) Sergey> parent(s): c846ebe9df9adc90a5c25be0dce816c5d4302794 Sergey> committer: Sergey Vojtovich Sergey> timestamp: 2017-02-07 13:27:42 +0400 Sergey> message: Sergey> MDEV-11379 - AliSQL: [Feature] Issue#8: SELECT FOR UPDATE WAIT Sergey> Extended syntax so that it is now possible to set lock_wait_timeout for the Sergey> following statements: Sergey> SELECT ... FOR UPDATE [WAIT n|NOWAIT] Sergey> SELECT ... LOCK IN SHARED MODE [WAIT n|NOWAIT] Sergey> LOCK TABLE ... [WAIT n|NOWAIT] Sergey> This is amended AliSQL patch. We prefer Oracle syntax for [WAIT n|NOWAIT] Sergey> instead of original [WAIT [n]|NO_WAIT]. Agree it's better to use a syntax that someone else is using. Sergey> diff --git a/include/mysql/plugin.h b/include/mysql/plugin.h Sergey> index b3c71c65..b0a5849 100644 Sergey> --- a/include/mysql/plugin.h Sergey> +++ b/include/mysql/plugin.h Sergey> @@ -633,6 +633,7 @@ void thd_storage_lock_wait(MYSQL_THD thd, long long value); Sergey> int thd_tx_isolation(const MYSQL_THD thd); Sergey> int thd_tx_is_read_only(const MYSQL_THD thd); Sergey> int thd_rpl_is_parallel(const MYSQL_THD thd); Sergey> +unsigned long thd_lex_lock_wait_timeout(const MYSQL_THD thd); Sergey> /** Sergey> Create a temporary file. <cut> Sergey> +++ b/sql/sql_base.cc Sergey> @@ -2723,13 +2723,18 @@ Open_table_context::Open_table_context(THD *thd, uint flags) Sergey> :m_thd(thd), Sergey> m_failed_table(NULL), Sergey> m_start_of_statement_svp(thd->mdl_context.mdl_savepoint()), Sergey> - m_timeout(flags & MYSQL_LOCK_IGNORE_TIMEOUT ? Sergey> - LONG_TIMEOUT : thd->variables.lock_wait_timeout), Sergey> m_flags(flags), Sergey> m_action(OT_NO_ACTION), Sergey> m_has_locks(thd->mdl_context.has_locks()), Sergey> m_has_protection_against_grl(FALSE) Sergey> -{} Sergey> +{ Sergey> + if (flags & MYSQL_LOCK_IGNORE_TIMEOUT) Sergey> + m_timeout= LONG_TIMEOUT; Sergey> + else if (thd->lex->lock_wait_timeout == ULONG_MAX) Sergey> + m_timeout= thd->variables.lock_wait_timeout; Sergey> + else Sergey> + m_timeout= thd->lex->lock_wait_timeout; Sergey> +} Instead of adding thd->lex->lock_wait_timeout I suggest that we instead use the existing facititliy of SET STATEMENT innodb_lock_wait_timeout=xxx SELECT This has the following benefits: - We only need one innodb timeout variable, not two - Probably simpler code. Very few changes needed in InnoDB - One will get exactly same behaviour independent of the syntax that is used This would change the task to be implemented the following wat: - If WAIT|NOWAIT is used, change in sql_yacc.yy to use the SET STATEMENT facility to set innodb_lock_wait_timeout and lock_wait_timeout - Add support for nowait (timeout == 0 would mean nowait) I agree that having the new syntax is good as it's used by other databases and simpler than having to set two variables. Regards, Monty

1 0

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