- developers - lists.mariadb.org

Re: [Maria-developers] MDEV-12506 Split Item_func_min_max::fix_length_and_dec() into methods in Type_handler
by Alexander Barkov 22 Apr '17

22 Apr '17

Hi Alexey, Thanks for your review! On 04/22/2017 06:49 PM, Alexey Botchkov wrote: > Ok to push. > I'd only move the bool Item_func_min_max::fix_attributes(Item **items, > uint nitems) function > to the item_func.h as it's just the typehandler method call now. I'm afraid this is not possible. This method uses current_thd->is_error(), and the structure of THD is not visible in item_func.h. > > Best regards. > HF > > > On Sat, Apr 15, 2017 at 7:23 AM, Alexander Barkov <bar(a)mariadb.org > <mailto:bar@mariadb.org>> wrote: > > Hello Alexey, > > Can you please review a patch for MDEV-12506? > > Thanks. > >

1 0

[Maria-developers] Refactoring in LEX_STRING and LEX_CSTRING
by Alexander Barkov 21 Apr '17

21 Apr '17

Hello all, Monty is now doing refactoring: 1. Replacing a lot of LEX_STRING to LEX_CSTRING. 2. Fixing functions and methods that have arguments of types: a. pointer "const LEX_STRING *name" and b. reference "const LEX_STRING &name". to the same style. Monty chose passing by pointer (2a). I think that: - #1 is good. - #2 is good, in the sense of changing to the same style. But I think that the choice of "passing by pointer" in #2 is bad. Passing by reference for LEX_STRING/LEX_CSTRING related things would be better. With the "passing by pointer" approach whenever one needs to pass a LEX_STRING to a function accepting LEX_CSTRING, one will have to do: void func1(const LEX_STRING *str) { LEX_CSTRING tmp= {str->str, str->length}; func2(&tmp); } I'd propose two things instead: 1. Add a class: class Lex_cstring: public LEX_CSTRING { public: Lex_cstring() {} /*Uninitialized */ Lex_cstring(const char *str, size_t length) { LEX_STRING::str= str; LEX_STRING::length= length; } Lex_cstring(const LEX_STRING *str) { LEX_STRING::str= str->str; LEX_STRING::length= str->length; } }; 2. Instead of "const LEX_CSTRING *str" we use "const Lex_cstring &str" all around C++ code. So the above example with passing a LEX_STRING to something that expect a LEX_CSTRING would be simplified to void func1(const LEX_STRING *str) { func2(str); } 3. In some classes now we have duplicate methods and constructors like this: class XXX { public: XXX(const char *str, size_t length) { ... } XXX(const LEX_STRING *str) { ... } void method(const char *str, size_t length) {...} void method(const LEX_STRING *str) {...} }; to make it possible to call like this: new XXX(str, length); new XXX(lex_str_ptr) new XXX(&lex_str) method(str, length); method(lex_str_ptr); method(&lex_str); We change this to "passing by reference" and remove duplicates: class XXX { public: XXX(const Lex_cstring &str) { ... } void method(const Lex_cstring &str) {...} }; After this change we can call like this: new XXX(Lex_cstring(str, length)); new XXX(*lex_str_ptr) new XXX(lex_str); method(Lex_cstring(str, length)); method(*lex_str_ptr); method(lex_str); Notice, not needs for separate methods accepting two arguments "const char *str, size_t length".

4 5

[Maria-developers] How to report progress for InnoDB background tasks (esp. at startup/shutdown)?
by Marko Mäkelä 18 Apr '17

18 Apr '17

Hi all, MariaDB Server features a nice-looking interface for reporting progress on long-running SQL operations: https://mariadb.com/kb/en/mariadb/progress-reporting/ MySQL 5.7 implemented something similar for reporting progress for ALTER TABLE…ALGORITHM=INPLACE. I should reimplement that for MariaDB using the existing MariaDB infrastructure. I wonder if the same interface could be used for tracking progress on periodic background tasks that might not be directly related to the execution of any SQL statements. InnoDB has a number of background tasks, such as: * purging unneeded history (removing undo log records and delete-marked index records) * rolling back recovered transactions (mainly, if the server was killed) * index defragmentation, encryption key rotation * redo log checkpoints (flush all dirty pages up to the checkpoint LSN; most interesting at shutdown) The above would need to be monitored also while the server is shutting down. I understand that at startup, we would probably need a different progress reporting interfaces for the very first InnoDB task of redo log apply. Everything else can be done in the background while the server is accepting connections. Related to this, I wonder if it is possible or feasible to change the SQL layer logic so that at shutdown, it will not issue ROLLBACK (as part of KILL QUERY) to the currently active connections, but let the subsequent startup do the rollback in the background. This should allow faster shutdowns and restarts. Best regards, Marko -- DON’T MISS M|17April 11 - 12, 2017 The Conrad Hotel New York City https://m17.mariadb.com/ Marko Mäkelä, Lead Developer InnoDB MariaDB Corporation

2 2

[Maria-developers] Item_func_group_concat::fix_fields - indentation and check positioning?
by Daniel Black 18 Apr '17

18 Apr '17

I came across the following gcc warning (6.3.1) sql/item_sum.cc: In member function ‘virtual bool Item_func_group_concat::fix_fields(THD*, Item**)’: sql/item_sum.cc:3478:5: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation] if ((!args[i]->fixed && ^~ sql/item_sum.cc:3482:7: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the ‘if’ with_subselect|= args[i]->with_subselect; ^~~~~~~~~~~~~~ The with_subselect line was added in 950abd526 In context it is: Item_func_group_concat::fix_fields(THD *thd, Item **ref) { ... for (i=0 ; i < arg_count ; i++) { if ((!args[i]->fixed && args[i]->fix_fields(thd, args + i)) || args[i]->check_cols(1)) return TRUE; with_subselect|= args[i]->with_subselect; } While removing the indent would fix the warning, I'd like someone to check this line should be after the IF statement rather than before.

2 1

Re: [Maria-developers] [JIRA] (MDEV-12499) ARCHIVE tables still don't show up in I_S.TABLES when storage engine is not loaded
by Jean-Francois B. Gagne 17 Apr '17

17 Apr '17

Hi Sergei, (sorry for cross-posting to MariaDB.dev and MariaDB.discuss: I think this interest both audiences.) Sergei Golubchik commented on MDEV-12499: > ----------------------------------------- > > This only happens if you created an ARCHIVE table and never used it before > the upgrade. No INSERT, no SELECT, not even {{SELECT * FROM > INFORMATION_SCHEMA.TABLES}} — nothing. > > ARCHIVE engine supports table discovery, that is, the engine is always the > authoritative source of table metadata, while {{.frm}} files are merely a > metadata cache. When you do {{CREATE TABLE}} the server does not know > whether the engine wants the table metadata to be cached. This happens when > the table is opened for the first time. > > If there is no {{.frm}} file and the ARCHIVE engine is not loaded, the > server has no way of knowing that the ARCHIVE table exists. That's why it's > not shown anywhere. > > If you do {{SELECT * FROM test.a1}} before the upgrade, the engine will > tell the server to create the {{.frm}} file and on the upgrade the plugin > will be loaded. In a practical use case, it's enough to have at least one > ARCHIVE table that was accessed at least once after it was created, and the > ARCHIVE plugin will be loaded on upgrade. > Thanks for the detailed info above. In this, you write ".frm files are merely a metadata cache", is there any sort of documentation about when .frm files are useful, useless, and might get out of sync ? This probably deserves a page in the KB (and Google does not know much on "mariadb frm file site:mariadb.com"). However, your comment above raises the following question: if "ARCHIVE engine supports table discovery, that is, the engine is always the authoritative source of table metadata", where is this information stored ? I guess that unless the plugin is loaded, this information is not accessible. This causes a chicken and egg problem which MDEV-12499 was trying to solve: how can mysql_upgrade knows if it should add BLACKHOLE and ARCHIVE to the list of plugin to load (see MDEV-11942). Finally, you marked MDEV-12499 as "Won't Fix", do you have another solution for ARCHIVE and MDEV-11942 ? Many thanks, JFG

2 1

[Maria-developers] MDEV-12514 Split Item_temporal_func::fix_length_and_dec()
by Alexander Barkov 17 Apr '17

17 Apr '17

Hi Sanja, Please review a patch for MDEV-12514, also fixing this bug: MDEV-12515 Wrong value when storing DATE_ADD() and ADDTIME() to a numeric field Thanks!

1 1

[Maria-developers] MDEV-12506 Split Item_func_min_max::fix_length_and_dec() into methods in Type_handler
by Alexander Barkov 15 Apr '17

15 Apr '17

Hello Alexey, Can you please review a patch for MDEV-12506? Thanks.

1 0

[Maria-developers] Please review patch for 12017 and 12018.
by Sachin Setiya 09 Apr '17

09 Apr '17

Hi Monty!, Please review patch for 12017 and 12018 . this patch solves both the problem. -- Regards Sachin Setiya Software Engineer at MariaDB

2 1

[Maria-developers] compilation failure: my_atomic.h
by Alexander Barkov 08 Apr '17

08 Apr '17

Hi Sergey, kvm-bintar-centos5-x86 fails to compile: http://buildbot.askmonty.org/buildbot/grid?category=main&branch=10.3 with this error: /home/buildbot/buildbot/build/include/my_atomic.h:121:2: error: #error atomic ops for this platform are not implemented Can you please fix this?

3 4

[Maria-developers] Adding "proxy protocol" support to MariaDB
by Jose 07 Apr '17

07 Apr '17

Hi! As you may know, the proxy protocol [1,2] is a way to communicate a client's IP address to a backend server sitting behind a proxy front-end. It is the equivalent of the X-Forwarded-For HTTP header, but done at Layer 4. This protocol was originally proposed by one of the HAProxy developers and its support is becoming more widespread. Percona, Amazon AWS, HAProxy, NGINX, and others. Mariadb's Jira shows an unassigned feature request for it, but with no one apparently assigned to it [3]. Percona supports proxy protocol [4]. For the configuration part, Percona defines a global variable that gives the IP address of the proxy front-ends that are authorized to send a proxy protocol header. Percona's handler code for the proxy protocol (minus the global variable handling) is in the vio/viosockets.c file [4] I looked at Percona's implementation. This file looks very similar to MariaDb's own viosockets.c file [5] so it may a potential good insertion point. Percona's software license seems to be permisive enough for reusing / adapting their code [6]. Please correct me if I'm wrong but it seems no one is currently working on this feature right now. I'd like to know if you would be interested in help side-porting Percona's proxy protocol code to MariaDB. I'll probably need some help or pointers on how to add a global variable, though. Best regards, --jose [1] Proxy protocol documentation https://blog.haproxy.com/haproxy/proxy-protocol/ [2] Proxy protocol specification www.haproxy.org/download/1.8/doc/proxy-protocol.txt [3] Mariadb feature request for proxy protocol https://jira.mariadb.org/browse/MDEV-11159?jql=project%20%3D%20MDEV%20AND%2… [3] Percona proxy protocol manual page https://www.percona.com/doc/percona-server/5.6/flexibility/proxy_protocol_s… [4] Percona proxy protocol handler code https://github.com/percona/percona-server/blob/1e2f003a5bd48763c27e37542d97… https://github.com/percona/percona-server/blob/1e2f003a5bd48763c27e37542d97… [5] mariadb potential insertion pode for proxy protocol code https://github.com/MariaDB/server/blob/bb2c1a52c61706dde8c525a8887f2d364c0d… [6] Percona software license https://www.percona.com/doc/percona-server/LATEST/copyright.html

3 3

Re: [Maria-developers] b4a223e3381: MDEV-10355 Weird error message upon CREATE TABLE with DEFAULT
by Sergei Golubchik 07 Apr '17

07 Apr '17

Hi, Jacob! On Mar 28, jacob.mathew(a)mariadb.com wrote: > revision-id: b4a223e338152c8764c980aeae59fb927eea1b0b (mariadb-10.2.4-98-gb4a223e3381) > parent(s): 3f7455c03008b2428fa9b364b1add4c36834ff71 > author: Jacob Mathew > committer: Jacob Mathew > timestamp: 2017-03-28 17:41:56 -0700 > message: > > MDEV-10355 Weird error message upon CREATE TABLE with DEFAULT > > Fixed handling of default values with cached temporal functions so that the > CREATE TABLE statement now succeeds. > Added clearing of cached function values to trigger function reevaluation > when updating default values and when updating virtual column values. > Fixed the error message. > Added quoting of date/time values in cases when this was omitted. > Added a test case. > Updated test result files that include date/time values that were > previously unquoted. > > diff --git a/mysql-test/r/func_default_between_temporal.result b/mysql-test/r/func_default_between_temporal.result > new file mode 100644 > index 00000000000..db9656f4d37 > --- /dev/null > +++ b/mysql-test/r/func_default_between_temporal.result > @@ -0,0 +1,15 @@ > +CREATE OR REPLACE TABLE t1 ( col INT DEFAULT ( 1 LIKE ( NOW() BETWEEN '2000-01-01' AND '2012-12-12' ) ) ); > +SHOW CREATE TABLE t1; > +Table Create Table > +t1 CREATE TABLE `t1` ( > + `col` int(11) DEFAULT (1 like (current_timestamp() between '2000-01-01 00:00:00' and '2012-12-12 00:00:00')) > +) ENGINE=MyISAM DEFAULT CHARSET=latin1 > +SET timestamp = UNIX_TIMESTAMP( '2004-04-04' ); > +INSERT INTO t1 VALUES( DEFAULT ); > +SET timestamp = DEFAULT; better use another constant here, don't rely on the current timestamp being between 2000 and 2012. I know, sounds crazy. But debian's reproducible builds (*) do everything with the system time seriously off. (*) https://wiki.debian.org/ReproducibleBuilds > +INSERT INTO t1 VALUES( DEFAULT ); > +SELECT * FROM t1; > +col > +1 > +0 > +DROP TABLE t1; > diff --git a/mysql-test/t/func_default_between_temporal.test b/mysql-test/t/func_default_between_temporal.test please, rename the test somehow or put it into default.test. "func_default" means it's about a DEFAULT() function, but your test is not. > new file mode 100644 > index 00000000000..6cbd5a342c6 > --- /dev/null > +++ b/mysql-test/t/func_default_between_temporal.test > @@ -0,0 +1,11 @@ Start the test with a header, like # # MDEV-10355 Weird error message upon CREATE TABLE with DEFAULT # > +CREATE OR REPLACE TABLE t1 ( col INT DEFAULT ( 1 LIKE ( NOW() BETWEEN '2000-01-01' AND '2012-12-12' ) ) ); > +SHOW CREATE TABLE t1; > + > +SET timestamp = UNIX_TIMESTAMP( '2004-04-04' ); > +INSERT INTO t1 VALUES( DEFAULT ); > +SET timestamp = DEFAULT; > +INSERT INTO t1 VALUES( DEFAULT ); > + > +SELECT * FROM t1; > + > +DROP TABLE t1; > diff --git a/sql/item.cc b/sql/item.cc > index c34d27fa63b..ee3e43b60ad 100644 > --- a/sql/item.cc > +++ b/sql/item.cc > @@ -616,8 +616,9 @@ void Item::print_value(String *str) > str->append("NULL"); > else > { > - switch (result_type()) { > + switch (cmp_type()) { > case STRING_RESULT: > + case TIME_RESULT: okay, that fixes quoting. good. > append_unescaped(str, ptr->ptr(), ptr->length()); > break; > case DECIMAL_RESULT: > @@ -9433,17 +9433,26 @@ void Item_cache::store(Item *item) > > void Item_cache::print(String *str, enum_query_type query_type) > { > - if (value_cached) > + if ( example && ( example->type() == FUNC_ITEM ) && // There is a cached function item > + ( query_type & QT_ITEM_IDENT_SKIP_TABLE_NAMES ) ) // Caller is show-create-table 1. better test for QT_ITEM_CACHE_WRAPPER_SKIP_DETAILS 2. Why the test for FUNC_ITEM? I don't see any logical reason why skipping of "<cache>" should only apply to functions. and please watch the spacing around parentheses, try to use the same coding style as the rest of the code > { > - print_value(str); > - return; > + // Instead of "cache" or the cached value, print the function name > + example->print( str, query_type ); > } > - str->append(STRING_WITH_LEN("<cache>(")); > - if (example) > - example->print(str, query_type); > else > - Item::print(str, query_type); > - str->append(')'); > + { > + if (value_cached) Indentation. please, try to use the same coding style as the rest of the code > + { > + print_value(str); > + return; > + } It's not very logical that the value is not printed if one doesn't want "<cache>". I think this method could be like this: ================================== void Item_cache::print(String *str, enum_query_type query_type) { if (value_cached && !(query_type & QT_NO_DATA_EXPANSION)) { print_value(str); return; } if (!(query_type & QT_ITEM_CACHE_WRAPPER_SKIP_DETAILS)) str->append(STRING_WITH_LEN("<cache>(")); if (example) example->print(str, query_type); else Item::print(str, query_type); if (!(query_type & QT_ITEM_CACHE_WRAPPER_SKIP_DETAILS)) str->append(')'); } ================================== That is, print the value unless QT_NO_DATA_EXPANSION is specified. Print "<cache>" unless QT_ITEM_CACHE_WRAPPER_SKIP_DETAILS is specified. And Virtual_column_info::print() should also specify QT_NO_DATA_EXPANSION. > + str->append(STRING_WITH_LEN("<cache>(")); > + if (example) > + example->print(str, query_type); > + else > + Item::print(str, query_type); > + str->append(')'); > + } > } > > /** > diff --git a/sql/item.h b/sql/item.h > index 67640ce5f4d..222f8580cb4 100644 > --- a/sql/item.h > +++ b/sql/item.h > @@ -1653,6 +1653,7 @@ class Item: public Value_source, > { > return mark_unsupported_function(full_name(), arg, VCOL_IMPOSSIBLE); > } > + virtual bool clear_cached_function_value(void *arg) { return 0; } > virtual bool check_field_expression_processor(void *arg) { return 0; } > virtual bool check_func_default_processor(void *arg) { return 0; } > /* > @@ -5450,8 +5451,21 @@ class Item_cache: public Item_basic_constant, > } > bool check_vcol_func_processor(void *arg) > { > + if ( example ) > + { > + return example->check_vcol_func_processor( arg ); whitespace around parentheses and indentation > + } > return mark_unsupported_function("cache", arg, VCOL_IMPOSSIBLE); > } > + bool clear_cached_function_value( void *arg ) > + { > + if ( example && example->type() == Item::FUNC_ITEM ) > + { > + // Clear the cached function value to trigger reevaluation > + clear(); > + } > + return false; > + } Why did you need a dedicated clear_cached_function_value() processor? There's Item::cleanup() that should be called from fix_session_vcol_expr(). Oh, okay, I see two issues here. First, item->cleanup() does not recurse into function arguments. That's my bug, I should've used something like - vcol->expr->cleanup(); + vcol->expr->walk(&Item::cleanup_excluding_fields_processor, 0, 0); It has to be fixed, it's a genuine bug. A test case could be create table t1 (n varchar(100), cu varchar(100) default current_user(), uc varchar(100) default concat(current_user())); create definer=foo@localhost view v1 as select * from t1; insert v1 (n) values ('v1'); select * from t1; This is a slightly modified test from default_session.test file. Second, it's incorrect for Item_cache to use simply example->check_vcol_func_processor(arg); because caching requires re-fixing, even if the underlying example item doesn't. So, this should be something like bool check_vcol_func_processor(void *arg) { if ( example ) { Item::vcol_func_processor_result *res= (Item::vcol_func_processor_result*)arg; example->check_vcol_func_processor(arg); if (res->errors & VCOL_NOT_STRICTLY_DETERMINISTIC) res->errors|= VCOL_SESSION_FUNC; return false; } return mark_unsupported_function("cache", arg, VCOL_IMPOSSIBLE); } this sets VCOL_SESSION_FUNC flag, which will require item to be re-fixed for every statement. But doesn't set it for true constants. With these two fixes Item_cache::cleanup() { clear(); } should works just fine. > /** > Check if saved item has a non-NULL value. > Will cache value of saved item if not already done. > diff --git a/sql/table.cc b/sql/table.cc > index 3a08d1e49ea..9ae035e41aa 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -2802,9 +2802,9 @@ static bool fix_and_check_vcol_expr(THD *thd, TABLE *table, > > int error= func_expr->walk(&Item::check_vcol_func_processor, 0, &res); > if (error || (res.errors & VCOL_IMPOSSIBLE)) > - { // this can only happen if the frm was corrupted > + { this comment is still true, don't delete it. and there's no need to pass the type inside anymore. > my_error(ER_VIRTUAL_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(0), res.name, > - "???", "?????"); > + vcol_type_name((enum_vcol_info_type) vcol_type), vcol->name.str); > DBUG_RETURN(1); > } > vcol->flags= res.errors; > @@ -7374,6 +7374,10 @@ int TABLE::update_virtual_fields(handler *h, enum_vcol_update_mode update_mode) > > if (update) > { > + // Clear any cached function values in the virtual field expression > + // to trigger their reevaluation > + vcol_info->expr->walk( &Item::clear_cached_function_value, 0, 0 ); // Always returns 0 There's no need to do that for every update_virtual_fields(), that is, no need to do it for every row, only needs to be done once per statement. And fix_session_vcol_expr() will do it for you if you implement Item_cache::cleanup() and set VCOL_SESSION_FUNC and fix fix_session_vcol_expr() to cleanup recursively. > + > int field_error __attribute__((unused)) = 0; > /* Compute the actual value of the virtual fields */ > if (vcol_info->expr->save_in_field(vf, 0)) Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 4

Re: [Maria-developers] b7aa15458db: MDEV-11117 CHECK constraint fails on intermediate step of ALTER
by Sergei Golubchik 06 Apr '17

06 Apr '17

Hi, Jacob! On Apr 03, jacob.mathew(a)mariadb.com wrote: > revision-id: b7aa15458db95149af4f473a90009844a16e9db8 (mariadb-10.2.4-103-gb7aa15458db) > parent(s): 3c422e60bbee79bb636e65910c26ac193de70a84 > author: Jacob Mathew > committer: Jacob Mathew > timestamp: 2017-04-03 17:54:32 -0700 > message: > > MDEV-11117 CHECK constraint fails on intermediate step of ALTER > > Fixed the bug by failing the statement with an error message that explains > that an auto-increment column may not be used in an expression for a > check constraint, a default value or a generated column. > Added a test case. > > new file mode 100644 > index 00000000000..e192fd16677 > --- /dev/null > +++ b/mysql-test/t/check_constraint_autoinc.test > @@ -0,0 +1,7 @@ > +# > +# An auto-increment column may not be used in an expression for > +# a check constraint: MDEV-11117 > +# > +--echo # AUTO_INCREMENT IN VCOL > +-- error ER_AUTOINC_IN_VIRTUAL_COLUMN_FUNCTION > +create or replace table t1( c1 int auto_increment primary key, check( c1 > 0 or c1 is null ) ); you could've added this 2-line test to the existing file check_constraint.test No strong arguments either way, though. Pro: mysqltest is executed for every test file, it's faster to have less test files. Don't know if the difference is big, though. Contra: it's easier to see what has failed with one test per file. It doesn't matter now, but might matter in the future, when we'll have more automatic failure analysys. > diff --git a/sql/field.h b/sql/field.h > index 50dcb397616..4bd43551648 100644 > --- a/sql/field.h > +++ b/sql/field.h > @@ -591,7 +591,8 @@ static inline const char *vcol_type_name(enum_vcol_info_type type) > #define VCOL_NON_DETERMINISTIC 2 > #define VCOL_SESSION_FUNC 4 /* uses session data, e.g. USER or DAYNAME */ > #define VCOL_TIME_FUNC 8 > -#define VCOL_IMPOSSIBLE 16 > +#define VCOL_AUTO_INC 16 > +#define VCOL_IMPOSSIBLE 32 > > #define VCOL_NOT_STRICTLY_DETERMINISTIC \ > (VCOL_NON_DETERMINISTIC | VCOL_TIME_FUNC | VCOL_SESSION_FUNC) > diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt > index 930cd2ceaa2..a78e97c2577 100644 > --- a/sql/share/errmsg-utf8.txt > +++ b/sql/share/errmsg-utf8.txt > @@ -7142,6 +7142,8 @@ ER_NO_EIS_FOR_FIELD > ER_WARN_AGGFUNC_DEPENDENCE > eng "Aggregate function '%-.192s)' of SELECT #%d belongs to SELECT #%d" > ukr "Агрегатна функція '%-.192s)' з SELECTу #%d належить до SELECTу #%d" > +ER_AUTOINC_IN_VIRTUAL_COLUMN_FUNCTION > + eng "Auto-increment column %`s may not be used in an expression for a check constraint, a default value or a generated column." First, please add error messages at the very end of the file. Second, there's an existing error ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED eng "Function or expression '%s' cannot be used in the %s clause of %`s" which is generally used like my_error(ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(0), "NOW()", "CHECK", field->name); or, for subqueries my_error(ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(0), "SELECT ...", "DEFAULT", field->name); It'd be best if you could use it. If not, at least rephrase your error message to use ... in the %s clause of %`s pattern, instead of enumerating all possible places where autoincrement is not allowed. > diff --git a/sql/table.cc b/sql/table.cc > index 6b15c06cb91..d1772933190 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -2811,6 +2811,19 @@ static bool fix_and_check_vcol_expr(THD *thd, TABLE *table, > "???", "?????"); > DBUG_RETURN(1); > } > + else if (res.errors & VCOL_AUTO_INC) > + { > + /* > + An auto_increment field may not be used in an expression for > + a check constraint, a default value or a generated column > + > + Note that this error condition is not detected during parsing > + of the statement because the field item does not have a field > + pointer at that time > + */ > + my_error(ER_AUTOINC_IN_VIRTUAL_COLUMN_FUNCTION, MYF(0), res.name); > + DBUG_RETURN(1); > + } > vcol->flags= res.errors; There is already a check for auto-increment in generated columns. I've added it in commit a418c9920047d5222a0d065343347312127b780f (see the change in item.cc). But the error message is somewhat confusing. And very confusing when applied to CHECK and DEFAULT. Yours is better, so please remove my check, no need to do it twice. Don't forget to re-run tests after that (at least vcol and gcol suites) as some error messages could change. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] mariadb-connector-c on mingw-w64 with pthreads
by Jeroen Ooms 06 Apr '17

06 Apr '17

For the language bindings for the R programming language (RMySQL) I am building mariadb-connector-c-2.3.2 on Windows using the mingw-w64 compiler. Our mingw-w64 compilers have been configured with pthreads support (same as msys2): - i686-493-posix-dwarf-rt_v3-s for win32 - x86_64-493-posix-seh-rt_v3-s for win64 Building libmariadbclient succeeds, however when I try to link the static library into the application I get these errors: C:/Rtools/mingw_64/lib/gcc/x86_64-w64-mingw32/lib/libpthread.a(libwinpthread_la-cond.o):cond.c:(.text+0x300): multiple definition of `pthread_cond_init' libmariadbclient.a(my_pthread.c.obj):my_pthread.c:(.text+0x0): first defined here Same error for 'pthread_cond_destroy', 'pthread_cond_wait', and 'pthread_cond_timedwait'. From the source code of my_pthread.c it seems that mariadb-connector-c is indeed redefining these functions on _WIN32. I guess this assumes MSVC which does not have native pthreads? Some questions: 1. What would be the correct way to configure/build mariadb-connector-c on mingw-w64 with native pthread support? 2. Is there a cmake option to disable threading alltogether? Thank you

2 1

[Maria-developers] Database corruption
by jerome brauge 05 Apr '17

05 Apr '17

Hello, We have a java tools to massively load data in a database (parallel load in different tables). With it, on bb-10.2-compatibility branch, I'm able to reproduce crash and sometimes database corruption: · on truncate (but delete works fine) · on parallel insert Crash seems occurred in innoDB code. See attached, logs of each case. Do you need more information ? Regards, Jérôme.

3 11

Re: [Maria-developers] JDBC getWarnings and stored procedure
by Daniel Black 05 Apr '17

05 Apr '17

On 03/04/17 20:21, jerome brauge wrote: > Hello, > > When we call a stored procedure in JDBC which emit warnings, > getWarnings() always returns null. > > I don’t really think that the problem is in MariaDB Connector J because > warning flag in the EOF packet is not set to true. > hasWarnings is set: https://github.com/MariaDB/mariadb-connector-j/blob/master/src/main/java/or… Its used here: https://github.com/MariaDB/mariadb-connector-j/blob/master/src/main/java/or… I suspect the bug is warningsCleared has a implicit default of false in the above class. As a workaround check, call reenableWarnings() before getWarnings(). > Mysql client behaves in the same way, by default it shows warnings count > issued by DML command but not for stored procedure. > > > > Is it a normal behavior ? > > > > Best regards, > > Jérôme. > > > > > > > > > > > > > > _______________________________________________ > Mailing list: https://launchpad.net/~maria-developers > Post to : maria-developers(a)lists.launchpad.net > Unsubscribe : https://launchpad.net/~maria-developers > More help : https://help.launchpad.net/ListHelp >

3 3

Re: [Maria-developers] [Commits] e446bf7: MDEV-12145: Wrong result (missing rows) on query with IN and EXISTS subqueries from InnoDB tables
by Igor Babaev 04 Apr '17

04 Apr '17

Varun, 1. The mdev # in the test case should be 12145 2. Could you please explain why do you need > @@ -9735,8 +9735,7 @@ make_join_select(JOIN *join,SQL_SELECT *select,COND *cond) > !(used_tables & tab->emb_sj_nest->sj_inner_tables)) > { > save_used_tables= used_tables; > - used_tables= join->const_table_map | OUTER_REF_TABLE_BIT | > - RAND_TABLE_BIT; > + used_tables= join->const_table_map | RAND_TABLE_BIT; > } ? I checked your test case on 5.5 only with ; > */ > if (tab == join->join_tab + join->top_join_tab_count - 1) > - current_map|= OUTER_REF_TABLE_BIT | RAND_TABLE_BIT; > + current_map|= RAND_TABLE_BIT; > used_tables|=current_map; > > if (tab->type == JT_REF && tab->quick && and it passed. If the first change is really needed you have to produce a test case where it is really critical. If this is just cosmetic change the comment should clearly say about it. 3. I wonder why the patch is applied to 10.2 if it's reported for 5.5-10.2. Regards, Igor. On 04/04/2017 05:55 AM, Varun wrote: > revision-id: e446bf7dc49b96a09e11199ad3e05fade6490279 (mariadb-10.2.3-396-ge446bf7) > parent(s): 6d417a0bad205a6bacfee10dbc46dd631b093e75 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2017-04-04 18:24:43 +0530 > message: > > MDEV-12145: Wrong result (missing rows) on query with IN and EXISTS subqueries from InnoDB tables > > Tables that are within SJ-Materialization nests cannot have their conditions referring to preceding non-const tables. But in this issue this was happening. > Fixed by not allowing used_tables to allow outer tables > > --- > mysql-test/r/subselect_sj2.result | 26 ++++++++++++++++++++++++++ > mysql-test/r/subselect_sj2_jcl6.result | 26 ++++++++++++++++++++++++++ > mysql-test/r/subselect_sj2_mat.result | 26 ++++++++++++++++++++++++++ > mysql-test/t/subselect_sj2.test | 24 ++++++++++++++++++++++++ > sql/sql_select.cc | 5 ++--- > 5 files changed, 104 insertions(+), 3 deletions(-) > > diff --git a/mysql-test/r/subselect_sj2.result b/mysql-test/r/subselect_sj2.result > index 948be57..14032ec 100644 > --- a/mysql-test/r/subselect_sj2.result > +++ b/mysql-test/r/subselect_sj2.result > @@ -1331,5 +1331,31 @@ WHERE a IN ( SELECT b FROM t2 LEFT JOIN v3 ON ( c = b ) ) ; > a pk b > DROP TABLE t1,t2,t3; > DROP VIEW v3; > +# > +# MDEV-11108: Assertion `uniq_tuple_length_arg <= table->file->max_key_length()' failed in SJ_TMP_TABLE::create_sj_weedout_tmp_table > +# > +CREATE TABLE t1 (f1 INT) ENGINE=InnoDB; > +INSERT INTO t1 VALUES (4),(6); > +CREATE TABLE t2 (i2 INT, KEY(i2)) ENGINE=InnoDB; > +INSERT INTO t2 VALUES (8),(7),(1); > +CREATE TABLE t3 (f3 INT, i3 INT, KEY(i3)) ENGINE=InnoDB; > +CREATE ALGORITHM=MERGE VIEW v3 AS SELECT * FROM t3; > +INSERT INTO t3 VALUES (8,0),(6,3),(2,8),(3,8),(1,6),(0,0),(1,0),(1,5); > +CREATE TABLE t4 (i4 INT, f4 INT, KEY(i4)) ENGINE=InnoDB; > +INSERT INTO t4 VALUES (0,0),(4,0),(0,2),(1,0),(2,1),(2,7),(6,3),(8,7),(8,1),(1,0),(7,2),(6,0),(8,1); > +SELECT * FROM t1 WHERE EXISTS ( SELECT * FROM t2, t3 WHERE i3 = i2 AND f1 IN ( SELECT f3 FROM v3, t4 WHERE i4 = f3 AND f4 <> f3 ) ); > +f1 > +6 > +ANALYZE TABLE t1,t2,t3,t4; > +Table Op Msg_type Msg_text > +test.t1 analyze status OK > +test.t2 analyze status OK > +test.t3 analyze status OK > +test.t4 analyze status OK > +SELECT * FROM t1 WHERE EXISTS ( SELECT * FROM t2, t3 WHERE i3 = i2 AND f1 IN ( SELECT f3 FROM v3, t4 WHERE i4 = f3 AND f4 <> f3 ) ); > +f1 > +6 > +DROP VIEW v3; > +DROP TABLE t1, t2, t3, t4; > # This must be the last in the file: > set optimizer_switch=@subselect_sj2_tmp; > diff --git a/mysql-test/r/subselect_sj2_jcl6.result b/mysql-test/r/subselect_sj2_jcl6.result > index 2955307..2aa0605 100644 > --- a/mysql-test/r/subselect_sj2_jcl6.result > +++ b/mysql-test/r/subselect_sj2_jcl6.result > @@ -1346,6 +1346,32 @@ WHERE a IN ( SELECT b FROM t2 LEFT JOIN v3 ON ( c = b ) ) ; > a pk b > DROP TABLE t1,t2,t3; > DROP VIEW v3; > +# > +# MDEV-11108: Assertion `uniq_tuple_length_arg <= table->file->max_key_length()' failed in SJ_TMP_TABLE::create_sj_weedout_tmp_table > +# > +CREATE TABLE t1 (f1 INT) ENGINE=InnoDB; > +INSERT INTO t1 VALUES (4),(6); > +CREATE TABLE t2 (i2 INT, KEY(i2)) ENGINE=InnoDB; > +INSERT INTO t2 VALUES (8),(7),(1); > +CREATE TABLE t3 (f3 INT, i3 INT, KEY(i3)) ENGINE=InnoDB; > +CREATE ALGORITHM=MERGE VIEW v3 AS SELECT * FROM t3; > +INSERT INTO t3 VALUES (8,0),(6,3),(2,8),(3,8),(1,6),(0,0),(1,0),(1,5); > +CREATE TABLE t4 (i4 INT, f4 INT, KEY(i4)) ENGINE=InnoDB; > +INSERT INTO t4 VALUES (0,0),(4,0),(0,2),(1,0),(2,1),(2,7),(6,3),(8,7),(8,1),(1,0),(7,2),(6,0),(8,1); > +SELECT * FROM t1 WHERE EXISTS ( SELECT * FROM t2, t3 WHERE i3 = i2 AND f1 IN ( SELECT f3 FROM v3, t4 WHERE i4 = f3 AND f4 <> f3 ) ); > +f1 > +6 > +ANALYZE TABLE t1,t2,t3,t4; > +Table Op Msg_type Msg_text > +test.t1 analyze status OK > +test.t2 analyze status OK > +test.t3 analyze status OK > +test.t4 analyze status OK > +SELECT * FROM t1 WHERE EXISTS ( SELECT * FROM t2, t3 WHERE i3 = i2 AND f1 IN ( SELECT f3 FROM v3, t4 WHERE i4 = f3 AND f4 <> f3 ) ); > +f1 > +6 > +DROP VIEW v3; > +DROP TABLE t1, t2, t3, t4; > # This must be the last in the file: > set optimizer_switch=@subselect_sj2_tmp; > # > diff --git a/mysql-test/r/subselect_sj2_mat.result b/mysql-test/r/subselect_sj2_mat.result > index a067422..1ea020a 100644 > --- a/mysql-test/r/subselect_sj2_mat.result > +++ b/mysql-test/r/subselect_sj2_mat.result > @@ -1333,6 +1333,32 @@ WHERE a IN ( SELECT b FROM t2 LEFT JOIN v3 ON ( c = b ) ) ; > a pk b > DROP TABLE t1,t2,t3; > DROP VIEW v3; > +# > +# MDEV-11108: Assertion `uniq_tuple_length_arg <= table->file->max_key_length()' failed in SJ_TMP_TABLE::create_sj_weedout_tmp_table > +# > +CREATE TABLE t1 (f1 INT) ENGINE=InnoDB; > +INSERT INTO t1 VALUES (4),(6); > +CREATE TABLE t2 (i2 INT, KEY(i2)) ENGINE=InnoDB; > +INSERT INTO t2 VALUES (8),(7),(1); > +CREATE TABLE t3 (f3 INT, i3 INT, KEY(i3)) ENGINE=InnoDB; > +CREATE ALGORITHM=MERGE VIEW v3 AS SELECT * FROM t3; > +INSERT INTO t3 VALUES (8,0),(6,3),(2,8),(3,8),(1,6),(0,0),(1,0),(1,5); > +CREATE TABLE t4 (i4 INT, f4 INT, KEY(i4)) ENGINE=InnoDB; > +INSERT INTO t4 VALUES (0,0),(4,0),(0,2),(1,0),(2,1),(2,7),(6,3),(8,7),(8,1),(1,0),(7,2),(6,0),(8,1); > +SELECT * FROM t1 WHERE EXISTS ( SELECT * FROM t2, t3 WHERE i3 = i2 AND f1 IN ( SELECT f3 FROM v3, t4 WHERE i4 = f3 AND f4 <> f3 ) ); > +f1 > +6 > +ANALYZE TABLE t1,t2,t3,t4; > +Table Op Msg_type Msg_text > +test.t1 analyze status OK > +test.t2 analyze status OK > +test.t3 analyze status OK > +test.t4 analyze status OK > +SELECT * FROM t1 WHERE EXISTS ( SELECT * FROM t2, t3 WHERE i3 = i2 AND f1 IN ( SELECT f3 FROM v3, t4 WHERE i4 = f3 AND f4 <> f3 ) ); > +f1 > +6 > +DROP VIEW v3; > +DROP TABLE t1, t2, t3, t4; > # This must be the last in the file: > set optimizer_switch=@subselect_sj2_tmp; > set optimizer_switch=default; > diff --git a/mysql-test/t/subselect_sj2.test b/mysql-test/t/subselect_sj2.test > index a948b08..5b745ca 100644 > --- a/mysql-test/t/subselect_sj2.test > +++ b/mysql-test/t/subselect_sj2.test > @@ -1464,5 +1464,29 @@ WHERE a IN ( SELECT b FROM t2 LEFT JOIN v3 ON ( c = b ) ) ; > DROP TABLE t1,t2,t3; > DROP VIEW v3; > > +--echo # > +--echo # MDEV-11108: Assertion `uniq_tuple_length_arg <= table->file->max_key_length()' failed in SJ_TMP_TABLE::create_sj_weedout_tmp_table > +--echo # > + > +CREATE TABLE t1 (f1 INT) ENGINE=InnoDB; > +INSERT INTO t1 VALUES (4),(6); > + > +CREATE TABLE t2 (i2 INT, KEY(i2)) ENGINE=InnoDB; > +INSERT INTO t2 VALUES (8),(7),(1); > + > +CREATE TABLE t3 (f3 INT, i3 INT, KEY(i3)) ENGINE=InnoDB; > +CREATE ALGORITHM=MERGE VIEW v3 AS SELECT * FROM t3; > +INSERT INTO t3 VALUES (8,0),(6,3),(2,8),(3,8),(1,6),(0,0),(1,0),(1,5); > + > +CREATE TABLE t4 (i4 INT, f4 INT, KEY(i4)) ENGINE=InnoDB; > +INSERT INTO t4 VALUES (0,0),(4,0),(0,2),(1,0),(2,1),(2,7),(6,3),(8,7),(8,1),(1,0),(7,2),(6,0),(8,1); > + > +SELECT * FROM t1 WHERE EXISTS ( SELECT * FROM t2, t3 WHERE i3 = i2 AND f1 IN ( SELECT f3 FROM v3, t4 WHERE i4 = f3 AND f4 <> f3 ) ); > +ANALYZE TABLE t1,t2,t3,t4; > +SELECT * FROM t1 WHERE EXISTS ( SELECT * FROM t2, t3 WHERE i3 = i2 AND f1 IN ( SELECT f3 FROM v3, t4 WHERE i4 = f3 AND f4 <> f3 ) ); > + > +DROP VIEW v3; > +DROP TABLE t1, t2, t3, t4; > + > --echo # This must be the last in the file: > set optimizer_switch=@subselect_sj2_tmp; > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 151e341..135ad9a 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -9735,8 +9735,7 @@ make_join_select(JOIN *join,SQL_SELECT *select,COND *cond) > !(used_tables & tab->emb_sj_nest->sj_inner_tables)) > { > save_used_tables= used_tables; > - used_tables= join->const_table_map | OUTER_REF_TABLE_BIT | > - RAND_TABLE_BIT; > + used_tables= join->const_table_map | RAND_TABLE_BIT; > } > > /* > @@ -9744,7 +9743,7 @@ make_join_select(JOIN *join,SQL_SELECT *select,COND *cond) > It solve problem with select like SELECT * FROM t1 WHERE rand() > 0.5 > */ > if (tab == join->join_tab + join->top_join_tab_count - 1) > - current_map|= OUTER_REF_TABLE_BIT | RAND_TABLE_BIT; > + current_map|= RAND_TABLE_BIT; > used_tables|=current_map; > > if (tab->type == JT_REF && tab->quick && > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

1 0

Re: [Maria-developers] MDEV-12238 Add Type_handler::Item_func_{plus|minus|mul|div|mod}_fix_length_and_dec()
by Alexander Barkov 04 Apr '17

04 Apr '17

Hello Alexey, Thanks for your review. Please find an updated version attached. On 03/19/2017 08:12 PM, Alexey Botchkov wrote: > Hi, Alexander. > > The patch is ok to push. > I only have two cosmetical recommendation. > 1. const Pair* find_pair() implementation belongs to sql_type.cc. A bit > too long for the .h file. Moved. > 2. I'd create the enum {NOT_COMMUTATIVE, COMMUTATIVE} and use it as the > parameter for the find_pair() and related functions. Makes code much > more understandable. As agreed on IRC, I created a new class Type_aggregator_commutative instead. Thanks! > > Best regards. > HF > > On Mon, Mar 13, 2017 at 3:41 PM, Alexander Barkov <bar(a)mariadb.org > <mailto:bar@mariadb.org>> wrote: > > Hello Alexey, > > Please review a patch for MDEV-12238. > > Thanks! > >

1 1

[Maria-developers] Please review a patch removing LEX::text_string_is_7bit
by Alexander Barkov 04 Apr '17

04 Apr '17

Hello Sergei, Can you please review a patch for bb-10.2-ext fixing the problem that in case of possible changes in look-ahead in rules using TEXT_STRING or NCHAR_STRING, Lex->text_string_is_7bit can get out of sync with $1/$2/$3, etc. Thanks!

2 2

[Maria-developers] JDBC getWarnings and stored procedure
by jerome brauge 03 Apr '17

03 Apr '17

Hello, When we call a stored procedure in JDBC which emit warnings, getWarnings() always returns null. I don't really think that the problem is in MariaDB Connector J because warning flag in the EOF packet is not set to true. Mysql client behaves in the same way, by default it shows warnings count issued by DML command but not for stored procedure. Is it a normal behavior ? Best regards, Jérôme.

1 0

[Maria-developers] Please review a patch for Mdev-12019
by Sachin Setiya 03 Apr '17

03 Apr '17

Hi Monty, Please review a patch for Mdev-12019. -- Regards Sachin Setiya Software Engineer at MariaDB

1 0

[Maria-developers] Errors in building debs
by Sergey Petrunia 01 Apr '17

01 Apr '17

Hi Vicențiu, I'm looking at the last build in bb-10.2-mariarocks http://buildbot.askmonty.org/buildbot/builders/kvm-deb-trusty-x86/builds/35… and I see this: + DH_BUILD_DDEBS=1 + echo + debian/autobake-deb.sh debian/autobake-deb.sh: line 80: [: i686: integer expression expected debian/autobake-deb.sh: line 80: [i686: command not found and then it eventually fails with this error: cp: cannot stat ‘debian/tmp/etc/mysql/conf.d/rocksdb_se.cnf’: No such file or I assume this is caused by your recent changes? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 1

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

[Maria-developers] Query on MDEC-7502
by Vishwanath Bharadwaj 27 Mar '17

27 Mar '17

Hello Mr. Sergei, What I propose is rather than dumping the table into a single bin log. What your idea proposes is we slide through the table based on primary key. This becomes a bit of a problem if there is some error on the receiver side. So we create 2 logs. We cut the chunk from one log and send it through the RB logging. This way we need not have different replication for same data. If some error occurs we can resend the file from the data that is dumped into bin logger. I hope I'm able to clear you query. If there is even a slightest doubt in my idea please feel free to contact me and suggest changes as I am still in my learning stage. Thank you:)

2 2

[Maria-developers] [GSoC2017]Project prosoal [MDEV-12296] Cassandra Storage Engine V2
by Neveralso 27 Mar '17

27 Mar '17

Hello. My name is Fangshuo Jiang. I am a first-year master student of computer technology, in Beijing University of Posts and Telecommunications. I would like to write a prososal and work on MDEV-12296:Cassandra Storage Engine V2. I was an intern at Tencent Company as a C++ back-end development engineer. I am familiar with C++ network programming, with some C++ projects holding on my GitHub. https://github.com/neveralso Look forward to more advice. Thank you. Best wishes. Fangshuo Jiang neveralso(a)gmail.com

2 1

Re: [Maria-developers] Gsoc project
by Vishwanath Bharadwaj 26 Mar '17

26 Mar '17

Hi hrishikesh, I guess we've to wait for someone who is authorized to make these decisions reply to your question. Ping Sergei Golubchik if you want to submit a proposal. Thank you:) On Mar 26, 2017 23:55, "Hrishikesh Paramane" <paramane1995hrishi(a)gmail.com> wrote: > MDEV-8100 > > > On Sun, Mar 26, 2017 at 11:54 PM, Vishwanath Bharadwaj < > bharadwajpullabhatla(a)gmail.com> wrote: > >> Hi hrishikesh, >> >> It would be helpful if you could provide us the project ticket number(Ex: >> MDEV-7502) >> >> Thank you:) >> >> On Mar 26, 2017 23:44, "Vishwanath Bharadwaj" < >> bharadwajpullabhatla(a)gmail.com> wrote: >> >>> Hi Hrishikesh, >>> >>> It would be helpful if you could provide us the project ticket number. >>> >>> Thank you:) >>> >>> On Mar 26, 2017 11:35 PM, "Hrishikesh Paramane" < >>> paramane1995hrishi(a)gmail.com> wrote: >>> >>>> Is Table function project removed from Google Summer of Code? >>>> >>>> _______________________________________________ >>>> Mailing list: https://launchpad.net/~maria-developers >>>> Post to : maria-developers(a)lists.launchpad.net >>>> Unsubscribe : https://launchpad.net/~maria-developers >>>> More help : https://help.launchpad.net/ListHelp >>>> >>>> >

1 0

[Maria-developers] Gsoc project
by Hrishikesh Paramane 26 Mar '17

26 Mar '17

Is Table function project removed from Google Summer of Code?

3 3

[Maria-developers] GSOC 2017 ideas
by Charles Cui 25 Mar '17

25 Mar '17

Hi guys, This is Charles Cui from Tsinghua University, China. I am pretty interested in the ideas listed in GSOC 2017 page on Maria DB, especially the idea "Cassandra Storage Engine V2". I have some experiences in Cassandra and MySQL, but quite new to Maria DB. I am wondering whether I can join this project and work with you guys? I speak fluent English and available during the whole summer. In case that this project is allocated to other people, I am also open to pick other projects in Maria DB. Let me know if you have questions or feedbacks. Thanks, Charles.

3 4

[Maria-developers] MariaDB Developers (Un)Conference
by Ian Gilfillan 24 Mar '17

24 Mar '17

Hi The schedule for the MariaDB Developers (Un)Conference (to be held in New York, April 9 and 10) is being fleshed out, so this is a last call to add or vote for any sessions. You can update the spreadsheet directly: https://docs.google.com/spreadsheets/d/1mUFk5gd0fd6amjxGhb7hG1xRHQdh-9qxavp… Please remember to register on the Meetup page if you're coming - our hosts BNY Mellon need a list of everyone attending in advance: https://www.meetup.com/MariaDB-Developers/events/234702611/ The MySQL New York Meetup group is also hosting an event on the evening of April 10 - see the details at: https://www.meetup.com/EffectiveMySQL/ Announcement: https://mariadb.org/2017-developers-meetup-new-york/

1 0

[Maria-developers] Why We can set binlog_format=mixed for session ?
by Sachin Setia 23 Mar '17

23 Mar '17

Hi Monty! When flashback is on why we can set binlog_format = Mixed/ statement for session scope, What is use of this and why this is allowed ? Regards sachin

1 0

[Maria-developers] Please review MDEV-12338 Split Item_type_holder::get_real_type() into virtual Item::real_type_handler()
by Alexander Barkov 23 Mar '17

23 Mar '17

Hi Vicențiu, can you please review a patch for MDEV-12338? Thanks!

1 0

Re: [Maria-developers] [Commits] 8ac272ab2c4: MDEV-12253: Buffer pool blocks are accessed after they have been freed
by Marko Mäkelä 23 Mar '17

23 Mar '17

On Mon, Mar 20, 2017 at 3:59 PM, jan <jan.lindstrom(a)mariadb.com> wrote: > diff --git a/mysql-test/suite/encryption/include/innodb-util.pl > b/mysql-test/suite/encryption/include/innodb-util.pl > index 241545dac18..5a7295682dd 100644 > --- a/mysql-test/suite/encryption/include/innodb-util.pl > +++ b/mysql-test/suite/encryption/include/innodb-util.pl > @@ -4,8 +4,11 @@ > # All the tables must be in the same database, you can call it like so: > # ib_backup_tablespaces("test", "t1", "blah", ...). > > +use strict; > +use warnings; > use File::Copy; > use File::Spec; > +use Fcntl qw(:DEFAULT :seek); > > sub ib_normalize_path { > my ($path) = @_; > @@ -124,3 +127,32 @@ sub ib_restore_ibd_files { > ib_restore_ibd_file($tmpd, $datadir, $db, $table); > } > } > + > +sub ib_corrupt_tablespace { > + my ($db, $table) = @_; > + my $datadir = $ENV{'MYSQLD_DATADIR'}; > + my $page_size = $ENV{'INNODB_PAGE_SIZE'}; > + my $ibd_file = sprintf("%s%s/%s.ibd", $datadir,$db,$table); > The sprintf() function and the separate parameters $db, $table look like obfuscation to me. Why not just write "$ENV{MYSQLD_DATADIR}/test/t1.ibd" when invoking this function in the .test file? > + print "file: $ibd_file $page_size\n"; > Why do you want to pollute the output with the page size? That would require rdiff files for different page sizes. > +# We try to corrupt FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION field > + my $len; > + > + sysopen IBD_FILE, $ibd_file, O_RDWR || die "Unable to open $ibd_file"; > + > + while ($len = sysread(IBD_FILE, $_, $page_size)) > + { > + sysseek(IBD_FILE, -$len, SEEK_CUR); > + substr($_, 26, 8) = pack("N",0xdeadbeef); > + print "Corrupted page with $chunk\n"; > + syswrite(IBD_FILE, $_, $len); > + } > + close IBD_FILE; > +} > Where is the variable $chunk defined? Why does this script execute issue output to stdout for every iteration of the loop? That would seem to make the test depend on the number of pages in the file, and thus cause result differences when using a different innodb_page_size. If the sysread() function happens to be interrupted by a signal, it could return a different amount than 0 or $page_size bytes. Thus, the operation of the loop would be nondeterministic. I would rather use a simple copying loop with a regular expression replacement, instead of this in-place modification. I would just do something like this: --exec perl -ei 's|(.{26}).{8}(.$ENV{INNODB_PAGE_SIZE})|"$1".pack("N",0xdeadbeef),"$2"|seg' $MYSQLD_DATADIR/test/t1.ibd Shorter and faster, and using fewer system calls. > diff --git a/mysql-test/suite/encryption/r/innodb-force-corrupt.result > b/mysql-test/suite/encryption/r/innodb-force-corrupt.result > new file mode 100644 > index 00000000000..bba2ec17bbc > --- /dev/null > +++ b/mysql-test/suite/encryption/r/innodb-force-corrupt.result > @@ -0,0 +1,38 @@ > +call mtr.add_suppression("InnoDB: Database page corruption on disk or a > failed.*"); > +call mtr.add_suppression("InnoDB: Corruption: Block in space_id .* in > file .* corrupted."); > +call mtr.add_suppression("InnoDB: Based on page type .*"); > +call mtr.add_suppression("InnoDB: Unable to read tablespace .* page no .* > into the buffer pool after .* attempts"); > +CALL mtr.add_suppression("InnoDB: Error: Unable to read tablespace .* > page no .* into the buffer pool after 100 attempts"); > +CALL mtr.add_suppression("InnoDB: Database page corruption on disk or a > failed"); > +CALL mtr.add_suppression("InnoDB: Space .* file test/t1 read of page .*"); > +CALL mtr.add_suppression("InnoDB: You may have to recover from a > backup."); > +CALL mtr.add_suppression("InnoDB: It is also possible that your > operatingsystem has corrupted its own file cache."); > +CALL mtr.add_suppression("InnoDB: and rebooting your computer removes the > error."); > +CALL mtr.add_suppression("InnoDB: If the corrupt page is an index page > you can also try to"); > +CALL mtr.add_suppression("InnoDB: fix the corruption by dumping, > dropping, and reimporting"); > +CALL mtr.add_suppression("InnoDB: the corrupt table. You can use CHECK"); > +CALL mtr.add_suppression("InnoDB: TABLE to scan your table for > corruption."); > +CALL mtr.add_suppression("InnoDB: See also .* about forcing recovery."); > +CALL mtr.add_suppression("InnoDB: However key management plugin or used > key_version 3221605118 is not found or used encryption algorithm or method > does not match."); > +CALL mtr.add_suppression("Marking tablespace as missing. You may drop > this table or install correct key management plugin and key file."); > +CALL mtr.add_suppression("InnoDB: Block in space_id .* in file .* > encrypted."); > Are all these suppressions really required? (The same problem is with multiple tests. +call mtr.add_suppression("mysqld: File .* not found .*"); > The suppression is looking for a substring by default (no implied $ at the end of the regular expression). The .* at the end of the regexp is clutter. > +call mtr.add_suppression(".*"); > This catch-all is absolutely not acceptable. > +--exec echo "wait" > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect > +--shutdown_server > +--source include/wait_until_disconnected.inc > Do not use the low-level commands directly. Use restart_mysqld.inc. See the encryption test cleanup that I did in 10.2 in the merge: https://github.com/MariaDB/server/commit/2d96d13ecdcab4ce16c0d8976753d8aa50… > ++--write_file $MYSQLTEST_VARDIR/keys1.txt > +1;770A8A65DA156D24EE2A093277530142 > +4;770A8A65DA156D24EE2A093277530143 > +EOF > This file can be created already before the shutdown (or restart). Why not use key files in std_data? What is the benefit of creating files with identical static content in multiple --suite=encryption tests? +--exec echo "restart:--innodb-encrypt-tables --innodb-stats-persistent > --plugin-load-add=file_key_management.so --file-key-management > --file-key-management-filename=$MYSQLTEST_VARDIR/keys1.txt" > > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect > +--enable_reconnect > +--source include/wait_until_connected_again.inc > This is missing disable_reconnect, which is part of restart_mysqld.inc. > +--exec echo "restart:--innodb-encrypt-tables --innodb-stats-persistent > --plugin-load-add=file_key_management.so --file-key-management > --file-key-management-filename=$MYSQLTEST_VARDIR/keys2.txt" > > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect > +--enable_reconnect > +--source include/wait_until_connected_again.inc > +--source include/restart_mysqld.inc > What is the purpose of restarting the server twice here? > +--error 1296 > +select count(*) from t1 FORCE INDEX (test) where b like 'secret%'; > +--error 1296 > +select count(*) from t2 FORCE INDEX (test) where b like 'secret%'; > +select count(*) from t3 FORCE INDEX (test) where b like 'secret%'; > Do not use numeric error codes. Refer to include/mysqld_ername.h in the build directory for the symbolic codes. 1296 is ER_GET_ERRMSG. +--remove_file $MYSQLTEST_VARDIR/keys1.txt > +--remove_file $MYSQLTEST_VARDIR/keys2.txt > + > +--write_file $MYSQLTEST_VARDIR/keys1.txt > +1;770A8A65DA156D24EE2A093277530142 > +4;770A8A65DA156D24EE2A093277530143 > +EOF > Why remove and rewrite the file with the identical content? Similar steps are being done in multiple encryption tests. Can some of the repetition be avoided? Or could we test multiple operations with one server restart? Instead of having one test to check if CHECK TABLE crashes when the keys are incorrect or not available, another to check if ALTER TABLE crashes, and so on, just restart the server once and then execute all the commands that are to be tested. That would make --suite=encryption run faster too. > +--exec echo "restart:--innodb-encrypt-tables --innodb-stats-persistent > --plugin-load-add=file_key_management.so --file-key-management > --file-key-management-filename=$MYSQLTEST_VARDIR/keys1.txt" > > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect > +--enable_reconnect > +--source include/wait_until_connected_again.inc > + > +drop table t1,t2,t3; > + > +--remove_file $MYSQLTEST_VARDIR/keys1.txt > The server was left running, tied to keys1.txt. Shouldn’t we rather restart the server at the end of the test, to have the same configuration as it was at the start of the test? > diff --git a/mysql-test/suite/encryption/t/innodb-force-corrupt.test > b/mysql-test/suite/encryption/t/innodb-force-corrupt.test > new file mode 100644 > index 00000000000..3127b8f4006 > --- /dev/null > +++ b/mysql-test/suite/encryption/t/innodb-force-corrupt.test > @@ -0,0 +1,122 @@ > +# > +# MDEV-11759: Encryption code in MariaDB 10.1/10.2 causes compatibility > problems > +# > + > +-- source include/have_innodb.inc > +-- source include/have_file_key_management_plugin.inc > +# Don't test under valgrind, memory leaks will occur > +source include/not_valgrind.inc; > Which memory leaks? Please run this with --valgrind and show. > +# Avoid CrashReporter popup on Mac > +source include/not_crashrep.inc; > Why? The test should not crash. > +# Don't test under embedded > +source include/not_embedded.inc; > +# Require InnoDB > +source include/have_innodb.inc; > I would omit this kind of redundant comments. I’d add a comment to explain that embedded server tests do not support restarting. > +# Test could open crash reporter on Windows > +# if compiler set up > +source include/not_windows.inc; > This is totally unacceptable. We must test under Windows on Buildbot. > +call mtr.add_suppression("InnoDB: Database page corruption on disk or a > failed.*"); > +call mtr.add_suppression("InnoDB: Corruption: Block in space_id .* in > file .* corrupted."); > +call mtr.add_suppression("InnoDB: Based on page type .*"); > +call mtr.add_suppression("InnoDB: Unable to read tablespace .* page no .* > into the buffer pool after .* attempts"); > +CALL mtr.add_suppression("InnoDB: Error: Unable to read tablespace .* > page no .* into the buffer pool after 100 attempts"); > +CALL mtr.add_suppression("InnoDB: Database page corruption on disk or a > failed"); > +CALL mtr.add_suppression("InnoDB: Space .* file test/t1 read of page .*"); > +CALL mtr.add_suppression("InnoDB: You may have to recover from a > backup."); > +CALL mtr.add_suppression("InnoDB: It is also possible that your > operatingsystem has corrupted its own file cache."); > +CALL mtr.add_suppression("InnoDB: and rebooting your computer removes the > error."); > +CALL mtr.add_suppression("InnoDB: If the corrupt page is an index page > you can also try to"); > +CALL mtr.add_suppression("InnoDB: fix the corruption by dumping, > dropping, and reimporting"); > +CALL mtr.add_suppression("InnoDB: the corrupt table. You can use CHECK"); > +CALL mtr.add_suppression("InnoDB: TABLE to scan your table for > corruption."); > +CALL mtr.add_suppression("InnoDB: See also .* about forcing recovery."); > +CALL mtr.add_suppression("InnoDB: However key management plugin or used > key_version 3221605118 is not found or used encryption algorithm or method > does not match."); > +CALL mtr.add_suppression("Marking tablespace as missing. You may drop > this table or install correct key management plugin and key file."); > +CALL mtr.add_suppression("InnoDB: Block in space_id .* in file .* > encrypted."); > Are all these suppressions really necessary? Some of the first 4 suppressions are matching same strings as some of the later suppressions. > +let $MYSQLD_DATADIR=`select @@datadir`; > +let INNODB_PAGE_SIZE=`select @@innodb_page_size`; > +let MYSQLD_DATADIR=`select @@datadir`; > Why is MYSQLD_DATADIR assigned both to an mtr internal variable and an environment variable? The latter should suffice. > +--source include/shutdown_mysqld.inc > +--source include/wait_until_disconnected.inc > The wait_until_disconnected.inc is redundant and should be removed. > +--disable_result_log > +--error 1296 > +SELECT * FROM t1; > +# > +# There is change that index is marked as corrupted here > +# > +--error 0,1712 > +SELECT * FROM t2; > chance != change. Why would t2 sometimes be not corrupted? In which way does it fail if the 0, is removed? Use symbolic error codes, not numeric ones. > +--error 1296 > +SELECT * FROM t3; > +--enable_result_log > + > +--source include/shutdown_mysqld.inc > +--source include/wait_until_disconnected.inc > Again, the wait_until_disconnected.inc should be removed. > + > +--echo # Restore the original t1.ibd > +--remove_file $MYSQLD_DATADIR/test/t1.ibd > +--move_file $MYSQLD_DATADIR/test/t1.ibd.backup > $MYSQLD_DATADIR/test/t1.ibd > +--move_file $MYSQLD_DATADIR/test/t2.ibd.backup > $MYSQLD_DATADIR/test/t2.ibd > +--move_file $MYSQLD_DATADIR/test/t3.ibd.backup > $MYSQLD_DATADIR/test/t3.ibd > + > +--source include/start_mysqld.inc > + > +DROP TABLE t1,t2,t3; > diff --git a/mysql-test/suite/encryption/t/innodb-redo-badkey.test b/mysql-test/suite/encryption/t/innodb-redo-badkey.test > new file mode 100644 > index 00000000000..34ffeb1b59a > --- /dev/null > +++ b/mysql-test/suite/encryption/t/innodb-redo-badkey.test > @@ -0,0 +1,121 @@ > +-- source include/have_innodb.inc > +-- source include/have_file_key_management_plugin.inc > +# embedded does not support restart > +-- source include/not_embedded.inc > +# Don't test this under valgrind, memory leaks will occur. > +--source include/not_valgrind.inc > +# Avoid CrashReporter popup on Mac > +--source include/not_crashrep.inc > +--source include/not_windows.inc > The not_windows.inc is not acceptable. I would say that crashes or memory leaks are not acceptable either. > +--source ../../suite/innodb/include/no_checkpoint_start.inc > + > +--disable_warnings > +SET GLOBAL innodb_file_format = `Barracuda`; > +SET GLOBAL innodb_file_per_table = ON; > +--enable_warnings > + > +create table t1(a int not null primary key auto_increment, c char(250), b > blob) engine=innodb row_format=compressed encrypted=yes > encryption_key_id=10; > +create table t2(a int not null primary key auto_increment, c char(250), b > blob) engine=innodb row_format=compressed; > +create table t3(a int not null primary key auto_increment, c char(250), b > blob) engine=innodb encrypted=yes encryption_key_id=10; > +create table t4(a int not null primary key auto_increment, c char(250), b > blob) engine=innodb; > +CREATE INDEX test ON t1 (b(10)); > +CREATE INDEX test ON t2 (b(10)); > +CREATE INDEX test ON t3 (b(10)); > +CREATE INDEX test ON t4 (b(10)); > + > +--disable_query_log > +--let $i = 20 > +begin; > +while ($i) > +{ > + insert into t1(c,b) values (repeat('secret1',20), > repeat('secret2',6000)); > + dec $i; > +} > +commit; > +--enable_query_log > + > +begin; > +insert into t2 select * from t1; > +insert into t3 select * from t1; > +insert into t4 select * from t1; > +commit; > + > +--remove_file $MYSQLTEST_VARDIR/keys1.txt > +let SEARCH_FILE= $MYSQLTEST_VARDIR/log/my_restart.err; > + > +# > +# We test redo log page read at recv_read_page using > +# incorrect keys from std_data/keys.txt. If checkpoint > +# happens we will skip this test. If no checkpoint > +# happens, InnoDB refuses to start as used > +# encryption key is incorrect. > +# > +SET GLOBAL innodb_buf_flush_list_now = 1; > +begin; > +update t1 set c = repeat('secret3', 20); > +update t2 set c = repeat('secret4', 20); > +update t3 set c = repeat('secret4', 20); > +update t4 set c = repeat('secret4', 20); > +insert into t1 (c,b) values (repeat('secret5',20), > repeat('secret6',6000)); > +insert into t2 (c,b) values (repeat('secret7',20), > repeat('secret8',6000)); > +insert into t3 (c,b) values (repeat('secret9',20), > repeat('secre10',6000)); > +insert into t4 (c,b) values (repeat('secre11',20), > repeat('secre12',6000)); > +COMMIT; > +let $cleanup= drop table t1,t2,t3,t4; > +--let CLEANUP_IF_CHECKPOINT= $cleanup; > +--source ../../suite/innodb/include/no_checkpoint_end.inc > Do we really need to execute all this code while not allowing any redo log checkpoint? It would seem to me that this test would be skipped most of the time. Where is the file my_restart.err created? +--write_file $MYSQLTEST_VARDIR/keys1.txt > +1;770A8A65DA156D24EE2A093277530997 > +10;770A8A65DA156D24EE2A093277530142 > +EOF > + > +--echo # restart > +--error 1 > +-- source include/start_mysqld.inc > +--source include/kill_mysqld.inc > This looks very suspicious. Why is the start and kill needed? And when would --source return an error? Why would we want to kill the server here? A shutdown would make the test more deterministic. I think that we would want to ensure that InnoDB refused to start up after the start_mysqld.inc. > diff --git a/mysql-test/suite/encryption/t/innodb-redo-nokeys.test > b/mysql-test/suite/encryption/t/innodb-redo-nokeys.test > new file mode 100644 > index 00000000000..bfe5cd78e5b > --- /dev/null > +++ b/mysql-test/suite/encryption/t/innodb-redo-nokeys.test > @@ -0,0 +1,98 @@ > +-- source include/have_innodb.inc > +-- source include/have_file_key_management_plugin.inc > +# embedded does not support restart > +-- source include/not_embedded.inc > +# Don't test this under valgrind, memory leaks will occur. > +--source include/not_valgrind.inc > +# Avoid CrashReporter popup on Mac > +--source include/not_crashrep.inc > +--source include/not_windows.inc > + > +--write_file $MYSQLTEST_VARDIR/keys1.txt > +1;770A8A65DA156D24EE2A093277530997 > +10;770A8A65DA156D24EE2A093277530999 > +EOF > This file is very similar to the other added tests. I would prefer to have one test that tests multiple things across a server restart, instead of having a large number of tests that duplicate a lot of boilerplate code and make it hard to follow the actual purpose of the test. Can you please merge some of the tests? +--source ../../suite/innodb/include/no_checkpoint_start.inc > + > +create table t1(a int not null primary key auto_increment, c char(200), b > blob) engine=innodb row_format=compressed encrypted=yes > encryption_key_id=10; > +create table t2(a int not null primary key auto_increment, c char(200), b > blob) engine=innodb row_format=compressed; > +create table t3(a int not null primary key auto_increment, c char(200), b > blob) engine=innodb encrypted=yes encryption_key_id=10; > +create table t4(a int not null primary key auto_increment, c char(200), b > blob) engine=innodb; > +CREATE INDEX test ON t1 (b(10)); > +CREATE INDEX test ON t2 (b(10)); > +CREATE INDEX test ON t3 (b(10)); > +CREATE INDEX test ON t4 (b(10)); > + > +--disable_query_log > +--let $i = 20 > +begin; > +while ($i) > +{ > + insert into t1(c,b) values (repeat('secret1',20), > repeat('secret2',6000)); > + dec $i; > +} > +commit; > +--enable_query_log > + > +begin; > +insert into t2 select * from t1; > +insert into t3 select * from t1; > +insert into t4 select * from t1; > +commit; > + > +--remove_file $MYSQLTEST_VARDIR/keys1.txt > + > +# > +# We test redo log page read at recv_read_page using > +# incorrect keys from std_data/keys.txt. If checkpoint > +# happens we will skip this test. If no checkpoint > +# happens, InnoDB refuses to start as used > +# encryption key is not found. > +# > +SET GLOBAL innodb_flush_log_at_trx_commit=1; > +begin; > +update t1 set c = repeat('secret3', 20); > +update t2 set c = repeat('secret4', 20); > +update t3 set c = repeat('secret4', 20); > +update t4 set c = repeat('secret4', 20); > +insert into t1 (c,b) values (repeat('secret5',20), > repeat('secret6',6000)); > +insert into t2 (c,b) values (repeat('secret7',20), > repeat('secret8',6000)); > +insert into t3 (c,b) values (repeat('secret9',20), > repeat('secre10',6000)); > +insert into t4 (c,b) values (repeat('secre11',20), > repeat('secre12',6000)); > +COMMIT; > +let $cleanup= drop table t1,t2,t3,t4; > +--let CLEANUP_IF_CHECKPOINT= $cleanup; > +--source ../../suite/innodb/include/no_checkpoint_end.inc > Is it really necessary to place the no_checkpoint_start.inc before the table creation statements? Why are there separate CREATE TABLE and CREATE INDEX statements for the empty tables? > diff --git a/mysql-test/suite/innodb/t/innodb_bug14147491.test > b/mysql-test/suite/innodb/t/innodb_bug14147491.test > index 5776b2c2e37..d16a44e8a55 100644 > --- a/mysql-test/suite/innodb/t/innodb_bug14147491.test > +++ b/mysql-test/suite/innodb/t/innodb_bug14147491.test > @@ -7,6 +7,7 @@ > call mtr.add_suppression("InnoDB: Database page corruption on disk or a > failed.*"); > call mtr.add_suppression("InnoDB: Corruption: Block in space_id .* in > file .* corrupted."); > call mtr.add_suppression("InnoDB: Based on page type .*"); > +call mtr.add_suppression("InnoDB: Unable to read tablespace .* page no .* > into the buffer pool after .* attempts"); > > # Don't test under valgrind, memory leaks will occur > source include/not_valgrind.inc; > I think that InnoDB is already noisy enough. Please fix the code instead of adding one more warning that is not really helping users. @@ -4096,6 +4096,11 @@ btr_estimate_number_of_different_key_vals( > > page = btr_cur_get_page(&cursor); > > + if (index->table->file_unreadable || > index->table->corrupted) { > + mtr_commit(&mtr); > + goto exit_loop; > + } > + > rec = page_rec_get_next(page_get_infimum_rec(page)); > > if (!page_rec_is_supremum(rec)) { > When is this code covered? Why are we not testing this before assigning page? That variable is not being used after the goto. In fact, the variable "page" should be declared inside the for loop body. diff --git a/storage/innobase/btr/btr0pcur.cc b/storage/innobase/btr/ > btr0pcur.cc > index 01d2e1bb8e2..237e4e8d109 100644 > --- a/storage/innobase/btr/btr0pcur.cc > +++ b/storage/innobase/btr/btr0pcur.cc > @@ -416,6 +416,11 @@ btr_pcur_move_to_next_page( > cursor->old_stored = BTR_PCUR_OLD_NOT_STORED; > > page = btr_pcur_get_page(cursor); > + > + if (!page) { > + return; > + } > + > next_page_no = btr_page_get_next(page, mtr); > space = buf_block_get_space(btr_pcur_get_block(cursor)); > zip_size = buf_block_get_zip_size(btr_pcur_get_block(cursor)); > @@ -425,11 +430,16 @@ btr_pcur_move_to_next_page( > next_block = btr_block_get(space, zip_size, next_page_no, > cursor->latch_mode, > btr_pcur_get_btr_cur(cursor)->index, > mtr); > + > + if (!next_block) { > + return; > + } > + > next_page = buf_block_get_frame(next_block); > #ifdef UNIV_BTR_DEBUG > ut_a(page_is_comp(next_page) == page_is_comp(page)); > ut_a(btr_page_get_prev(next_page, mtr) > - == buf_block_get_page_no(btr_pcur_get_block(cursor))); > + == buf_block_get_page_no(btr_pcur_get_block(cursor))); > #endif /* UNIV_BTR_DEBUG */ > next_block->check_index_page_at_flush = TRUE; > Are these changes really necessary? In which tests would the added "return" statements be executed? These conditions look like they could hurt performance. > diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/ > buf0buf.cc > index c7e2183fa7b..066a68da8c0 100644 > --- a/storage/innobase/buf/buf0buf.cc > +++ b/storage/innobase/buf/buf0buf.cc > @@ -2358,7 +2358,18 @@ buf_page_get_zip( > /* Page not in buf_pool: needs to be read from file */ > > ut_ad(!hash_lock); > - buf_read_page(space, zip_size, offset, NULL); > + dberr_t err = buf_read_page(space, zip_size, offset); > + > + if (err != DB_SUCCESS) { > + ib_logf(IB_LOG_LEVEL_ERROR, > + "Reading compressed page " ULINTPF > + " from tablespace " ULINTPF > + " failed with error: %s (%d).", > + offset, space, ut_strerr(err), err); > + > + goto err_exit; > + } > + > > #if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG > ut_a(++buf_dbg_counter % 5771 || buf_validate()); > Return the error to the only caller of this function, and let that function report the error. With the patch as it is, 2 messages would be reported for the single error. Reporting the ut_strerr(err) alone is sufficient; nobody is interested in the enum dberr_t numeric values. DBUG_EXECUTE_IF( > "innodb_page_corruption_retries", > - retries = BUF_PAGE_READ_MAX_RETRIES; > + goto force_fail; > ); > This change is wrong. The idea of the debug instrumentation is to speed up the failure (not retry reading the corrupted page for 100 times). The test must not crash even when this instrumentation is not present. > + /* Try to set table as corrupted instead of > + asserting. */ > + if (space > TRX_SYS_SPACE && > + dict_set_corrupted_by_space(space)) { > + return (NULL); > } > Why should we commit suicide when a page in the system tablespace is corrupted? I would just return NULL from here in any case. - if (!bpage->encrypted) { > - ut_ad(buf_pool->n_pend_reads > 0); > - buf_pool->n_pend_reads--; > + /* After this point bpage can't be referenced. */ > + buf_LRU_free_one_page(bpage); > > - buf_pool_mutex_exit(buf_pool); > - } > + ut_ad(buf_pool->n_pend_reads > 0); > + buf_pool->n_pend_reads--; > > - return(ret); > + buf_pool_mutex_exit(buf_pool); > } > Elsewhere, MariaDB 10.1 is using atomic memory access for the field buf_pool->n_pend_reads. We must be consistent. If some reader or writer is using atomic memory access and no buf_pool->mutex at all, then every access must use atomic memory access, no matter if holding buf_pool->mutex. @@ -4708,31 +4696,12 @@ buf_page_io_complete( > /* If page space id is larger than > TRX_SYS_SPACE > (0), we will attempt to mark the > corresponding > table as corrupted instead of crashing > server */ > - if (bpage->space > TRX_SYS_SPACE > - && buf_mark_space_corrupt(bpage)) { > - return(false); > + if (bpage->space > TRX_SYS_SPACE) { > + buf_mark_space_corrupt(bpage); > + return(err); > } else { > - if (!bpage->encrypted) { > - ib_logf(IB_LOG_LEVEL_ERROR, > - "Ending processing > because of a corrupt database page."); > - > - ut_error; > - } > - > - ib_push_warning(innobase_get_trx(), > DB_DECRYPTION_FAILED, > - "Table in tablespace %lu > encrypted." > - "However key management > plugin or used key_id %lu is not found or" > - " used encryption > algorithm or method does not match." > - " Can't continue opening > the table.", > - bpage->space, > bpage->key_version); > - > - if (bpage->encrypted && > bpage->space > TRX_SYS_SPACE) { > - > buf_mark_space_corrupt(bpage); > - } else { > - ut_error; > - } > - > - return(false); > + ib_logf(IB_LOG_LEVEL_FATAL, > + "Ending processing because > of a corrupt database page."); > } > } > } > Again, why commit suicide if a page in the system tablespace cannot be read? I think that we should try our best to allow any data to be rescue, and not crash early. diff --git a/storage/innobase/buf/buf0flu.cc b/storage/innobase/buf/ > buf0flu.cc > index 3f03ee8256f..e805585ed1e 100644 > --- a/storage/innobase/buf/buf0flu.cc > +++ b/storage/innobase/buf/buf0flu.cc > @@ -956,7 +956,10 @@ buf_flush_write_block_low( > > /* true means we want to evict this page from the > LRU list as well. */ > - buf_page_io_complete(bpage, true); > + dberr_t err = buf_page_io_complete(bpage, true); > + > + /* This is write path, encryption should not fail here. */ > + ut_ad(err == DB_SUCCESS); > } > > /* Increment the counter of I/O operations used > I think I already noted in an earlier review that a warning for the unused variable "err" would be issued for the non-debug build. Please build both non-debug and debug, and pay attention to the warnings! @@ -444,23 +464,23 @@ High-level function which reads a page asynchronously > from a file to the > buffer buf_pool if it is not already there. Sets the io_fix flag and sets > an exclusive lock on the buffer frame. The flag is cleared and the x-lock > released by the i/o-handler thread. > -@return TRUE if page has been read in, FALSE in case of failure */ > +@param[in] space Tablespace id > +@param[in] offset Page no */ > UNIV_INTERN > -ibool > +void > buf_read_page_async( > -/*================*/ > - ulint space, /*!< in: space id */ > - ulint offset) /*!< in: page number */ > + ulint space, > + ulint offset) > Add a function comment that this function is only used for restoring buffer pool dumps, and that we do not care if the page exists or if the read succeeded. @@ -728,16 +748,30 @@ buf_read_ahead_linear( > count += buf_read_page_low( > &err, false, > ibuf_mode, > - space, zip_size, FALSE, > tablespace_version, i, NULL); > - if (err == DB_TABLESPACE_DELETED) { > - ut_print_timestamp(stderr); > - fprintf(stderr, > - " InnoDB: Warning: in" > - " linear readahead trying to > access\n" > - "InnoDB: tablespace %lu page > %lu,\n" > - "InnoDB: but the tablespace does > not" > - " exist or is just being > dropped.\n", > - (ulong) space, (ulong) i); > + space, zip_size, FALSE, > tablespace_version, i); > + > + switch(err) { > + case DB_SUCCESS: > + break; > + case DB_TABLESPACE_DELETED: > + ib_logf(IB_LOG_LEVEL_WARN, > + "In random" > + "readahead trying to access" > + "tablespace " ULINTPF " page " > ULINTPF > + " but the tablespace does not" > + " exist or is just being dropped.", > + space, i); > + break; > + > + case DB_DECRYPTION_FAILED: > + ib_logf(IB_LOG_LEVEL_ERROR, > + "In random readahead read page " > ULINTPF > + " from tablespace " ULINTPF > + " but page is still encrypted.", > + i, space); > + break; > + default: > + ut_error; > } > You seem to have missed my earlier review comment where I noted that the message about "linear readahead" is misleadingly replaced by messages mentioning "random readahead". > @@ -829,6 +863,13 @@ buf_read_ibuf_merge_pages( > page_nos[i], > zip_size, FALSE); > } > + > + if (err == DB_DECRYPTION_FAILED) { > + ib_logf(IB_LOG_LEVEL_ERROR, > + "Read page " ULINTPF " from tablespace " > ULINTPF > + " for insert buffer but page was > encrypted.", > + page_nos[i], space_ids[i]); > + } > } > > os_aio_simulated_wake_handler_threads(); > It would be clearer to say in the error message that we were unable to decrypt the page. > @@ -2192,7 +2192,7 @@ dict_load_table_low( > > (*table)->id = mach_read_from_8(field); > > - (*table)->ibd_file_missing = FALSE; > + (*table)->file_unreadable = FALSE; > > return(NULL); > } > @@ -2379,7 +2379,7 @@ dict_load_table( > "Table '%s' tablespace is set as discarded.", > table_name); > > - table->ibd_file_missing = TRUE; > + table->file_unreadable = true; > > } else if (!fil_space_for_table_exists_in_mem( > table->space, name, false, true, heap, > @@ -2387,7 +2387,7 @@ dict_load_table( > > if (DICT_TF2_FLAG_IS_SET(table, DICT_TF2_TEMPORARY)) { > /* Do not bother to retry opening temporary > tables. */ > - table->ibd_file_missing = TRUE; > + table->file_unreadable = true; > > } else { > if (!(ignore_err & DICT_ERR_IGNORE_RECOVER_LOCK)) { > @@ -2422,7 +2422,7 @@ dict_load_table( > /* We failed to find a sensible > tablespace file */ > > - table->ibd_file_missing = TRUE; > + table->file_unreadable = TRUE; > } > if (filepath) { > mem_free(filepath); > This is not consistent. Sometimes you are using true, sometimes TRUE or FALSE for the unsigned:1 bit-field. Either way is fine, but be consistent. > diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/ > fil0crypt.cc > index 7bef91f47ff..6a54ec6df35 100644 > --- a/storage/innobase/fil/fil0crypt.cc > +++ b/storage/innobase/fil/fil0crypt.cc > @@ -541,6 +541,11 @@ fil_parse_write_crypt_data( > fil_space_release(space); > } > > + /* Check is used key found from encryption plugin */ > + if (crypt_data->should_encrypt() && !crypt_data->is_key_found()) { > + return NULL; > + } > + > return ptr; > } > > As far as I can understand, this is prematurely ending the redo log recovery, without returning any error! This is not acceptable. Please add a proper test for this and ensure that an error is being returned. > diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/ > fil0fil.cc > index 627a63aaae0..079803eedf3 100644 > --- a/storage/innobase/fil/fil0fil.cc > +++ b/storage/innobase/fil/fil0fil.cc > @@ -6292,6 +6292,8 @@ fil_aio_wait( > mutex_enter(&fil_system->mutex); > > fil_node_complete_io(fil_node, fil_system, type); > + ulint purpose = fil_node->space->purpose; > + ulint space_id = fil_node->space->id; > > mutex_exit(&fil_system->mutex); > > Why assign fil_node->space->purpose to a local variable? It is only referenced once. > @@ -6303,9 +6305,29 @@ fil_aio_wait( > deadlocks in the i/o system. We keep tablespace 0 data files always > open, and use a special i/o thread to serve insert buffer > requests. */ > > - if (fil_node->space->purpose == FIL_TABLESPACE) { > + if (purpose == FIL_TABLESPACE) { > srv_set_io_thread_op_info(segment, "complete io for buf > page"); > - buf_page_io_complete(static_cast<buf_page_t*>(message)); > + buf_page_t* bpage = static_cast<buf_page_t*>(message); > + ulint offset = bpage ? bpage->offset : ULINT_UNDEFINED; > + > + dberr_t err = buf_page_io_complete(bpage); > + > + if (err != DB_SUCCESS) { > + ib_log_level_t level = IB_LOG_LEVEL_FATAL; > + > + /* In crash recovery set log corruption on > + and produce only an error to fail InnoDB startup. > */ > + if (recv_recovery_is_on()) { > + recv_sys->found_corrupt_log = true; > + level = IB_LOG_LEVEL_ERROR; > + } > + > + ib_logf(level, > + "%s operation failed for tablespace " > + ULINTPF " offset " ULINTPF " error=%s > (%d).", > + type == OS_FILE_WRITE ? "Write" : "Read", > + space_id, offset, ut_strerr(err),err); > + } > } else { > srv_set_io_thread_op_info(segment, "complete io for log"); > log_io_complete(static_cast<log_group_t*>(message)); > Can this really ever fail for writes? Please add a debug assertion and simplify the message. There is no need to display the numeric value of dberr_t; the ut_strerr() suffices. diff --git a/storage/innobase/include/btr0pcur.ic > b/storage/innobase/include/btr0pcur.ic > index 1cd13824542..87b48de6389 100644 > --- a/storage/innobase/include/btr0pcur.ic > +++ b/storage/innobase/include/btr0pcur.ic > @@ -357,6 +357,11 @@ btr_pcur_move_to_next( > return(FALSE); > } > > + if (cursor->btr_cur.index->table->corrupted || > + cursor->btr_cur.index->table->file_unreadable) { > + return(FALSE); > + } > + > btr_pcur_move_to_next_page(cursor, mtr); > > return(TRUE); > Is this change really needed? Can we merge the "corrupted" flag to "file_unreadable" to make the check less expensive? diff --git a/storage/innobase/include/log0recv.h b/storage/innobase/include/ > log0recv.h > index 9ca1c46d72b..1ff5cf721bf 100644 > --- a/storage/innobase/include/log0recv.h > +++ b/storage/innobase/include/log0recv.h > @@ -274,9 +274,10 @@ recv_sys_var_init(void); > #endif /* !UNIV_HOTBACKUP */ > /** Apply the hash table of stored log records to persistent data pages. > @param[in] last_batch whether the change buffer merge will be > - performed as part of the operation */ > + performed as part of the operation > +@return DB_SUCCESS or DB_DECRYPTION_FAILED */ > UNIV_INTERN > -void > +dberr_t > recv_apply_hashed_log_recs(bool last_batch); > Please try to keep the return value as void, or alternatively add MY_ATTRIBUTE((warn_unused_result)) and fix all callers. I do not think it is easy to fix log_checkpoint(), > diff --git a/storage/innobase/log/log0recv.cc b/storage/innobase/log/ > log0recv.cc > index 45fd42848e8..e56d4a46650 100644 > --- a/storage/innobase/log/log0recv.cc > +++ b/storage/innobase/log/log0recv.cc > @@ -1298,6 +1298,10 @@ recv_parse_or_apply_log_rec_body( > break; > case MLOG_FILE_WRITE_CRYPT_DATA: > ptr = const_cast<byte*>(fil_parse_write_crypt_data(ptr, > end_ptr, block)); > + > + if (!ptr) { > + recv_sys->found_corrupt_log = TRUE; > + } > break; > default: > ptr = NULL; > If block==NULL, ptr=NULL is returned on EOF. We do not want to have random bogus failures in redo log recovery! Even if a redo log record happens to be split across multiple redo log segments, we must parse the whole redo log! > @@ -3549,7 +3553,7 @@ row_truncate_table_for_mysql( > "create a new tablespace", > table->name); > > - table->ibd_file_missing = 1; > + table->file_unreadable = 1; > err = DB_ERROR; > goto funct_exit; > } > Here we are assigning yet another literal to the field. Earlier, FALSE, TRUE, and true have been observed. Be consistent. Or even better, add a method void dict_table_t::set_unreadable(bool unreadable = true) and use it. > diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/ > row0sel.cc > index dab1bc58db5..8f73ed4d49a 100644 > --- a/storage/innobase/row/row0sel.cc > +++ b/storage/innobase/row/row0sel.cc > @@ -3714,11 +3714,12 @@ row_search_for_mysql( > > return(DB_TABLESPACE_DELETED); > > - } else if (prebuilt->table->ibd_file_missing) { > + } else if (prebuilt->table->file_unreadable && > + fil_space_get(prebuilt->table->space) == NULL) { > > return(DB_TABLESPACE_NOT_FOUND); > > - } else if (prebuilt->table->is_encrypted) { > + } else if (prebuilt->table->file_unreadable) { > > return(DB_DECRYPTION_FAILED); > } else if (!prebuilt->index_usable) { > I would add an method bool dict_table_t::is_file_missing() const and use it instead of duplicating the code. 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] Queries on ticket MDEV-7502(GSOC)
by Vishwanath Bharadwaj 22 Mar '17

22 Mar '17

Hi, Vishwanath! Hello MR. Sergei, I am Vishwanath Bharadwaj and I've contacted you earlier regarding the project. With the help of the code Kristian provided in the comment section of the ticket I was able to review the code of the project proceedings till date. I was very much impressed by the way it has been implemented. I don't have any major queries (just some minor issues on the implementation of the semaphores). I've also familiarized myself with some code of both statement based binary logging and row- based logging. I would also like to get into the details of what the provisioning mode permissions are? And the method of using the primary key to identify the remaining chunk is not quite effective because weare limiting the options only to tables with primary keys. This is quite a unique and interesting project and I'm looking forward to working and discussing with you on the same. Thank you:)

2 1

[Maria-developers] MDEV-10598 - bb-10.2-compatibility
by jerome brauge 22 Mar '17

22 Mar '17

Alexander, As requested, this is my last patch for MDEV-10598. Of course, I share this code under MCA. Best regards, Jérôme. > -----Message d'origine----- > De : Alexander Barkov [mailto:bar@mariadb.org] > Envoyé : mercredi 22 mars 2017 12:00 > À : jerome brauge > Objet : Re: MDEV-10598 - bb-10.2-compatibility > > Hello Jerome, > > > On 03/22/2017 01:37 PM, jerome brauge wrote: > > Hello Alexander, > > All works fine, thank you very much. > > Thanks for confirming! > > Can I ask you for some bureaucracy? > > Can you please send your last patch for "MDEV-10598 - bb-10.2- > compatibility" to maria-developers and specify that you share it under BSD > license? > > This is needed to officially accept your code into our trees. > > Thanks! > > > > Now, we are blocked by MDEV-12137 (delete or update statement with > the same source and target). > > When do you think you can work on it? > > Sorry, I don't have expertise in this part of the code. > > Somebody else will be working on it. > But I'm afraid this is not something of high priority at the moment. > > > > > > Regards, > > Jérôme. > > > >> -----Message d'origine----- > >> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : jeudi 16 mars > >> 2017 13:03 À : jerome brauge Cc : MariaDB Developers > >> (maria-developers(a)lists.launchpad.net) > >> Objet : Re: MDEV-10598 - bb-10.2-compatibility > >> > >> Hello Jerome, > >> > >> > >> On 03/14/2017 06:07 PM, jerome brauge wrote: > >>> Hi Alexander, > >>> Can you review this patch ? > >>> I could add more tests when your others points will be corrected. > >> > >> I tried to simplify your patch slightly. > >> > >> sp_instr_cpush instructions are now created from the array > >> sp_pcontext::m_cursors, which already stores all cursors declared on > >> the current frame. > >> > >> So there is now no a need for a separate list sp_head::m_delayed_cpush. > >> > >> This seems to work fine. > >> > >> Also, I changed the meaning of the last argument of > >> sp_declare_cursor() to the opposite: from "delay_instr" to > "add_cpush_instr". > >> I find "bool do_something" easier to read than "bool > >> dont_do_something", especially in this context: > >> > >> if (do_something) > >> > >> vs > >> > >> if (!dont_do_something) > >> > >> > >> but this is probably my personal preference :) > >> > >> Also I removed overloading (and fixed sql_yacc.yy instead). > >> > >> > >> Can you please review the attached patch? > >> > >> (It also includes my previous suggestions) > >> > >> Many thanks!!! > >> > >>> > >>> Thanks. > >>> Jérôme. > >>> > >>> > >>>> -----Message d'origine----- > >>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : mardi 14 > >>>> mars > >>>> 2017 11:54 À : jerome brauge Cc : MariaDB Developers > >>>> (maria-developers(a)lists.launchpad.net) > >>>> Objet : Re: MDEV-10598 - bb-10.2-compatibility > >>>> > >>>> Hi Jerome, > >>>> > >>>> On 03/14/2017 02:29 PM, jerome brauge wrote: > >>>>> Thanks Alexander, > >>>>> > >>>>> I have two other problems to finalize my patch: > >>>>> - Attached script cur_err_warning.sql produce a warning 1931 > >>>>> (Query > >>>> execution was interrupted. The query examined at least 2 rows, > >>>> which exceeds LIMIT ROWS EXAMINED (0). The query result may be > >>>> incomplete) and I don't see why. > >>>> > >>>> That's my fault. Please ignore this warning for now. > >>>> This warning happens when the procedure opens a cursor to resolve > >>>> its structure. I will suppress this warning later. > >>>> > >>>> > >>>>> > >>>>> - Attached script cur_err_field_name.sql produce an error 4057 > >>>>> (HY000) at > >>>> line 23: Row variable 'rec2' does not have a field 'a' > >>>> > >>>> This is also my bug. > >>>> The problem is that rec2 has a name "rec1.a" instead of just "a". > >>>> > >>>> > >>>> > >>>> Please change > >>>> > >>>> CURSOR cur2 IS SELECT rec1.a ; > >>>> > >>>> to > >>>> > >>>> CURSOR cur2 IS SELECT rec1.a AS a; > >>>> > >>>> > >>>>> > >>>>> These two scripts work fine on oracle. > >>>>> > >>>>> Jérôme. > >>>>> > >>>>> > >>>>>> -----Message d'origine----- > >>>>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : mardi 14 > >>>>>> mars > >>>>>> 2017 11:11 À : jerome brauge Objet : Re: MDEV-10598 - > >>>>>> bb-10.2-compatibility > >>>>>> > >>>>>> Hello Jerome, > >>>>>> > >>>>>> > >>>>>> > >>>>>> On 03/13/2017 07:46 PM, jerome brauge wrote: > >>>>>>> Alexander, > >>>>>>> I think there is a little bug with ROWTYPE: when I affect a > >>>>>>> variable with a > >>>>>> field value, the server crash. > >>>>>>> I attached the script which cause the issue. > >>>>>> > >>>>>> Thanks for reporting this. > >>>>>> > >>>>>> > >>>>>> This quick patch fixes the problem: > >>>>>> > >>>>>> > >>>>>> diff --git a/sql/sp_head.cc b/sql/sp_head.cc index > >>>>>> faf9f5f..745f982 > >>>>>> 100644 > >>>>>> --- a/sql/sp_head.cc > >>>>>> +++ b/sql/sp_head.cc > >>>>>> @@ -347,6 +347,12 @@ Item * > >>>>>> sp_prepare_func_item(THD* thd, Item **it_addr, uint cols) { > >>>>>> DBUG_ENTER("sp_prepare_func_item"); > >>>>>> + if (!(*it_addr)->fixed && > >>>>>> + (*it_addr)->fix_fields(thd, it_addr)) { > >>>>>> + DBUG_PRINT("info", ("fix_fields() failed")); > >>>>>> + DBUG_RETURN(NULL); > >>>>>> + } > >>>>>> it_addr= (*it_addr)->this_item_addr(thd, it_addr); > >>>>>> > >>>>>> if ((!(*it_addr)->fixed && > >>>>>> > >>>>>> > >>>>>> > >>>>>> But I'm still thinking. > >>>>>> Perhaps I'll end some with some different patch. > >>>>>> I let you know when the final fix is pushed. > >>>>>> > >>>>>> > >>>>>>> > >>>>>>> This is the call stack : > >>>>>>> > >>>>>>>> mysqld.exe!my_sigabrt_handler(int sig) Line 477 C > >>>>>>> mysqld.exe!raise(int signum) Line 516 C++ > >>>>>>> mysqld.exe!abort() Line 64 C++ > >>>>>>> mysqld.exe!common_assert_to_stderr_direct(const wchar_t > >> * const > >>>>>> expression, const wchar_t * const file_name, const unsigned int > >>>>>> line_number) Line 124 C++ > >>>>>>> mysqld.exe!common_assert_to_stderr<wchar_t>(const > >> wchar_t * > >>>>>> const expression, const wchar_t * const file_name, const unsigned > int > >>>>>> line_number) Line 138 C++ > >>>>>>> mysqld.exe!common_assert<wchar_t>(const wchar_t * > >> const > >>>>>> expression, const wchar_t * const file_name, const unsigned int > >>>>>> line_number, void * const return_address) Line 383 C++ > >>>>>>> mysqld.exe!_wassert(const wchar_t * expression, const > >> wchar_t * > >>>>>> file_name, unsigned int line_number) Line 404 C++ > >>>>>>> > >> mysqld.exe!Item_splocal_row_field_by_name::this_item_addr(THD > >>>>>> * thd, Item * * it) Line 1814 C++ > >>>>>>> mysqld.exe!sp_prepare_func_item(THD * thd, Item * * > >> it_addr, > >>>>>> unsigned int cols) Line 350 C++ > >>>>>>> mysqld.exe!sp_eval_expr(THD * thd, Item * result_item, > >> Field * > >>>>>> result_field, Item * * expr_item_ptr) Line 391 C++ > >>>>>>> mysqld.exe!sp_rcontext::set_variable(THD * thd, unsigned > >> int > >>>>>>> idx, > >>>>>> Item * * value) Line 566 C++ > >>>>>>> mysqld.exe!sp_instr_set::exec_core(THD * thd, unsigned int > >> * > >>>>>> nextp) Line 3378 C++ > >>>>>>> mysqld.exe!sp_lex_keeper::reset_lex_and_exec_core(THD > >> * thd, > >>>>>> unsigned int * nextp, bool open_tables, sp_instr * instr) Line > >>>>>> 3097 > >> C++ > >>>>>>> mysqld.exe!sp_instr_set::execute(THD * thd, unsigned int * > >>>>>>> nextp) > >>>>>> Line 3371 C++ > >>>>>>> mysqld.exe!sp_head::execute(THD * thd, bool > >>>>>> merge_da_on_success) Line 1261 C++ > >>>>>>> mysqld.exe!sp_head::execute_procedure(THD * thd, > >> List<Item> * > >>>>>> args) Line 2086 C++ > >>>>>>> mysqld.exe!do_execute_sp(THD * thd, sp_head * sp) Line > >> 2890 > >>>>>> C++ > >>>>>>> mysqld.exe!mysql_execute_command(THD * thd) Line 5919 > >> C++ > >>>>>>> mysqld.exe!mysql_parse(THD * thd, char * rawbuf, unsigned > >> int > >>>>>> length, Parser_state * parser_state, bool is_com_multi, bool > >>>>>> is_next_command) Line 8006 C++ > >>>>>>> mysqld.exe!dispatch_command(enum_server_command > >> command, > >>>>>> THD * thd, char * packet, unsigned int packet_length, bool > >> is_com_multi, > >>>>>> bool is_next_command) Line 1821 C++ > >>>>>>> mysqld.exe!do_command(THD * thd) Line 1369 C++ > >>>>>>> mysqld.exe!threadpool_process_request(THD * thd) Line > >> 346 > >>>>>> C++ > >>>>>>> mysqld.exe!tp_callback(TP_connection * c) Line 192 C++ > >>>>>>> mysqld.exe!tp_callback(_TP_CALLBACK_INSTANCE * > >> instance, void * > >>>>>> context) Line 377 C++ > >>>>>>> mysqld.exe!work_callback(_TP_CALLBACK_INSTANCE * > >> instance, > >>>>>> void * context, _TP_WORK * work) Line 451 C++ > >>>>>>> > >>>>>>> Can you reproduce this ? > >>>>>>> > >>>>>>> Jérôme. > >>>>>>> > >>>>>>>> -----Message d'origine----- > >>>>>>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : lundi > >>>>>>>> 13 mars > >>>>>>>> 2017 14:47 À : jerome brauge Objet : Re: MDEV-10598 - > >>>>>>>> bb-10.2-compatibility > >>>>>>>> > >>>>>>>> Jérôme, > >>>>>>>> > >>>>>>>> On 03/13/2017 05:43 PM, jerome brauge wrote: > >>>>>>>>> Hello Alexander, > >>>>>>>>> I have to do some changes in the patch and add some tests > >>>>>>>>> cases (with row type) I think it will be ready this afternoon (CET). > >>>>>>>> > >>>>>>>> Excellent. You rock! > >>>>>>>> > >>>>>>>>> > >>>>>>>>> Jérôme. > >>>>>>>>> > >>>>>>>>> > >>>>>>>>>> -----Message d'origine----- > >>>>>>>>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : lundi > >>>>>>>>>> 13 mars > >>>>>>>>>> 2017 14:38 À : jerome brauge Cc : maria-developers Objet : Re: > >>>>>>>>>> MDEV-10598 - bb-10.2-compatibility > >>>>>>>>>> > >>>>>>>>>> Hello Jerome, > >>>>>>>>>> > >>>>>>>>>> will you try to apply your patch on top of the current > >>>>>>>>>> bb-10.2- > >>>>>>>> compatibility? > >>>>>>>>>> > >>>>>>>>>> Or should I do that? > >>>>>>>>>> > >>>>>>>>>> Thanks! > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> On 03/10/2017 02:36 PM, Alexander Barkov wrote: > >>>>>>>>>>> Hello Jerome, > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> On 02/27/2017 11:39 PM, jerome brauge wrote: > >>>>>>>>>>>> Hello Alexander, > >>>>>>>>>>>> Thanks for the explanation. > >>>>>>>>>>>> It's something we do not use. I did not think about it. > >>>>>>>>>>>> I look forward to your patch. > >>>>>>>>>>> > >>>>>>>>>>> I pushed these tasks: > >>>>>>>>>>> > >>>>>>>>>>> MDEV-10581 sql_mode=ORACLE: Explicit cursor FOR LOOP > >>>>>>>>>>> MDEV-12098 sql_mode=ORACLE: Implicit cursor FOR loop > >>>>>>>>>>> MDEV-12011 sql_mode=ORACLE: cursor%ROWTYPE in variable > >>>>>>>> declarations > >>>>>>>>>>> MDEV-12133 sql_mode=ORACLE: table%ROWTYPE in variable > >>>>>>>> declarations > >>>>>>>>>>> > >>>>>>>>>>> Please clone the branch again. > >>>>>>>>>>> Git pull will not work, because I recently rebased > >>>>>>>>>>> bb-10.2-compatibility on top of the latest 10.2. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> When implementing cursor%ROWTYPE, I had your patch in > mind > >>>> and > >>>>>>>> made > >>>>>>>>>>> some refactoring to help us apply MDEV-10598 easier. > >>>>>>>>>>> Please see a comment to MDEV-12011 in "git log". > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> Now the tricky thing (when adding your patch) is to make > >>>>>>>>>>> sure that this work fine: > >>>>>>>>>>> > >>>>>>>>>>> CREATE PROCEDURE p1 > >>>>>>>>>>> AS > >>>>>>>>>>> a INT:=10; > >>>>>>>>>>> CURSOR cur1 IS SELECT a; > >>>>>>>>>>> rec1 cur1%ROWTYPE; > >>>>>>>>>>> CURSOR cur2 IS SELECT rec1.a; > >>>>>>>>>>> rec2 cur2%ROWTYPE; > >>>>>>>>>>> BEGIN > >>>>>>>>>>> OPEN cur2; > >>>>>>>>>>> FETCH cur2 INTO rec2; > >>>>>>>>>>> CLOSE cur2; > >>>>>>>>>>> SELECT rec2.a; > >>>>>>>>>>> END; > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> I.e. a set of intermixed CURSOR and cursor%ROWTYPE > variable > >>>>>>>>>>> declarations referencing each other recursively. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> Thanks. > >>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> Regards, > >>>>>>>>>>>> Jérôme. > >>>>>>>>>>>> > >>>>>>>>>>>>> -----Message d'origine----- De : Alexander Barkov > >>>>>>>>>>>>> [mailto:bar@mariadb.org] Envoyé : > >>>>>>>>>>>>> lundi > >>>>>>>>>>>>> 27 février 2017 11:28 À : jerome brauge Cc : > >>>>>>>>>>>>> maria-developers > >>>> Objet : > >>>>>>>>>>>>> Re: MDEV-10598 - bb-10.2-compatibility > >>>>>>>>>>>>> > >>>>>>>>>>>>> Hello Jerome, > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> On 02/21/2017 07:18 PM, jerome brauge wrote: > >>>>>>>>>>>>>> Hello Alexander, > >>>>>>>>>>>>>> I've done this patch for MDEV-10598. > >>>>>>>>>>>>>> Can you review it ? > >>>>>>>>>>>>> > >>>>>>>>>>>>> It seems we'll have to postpone this patch. > >>>>>>>>>>>>> > >>>>>>>>>>>>> I'm currently working on: > >>>>>>>>>>>>> > >>>>>>>>>>>>> MDEV-10598 Variable declarations can go after cursor > >>>>>>>>>>>>> declarations > >>>>>>>>>>>>> MDEV-12011 sql_mode=ORACLE: cursor%ROWTYPE in > variable > >>>>>>>>>> declarations > >>>>>>>>>>>>> So the trick with postponing variable declarations using a > >>>>>>>>>>>>> temporary list might not work properly after adding > >>>>>>>>>>>>> MDEV-10598 abd MDEV-12011, the order of cursors and > >> variables is important. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Example: > >>>>>>>>>>>>> > >>>>>>>>>>>>> DECLARE > >>>>>>>>>>>>> CURSOR cur1 IS SELECT a,b FROM t1; > >>>>>>>>>>>>> v cur1%ROWTYPE; > >>>>>>>>>>>>> CURSOR cur2 IS SELECT v.a, v.b FROM DUAL; BEGIN > >>>>>>>>>>>>> ... > >>>>>>>>>>>>> END; > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> So the order of cur1, v and cur2 is important. > >>>>>>>>>>>>> > >>>>>>>>>>>>> I'll let you known when I'm ready with %ROWTYPE tasks. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Thanks! > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Regards, > >>>>>>>>>>>>>> Jérôme. > >>>>>>>>>>>>>>

2 1

Re: [Maria-developers] 85058bf: MDEV-9255 Add generation_expression to information_schema.columns.
by Sergei Golubchik 22 Mar '17

22 Mar '17

Hi, Alexey! On Mar 22, Alexey Botchkov wrote: > revision-id: 85058bf17953dc313731f4b22f0ec6037380f42b (mariadb-10.2.4-77-g85058bf) > parent(s): 1ca8637ae35061d2567728a1c7aede11e68e5fff > committer: Alexey Botchkov > timestamp: 2017-03-22 01:31:47 +0400 > message: > > MDEV-9255 Add generation_expression to information_schema.columns. > > Added IS_GENERATED and GENERATION_EXPRESSION columns required by > the SQL standard. Ouch, 1M for a commit email for a few-liner... > revision-id: 85058bf17953dc313731f4b22f0ec6037380f42b (mariadb-10.2.4-77-g85058bf) > parent(s): 1ca8637ae35061d2567728a1c7aede11e68e5fff > committer: Alexey Botchkov > timestamp: 2017-03-22 01:31:47 +0400 > message: > > MDEV-9255 Add generation_expression to information_schema.columns. > > diff --git a/mysql-test/suite/funcs_1/r/is_columns.result b/mysql-test/suite/funcs_1/r/is_columns.result > index 63c6e11..382a634 100644 > --- a/mysql-test/suite/funcs_1/r/is_columns.result > +++ b/mysql-test/suite/funcs_1/r/is_columns.result > @@ -70,7 +72,9 @@ COLUMNS CREATE TEMPORARY TABLE `COLUMNS` ( > `COLUMN_KEY` varchar(3) NOT NULL DEFAULT '', > `EXTRA` varchar(30) NOT NULL DEFAULT '', > `PRIVILEGES` varchar(80) NOT NULL DEFAULT '', > - `COLUMN_COMMENT` varchar(1024) NOT NULL DEFAULT '' > + `COLUMN_COMMENT` varchar(1024) NOT NULL DEFAULT '', > + `IS_GENERATED` varchar(6) NOT NULL DEFAULT '', > + `GENERATION_EXPRESSION` varchar(766) DEFAULT NULL does it mean that longer expressions will be truncated? we suport up to 64K expressions. should this be SMALLTEXT or long varchar, perhaps? > ) DEFAULT CHARSET=utf8 > SHOW COLUMNS FROM information_schema.COLUMNS; > Field Type Null Key Default Extra > diff --git a/mysql-test/suite/gcol/t/gcol_bugfixes.test b/mysql-test/suite/gcol/t/gcol_bugfixes.test > index 7ca50e6..1318abe 100644 > --- a/mysql-test/suite/gcol/t/gcol_bugfixes.test > +++ b/mysql-test/suite/gcol/t/gcol_bugfixes.test > @@ -536,3 +536,17 @@ SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED ; > SELECT 1 FROM t WHERE c GROUP BY b; > COMMIT; > DROP TABLE t; > + > +--echo # > +--echo # MDEV-9255 Add generation_expression to information_schema.columns. > +--echo # > + > +CREATE TABLE gcol_t1 ( > + sidea DOUBLE, > + sideb DOUBLE, > + sidec DOUBLE AS (SQRT(sidea * sidea + sideb * sideb)) > +); > + > +SELECT * FROM information_schema.columns WHERE table_name='gcol_t1'; Great. Just one tiny detail - there is no gcol_bugfixes.result in your commit :) > + > +DROP TABLE gcol_t1; Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

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

22 Mar '17

Thanks Alexander, I have two other problems to finalize my patch: - Attached script cur_err_warning.sql produce a warning 1931 (Query execution was interrupted. The query examined at least 2 rows, which exceeds LIMIT ROWS EXAMINED (0). The query result may be incomplete) and I don't see why. - Attached script cur_err_field_name.sql produce an error 4057 (HY000) at line 23: Row variable 'rec2' does not have a field 'a' These two scripts work fine on oracle. Jérôme. > -----Message d'origine----- > De : Alexander Barkov [mailto:bar@mariadb.org] > Envoyé : mardi 14 mars 2017 11:11 > À : jerome brauge > Objet : Re: MDEV-10598 - bb-10.2-compatibility > > Hello Jerome, > > > > On 03/13/2017 07:46 PM, jerome brauge wrote: > > Alexander, > > I think there is a little bug with ROWTYPE: when I affect a variable with a > field value, the server crash. > > I attached the script which cause the issue. > > Thanks for reporting this. > > > This quick patch fixes the problem: > > > diff --git a/sql/sp_head.cc b/sql/sp_head.cc index faf9f5f..745f982 100644 > --- a/sql/sp_head.cc > +++ b/sql/sp_head.cc > @@ -347,6 +347,12 @@ Item * > sp_prepare_func_item(THD* thd, Item **it_addr, uint cols) { > DBUG_ENTER("sp_prepare_func_item"); > + if (!(*it_addr)->fixed && > + (*it_addr)->fix_fields(thd, it_addr)) { > + DBUG_PRINT("info", ("fix_fields() failed")); > + DBUG_RETURN(NULL); > + } > it_addr= (*it_addr)->this_item_addr(thd, it_addr); > > if ((!(*it_addr)->fixed && > > > > But I'm still thinking. > Perhaps I'll end some with some different patch. > I let you know when the final fix is pushed. > > > > > > This is the call stack : > > > >> mysqld.exe!my_sigabrt_handler(int sig) Line 477 C > > mysqld.exe!raise(int signum) Line 516 C++ > > mysqld.exe!abort() Line 64 C++ > > mysqld.exe!common_assert_to_stderr_direct(const wchar_t * const > expression, const wchar_t * const file_name, const unsigned int > line_number) Line 124 C++ > > mysqld.exe!common_assert_to_stderr<wchar_t>(const wchar_t * > const expression, const wchar_t * const file_name, const unsigned int > line_number) Line 138 C++ > > mysqld.exe!common_assert<wchar_t>(const wchar_t * const > expression, const wchar_t * const file_name, const unsigned int > line_number, void * const return_address) Line 383 C++ > > mysqld.exe!_wassert(const wchar_t * expression, const wchar_t * > file_name, unsigned int line_number) Line 404 C++ > > mysqld.exe!Item_splocal_row_field_by_name::this_item_addr(THD > * thd, Item * * it) Line 1814 C++ > > mysqld.exe!sp_prepare_func_item(THD * thd, Item * * it_addr, > unsigned int cols) Line 350 C++ > > mysqld.exe!sp_eval_expr(THD * thd, Item * result_item, Field * > result_field, Item * * expr_item_ptr) Line 391 C++ > > mysqld.exe!sp_rcontext::set_variable(THD * thd, unsigned int idx, > Item * * value) Line 566 C++ > > mysqld.exe!sp_instr_set::exec_core(THD * thd, unsigned int * > nextp) Line 3378 C++ > > mysqld.exe!sp_lex_keeper::reset_lex_and_exec_core(THD * thd, > unsigned int * nextp, bool open_tables, sp_instr * instr) Line 3097 C++ > > mysqld.exe!sp_instr_set::execute(THD * thd, unsigned int * nextp) > Line 3371 C++ > > mysqld.exe!sp_head::execute(THD * thd, bool > merge_da_on_success) Line 1261 C++ > > mysqld.exe!sp_head::execute_procedure(THD * thd, List<Item> * > args) Line 2086 C++ > > mysqld.exe!do_execute_sp(THD * thd, sp_head * sp) Line 2890 > C++ > > mysqld.exe!mysql_execute_command(THD * thd) Line 5919 C++ > > mysqld.exe!mysql_parse(THD * thd, char * rawbuf, unsigned int > length, Parser_state * parser_state, bool is_com_multi, bool > is_next_command) Line 8006 C++ > > mysqld.exe!dispatch_command(enum_server_command command, > THD * thd, char * packet, unsigned int packet_length, bool is_com_multi, > bool is_next_command) Line 1821 C++ > > mysqld.exe!do_command(THD * thd) Line 1369 C++ > > mysqld.exe!threadpool_process_request(THD * thd) Line 346 > C++ > > mysqld.exe!tp_callback(TP_connection * c) Line 192 C++ > > mysqld.exe!tp_callback(_TP_CALLBACK_INSTANCE * instance, void * > context) Line 377 C++ > > mysqld.exe!work_callback(_TP_CALLBACK_INSTANCE * instance, > void * context, _TP_WORK * work) Line 451 C++ > > > > Can you reproduce this ? > > > > Jérôme. > > > >> -----Message d'origine----- > >> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : lundi 13 mars > >> 2017 14:47 À : jerome brauge Objet : Re: MDEV-10598 - > >> bb-10.2-compatibility > >> > >> Jérôme, > >> > >> On 03/13/2017 05:43 PM, jerome brauge wrote: > >>> Hello Alexander, > >>> I have to do some changes in the patch and add some tests cases > >>> (with row type) I think it will be ready this afternoon (CET). > >> > >> Excellent. You rock! > >> > >>> > >>> Jérôme. > >>> > >>> > >>>> -----Message d'origine----- > >>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : lundi 13 > >>>> mars > >>>> 2017 14:38 À : jerome brauge Cc : maria-developers Objet : Re: > >>>> MDEV-10598 - bb-10.2-compatibility > >>>> > >>>> Hello Jerome, > >>>> > >>>> will you try to apply your patch on top of the current bb-10.2- > >> compatibility? > >>>> > >>>> Or should I do that? > >>>> > >>>> Thanks! > >>>> > >>>> > >>>> On 03/10/2017 02:36 PM, Alexander Barkov wrote: > >>>>> Hello Jerome, > >>>>> > >>>>> > >>>>> On 02/27/2017 11:39 PM, jerome brauge wrote: > >>>>>> Hello Alexander, > >>>>>> Thanks for the explanation. > >>>>>> It's something we do not use. I did not think about it. > >>>>>> I look forward to your patch. > >>>>> > >>>>> I pushed these tasks: > >>>>> > >>>>> MDEV-10581 sql_mode=ORACLE: Explicit cursor FOR LOOP > >>>>> MDEV-12098 sql_mode=ORACLE: Implicit cursor FOR loop > >>>>> MDEV-12011 sql_mode=ORACLE: cursor%ROWTYPE in variable > >> declarations > >>>>> MDEV-12133 sql_mode=ORACLE: table%ROWTYPE in variable > >> declarations > >>>>> > >>>>> Please clone the branch again. > >>>>> Git pull will not work, because I recently rebased > >>>>> bb-10.2-compatibility on top of the latest 10.2. > >>>>> > >>>>> > >>>>> When implementing cursor%ROWTYPE, I had your patch in mind and > >> made > >>>>> some refactoring to help us apply MDEV-10598 easier. > >>>>> Please see a comment to MDEV-12011 in "git log". > >>>>> > >>>>> > >>>>> Now the tricky thing (when adding your patch) is to make sure that > >>>>> this work fine: > >>>>> > >>>>> CREATE PROCEDURE p1 > >>>>> AS > >>>>> a INT:=10; > >>>>> CURSOR cur1 IS SELECT a; > >>>>> rec1 cur1%ROWTYPE; > >>>>> CURSOR cur2 IS SELECT rec1.a; > >>>>> rec2 cur2%ROWTYPE; > >>>>> BEGIN > >>>>> OPEN cur2; > >>>>> FETCH cur2 INTO rec2; > >>>>> CLOSE cur2; > >>>>> SELECT rec2.a; > >>>>> END; > >>>>> > >>>>> > >>>>> I.e. a set of intermixed CURSOR and cursor%ROWTYPE variable > >>>>> declarations referencing each other recursively. > >>>>> > >>>>> > >>>>> Thanks. > >>>>> > >>>>>> > >>>>>> Regards, > >>>>>> Jérôme. > >>>>>> > >>>>>>> -----Message d'origine----- > >>>>>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : lundi 27 > >>>>>>> février 2017 11:28 À : jerome brauge Cc : maria-developers Objet : > >>>>>>> Re: MDEV-10598 - bb-10.2-compatibility > >>>>>>> > >>>>>>> Hello Jerome, > >>>>>>> > >>>>>>> > >>>>>>> On 02/21/2017 07:18 PM, jerome brauge wrote: > >>>>>>>> Hello Alexander, > >>>>>>>> I've done this patch for MDEV-10598. > >>>>>>>> Can you review it ? > >>>>>>> > >>>>>>> It seems we'll have to postpone this patch. > >>>>>>> > >>>>>>> I'm currently working on: > >>>>>>> > >>>>>>> MDEV-10598 Variable declarations can go after cursor > >>>>>>> declarations > >>>>>>> MDEV-12011 sql_mode=ORACLE: cursor%ROWTYPE in variable > >>>> declarations > >>>>>>> So the trick with postponing variable declarations using a > >>>>>>> temporary list might not work properly after adding MDEV-10598 > >>>>>>> abd MDEV-12011, the order of cursors and variables is important. > >>>>>>> > >>>>>>> Example: > >>>>>>> > >>>>>>> DECLARE > >>>>>>> CURSOR cur1 IS SELECT a,b FROM t1; > >>>>>>> v cur1%ROWTYPE; > >>>>>>> CURSOR cur2 IS SELECT v.a, v.b FROM DUAL; BEGIN > >>>>>>> ... > >>>>>>> END; > >>>>>>> > >>>>>>> > >>>>>>> So the order of cur1, v and cur2 is important. > >>>>>>> > >>>>>>> I'll let you known when I'm ready with %ROWTYPE tasks. > >>>>>>> > >>>>>>> Thanks! > >>>>>>> > >>>>>>> > >>>>>>>> > >>>>>>>> Regards, > >>>>>>>> Jérôme. > >>>>>>>>

2 9

Re: [Maria-developers] 1ee6b0ff748: MDEV-11115 CHECK constraints are not shown in I_S.TABLE_CONSTRAINTS
by Sergei Golubchik 21 Mar '17

21 Mar '17

Hi, jacob.mathew! On Mar 16, jacob.mathew(a)mariadb.com wrote: > revision-id: 1ee6b0ff74858dccd779c7974858c6d084c1cde5 (mariadb-10.2.3-246-g1ee6b0ff748) > parent(s): 9f33288dde950343c46e6c5d6a4f233de191f477 > author: Jacob Mathew > committer: Jacob Mathew > timestamp: 2017-03-16 18:31:11 -0700 > message: > > MDEV-11115 CHECK constraints are not shown in I_S.TABLE_CONSTRAINTS > > Fixed handling of column-level CHECK constraints in I_S.TABLE_CONSTRAINTS. > Fixed a bug regarding virtual column definitions whose name is the field name. > Fixed column-level CHECK constraint syntax in the test case. Ah, good. I had a feeling there was something wrong with field-level constraints in your first patch. This looks great now. Did you push it into a stage tree already? Like bb-10.2-jacob or something? If not - please do. If you did and buildbot didn't show anything bad, please squash your three commits into one, rebase it on top of the current 10.2 and push. Thanks! Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

3 4

Re: [Maria-developers] [Commits] 06dac6f23f6: MDEV-10355 Weird error message upon CREATE TABLE with DEFAULT
by Sergei Golubchik 21 Mar '17

21 Mar '17

Hi, Jacob! Even if MDEV is not "In Review", here're few early thoughts to consider: On Mar 20, jacob.mathew(a)mariadb.com wrote: > revision-id: 06dac6f23f662d6892fddd47c4566c96a3d044d3 (mariadb-10.2.3-180-g06dac6f23f6) > parent(s): cd53525d7c97a7c2b69873fbbd28982844e3cae3 > author: Jacob Mathew > committer: Jacob Mathew > timestamp: 2017-03-20 16:35:31 -0700 > message: > > MDEV-10355 Weird error message upon CREATE TABLE with DEFAULT > > Fixed handling of default values with BETWEEN and temporal functions. > Added test case. > > diff --git a/mysql-test/r/func_default_between_temporal.result b/mysql-test/r/func_default_between_temporal.result > new file mode 100644 > index 00000000000..a1d11f56d4d > --- /dev/null > +++ b/mysql-test/r/func_default_between_temporal.result > @@ -0,0 +1,11 @@ > +CREATE OR REPLACE TABLE t1 ( col INT DEFAULT ( 1 LIKE ( NOW() BETWEEN '2000-01-01' AND '2012-12-12' ) ) ); > +SHOW CREATE TABLE t1; > +Table Create Table > +t1 CREATE TABLE `t1` ( > + `col` int(11) DEFAULT (1 like (current_timestamp() between 2000-01-01 00:00:00 and 2012-12-12 00:00:00)) Note, that the generated create table statement is invalid. Temporal constants are printed unquoted. > +) ENGINE=MyISAM DEFAULT CHARSET=latin1 > +INSERT INTO t1 VALUES( DEFAULT ); > +SELECT * FROM t1; > +col > +0 > +DROP TABLE t1; > diff --git a/sql/item.cc b/sql/item.cc > index dd2c4fd75a5..5c0afc526f8 100644 > --- a/sql/item.cc > +++ b/sql/item.cc > @@ -9378,17 +9378,30 @@ void Item_cache::store(Item *item) > > void Item_cache::print(String *str, enum_query_type query_type) > { > - if (value_cached) > - { > - print_value(str); > - return; > - } > - str->append(STRING_WITH_LEN("<cache>(")); > - if (example) > - example->print(str, query_type); > - else > - Item::print(str, query_type); > - str->append(')'); > + if ( ( query_type & QT_ITEM_ORIGINAL_FUNC_NULLIF ) && > + example && ( example->type() == FUNC_ITEM ) ) > + { > + // Instead of "cache" or the cached value, print the function name, > + // which is more meaningful to the end user > + const char *func_name = ( ( Item_func* ) example )->func_name(); > + > + str->append( func_name ); > + str->append( "()" ); 1. Why a check for QT_ITEM_ORIGINAL_FUNC_NULLIF? Is it how you distinguish SHOW CREATE from EXPLAIN? 2. Why do you do that explicitly, instead of example->print()? The item knows how to print itself. You don't. For example, the function can have arguments, not "()", or may be it needs to be printed using infix notation (like Item_func_plus, and some others). > + } > + else > + { > + if (value_cached) > + { > + print_value(str); > + return; > + } > + str->append(STRING_WITH_LEN("<cache>(")); > + if (example) > + example->print(str, query_type); > + else > + Item::print(str, query_type); > + str->append(')'); > + } Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] Google Summer of Code
by Daniel Black 21 Mar '17

21 Mar '17

I got referred to another GSoC documentation site: https://code.timvideos.us/summer-of-code/ I think its a pretty good model of what sort of documentation students should read as largely it applies to projects in general.

1 0

Re: [Maria-developers] 23959479689: MDEV-11419: Report all INSERT ID for bulk operation INSERT
by Sergei Golubchik 20 Mar '17

20 Mar '17

Hi, Oleksandr! On Jan 17, Oleksandr Byelkin wrote: > revision-id: 23959479689f47bdfe5aa33cb6cd5e1171b5f8a8 (mariadb-10.2.2-128-g23959479689) > parent(s): 9ea5de30963dd16cec7190b8b2f77858f4a82545 > committer: Oleksandr Byelkin > timestamp: 2017-01-17 15:47:17 +0100 > message: > > MDEV-11419: Report all INSERT ID for bulk operation INSERT > > Send all Insert IDs of the buld operation to client (JDBC need it) > > --- > include/mysql.h.pp | 3 +- > include/mysql_com.h | 3 +- > sql/protocol.cc | 22 +++++++--- > sql/protocol.h | 6 +++ > sql/sql_class.cc | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > sql/sql_class.h | 8 ++++ > sql/sql_insert.cc | 11 ++++- > sql/sql_prepare.cc | 21 ++++++++-- Tests are in C/C repository, I hope? > diff --git a/include/mysql_com.h b/include/mysql_com.h > index c399520022d..9fac5edd1bc 100644 > --- a/include/mysql_com.h > +++ b/include/mysql_com.h > @@ -551,7 +551,8 @@ enum enum_cursor_type > CURSOR_TYPE_NO_CURSOR= 0, > CURSOR_TYPE_READ_ONLY= 1, > CURSOR_TYPE_FOR_UPDATE= 2, > - CURSOR_TYPE_SCROLLABLE= 4 > + CURSOR_TYPE_SCROLLABLE= 4, > + INSERT_ID_REQUEST= 128 That's very weird. Why would "INSERT_ID_REQUEST" be a flag in the "cursor_type"? Don't put this flag into enum_cursor_type > }; > > > diff --git a/sql/protocol.cc b/sql/protocol.cc > index f8b68c02fff..608c06da2df 100644 > --- a/sql/protocol.cc > +++ b/sql/protocol.cc > @@ -562,6 +562,7 @@ void Protocol::end_statement() > > switch (thd->get_stmt_da()->status()) { > case Diagnostics_area::DA_ERROR: > + thd->stop_collecting_insert_id(); > /* The query failed, send error to log and abort bootstrap. */ > error= send_error(thd->get_stmt_da()->sql_errno(), > thd->get_stmt_da()->message(), > @@ -573,12 +574,21 @@ void Protocol::end_statement() > break; > case Diagnostics_area::DA_OK: > case Diagnostics_area::DA_OK_BULK: > - error= send_ok(thd->server_status, > - thd->get_stmt_da()->statement_warn_count(), > - thd->get_stmt_da()->affected_rows(), > - thd->get_stmt_da()->last_insert_id(), > - thd->get_stmt_da()->message(), > - thd->get_stmt_da()->skip_flush()); > + if (thd->report_collected_insert_id()) > + if (thd->is_error()) > + error= send_error(thd->get_stmt_da()->sql_errno(), > + thd->get_stmt_da()->message(), > + thd->get_stmt_da()->get_sqlstate()); > + else > + error= send_eof(thd->server_status, > + thd->get_stmt_da()->statement_warn_count()); > + else > + error= send_ok(thd->server_status, > + thd->get_stmt_da()->statement_warn_count(), > + thd->get_stmt_da()->affected_rows(), > + thd->get_stmt_da()->last_insert_id(), > + thd->get_stmt_da()->message(), > + thd->get_stmt_da()->skip_flush()); > break; > case Diagnostics_area::DA_DISABLED: > break; > diff --git a/sql/protocol.h b/sql/protocol.h > index 6397e3dd5e6..bf74f52fa98 100644 > --- a/sql/protocol.h > +++ b/sql/protocol.h > @@ -30,6 +30,12 @@ class Item_param; > typedef struct st_mysql_field MYSQL_FIELD; > typedef struct st_mysql_rows MYSQL_ROWS; > > +struct insert_id_desc > +{ > + ulonglong first_id; > + ulonglong sequence; > +}; > + > class Protocol > { > protected: > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index 8fabc8f593e..53591c371c0 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -1474,6 +1474,7 @@ void THD::init(void) > #endif //EMBEDDED_LIBRARY > > apc_target.init(&LOCK_thd_data); > + insert_ids= NULL; > DBUG_VOID_RETURN; > } > > @@ -7365,4 +7366,119 @@ bool Discrete_intervals_list::append(Discrete_interval *new_interval) > DBUG_RETURN(0); > } > > +bool THD::init_collecting_insert_id() > +{ > + if (!insert_ids) > + { > + void *buff; > + if (!(my_multi_malloc(MYF(MY_WME), &insert_ids, sizeof(DYNAMIC_ARRAY), > + &buff, sizeof(insert_id_desc) * 10, > + NullS)) || > + my_init_dynamic_array2(insert_ids, sizeof(insert_id_desc), > + buff, 10, 100, MYF(MY_WME))) Huh? 1. You allocate DYNAMIC_ARRAY, on the heap, for every (insert-id generating) statement. Why? 2. You preallocate a buffer, but dynamic array cannot reallocate it, so on the 11'th element, it'll allocate *another* buffer and won't use yours. Better: put DYNAMIC_ARRAY in the THD, or (even better) allocate it with thd->alloc. And let DYNAMIC_ARRAY do it's own memory management. > + { > + if (insert_ids) > + my_free(insert_ids); > + insert_ids= NULL; > + return TRUE; > + } > + collect_auto_increment_increment= variables.auto_increment_increment; > + } > + return FALSE; > +} > + > +void THD::stop_collecting_insert_id() > +{ > + if (insert_ids) > + { > + delete_dynamic(insert_ids); > + my_free(insert_ids); > + insert_ids= NULL; > + } > +} > + > +bool THD::collect_insert_id(ulonglong id) > +{ > + if (insert_ids) > + { > + if (insert_ids->elements) > + { > + insert_id_desc *last= > + (insert_id_desc *)dynamic_array_ptr(insert_ids, > + insert_ids->elements - 1); > + if (id == last->first_id) > + { > + return FALSE; // no new insert id > + } > + if (id == last->first_id + (last->sequence * > + collect_auto_increment_increment)) > + { > + last->sequence++; > + return FALSE; > + } > + } > + insert_id_desc el; > + el.first_id= id; > + el.sequence= 1; > + if (insert_dynamic(insert_ids, &el)) > + { > + return TRUE; > + } > + } > + return FALSE; > +} This definitely needs a comment, explaining what you're doing and how you're storing your insert id values. > + > + > +bool THD::report_collected_insert_id() > +{ > + if (insert_ids) > + { > + List<Item> field_list; > + MEM_ROOT tmp_mem_root; > + Query_arena arena(&tmp_mem_root, Query_arena::STMT_INITIALIZED), backup; > + > + init_alloc_root(arena.mem_root, 2048, 4096, MYF(0)); > + set_n_backup_active_arena(&arena, &backup); > + DBUG_ASSERT(mem_root == &tmp_mem_root); > + > + field_list.push_back(new (mem_root) > + Item_int(this, "Id", 0, MY_INT64_NUM_DECIMAL_DIGITS), > + mem_root); > + field_list.push_back(new (mem_root) > + Item_int(this, "Len", 0, MY_INT64_NUM_DECIMAL_DIGITS), > + mem_root); > + field_list.push_back(new (mem_root) > + Item_return_int(this, "Inc", 0, MYSQL_TYPE_LONG), > + mem_root); > + > + if (protocol_binary.send_result_set_metadata(&field_list, > + Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF)) > + goto error; > + > + for (ulonglong i= 0; i < insert_ids->elements; i++) > + { > + insert_id_desc *last= > + (insert_id_desc *)dynamic_array_ptr(insert_ids, i); > + if (insert_ids->elements == 1 && last->first_id == 0 && > + get_stmt_da()->affected_rows() != 1) > + continue; // No insert IDs > + protocol_binary.prepare_for_resend(); > + protocol_binary.store_longlong(last->first_id, TRUE); > + protocol_binary.store_longlong(last->sequence, TRUE); > + protocol_binary.store_long(collect_auto_increment_increment); > + if (protocol_binary.write()) > + goto error; > + } > +error: > + restore_active_arena(&arena, &backup); > + DBUG_ASSERT(arena.mem_root == &tmp_mem_root); > + // no need free Items because they was only constants > + free_root(arena.mem_root, MYF(0)); > + stop_collecting_insert_id(); > + return TRUE; > + } > + return FALSE; > + > +} > + > #endif /* !defined(MYSQL_CLIENT) */ > diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc > index 7c0c1b68a2b..20b775c9273 100644 > --- a/sql/sql_insert.cc > +++ b/sql/sql_insert.cc > @@ -1007,6 +1007,12 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list, > } > its.rewind(); > iteration++; > + > + if (!error && thd->bulk_param) > + { > + thd->collect_insert_id(table->file->insert_id_for_cur_row); > + } 1. Why are you doing it here, and not per row, inside `while ((values= its++))` loop? 2. What about INSERT ... SELECT? 3. What about a insert-id generated in one statement but for many tables? (stored function, trigger, whatever) > + > } while (iteration < bulk_iterations); Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

3 4

Re: [Maria-developers] b6f4d16: MDEV-10436 non-deterministic vcol does not force rbr
by Sergei Golubchik 20 Mar '17

20 Mar '17

Hi, Sachin! On Feb 10, Sachin Setiya wrote: > revision-id: b6f4d16a015fc778186ae4d2c1820a78cd9645d1 (mariadb-10.2.3-157-gb6f4d16) > parent(s): 5285504857df6caf417c8a56601913a95c9f7abc > author: Sachin Setiya > committer: Sachin Setiya > timestamp: 2017-02-10 18:24:12 +0530 > message: > > MDEV-10436 non-deterministic vcol does not force rbr > > Problem:- Since 10.2.1 one can create virtual generated columns with > non-deterministic functions. Arguably, in cases like > INSERT t1 SELECT * FROM t2 > if t2 contains such non-deterministic generated columns and replication > mode is MIXED, the statement should be logged in the row format. But it is > logged in statement format. Some explanation of the fix would be good here. Especially if it's not trivial. Why did you need set_cs_bin_format_row_if_mixed_and_nd_vcol() function and why are you doing something in end_send()? You've fixed THD::decide_logging_format(), why was that not enough? Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

[Maria-developers] MDEV-12303 Add Type_handler::Item_xxx_fix_length_and_dec()
by Alexander Barkov 20 Mar '17

20 Mar '17

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

1 0

Re: [Maria-developers] Filtered transactions coming through mysql/mariadb replication to Galera cluster
by Michael Widenius 20 Mar '17

20 Mar '17

Hi! > Hi, > > Galera cluster (https://github.com/codership/mysql-wsrep/commits/5.6) has new feature : > > commit 14301279075dc2b420f3173cc9c2eb7912db63de > Author: sjaakola <seppo.jaakola(a)iki.fi> > Date: Fri Nov 18 12:45:42 2016 +0200 > > Refs: MW-319 > * replicating write set containing only GTID event of totally filtered transaction > coming in through mysql replication > * note that replicated WS will have no keys, and current debug version of Galera 3.19 > will assert for no keys > > My questions: > > (1) If MySQL (5.6/5.7) replicates transactions to Galera cluster (assuming here that this cluster will contain MariaDB 10.0-10.2 nodes) does the server somehow see filtered GTID events ? > > (2) If MariaDB replicates transactions to Galera cluster are filtered transactions seen by Galera ? What exactly is the definition of filtered transactions? In MariaDB replication, we automatically remove all MySQL GTID's and what the slaves sees is basically a replication stream without any GTID, like it would come from an old slave. The slave will add it's own GTID's to it's own binary log, if logging is enabled. Seppo, is this still the case for new Galera code that will come to MariaDB? If not, what has changed? Regards, Monty

1 0

[Maria-developers] [GSOC] Table functions (aka SQL functions returning tables)
by Pranjal Rai 20 Mar '17

20 Mar '17

Hello, My name is Pranjal and I am a 4th year student at IIIT Hyderabad. I am interested in working on the project MDEV-8100 <https://jira.mariadb.org/browse/MDEV-8100> "Table functions (aka SQL functions returning tables)". I read the description of the task and it is quite clear to me. In the comments it is advised to consider creating TABLE datatype for the task. Is this task along with creation of TABLE datatype doable in the given timeline? Also, can someone please suggest what part of the source code should I read. Regards, Pranjal

2 1

[Maria-developers] prospective GSOC 2017 student [MDEV-7502]
by ibrar arshad 19 Mar '17

19 Mar '17

Hi, My name is Ibrar Arshad and I am interested in working on the task of automatic slave provisioning(ticket: MDEV-7502 <https://jira.mariadb.org/browse/MDEV-7502>) during GSOC 2017. I have read the summary on the ticket and have achieved a fair understanding of the problem and I am working towards ironing out the implementation details. The use-case as I understand is that we want the slave to auto-replicate the data from master once pointed the master and we want to do it in such a manner that the binlog events from current master position as well as the old data chunks are relayed to the slave in a parallel fashion. I have a few questions related to the proposal: 1. After reading a few pages on replication, my understanding is that after "CHANGE MASTER TO" and "START SLAVE", master starts sending binlog events from its current position to the slave which slave starts applying. The usual replication approach is to get the current binlog position on master, backup all the data till this position from master to slave, point slave to this position(or GTID) via "CHANGE MASTER TO", and START SLAVE to start replicating bin events from master. But for MDEV-7502, we want the normal events and old data chunks to be transmitted in parallel. The ticket summary mentions using separate domain_ids to send the new and old data in parallel, does there exist a way to do so currently? How can domain id be used here? Can we currently point the slave to 2 different bin positions on a single master and expect the master to send events from both positions? Or will this require some sort of new process/thread implementation on master to do so? 2. There are at-least two other approaches mentioned in the ticket's comments section. It doesn't seem like that a single approach has been finalized. This project doesn't seem to have a mentor yet to provide guidance so which approach should an applicant pursue further? I would like to discuss the project approaches and implementation further in detail before submitting a proposal so can somebody please answer my queries and further suggest pointers to this project specific material which I can go through to get a deeper understanding? Thanks.

3 2

Re: [Maria-developers] 9f33288dde9: MDEV-11115 CHECK constraints are not shown in I_S.TABLE_CONSTRAINTS
by Sergei Golubchik 17 Mar '17

17 Mar '17

Hi, Jacob! On Mar 16, jacob.mathew(a)mariadb.com wrote: > revision-id: 9f33288dde950343c46e6c5d6a4f233de191f477 (mariadb-10.2.3-245-g9f33288dde9) > parent(s): 7c152ac68a7993bca99ef23e073b0a0974b087a1 > author: Jacob Mathew > committer: Jacob Mathew > timestamp: 2017-03-16 14:41:41 -0700 > message: > > MDEV-11115 CHECK constraints are not shown in I_S.TABLE_CONSTRAINTS > > Add test case: check_constraint_show > > diff --git a/mysql-test/t/check_constraint_show.test b/mysql-test/t/check_constraint_show.test > new file mode 100644 > index 00000000000..0e2f45883df > --- /dev/null > +++ b/mysql-test/t/check_constraint_show.test > @@ -0,0 +1,8 @@ > +# Table with 2 column-level check constraints and 1 table-level check constraint > +create or replace table t1( c1 int, c2 int, c3 int, primary key( c1 > ), constraint `c1_min` check( c1 > 0 ), constraint `c3_min` check( c3 > > 0 ), constraint `range` check( ( c2 >= c1 ) and ( c2 <= c3 ) ) ); > +show create table t1; > + > +# Show all constraints, including check constraints > +select * from information_schema.table_constraints where table_name = 't1'; > + > +drop table t1; This is good, thanks! But please, test column-level constraints too, as I've asked in a review. Something like create table t1 (c1 int check(c1 > 10), c2 int check (c2 < c1), ...) Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] cbfa5f6: MDEV-11177 mysqlbinlog exits silently without error when another
by Sergei Golubchik 15 Mar '17

15 Mar '17

Hi, Alexey! On Mar 13, Alexey Botchkov wrote: > revision-id: cbfa5f6a6d48b3b755462eb0df5e34c63de78f98 (mariadb-10.2.4-46-gcbfa5f6) > parent(s): eded6243bc4796ab44e70403edd059d32225f589 > committer: Alexey Botchkov > timestamp: 2017-03-13 18:26:37 +0400 > message: > > MDEV-11177 mysqlbinlog exits silently without error when another > instance connects to server. > > New thread kill status added KILL_SLAVE_SAME_ID, and the related > error message. > > diff --git a/sql/sql_class.h b/sql/sql_class.h > index 22895d7..986cb1c 100644 > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -478,16 +478,22 @@ enum killed_state > KILL_TIMEOUT= 8, > KILL_TIMEOUT_HARD= 9, > /* > + When binlog reading thread connects to the server it kills > + all the binlog threads with the same ID. > + */ > + KILL_SLAVE_SAME_ID= 10, > + /* > All of the following killed states will kill the connection > KILL_CONNECTION must be the first of these and it must start with > an even number (becasue of HARD bit)! > */ > - KILL_CONNECTION= 10, > - KILL_CONNECTION_HARD= 11, > - KILL_SYSTEM_THREAD= 12, > - KILL_SYSTEM_THREAD_HARD= 13, > - KILL_SERVER= 14, > - KILL_SERVER_HARD= 15 > + KILL_CONNECTION= 12, > + KILL_CONNECTION_HARD= 13, > + KILL_SYSTEM_THREAD= 14, > + KILL_SYSTEM_THREAD_HARD= 15, > + KILL_SERVER= 16, > + KILL_SERVER_HARD= 17, Okay. I don't really like it, because KILL_CONNECTION/KILL_QUERY is how a user can specify what to kill, and KILL_SLAVE_SAME_ID is not that. But looking at the current enum killed_state, I see that its values already tell * _what_ to kill: KILL_CONNECTION, KILL_QUERY * _how_ to kill: KILL_HARD * _why_ to kill: KILL_TIMEOUT, KILL_SERVER, etc all in one enum. Your KILL_SLAVE_SAME_ID is another one in the "why to kill" list, it adds to this mess, but it doesn't make it much worse, I'd say. This needs to be cleaned up, though. A question - are you sure there enum values are not visible to clients, not part of the API? If you've checked that and they are not - ok to push. If you'd like to clean up KILL_* values (in a separate commit, of course), a very simple change would be to make it into a bitmap. Like first bit is "how to kill" (hard/not hard). Then one bit for "what to kill" - connection/query. The rest is for why. There'll be helpers like #define is_hard_kill(X) ((X) & HARD_KILL_BIT) #define is_kill_query(X) ((X) & KILL_QUERY_BIT) #define is_kill_connection(X) (!is_kill_query(X)) #define kill_reason(X) ((enum kill_reason)((X) >> 2)) Something like that. This doesn't change much internally, only the presentation and the way of thinking about it changes. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] Adding ApplicationName, ClientUser and ClientHostname to Information_schema.processlist
by jerome brauge 14 Mar '17

14 Mar '17

Hello, Apparently, no one is interested by this request. I think it is still interesting for project "MDEV-10137 Providing compatibility to other databases". Major databases allow to see these informations: - master..sysprocesses on Sybase and SQLserver - v$session or gv$session on Oracle - SYSIBMADM.SNAPAPPL_INFO on DB2 LUW - pg_stat_activity on postgresql I can make a patch to add ApplicationName, ClientUser and ClientHostname to Information_schema.processlist. Must I open a feature request on jira and wait your agreement ? Best regards, Jérôme. -----Message d'origine----- De : Maria-developers [mailto:maria-developers-bounces+j.brauge=qualiac.com@lists.launchpad.net] De la part de jerome brauge Envoyé : mardi 7 mars 2017 15:49 À : MariaDB Developers (maria-developers(a)lists.launchpad.net) Objet : [Maria-developers] Adding information de Information_schema.processlist Hello, Connector/J support the method setClientInfo and standard information : ApplicationName, ClientUser and ClientHostname. (https://docs.oracle.com/javase/7/docs/api/java/sql/Connection.html#setClien… It's seem that there is no way to see these information by a select because session variables are used to store these values. Can we add these information as columns in Information_schema.processlist ? Regards, Jérôme. _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers(a)lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp

1 0

[Maria-developers] bb-10.2-compatibility - Updatable cursor
by jerome brauge 14 Mar '17

14 Mar '17

Hello Alexander, Can you add a task for updatable cursor (for positioned update and delete). Regards, Jérôme.

2 5

[Maria-developers] Filtered transactions coming through mysql/mariadb replication to Galera cluster
by Jan Lindström 14 Mar '17

14 Mar '17

Hi, Galera cluster (https://github.com/codership/mysql-wsrep/commits/5.6) has new feature : commit 14301279075dc2b420f3173cc9c2eb7912db63de Author: sjaakola <seppo.jaakola(a)iki.fi> Date: Fri Nov 18 12:45:42 2016 +0200 Refs: MW-319 * replicating write set containing only GTID event of totally filtered transaction coming in through mysql replication * note that replicated WS will have no keys, and current debug version of Galera 3.19 will assert for no keys My questions: (1) If MySQL (5.6/5.7) replicates transactions to Galera cluster (assuming here that this cluster will contain MariaDB 10.0-10.2 nodes) does the server somehow see filtered GTID events ? (2) If MariaDB replicates transactions to Galera cluster are filtered transactions seen by Galera ? R: Jan

1 0

Re: [Maria-developers] [Commits] [PATCH] Use size_t in thd_alloc() and friends.
by Sergei Golubchik 13 Mar '17

13 Mar '17

Hi, Marko! On Mar 11, Marko Mäkelä wrote: > THD::alloc() and related member functions use size_t parameters. > > thd_alloc(), thd_calloc(), thd_strmake(), thd_memdup(): > Define the interface with size_t instead of unsigned int, to avoid > truncating integer conversions on 64-bit Windows, where size_t > is longer than unsigned int. > --- > include/mysql/plugin_audit.h.pp | 20 ++++++++++---------- > include/mysql/plugin_auth.h.pp | 20 ++++++++++---------- > include/mysql/plugin_encryption.h.pp | 20 ++++++++++---------- > include/mysql/plugin_ftparser.h.pp | 20 ++++++++++---------- > include/mysql/plugin_password_validation.h.pp | 20 ++++++++++---------- > include/mysql/service_thd_alloc.h | 21 +++++++++++---------- > sql/sql_class.cc | 10 +++++----- > sql/sql_class.h | 2 +- > storage/innobase/handler/ha_innodb.cc | 18 ++++++++---------- > 9 files changed, 75 insertions(+), 76 deletions(-) Marko, when you change the ABI of services you absolutely must update coresponding service versions in include/service_versions.h. May be, it's best not to break the ABI for such a minor reason and only change THD methods, not service functions. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1

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

13 Mar '17

Hello Alexander, When I compiled some real procedures, I find some tables alias not supported by the current grammar. Example : create table t1 (col1 int); select dec.* from t1 dec; select mod.* from t1 mod; I have done a complete test on Oracle and Mariadb with all keywords used as variables, table alias and goto/block labels. You can find result in attached excel file. In addition, schema object names and qualifiers must complied with : https://docs.oracle.com/cd/B28359_01/server.111/b28286/sql_elements008.htm#… I tried to add these keywords in rules keyword_directly_assignable, keyword_sp_not_data_type and keyword_sp_data_type, but this generates too many conflicts. I think that some refactoring of the grammar rules must be done, but I don't know how to do this. Regards, Jérôme.

2 4

[Maria-developers] MDEV-10598 - bb-10.2-compatibility
by jerome brauge 13 Mar '17

13 Mar '17

Hello Alexander, I've done this patch for MDEV-10598. Can you review it ? Regards, Jérôme.

2 11

[Maria-developers] Please review MDEV-12239 Add Type_handler::Item_sum_{sum|avg|variance}_mod_length_and_dec()
by Alexander Barkov 13 Mar '17

13 Mar '17

Hello Alexey, Please review a patch for MDEV-12239. Thanks!

1 0

[Maria-developers] MDEV-12238 Add Type_handler::Item_func_{plus|minus|mul|div|mod}_fix_length_and_dec()
by Alexander Barkov 13 Mar '17

13 Mar '17

Hello Alexey, Please review a patch for MDEV-12238. Thanks!

1 0

Re: [Maria-developers] 23d1188: MDEV-10555: Server crashes in mysql_admin_table upon killing ANALYZE
by Sergei Golubchik 13 Mar '17

13 Mar '17

Hi, Oleksandr! On Mar 08, Oleksandr Byelkin wrote: > revision-id: 23d118899fe9fa030e61b6a9ac06df5210d36294 (mariadb-10.2.4-29-g23d1188) > parent(s): 5da6bd7b959af4a9755556afedde8f34d95c679a > committer: Oleksandr Byelkin > timestamp: 2017-03-08 19:05:44 +0100 > message: > > MDEV-10555: Server crashes in mysql_admin_table upon killing ANALYZE > > Take into acount result of open table operation in mysql_admin_table. Where's the test case? Otherwise ok to push. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] GSoC 2016 Student Introduction. [ MDEV-4989 ]
by Mike Reiss 13 Mar '17

13 Mar '17

Hi, I am Mike Reiss a MS Computer Science student and I would love to work on enhancing the mysqlbinlog tool during GSoC 2017 [MDEV-4989 <https://jira.mariadb.org/browse/MDEV-4989>]. I have studied the mysqlbinlog.cc file and experimented with the various options available in the my_options array of the tool. The proposed idea is straight forward to implement: We add the --start-gtid and --stop-gtid options to the array of options then parse the GTID passed to these options to get the GTID sequence number for the binlog. One quick question. Is there any relationship between a GTID sequence number and the argument passed to the --start-position option? that way we need to set the start position to the GTID sequence number. I am still doing more research into the source files and I will love to discuss this more with the mentor. I am having difficulties debugging mysqlbinlog on OS X sierra because it does not support GDB any ideas please? Thanks.

2 1

[Maria-developers] Knowing the process
by Sagar Shivani 11 Mar '17

11 Mar '17

Dear sir / ma'am, > My name is sagar. I am interested in contributing into the project > idea of allowing different authentication processes per user. > I have skills in c and c++ programming language. Also i have learned SQL > previous semester. I would really like to know about the process to > pursue this idea. Please guide me .

2 1

Re: [Maria-developers] Please review MDEV-12199 Split Item_func_{abs|neg|int_val}::fix_length_and_dec() into methods in Type_handler
by Alexander Barkov 10 Mar '17

10 Mar '17

Hello Alexey, On 03/10/2017 10:56 AM, Alexey Botchkov wrote: > Ok, i see. > > Last question then. > - item->cast_to_int_type() == cast_to_int_type() && > + item->cast_to_int_type_handler() == > cast_to_int_type_handler() && > Are you sure we should compare typehandlers here, not their ->cmp_type()? Item_hex_constant can be: 1. Either Item_hex_hybrid, which can act as a string or as a number, depending on context: SELECT 0x30, 0x30+1; +------+--------+ | 0x30 | 0x30+1 | +------+--------+ | 0 | 49 | +------+--------+ (the first 0x30 is a string '0', the second 0x30 is an integer 0x30, which is decimal 48) 2. Or Item_hex_string, which is always a string: SELECT X'30', X'30'+1; +-------+---------+ | X'30' | X'30'+1 | +-------+---------+ | 0 | 1 | +-------+---------+ (both X'30's are strings '0') The method Item_hex_constant::eq() needs to make sure that X'30' and 0x30 are considered as different items, and thus don't replace each other, e.g. in equal field propagation. Item_hex_hybrid::cast_to_int_type_handler() returns &type_handler_longlong. Item_hex_string::cast_to_int_type_handler() returns &type_handler_varchar. Comparing type handlers should be enough here. Comparing cmp_type() would be also correct, but would involve one extra virtual call, which is nice to avoid. > > Also i don't like that we have the 'val_int_signed_typecast' as a > virtual functions. Code would > be much nicer without that and all it was done just for Item_dyncol_get. > Not that simple to fix it though and I agree that it shouldn't be > touched as a part of this patch. > Probably sometime later... Maybe. Perhaps some related code will also migrate to Type_handler. Probably this would do: virtual longlong Type_handler::val_int_signed_typecast(Item *item); But I'm not 100% sure. Let's return to this later. Thanks! > > Best regards. > HF > > > > On Fri, Mar 10, 2017 at 1:10 AM, Alexander Barkov <bar(a)mariadb.org > <mailto:bar@mariadb.org>> wrote: > > Hello Alexey, > > > On 03/09/2017 11:38 PM, Alexey Botchkov wrote: > > Hi, Alexander. > > > > Some questions about the patch. > > > > I don't get why we have two sets of functions > > Type_handler_**::+bool > > Type_handler_int_result::Item_func_abs_fix_length_and_dec > > and > > Type_handler_**::+bool > > Type_handler_int_result::Item_func_neg_fix_length_and_dec > > > > As far as i can see these are equal and we just can have one > > Type_handler_**::+bool > > Type_handler_int_result::Item_func_abs_neg_fix_length_and_dec > > Did I miss something? > > > They might look similar: > > > bool Type_handler_int_result:: > Item_func_abs_fix_length_and_dec(Item_func_abs *item) const > { > item->fix_length_and_dec_int(); > return false; > } > > bool Type_handler_int_result:: > Item_func_neg_fix_length_and_dec(Item_func_neg *item) const > { > item->fix_length_and_dec_int(); > return false; > } > > > But they have arguments of different pointer types! > > > The former calls Item_func_abs::fix_length_and_dec_int(), and > the latter calls Item_func_neg::fix_length_and_dec_int(), > > and those are different. > > > > > Secondly, since Item_func::fix_length_and_dec() was historically 'void' > > and i don't see you're > > changing it, why do all these typehandler's 'xxx_fix_length_and_dec' > > return something? > > Nobody checks their return anyway. > > > Currently fix_fields() does this: > > fix_length_and_dec(); > if (thd->is_error()) // An error inside fix_length_and_dec occurred > return TRUE; > > I'm thinking of eventually changing this to: > > if (fix_length_and_dec()) > return true; > > > So I decided to make all Type_handler::xxxx_fix_length_and_dec() > return bool right now, so we have to do less changes in the future. > > > Thanks! > > > > > Best regards. > > HF > > > > > > On Tue, Mar 7, 2017 at 6:15 PM, Alexander Barkov <bar(a)mariadb.org <mailto:bar@mariadb.org> > > <mailto:bar@mariadb.org <mailto:bar@mariadb.org>>> wrote: > > > > Hello Alexey, > > > > Please review a patch for MDEV-12199. > > > > Thanks! > > > > > >

1 0

[Maria-developers] GTID and Optimize, Analyze etc
by Peter Mclarty 10 Mar '17

10 Mar '17

Hi I have some very large tables and I would like to give them a cleanup. If I run OPTIMIZE and ANALYZE on a slave does that affect the GTID of that slave and cause me to eventually get the dreaded Slave has diverged too far from the master type error Can I set a session variable to block any change Cheers -- *Cheers* *Peter McLarty* -- Compare The Market is a brand and trading name of Compare The Market (Pty) Ltd (CTM). This email is for the intended addressee and is confidential and subject to copyright. If you are not the intended addressee, confidentiality has not been waived and any use, interference with, or disclosure of this email is unauthorised. If you are not the intended addressee please immediately notify CTM and then delete the email. CTM does not warrant that this email is error or virus free.

2 1

Re: [Maria-developers] Please review MDEV-12199 Split Item_func_{abs|neg|int_val}::fix_length_and_dec() into methods in Type_handler
by Alexander Barkov 09 Mar '17

09 Mar '17

Hello Alexey, On 03/09/2017 11:38 PM, Alexey Botchkov wrote: > Hi, Alexander. > > Some questions about the patch. > > I don't get why we have two sets of functions > Type_handler_**::+bool > Type_handler_int_result::Item_func_abs_fix_length_and_dec > and > Type_handler_**::+bool > Type_handler_int_result::Item_func_neg_fix_length_and_dec > > As far as i can see these are equal and we just can have one > Type_handler_**::+bool > Type_handler_int_result::Item_func_abs_neg_fix_length_and_dec > Did I miss something? They might look similar: bool Type_handler_int_result:: Item_func_abs_fix_length_and_dec(Item_func_abs *item) const { item->fix_length_and_dec_int(); return false; } bool Type_handler_int_result:: Item_func_neg_fix_length_and_dec(Item_func_neg *item) const { item->fix_length_and_dec_int(); return false; } But they have arguments of different pointer types! The former calls Item_func_abs::fix_length_and_dec_int(), and the latter calls Item_func_neg::fix_length_and_dec_int(), and those are different. > > Secondly, since Item_func::fix_length_and_dec() was historically 'void' > and i don't see you're > changing it, why do all these typehandler's 'xxx_fix_length_and_dec' > return something? > Nobody checks their return anyway. Currently fix_fields() does this: fix_length_and_dec(); if (thd->is_error()) // An error inside fix_length_and_dec occurred return TRUE; I'm thinking of eventually changing this to: if (fix_length_and_dec()) return true; So I decided to make all Type_handler::xxxx_fix_length_and_dec() return bool right now, so we have to do less changes in the future. Thanks! > > Best regards. > HF > > > On Tue, Mar 7, 2017 at 6:15 PM, Alexander Barkov <bar(a)mariadb.org > <mailto:bar@mariadb.org>> wrote: > > Hello Alexey, > > Please review a patch for MDEV-12199. > > Thanks! > >

1 0

[Maria-developers] INFORMATION_SCHEMA.SCHEMATA and INFORMATION_SCHEMA.SCHEMA_PRIVILEGES
by Witold Filipczyk 09 Mar '17

09 Mar '17

Hi, I'm newbie, I have problem with INFORMATION_SCHEMA. There is mariadb-10.0 with around 100000 databases. Disks are slow, but most users have one database, a few have more than one, but it is not a big number. Queries like SHOW DATABASES or SELECT * FROM INFORMATION_SCHEMA.SCHEMATA bring to knees whole server. The idea to resolve it is: in find_files(...) instead of the for loop execute query: SELECT REPLACE(TABLE_SCHEMA, '\\', '') FROM INFORMATION_SCHEMA.SCHEMA_PRIVILEGES WHERE PRIVILEGE_TYPE = 'SELECT'; There are databases with names with _ in them and \_ in GRANTS. This is theory, but I don't know how to do it. That's mean how to freeze the current query, how to run new query, read results, and fill files in the find_files function and resume the current query. Could you help? Or do you have ideas how to solve it? -- Witold Filipczyk

2 3

[Maria-developers] Maria db read/write configuration issues
by Chaitanya Cheekate 09 Mar '17

09 Mar '17

Hi We have configured maxscale in our environment using the links http://karlssonondatabases.blogspot.in/2014/02/maxscale-for-rest-of-us-part…, 2, 3 . We have two slaves and one master and the replication between them is working well. But after configuring read/write split the select queries are being executed in master itself, not routing to the slaves. Please find the attached documents of maxscale configuration, maxscale commands maxscale log and. Please help with maxscale read/write split. Attached documentsdetails 1. maxsconf.txt from the location /etc/maxscale.cnf 2. maxscale-commands.txt, coomands like maxadmin -pmariadb list servers, maxadmin -pmariadb list services, maxadmin -pmariadb show service "Splitter Service", 3. maxscale-log.txt from /var/log/maxscale Thanks Chaitu -- Thanks.

3 3

Re: [Maria-developers] 440e040: MDEV-11941 - Lintian complains about executable bits
by Sergei Golubchik 07 Mar '17

07 Mar '17

Hi, Sergey! Ok to push, of course. On Mar 03, Sergey Vojtovich wrote: > revision-id: 440e040a2421ad0d3ad3a059f1c4d5ffbf94a271 (mariadb-10.1.21-44-g440e040) > parent(s): 88b5eedef2b1f235346adf9aeca05c4ebab9921c > committer: Sergey Vojtovich > timestamp: 2017-03-03 12:33:24 +0400 > message: > > MDEV-11941 - Lintian complains about executable bits Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] c6a38fd: MDEV-10646 - System Unit File After network-online
by Sergei Golubchik 07 Mar '17

07 Mar '17

Hi, Sergey! On Mar 07, Sergey Vojtovich wrote: > revision-id: c6a38fd974d43aa9094e215969fa3598c595f7b0 (mariadb-10.1.21-50-gc6a38fd) > parent(s): e6aa08ced29d4dab120ae2dc8c08f6b3b81e0c93 > committer: Sergey Vojtovich > timestamp: 2017-03-07 15:06:01 +0400 > message: > > MDEV-10646 - System Unit File After network-online > > Bind to an address even if it is not yet configured. Looks ok to me Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] Adding information de Information_schema.processlist
by jerome brauge 07 Mar '17

07 Mar '17

Hello, Connector/J support the method setClientInfo and standard information : ApplicationName, ClientUser and ClientHostname. (https://docs.oracle.com/javase/7/docs/api/java/sql/Connection.html#setClien… It's seem that there is no way to see these information by a select because session variables are used to store these values. Can we add these information as columns in Information_schema.processlist ? Regards, Jérôme.

1 0

[Maria-developers] Please review MDEV-12199 Split Item_func_{abs|neg|int_val}::fix_length_and_dec() into methods in Type_handler
by Alexander Barkov 07 Mar '17

07 Mar '17

Hello Alexey, Please review a patch for MDEV-12199. Thanks!

1 0

[Maria-developers] MDEV-11829 RFC3339 Support
by Seth Shelnutt 05 Mar '17

05 Mar '17

Hello, This is in response to the comments on the initial pull request I made for adding RFC3339 support [1]. The proposal is to add support for parsing RFC3339 and then use the given timezone offset to shift the datetime. I think most users would expect the behavior to be converted to the timezone of the mariadb server / session as defined in THD. Most timezone operations are depended on the session specified timezone, whether that is set or inherited from global timezone, or system timezone. In order to convert from RFC3339 timezone to session timezone, we will need to return the timezone from the str_to_datetime function. As mentioned there is no timezone member of the MY_TIME struct. I believe a change to the MY_TIME struct would be the most beneficial and cleanest approach. This would allow for handling RFC3339 and future additions, such as a datetime with time zone field, similar to the ansi timestamp with time zone. The struct is used in a several ways, including packing into longlong, all of those cases would need to be handled. This would be a much larger and more involved patchset. An alternative approach, that is less involved, would be use the MYSQL_TIME_STATUS struct to indicate that a timezone offset exists. An offset field and indicator could be added right to the status struct, or a new warning can be used to indicate it exists in the raw string. Then the caller of str_to_datetime can handle the timezone conversion. A third approach, would be to change the function signature of str_to_datetime and provide a variable to store the offset in, if exists. I'm sure what the general stance is on altering entire function calls, especially one that is older and quite important. With any approach, the timezone could then be handled outside of my_time.c, where we have THD and the timezone class, which can easily handle the conversion. I will start working on this once I receive some feedback on the general direction that would result in an acceptable patchset. [1] https://github.com/MariaDB/server/pull/290 Thank you, Seth Shelnutt

3 4

[Maria-developers] About help on mysqltest
by Amol Holani 03 Mar '17

03 Mar '17

Hi, I'm amol holani, computer science student from College of Engineering, Pune(COEP, India). I would love to contribute to project related to mysqltest (MDEV-12130) . Can you explain more about ad hoc parser. Also, How can I get some documentation about mysqltest? -- Regards, Amol Holani

2 1

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

03 Mar '17

Hi Varun, 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 It would be nice to elaborate where uninitialized data comes from. Am I right that it comes from record? It would be nice to explain why NULL values and CHAR type are not affected. As well as why 10.1 is unaffected, ideally point to revision that caused this. > > --- > 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; Why you declare offset here... > uint *blob, *end; > uchar *pos; > DBUG_ENTER("pack_row_v1"); > memcpy(record_buffer->buffer, record, table->s->reclength); > + for(Field** field= table->field; (*field) ; field++) > + { > + 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(); And use it here? Why not declare it inline with start and field_var? > + bzero(record_buffer->buffer + start + offset, fld->field_length - offset + 1); According to man bzero: This function is deprecated (marked as LEGACY in POSIX.1-2001): use memset(3) in new programs. POSIX.1-2008 removes the specification of bzero(). Please use memset() instead. Regards, Sergey

1 0

[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

Re: [Maria-developers] e8ca684: MDEV-11221 - main.events_restart failed in bb
by Sergei Golubchik 28 Feb '17

28 Feb '17

Hi, Sergey! Thanks! Ok to push! On Feb 28, Sergey Vojtovich wrote: > revision-id: e8ca684c6d5ce14eb021b4b691be50a8bf9c8281 (mariadb-10.0.29-38-ge8ca684) > parent(s): b54566d73bb96ef9eb7d734742d3743fb3369a40 > committer: Sergey Vojtovich > timestamp: 2017-02-28 15:23:44 +0400 > message: > > MDEV-11221 - main.events_restart failed in bb > > Applied lost in a merge revision: next time, please specify a commit you're applying. Like Applied commit 7f38a070baa that was lost in a merge: > MDEV-10043 - main.events_restart fails sporadically in buildbot (crashes upon > shutdown) > > There was race condition between shutdown thread and event worker threads. > > Shutdown thread waits for thread_count to become 0 in close_connections(). It > may happen so that event worker thread was started but didn't increment > thread_count by this time. In this case shutdown thread may miss wait for this > working thread and continue deinitialization. Worker thread in turn may continue > execution and crash on deinitialized data. > > Fixed by incrementing thread_count before thread is actually created like it is > done for connection threads. > > Also let event scheduler not to inc/dec running threads counter for symmetry > with other "service" threads. > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] b13cee8: MDEV-11078: NULL NOT IN (non-empty subquery) should never return results
by Sergei Golubchik 27 Feb '17

27 Feb '17

Hi, Varun! Really, copy-pasted *8* times? Why didn't you make it a method of Item_in_subselect? On Feb 27, Varun wrote: > revision-id: b13cee83f9cfcffa98d227c492411a9acec85f42 (mariadb-10.2.3-282-gb13cee8) > parent(s): b513e3711743e3ff24b3d14794a0fba87fd18c31 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2017-02-27 22:10:35 +0530 > message: > > MDEV-11078: NULL NOT IN (non-empty subquery) should never return results > > Setting the value of cond guard variables during the creation of Tricond Item for null values > > --- > mysql-test/r/subselect4.result | 34 +++++++++++++++++++++++++++++ > mysql-test/t/subselect4.test | 20 +++++++++++++++++ > sql/item_subselect.cc | 49 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 103 insertions(+) > > diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc > index 94bc71c..acdbabd 100644 > --- a/sql/item_subselect.cc > +++ b/sql/item_subselect.cc > @@ -2113,6 +2113,13 @@ Item_in_subselect::create_single_in_to_exists_cond(JOIN *join, > We can encounter "NULL IN (SELECT ...)". Wrap the added condition > within a trig_cond. > */ > + if(left_expr->basic_const_item() && !left_expr->is_expensive()) > + { > + left_expr->is_null(); > + if(left_expr->null_value) > + set_cond_guard_var(0,FALSE); > + } > + > item= new (thd->mem_root) Item_func_trig_cond(thd, item, get_cond_guard(0)); > } > > @@ -2137,6 +2144,12 @@ Item_in_subselect::create_single_in_to_exists_cond(JOIN *join, > having= new (thd->mem_root) Item_is_not_null_test(thd, this, having); > if (left_expr->maybe_null) > { > + if(left_expr->basic_const_item() && !left_expr->is_expensive()) > + { > + left_expr->is_null(); > + if(left_expr->null_value) > + set_cond_guard_var(0,FALSE); > + } > if (!(having= new (thd->mem_root) Item_func_trig_cond(thd, having, > get_cond_guard(0)))) > DBUG_RETURN(true); > @@ -2155,6 +2168,12 @@ Item_in_subselect::create_single_in_to_exists_cond(JOIN *join, > */ > if (!abort_on_null && left_expr->maybe_null) > { > + if(left_expr->basic_const_item() && !left_expr->is_expensive()) > + { > + left_expr->is_null(); > + if(left_expr->null_value) > + set_cond_guard_var(0,FALSE); > + } > if (!(item= new (thd->mem_root) Item_func_trig_cond(thd, item, > get_cond_guard(0)))) > DBUG_RETURN(true); > @@ -2184,6 +2203,12 @@ Item_in_subselect::create_single_in_to_exists_cond(JOIN *join, > (char *)"<result>")); > if (!abort_on_null && left_expr->maybe_null) > { > + if(left_expr->basic_const_item() && !left_expr->is_expensive()) > + { > + left_expr->is_null(); > + if(left_expr->null_value) > + set_cond_guard_var(0,FALSE); > + } > if (!(new_having= new (thd->mem_root) Item_func_trig_cond(thd, new_having, > get_cond_guard(0)))) > DBUG_RETURN(true); > @@ -2383,6 +2408,12 @@ Item_in_subselect::create_row_in_to_exists_cond(JOIN * join, > Item_cond_or(thd, item_eq, item_isnull); > if (!abort_on_null && left_expr->element_index(i)->maybe_null) > { > + if(left_expr->element_index(i)->basic_const_item() > + && !left_expr->element_index(i)->is_expensive()) > + { > + if(left_expr->element_index(i)->is_null()) > + set_cond_guard_var(i,FALSE); > + } > if (!(col_item= new (thd->mem_root) > Item_func_trig_cond(thd, col_item, get_cond_guard(i)))) > DBUG_RETURN(true); > @@ -2400,6 +2431,12 @@ Item_in_subselect::create_row_in_to_exists_cond(JOIN * join, > (char *)"<list ref>")); > if (!abort_on_null && left_expr->element_index(i)->maybe_null) > { > + if(left_expr->element_index(i)->basic_const_item() > + && !left_expr->element_index(i)->is_expensive()) > + { > + if(left_expr->element_index(i)->is_null()) > + set_cond_guard_var(i,FALSE); > + } > if (!(item_nnull_test= > new (thd->mem_root) > Item_func_trig_cond(thd, item_nnull_test, get_cond_guard(i)))) > @@ -2460,6 +2497,12 @@ Item_in_subselect::create_row_in_to_exists_cond(JOIN * join, > item= new (thd->mem_root) Item_cond_or(thd, item, item_isnull); > if (left_expr->element_index(i)->maybe_null) > { > + if(left_expr->element_index(i)->basic_const_item() > + && !left_expr->element_index(i)->is_expensive()) > + { > + if(left_expr->element_index(i)->is_null()) > + set_cond_guard_var(i,FALSE); > + } > if (!(item= new (thd->mem_root) > Item_func_trig_cond(thd, item, get_cond_guard(i)))) > DBUG_RETURN(true); > @@ -2471,6 +2514,12 @@ Item_in_subselect::create_row_in_to_exists_cond(JOIN * join, > } > if (!abort_on_null && left_expr->element_index(i)->maybe_null) > { > + if(left_expr->element_index(i)->basic_const_item() > + && !left_expr->element_index(i)->is_expensive()) > + { > + if(left_expr->element_index(i)->is_null()) > + set_cond_guard_var(i,FALSE); > + } > if (!(item= new (thd->mem_root) > Item_func_trig_cond(thd, item, get_cond_guard(i)))) > DBUG_RETURN(true); Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] On constructing ref access from mis-matched charset comparisons
by Sergey Petrunia 27 Feb '17

27 Feb '17

Hi Alexander, I've got a question about mis-matched charset comparisons and ref access method. == Short form == I know that VARCHAR comparisons over mis-matching charsets cannot be used for constructing index lookups into the index over the "narrower" character set column. But is this a real limitation or just an optimizer deficiency? If it is the latter, do people hit it in the real world sufficiently often to warrant lifting it? == Long form == === Example dataset === create table t0 ( a varchar(32) character set utf8 collate utf8_general_ci ); create table t1 ( a varchar(32) character set latin1 collate latin1_swedish_ci, col1 varchar(32), key(a) ); create table t2 ( a varchar(32) character set utf8 collate utf8_general_ci, col1 varchar(32), key(a) ); insert into t0 select a from ten; insert into t1 select a,a from one_k; insert into t2 select a,a from one_k; === Queries === MariaDB [test]> explain -> select * from t0, t2 where t0.a=t2.a; +------+-------------+-------+------+---------------+------+---------+-----------+------+-------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+-------+------+---------------+------+---------+-----------+------+-------------+ | 1 | SIMPLE | t0 | ALL | NULL | NULL | NULL | NULL | 10 | Using where | | 1 | SIMPLE | t2 | ref | a | a | 99 | test.t0.a | 1 | | +------+-------------+-------+------+---------------+------+---------+-----------+------+-------------+ Ok, good. MariaDB [test]> explain -> select * from t0, t1 where t0.a=t1.a; +------+-------------+-------+------+---------------+------+---------+------+------+-------------------------------------------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+-------+------+---------------+------+---------+------+------+-------------------------------------------------+ | 1 | SIMPLE | t0 | ALL | NULL | NULL | NULL | NULL | 10 | | | 1 | SIMPLE | t1 | ALL | NULL | NULL | NULL | NULL | 1000 | Using where; Using join buffer (flat, BNL join) | +------+-------------+-------+------+---------------+------+---------+------+------+-------------------------------------------------+ 2 rows in set (0.00 sec) Oops, the index t1(a) is not used. FORMAT=JSON shows the reason: "attached_condition": "(t0.a = convert(t1.a using utf8))" Still it feels like the optimizer could * Try to convert the value of t0.a into latin1. * If it can't be represented in latin1, then we know that no row in t1 has t1.a=t0.a. * make a lookup on t1.a= convert(t0.a using latin1) , using t1.a's collation rules. Is this true [for some charsets] ? If yes, is it worth doing? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

3 2

[Maria-developers] bb-10.2-compatibility : delete compatibility
by jerome brauge 27 Feb '17

27 Feb '17

Hello Alexander, Can you add a task for delete statement with same table as source and target in sql_mode=oracle Example : create or replace table t1 (c1 int, c2 int); delete from t1 where c1 in (select b.c1 from t1 b where b.c2=0); Failed with : Error: Table 't1' is specified twice, both as a target for 'DELETE' and as a separate source for data SQLState: HY000 ErrorCode: 1093 Regards, Jérôme.

2 1

Re: [Maria-developers] [Commits] c6d172f: MDEV-11078: NULL NOT IN (non-empty subquery) should never return results
by Sergey Petrunia 27 Feb '17

27 Feb '17

Hi Varun, Things that look apparently wrong: 1. The patch checks left_expr->null_value without first having made a call that will fill that member. The calling convention is something like this: item->val_XXX() // val_str or val_int() // check item->null_value. If one doesn't need string or integer value, there is item->is_null() call. 2. The patch checks null_value for any kind of left_expr. This is wrong. For example, consider a query: select t1.col IN (select t2.col FROM t2 ) from t1 Here, it doesn't make any sense to check {t1.col}->null_value at query planning/rewrite time. The value (and NULL-ness) of t1.col is different for different rows of t1. As discussed on Slack: please only check for NULL value for items which are - constant, - and "not expensive" and so have a fixed value which we are allowed to compute at query optimization phase. 3. I see the patch handles single-value comparisons like expr IN (select inner_expr FROM ...) but what about tuple-based comparisons like: (expr1, expr2) IN (select inner_expr1, inner_expr2 FROM ...) ? I guess they should have a similar issue. If they do not, we need - an explanation why - test coverage On Sun, Feb 26, 2017 at 09:04:40PM +0530, Varun wrote: > revision-id: c6d172f785f31641832087a432d1966b67efc47a (mariadb-10.2.3-283-gc6d172f) > parent(s): 78153cf641aea41166d3e79ae99b57b154f6a027 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2017-02-26 21:03:29 +0530 > message: > > MDEV-11078: NULL NOT IN (non-empty subquery) should never return results > > Setting the value of cond guard variables during the creation of Tricond Item for null values > > --- > mysql-test/r/subselect4.result | 31 +++++++++++++++++++++++++++++++ > mysql-test/t/subselect4.test | 18 ++++++++++++++++++ > sql/item_subselect.cc | 10 ++++++++++ > 3 files changed, 59 insertions(+) > > diff --git a/mysql-test/r/subselect4.result b/mysql-test/r/subselect4.result > index 6bbb80c..c3bd50a 100644 > --- a/mysql-test/r/subselect4.result > +++ b/mysql-test/r/subselect4.result > @@ -2454,3 +2454,34 @@ x > c1 > 1 > drop table t1; > +# > +# MDEV-11078: NULL NOT IN (non-empty subquery) should never return results > +# > +create table t1(a int,b int); > +create table t2(a int,b int); > +insert into t1 value (NULL,2); > +select 1 in (1,2, NULL); > +1 in (1,2, NULL) > +1 > +select (null) in (select 1 from t1); > +(null) in (select 1 from t1) > +NULL > +select (null) in (select 1 from t2); > +(null) in (select 1 from t2) > +0 > +select 1 in (select 1 from t1); > +1 in (select 1 from t1) > +1 > +select 1 in (select 1 from t2); > +1 in (select 1 from t2) > +0 > +select 1 from dual where null in (select 1 from t1); > +1 > +select 1 from dual where null in (select 1 from t2); > +1 > +select 1 from dual where null not in (select 1 from t1); > +1 > +select 1 from dual where null not in (select 1 from t2); > +1 > +1 > +drop table t1,t2; > diff --git a/mysql-test/t/subselect4.test b/mysql-test/t/subselect4.test > index 253160c..785b79d 100644 > --- a/mysql-test/t/subselect4.test > +++ b/mysql-test/t/subselect4.test > @@ -2009,3 +2009,21 @@ insert into t1 values(2,1),(1,2); > select (select c1 from t1 group by c1,c2 order by c1 limit 1) as x; > (select c1 from t1 group by c1,c2 order by c1 limit 1); > drop table t1; > + > +--echo # > +--echo # MDEV-11078: NULL NOT IN (non-empty subquery) should never return results > +--echo # > + > +create table t1(a int,b int); > +create table t2(a int,b int); > +insert into t1 value (NULL,2); > +select 1 in (1,2, NULL); > +select (null) in (select 1 from t1); > +select (null) in (select 1 from t2); > +select 1 in (select 1 from t1); > +select 1 in (select 1 from t2); > +select 1 from dual where null in (select 1 from t1); > +select 1 from dual where null in (select 1 from t2); > +select 1 from dual where null not in (select 1 from t1); > +select 1 from dual where null not in (select 1 from t2); > +drop table t1,t2; > diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc > index 94bc71c..b9c798e 100644 > --- a/sql/item_subselect.cc > +++ b/sql/item_subselect.cc > @@ -2113,6 +2113,8 @@ Item_in_subselect::create_single_in_to_exists_cond(JOIN *join, > We can encounter "NULL IN (SELECT ...)". Wrap the added condition > within a trig_cond. > */ > + if(left_expr->null_value) > + set_cond_guard_var(0,FALSE); > item= new (thd->mem_root) Item_func_trig_cond(thd, item, get_cond_guard(0)); > } > > @@ -2137,6 +2139,9 @@ Item_in_subselect::create_single_in_to_exists_cond(JOIN *join, > having= new (thd->mem_root) Item_is_not_null_test(thd, this, having); > if (left_expr->maybe_null) > { > + if(left_expr->null_value) > + set_cond_guard_var(0,FALSE); > + > if (!(having= new (thd->mem_root) Item_func_trig_cond(thd, having, > get_cond_guard(0)))) > DBUG_RETURN(true); > @@ -2155,6 +2160,9 @@ Item_in_subselect::create_single_in_to_exists_cond(JOIN *join, > */ > if (!abort_on_null && left_expr->maybe_null) > { > + if(left_expr->null_value) > + set_cond_guard_var(0, FALSE); > + > if (!(item= new (thd->mem_root) Item_func_trig_cond(thd, item, > get_cond_guard(0)))) > DBUG_RETURN(true); > @@ -2184,6 +2192,8 @@ Item_in_subselect::create_single_in_to_exists_cond(JOIN *join, > (char *)"<result>")); > if (!abort_on_null && left_expr->maybe_null) > { > + if(left_expr->null_value) > + set_cond_guard_var(0,FALSE); > if (!(new_having= new (thd->mem_root) Item_func_trig_cond(thd, new_having, > get_cond_guard(0)))) > DBUG_RETURN(true); > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits -- BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] an observation
by AugustQ 22 Feb '17

22 Feb '17

Hi, by playing with the code I think I found something interesting. My environment: MariaDB 10.0.10, MyISAM-engine I played with a table-scan, no index is defined on this table. When I execute a SQL-statement that forces the server to do a second table- scan on a table this 2nd table-scan will be slow. The reason for this behaviour is the usage of a buffer: during the 1st scan this buffer is filled, used and filled again until the whole table is processed. At the end of the 1st scan it contains the last bytes of the file. When a 2nd scan is started the reading of the table starts from the beginning of the file but the buffer and all associated variables are not reset: the buffer still contains the bytes from the end of the file, the request cannot be fulfilled by the buffer so the request has to be handled by reading the bytes directly from the file using the read()-function of the Std-library. This takes much more time then simply copying the bytes from the internal buffer. My idea is: somewhere in the code this situation must be detected and the buffer (and all associated variables) reset to initial values. rein it_io_cache() looks like the right candidate for this. I assume the engine Maria will show the same effect (I didn't check other engines). Some questions: is my observation correct? Do you think this needs to be fixed? Or is there no further development on this part of the code? Thanks Regards AugustQ PS: you will find more details on this topic here: http://augustq.blogs pot.com/2017/01/subselect-execution-3.html

3 6

[Maria-developers] engine condition pushdown
by AugustQ 22 Feb '17

22 Feb '17

Hi, by playing with a SQL-statement I encountered a problem. I'm not sure if this is a bug or if I did not use the correct method to get the information I expected. So let me please explain the situation in which I encountered this problem. But it will take some lines of text to come to the point. My environment: MariaDB 10.0.10 source-code-version on Linux. The statement: select SQL_NO_CACHE B.PZN, B.ArtikelText from TestSmall B where exists ( select 1 from TestBig A where A.Hersteller = '36367' and A.PZN = B.PZN) and B.Hersteller = '00020' Engine used is MyISAM but the effect should be independent of any engine (not checked). No indexes exist on the tables involved. You will find a lot more of details here: http://augustq.blogspot.com/2 017/02/subselect-execution-4.html Execution of this (silly) statement is done by a table-scan on the table TestSmall. When a record is found the matches the WHERE-condition the server switches over to the table TestBig and does a table-scan on this table. When a match is found it returns to the table TestSmall, otherwise the table TestBig is read to the end and then the server returns to the table TestSmall. The server continues scanning the table TestSmall, switching over to TestBig, returning to TestSmall, continues reading TestSmall and so on, until it reaches the end of the table TestSmall. Of interest for me was the condition tree which can be inspected by looking at the function cond_push() (in my case ha_myisam::cond_push()). So let's start. The function cond_push() is initally called for the tables TestSmall and TestBig, this looks good. Then the server starts reading the table TestSmall and therefore the function cond_push() is called again. This looks good too. Now a matching record is found in TestSmall: it switches over to the table TestBig and calls cond_push() with the condition-tree for this table. This tree looks like this: COND-ITEM args: 0 type=[COND_AND_FUNC] FUNC-ITEM [=] args: 2 type=[EQ_FUNC] FIELD-ITEM [TestOpt] [A] [Hersteller] name=<Hersteller> STRING-ITEM str_length=<5> str_value=<36367> name=<36367> FUNC-ITEM [=] args: 2 type=[EQ_FUNC] FIELD-ITEM [TestOpt] [A] [PZN] name=<PZN> FIELD-ITEM [TestOpt] [B] [PZN] name=<PZN> The last line is of interest (I've marked it in bold). For the table with the alias B (= TestSmall) no value is given for the column PZN but this value is available. This is the first point of interest. Let's continue with the operation. When the table TestBig is read to the end, the scanning of TestSmall continues and a matching record is found over there the server again switches over to TestBig. Before it starts reading the records from this table the function cond_push() is called again. This is the output of the condition-tree, restricted to the line of interest (the last line): FIELD-ITEM [TestOpt] [B] [PZN] str_length=<2> str_value=<37> name=<PZN> So let's continue: when it switches to TestBig for the 3rd time this output looks like: FIELD-ITEM [TestOpt] [B] [PZN] str_length=<7> str_value=<222 > name=<PZN> Enough of data. I can deliver more examples if needed or you may look into my text. Judgement: The output of the first case des not show the value of the column PZN from the current record in TestSmall although the value is available. The output in the second case shows the value 37 (because of the length eq. 2) but the correct value would be 3717968 (and length eq. 7). The output in the third case shows the value 222 followed by 4 spaces (length eq. 7), but the correct value is 222 (length eq. 3). What I see is the correct value (exclude the first example presented here) but the length-information is one row behind (one row of TestSmall). For accessing the information presented here I used COND->str_value. So the question is: - the information is available but I used the wrong function/variable? OR - is this a bug in the code? What do you think? Thanks AugustQ

2 4

Re: [Maria-developers] 2c36ba67240: MySQL Bug#12766319 - 61865: RENAME USER DOES NOT WORK CORRECTLY - REQUIRES FLUSH PRIVILEGES
by Sergei Golubchik 22 Feb '17

22 Feb '17

Hi, Oleksandr! On Jan 10, Oleksandr Byelkin wrote: > revision-id: 2c36ba67240850d909bf75fc36346261f37019fb (mariadb-10.0.28-43-g2c36ba67240) > parent(s): eed319b6fb543849046c8009c38575455e173dc2 > committer: Oleksandr Byelkin > timestamp: 2017-01-10 16:02:42 +0100 > message: > > MySQL Bug#12766319 - 61865: RENAME USER DOES NOT WORK CORRECTLY - REQUIRES FLUSH PRIVILEGES > > Port & fix of MySQL patch for the problem. Don't mix the refactoring and a bug fix. I don't see from this big patch what the bug fix was. Please split in two (or more) commits. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 0310b44: MDEV-10418 Assertion `m_extra_cache' failed in ha_partition::late_extra_cache(uint)
by Sergei Golubchik 21 Feb '17

21 Feb '17

Hi, Alexey! On Dec 20, Alexey Botchkov wrote: > revision-id: 0310b444dba1b9cbd6921963dd3bff689ca23a24 (mariadb-5.5.53-34-g0310b44) > parent(s): f23b41b9b8a30e0e54a1ec7a8923057b0e57e0f5 > committer: Alexey Botchkov > timestamp: 2016-12-20 00:24:20 +0400 > message: > > MDEV-10418 Assertion `m_extra_cache' failed in ha_partition::late_extra_cache(uint) > > The m_extra_prepare_for_update parameter should be set to FALSE > as the query ends. This seems fairly obvious, that it should. But why MySQL doesn't have this bug (despite not resetting m_extra_prepare_for_update)? > diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc > index 15fa7d1..4374354 100644 > --- a/sql/ha_partition.cc > +++ b/sql/ha_partition.cc > @@ -6667,6 +6667,7 @@ int ha_partition::reset(void) > DBUG_ENTER("ha_partition::reset"); > if (m_part_info) > bitmap_set_all(&m_part_info->used_partitions); > + m_extra_prepare_for_update= FALSE; > file= m_file; > do > { Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

Re: [Maria-developers] [Commits] e0a1e63: MDEV-9619: Assertion `null_ref_table' failed in virtual table_map Item_direct_view_ref::used_tables() const on 2nd execution of PS
by Sergey Petrunia 21 Feb '17

21 Feb '17

Hi Sanja, Two questions: 1. Why does the patch define Item_ptr type twice? 2. (The big one) In class Item_in_subselect, I see your comment made in 2015: ... public: Item *left_expr; /* Important for PS/SP: left_expr_orig is the item that left_expr originally pointed at. That item is allocated on the statement arena, while left_expr could later be changed to something on the execution arena. */ Item *left_expr_orig; Considering this comment, is it really safe to keep pointers to left_expr? Should left_expr_orig be used instead? On Sat, Feb 18, 2017 at 05:47:32PM +0100, Oleksandr Byelkin wrote: > revision-id: e0a1e632e4e6084cd989f217943618f4e9ac9179 (mariadb-5.5.54-25-ge0a1e63) > parent(s): 29d78dbb44ee9890b6bc28873344f20fc9157928 > committer: Oleksandr Byelkin > timestamp: 2017-02-18 17:47:31 +0100 > message: > > MDEV-9619: Assertion `null_ref_table' failed in virtual table_map Item_direct_view_ref::used_tables() const on 2nd execution of PS > > Refer left expression indirectly in case it changes from execution to execution. > > --- > mysql-test/r/ps.result | 24 ++++++++++++++++++++++++ > mysql-test/t/ps.test | 22 ++++++++++++++++++++++ > sql/item.h | 2 ++ > sql/opt_subselect.cc | 10 +++++----- > sql/table.h | 3 ++- > 5 files changed, 55 insertions(+), 6 deletions(-) > > diff --git a/mysql-test/r/ps.result b/mysql-test/r/ps.result > index f954583..7233fe8 100644 > --- a/mysql-test/r/ps.result > +++ b/mysql-test/r/ps.result > @@ -4173,4 +4173,28 @@ Warnings: > Note 1003 select `test`.`t1`.`id` AS `id`,`test`.`t1`.`c` AS `c` from `test`.`t1` where 0 > deallocate prepare stmt2; > drop table t1; > +# > +# MDEV-9619: Assertion `null_ref_table' failed in virtual > +# table_map Item_direct_view_ref::used_tables() const on 2nd > +# execution of PS > +# > +CREATE TABLE t1 (f1 VARCHAR(10)) ENGINE=MyISAM; > +CREATE ALGORITHM=MERGE VIEW v1 AS SELECT * FROM t1; > +INSERT INTO t1 VALUES ('a'),('b'); > +CREATE TABLE t2 (f2 VARCHAR(10)) ENGINE=MyISAM; > +INSERT INTO t2 VALUES ('c'),('d'); > +PREPARE stmt FROM "SELECT * FROM v1 WHERE f1 = SOME ( SELECT f2 FROM t2 )"; > +EXECUTE stmt; > +f1 > +EXECUTE stmt; > +f1 > +insert into t1 values ('c'); > +EXECUTE stmt; > +f1 > +c > +EXECUTE stmt; > +f1 > +c > +drop view v1; > +drop table t1,t2; > # End of 5.5 tests > diff --git a/mysql-test/t/ps.test b/mysql-test/t/ps.test > index cfd810f..627a9bf 100644 > --- a/mysql-test/t/ps.test > +++ b/mysql-test/t/ps.test > @@ -3714,4 +3714,26 @@ deallocate prepare stmt2; > > drop table t1; > > +--echo # > +--echo # MDEV-9619: Assertion `null_ref_table' failed in virtual > +--echo # table_map Item_direct_view_ref::used_tables() const on 2nd > +--echo # execution of PS > +--echo # > + > +CREATE TABLE t1 (f1 VARCHAR(10)) ENGINE=MyISAM; > +CREATE ALGORITHM=MERGE VIEW v1 AS SELECT * FROM t1; > +INSERT INTO t1 VALUES ('a'),('b'); > + > +CREATE TABLE t2 (f2 VARCHAR(10)) ENGINE=MyISAM; > +INSERT INTO t2 VALUES ('c'),('d'); > + > +PREPARE stmt FROM "SELECT * FROM v1 WHERE f1 = SOME ( SELECT f2 FROM t2 )"; > +EXECUTE stmt; > +EXECUTE stmt; > +insert into t1 values ('c'); > +EXECUTE stmt; > +EXECUTE stmt; > + > +drop view v1; > +drop table t1,t2; > --echo # End of 5.5 tests > diff --git a/sql/item.h b/sql/item.h > index 89155ac..dff7bc4 100644 > --- a/sql/item.h > +++ b/sql/item.h > @@ -1543,6 +1543,8 @@ class Item { > virtual void mark_as_condition_AND_part(TABLE_LIST *embedding) {}; > }; > > +typedef Item (*Item_ptr); > + > > /** > Compare two Items for List<Item>::add_unique() > diff --git a/sql/opt_subselect.cc b/sql/opt_subselect.cc > index 5137d8a..7f26d01 100644 > --- a/sql/opt_subselect.cc > +++ b/sql/opt_subselect.cc > @@ -1602,7 +1602,7 @@ static bool convert_subq_to_sj(JOIN *parent_join, Item_in_subselect *subq_pred) > > if (subq_pred->left_expr->cols() == 1) > { > - nested_join->sj_outer_expr_list.push_back(subq_pred->left_expr); > + nested_join->sj_outer_expr_list.push_back(&subq_pred->left_expr); > /* > Create Item_func_eq. Note that > 1. this is done on the statement, not execution, arena > @@ -1624,7 +1624,7 @@ static bool convert_subq_to_sj(JOIN *parent_join, Item_in_subselect *subq_pred) > for (uint i= 0; i < subq_pred->left_expr->cols(); i++) > { > nested_join->sj_outer_expr_list.push_back(subq_pred->left_expr-> > - element_index(i)); > + addr(i)); > Item_func_eq *item_eq= > new Item_func_eq(subq_pred->left_expr->element_index(i), > subq_lex->ref_pointer_array[i]); > @@ -3181,8 +3181,8 @@ void restore_prev_sj_state(const table_map remaining_tables, > ulonglong get_bound_sj_equalities(TABLE_LIST *sj_nest, > table_map remaining_tables) > { > - List_iterator<Item> li(sj_nest->nested_join->sj_outer_expr_list); > - Item *item; > + List_iterator<Item_ptr> li(sj_nest->nested_join->sj_outer_expr_list); > + Item **item; > uint i= 0; > ulonglong res= 0; > while ((item= li++)) > @@ -3193,7 +3193,7 @@ ulonglong get_bound_sj_equalities(TABLE_LIST *sj_nest, > class and see if there is an element that is bound? > (this is an optional feature) > */ > - if (!(item->used_tables() & remaining_tables)) > + if (!(item[0]->used_tables() & remaining_tables)) > { > res |= 1ULL << i; > } > diff --git a/sql/table.h b/sql/table.h > index dde01a8..c981243 100644 > --- a/sql/table.h > +++ b/sql/table.h > @@ -33,6 +33,7 @@ > /* Structs that defines the TABLE */ > > class Item; /* Needed by ORDER */ > +typedef Item (*Item_ptr); > class Item_subselect; > class Item_field; > class GRANT_TABLE; > @@ -2348,7 +2349,7 @@ typedef struct st_nested_join > table_map sj_depends_on; > /* Outer non-trivially correlated tables */ > table_map sj_corr_tables; > - List<Item> sj_outer_expr_list; > + List<Item_ptr> sj_outer_expr_list; > /** > True if this join nest node is completely covered by the query execution > plan. This means two things. > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits -- BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] MDEV-10697 - bb-10.2-compatibility
by jerome brauge 20 Feb '17

20 Feb '17

Hello, I've wrote a patch for MDEV-10697 (goto statement for sql_mode=oracle). If someone can review it. Thanks. Best regards, Jérôme.

2 8

Re: [Maria-developers] [Commits] 13ebc9c: MDEV-11645: archive.archive fails in buildbot with valgrind (Use of uninitialised value)
by Sergey Petrunia 20 Feb '17

20 Feb '17

Hi Varun, On Mon, Feb 13, 2017 at 03:47:57AM +0530, Varun wrote: > revision-id: 13ebc9c826de080e416071b7881585b45922bef9 (mariadb-10.2.3-185-g13ebc9c) > parent(s): 3ae038b732ce503fb839e9095355e05f5c6866f9 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2017-02-13 03:45:59 +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 > > --- > storage/archive/ha_archive.cc | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/storage/archive/ha_archive.cc b/storage/archive/ha_archive.cc > index 9d6d100..329e3d5 100644 > --- a/storage/archive/ha_archive.cc > +++ b/storage/archive/ha_archive.cc > @@ -372,10 +372,31 @@ int Archive_share::write_v1_metafile() > > unsigned int ha_archive::pack_row_v1(uchar *record) > { > + uint offset= 0,length; > uint *blob, *end; > uchar *pos; > DBUG_ENTER("pack_row_v1"); > memcpy(record_buffer->buffer, record, table->s->reclength); > + for(Field** field= table->field; (*field) ; field++) > + { > + > + Field *fld= *field; > + > + if(fld->type() == MYSQL_TYPE_VARCHAR) > + { > + uint start= (fld->ptr - table->record[0]); > + uint index_end= start+fld->field_length; There is no reason to compute the above variables when the field value is an SQL NULL. Please move them into the if. Please also use ptrdiff_t datatype instead of uint. A different question: did you check what happens when one attempts to write an SQL NULL value? > + > + if(!(fld->is_real_null(record - table->record[0]))) > + { > + Field_varstring *const field_var = (Field_varstring *)fld; > + offset = field_var->data_length() + field_var->length_size(); Coding style says there should be no space before the assignment. Please fix the above two lines. > + > + for(uint index= start + offset; index <= index_end ; index++) > + record_buffer->buffer[index]=0; Is there any reason you're not using bzero or memset call here instead? (please check with Serg which one is guaranteed to be available) > + } > + } > + } > 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

Re: [Maria-developers] [Commits] 01d5d6db4c3: Fix GCC 6.3.0 warnings.
by Jan Lindström 16 Feb '17

16 Feb '17

ok to push. On Thu, Feb 16, 2017 at 11:17 AM, <marko.makela(a)mariadb.com> wrote: > revision-id: 01d5d6db4c337daae0aa593341e081cc6bfebb8a (mariadb-10.0.29-25- > g01d5d6db4c3) > parent(s): 6011fb6daa5ac9e6cb97a23b877e4d2568c85d94 > author: Marko Mäkelä > committer: Marko Mäkelä > timestamp: 2017-02-16 11:16:27 +0200 > message: > > Fix GCC 6.3.0 warnings. > > --- > storage/innobase/fsp/fsp0fsp.cc | 1 + > storage/innobase/row/row0ftsort.cc | 4 ---- > storage/xtradb/fsp/fsp0fsp.cc | 1 + > 3 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/storage/innobase/fsp/fsp0fsp.cc b/storage/innobase/fsp/ > fsp0fsp.cc > index afdd6667711..836ed2de27a 100644 > --- a/storage/innobase/fsp/fsp0fsp.cc > +++ b/storage/innobase/fsp/fsp0fsp.cc > @@ -2801,6 +2801,7 @@ fsp_reserve_free_extents( > } > } else { > ut_a(alloc_type == FSP_CLEANING); > + reserve = 0; > } > > success = fil_space_reserve_free_extents(space, n_free, n_ext); > diff --git a/storage/innobase/row/row0ftsort.cc b/storage/innobase/row/ > row0ftsort.cc > index f662ef6b461..4865c8e850a 100644 > --- a/storage/innobase/row/row0ftsort.cc > +++ b/storage/innobase/row/row0ftsort.cc > @@ -620,7 +620,6 @@ fts_parallel_tokenization( > mem_heap_t* blob_heap = NULL; > fts_doc_t doc; > dict_table_t* table = psort_info->psort_common->new_ > table; > - dict_field_t* idx_field; > fts_tokenize_ctx_t t_ctx; > ulint retried = 0; > dberr_t error = DB_SUCCESS; > @@ -642,9 +641,6 @@ fts_parallel_tokenization( > doc.charset = fts_index_get_charset( > psort_info->psort_common->dup->index); > > - idx_field = dict_index_get_nth_field( > - psort_info->psort_common->dup->index, 0); > - > block = psort_info->merge_block; > zip_size = dict_table_zip_size(table); > > diff --git a/storage/xtradb/fsp/fsp0fsp.cc b/storage/xtradb/fsp/fsp0fsp.cc > index 0bdb2ea0d4c..64576c93545 100644 > --- a/storage/xtradb/fsp/fsp0fsp.cc > +++ b/storage/xtradb/fsp/fsp0fsp.cc > @@ -2810,6 +2810,7 @@ fsp_reserve_free_extents( > } > } else { > ut_a(alloc_type == FSP_CLEANING); > + reserve = 0; > } > > success = fil_space_reserve_free_extents(space, n_free, n_ext); > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

1 0

Re: [Maria-developers] [Commits] ab0943d5b11: Fix gcc 6.3.x compiler warnings.
by Marko Mäkelä 16 Feb '17

16 Feb '17

On Thu, Feb 16, 2017 at 10:26 AM, jan <jan.lindstrom(a)mariadb.com> wrote: > revision-id: ab0943d5b11f01fb3f4638134bd1b8fb6a14a5e0 (mariadb-5.5.54-23- > gab0943d5b11) > parent(s): 2e8fa1c2b2867b99b5cc7f987ad64b666e0af2c4 > author: Jan Lindström > committer: Jan Lindström > timestamp: 2017-02-16 10:25:27 +0200 > message: > > Fix gcc 6.3.x compiler warnings. > > These are caused by fact that functions are declared with > __attribute__((nonnull)) or left shit like ~0 << macro > when ~0U << macro should be used. > OK to push, except for the following unrelated changes (which are OK in a separate commit): diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/ > trx0trx.h > index 4ade245f03e..94707ecf7cb 100644 > --- a/storage/innobase/include/trx0trx.h > +++ b/storage/innobase/include/trx0trx.h > @@ -481,6 +481,7 @@ struct trx_struct{ > of view of concurrency control: > TRX_ACTIVE, > TRX_COMMITTED_IN_MEMORY, > ... */ > + os_thread_id_t thread_id; > /*------------------------------*/ > /* MySQL has a transaction coordinator to coordinate two phase > commit between multiple storage engines and the binary > log. When > diff --git a/storage/innobase/trx/trx0trx.c b/storage/innobase/trx/ > trx0trx.c > index 2d585b7507f..9a80478f6f1 100644 > --- a/storage/innobase/trx/trx0trx.c > +++ b/storage/innobase/trx/trx0trx.c > @@ -40,6 +40,7 @@ Created 3/26/1996 Heikki Tuuri > #include "srv0srv.h" > #include "btr0sea.h" > #include "os0proc.h" > +#include "os0thread.h" > #include "trx0xa.h" > #include "trx0purge.h" > #include "ha_prototypes.h" > @@ -97,7 +98,6 @@ trx_create( > trx_t* trx; > > ut_ad(mutex_own(&kernel_mutex)); > - ut_ad(sess); > > trx = mem_alloc(sizeof(trx_t)); > > @@ -1770,7 +1770,7 @@ trx_print( > { > ibool newline; > > - fprintf(f, "TRANSACTION " TRX_ID_FMT, (ullint) trx->id); > + fprintf(f, "TRANSACTION " TRX_ID_FMT " OS thread %lu", (ullint) > trx->id, trx->thread_id); > > switch (trx->conc_state) { > case TRX_NOT_STARTED: > Furthermore, I think that in that separate commit for adding these, a macro could be useful instead of "%lu", because pthread_t may be incompatible with "%lu". 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] f380e7b5535: MDEV-12072 Do not unnecessarily construct rec_printer objects in debug builds
by Jan Lindström 15 Feb '17

15 Feb '17

ok to push. On Wed, Feb 15, 2017 at 4:02 PM, <marko.makela(a)mariadb.com> wrote: > revision-id: f380e7b5535358bd3d7caf81d22579ef97f6ca24 (mariadb-10.2.3-271- > gf380e7b5535) > parent(s): 24911cee4e3be988848f6eab4d768849709f5256 > author: Marko Mäkelä > committer: Marko Mäkelä > timestamp: 2017-02-15 16:02:00 +0200 > message: > > MDEV-12072 Do not unnecessarily construct rec_printer objects in debug > builds > > I introduced the rec_printer object in MySQL to pretty-print raw InnoDB > records and index tuples in diagnostic messages. These objects are being > constructed unconditionally, even though the DBUG_PRINT is not enabled. > > The unnecessary work would be avoided by simply passing > rec_printer(…).str() to the DBUG_LOG that was introduced in MDEV-11713. > > --- > storage/innobase/btr/btr0cur.cc | 54 ++++++++---------------- > storage/innobase/row/row0log.cc | 58 ++++++++------------------ > storage/innobase/row/row0merge.cc | 88 +++++++++++++++--------------- > --------- > 3 files changed, 68 insertions(+), 132 deletions(-) > > diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/ > btr0cur.cc > index 9033c399ce2..bdf3bd2eca7 100644 > --- a/storage/innobase/btr/btr0cur.cc > +++ b/storage/innobase/btr/btr0cur.cc > @@ -3012,18 +3012,10 @@ btr_cur_optimistic_insert( > > page_cursor = btr_cur_get_page_cur(cursor); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(entry); > - DBUG_PRINT("ib_cur", ("insert %s (" IB_ID_FMT ") by " > IB_ID_FMT " %s", > - index->name(), index->id, > - thr != NULL > - ? trx_get_id_for_print(thr_get_trx(thr)) > - : 0, > - p.str().c_str())); > - } > -#endif > - > + DBUG_LOG("ib_cur", > + "insert " << index->name << " (" << index->id << ") by " > + << ib::hex(thr ? trx_get_id_for_print(thr_get_trx(thr)) > : 0) > + << ' ' << rec_printer(entry).str()); > DBUG_EXECUTE_IF("do_page_reorganize", > btr_page_reorganize(page_cursor, index, mtr);); > > @@ -3635,14 +3627,10 @@ btr_cur_update_in_place( > ut_ad(fil_page_index_page_check(btr_cur_get_page(cursor))); > ut_ad(btr_page_get_index_id(btr_cur_get_page(cursor)) == > index->id); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(rec, offsets); > - DBUG_PRINT("ib_cur", ("update-in-place %s (" IB_ID_FMT ") > by " IB_ID_FMT ": %s", > - index->name(), index->id, trx_id, > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_cur", > + "update-in-place " << index->name << " (" << index->id > + << ") by " << ib::hex(trx_id) << ": " > + << rec_printer(rec, offsets).str()); > > block = btr_cur_get_block(cursor); > page_zip = buf_block_get_page_zip(block); > @@ -3842,14 +3830,10 @@ btr_cur_optimistic_update( > } > } > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(rec, *offsets); > - DBUG_PRINT("ib_cur", ("update %s (" IB_ID_FMT ") by " > IB_ID_FMT ": %s", > - index->name(), index->id, trx_id, > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_cur", > + "update " << index->name << " (" << index->id << ") by " > + << ib::hex(trx_id) << ": " > + << rec_printer(rec, *offsets).str()); > > page_cursor = btr_cur_get_page_cur(cursor); > > @@ -4669,15 +4653,11 @@ btr_cur_del_mark_set_clust_rec( > ut_ad(trx_state_eq(trx, TRX_STATE_ACTIVE)); > ut_ad(!trx->in_rollback); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(rec, offsets); > - DBUG_PRINT("ib_cur", ("delete-mark clust %s (" IB_ID_FMT > ") by " IB_ID_FMT ": %s", > - index->table_name, index->id, > - trx_get_id_for_print(trx), > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_cur", > + "delete-mark clust " << index->table->name > + << " (" << index->id << ") by " > + << ib::hex(trx_get_id_for_print(trx)) << ": " > + << rec_printer(rec, offsets).str()); > > if (dict_index_is_online_ddl(index)) { > row_log_table_delete(rec, entry, index, offsets, NULL); > diff --git a/storage/innobase/row/row0log.cc b/storage/innobase/row/ > row0log.cc > index d43fb333cdc..0fc1dd097cc 100644 > --- a/storage/innobase/row/row0log.cc > +++ b/storage/innobase/row/row0log.cc > @@ -1,6 +1,7 @@ > /*********************************************************** > ****************** > > Copyright (c) 2011, 2016, 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 the Free > Software > @@ -1641,15 +1642,9 @@ row_log_table_apply_insert_low( > ut_ad(dtuple_validate(row)); > ut_ad(trx_id); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(row); > - DBUG_PRINT("ib_alter_table", > - ("insert table " IB_ID_FMT " (index " IB_ID_FMT > "): %s", > - index->table->id, index->id, > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_alter_table", > + "insert table " << index->table->id << " (index " > + << index->id << "): " << rec_printer(row).str()); > > static const ulint flags > = (BTR_CREATE_FLAG > @@ -1777,15 +1772,10 @@ row_log_table_apply_delete_low( > > ut_ad(dict_index_is_clust(index)); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(btr_pcur_get_rec(pcur), offsets); > - DBUG_PRINT("ib_alter_table", > - ("delete table " IB_ID_FMT " (index " IB_ID_FMT > "): %s", > - index->table->id, index->id, > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_alter_table", > + "delete table " << index->table->id << " (index " > + << index->id << "): " > + << rec_printer(btr_pcur_get_rec(pcur), offsets).str()); > > if (dict_table_get_next_index(index)) { > /* Build a row template for purging secondary index > entries. */ > @@ -2266,17 +2256,11 @@ row_log_table_apply_update( > cur_offsets, NULL, NULL, NULL, &old_ext, heap); > ut_ad(old_row); > > -#ifdef UNIV_DEBUG > - { > - rec_printer old(old_row); > - rec_printer new_row(row); > - DBUG_PRINT("ib_alter_table", > - ("update table " IB_ID_FMT " (index " > IB_ID_FMT "): %s to %s", > - index->table->id, index->id, > - old.str().c_str(), > - new_row.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_alter_table", > + "update table " << index->table->id > + << " (index " << index->id > + << ": " << rec_printer(old_row).str() > + << " to " << rec_printer(row).str()); > } else { > old_row = NULL; > old_ext = NULL; > @@ -3298,17 +3282,11 @@ row_log_apply_op_low( > ut_ad(!dict_index_is_corrupted(index)); > ut_ad(trx_id != 0 || op == ROW_OP_DELETE); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(entry); > - DBUG_PRINT("ib_create_index", > - ("%s %s index " IB_ID_FMT "," IB_ID_FMT ": > %s", > - op == ROW_OP_INSERT ? "insert" : "delete", > - has_index_lock ? "locked" : "unlocked", > - index->id, trx_id, > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_create_index", > + (op == ROW_OP_INSERT ? "insert " : "delete ") > + << (has_index_lock ? "locked index " : "unlocked index ") > + << index->id << ',' << ib::hex(trx_id) << ": " > + << rec_printer(entry).str()); > > mtr_start(&mtr); > mtr.set_named_space(index->space); > diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/ > row0merge.cc > index 57dd8f75e8e..85f1e0b6ecc 100644 > --- a/storage/innobase/row/row0merge.cc > +++ b/storage/innobase/row/row0merge.cc > @@ -1,6 +1,7 @@ > /*********************************************************** > ****************** > > Copyright (c) 2005, 2016, 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 the Free > Software > @@ -1100,16 +1101,11 @@ row_merge_buf_write( > row_merge_buf_encode(&b, index, entry, n_fields); > ut_ad(b < &block[srv_sort_buf_size]); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(entry->fields, n_fields); > - DBUG_PRINT("ib_merge_sort", > - ("%p,fd=%d,%lu %lu: %s", > - reinterpret_cast<const void*>(b), > of->fd, > - (ulint)of->offset, (ulint)i, > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_merge_sort", > + reinterpret_cast<const void*>(b) << ',' > + << of->fd << ',' << of->offset << ' ' << > + i << ": " << > + rec_printer(entry->fields, n_fields).str()); > } > > /* Write an "end-of-chunk" marker. */ > @@ -1121,10 +1117,9 @@ row_merge_buf_write( > to avoid bogus warnings. */ > memset(b, 0xff, &block[srv_sort_buf_size] - b); > #endif /* UNIV_DEBUG_VALGRIND */ > - DBUG_PRINT("ib_merge_sort", > - ("write %p,%d,%lu EOF", > - reinterpret_cast<const void*>(b), of->fd, > - (ulint)of->offset)); > + DBUG_LOG("ib_merge_sort", > + "write " << b << ',' << of->fd << ',' << of->offset << > + " EOF"); > DBUG_VOID_RETURN; > } > > @@ -1177,7 +1172,7 @@ row_merge_read( > os_offset_t ofs = ((os_offset_t) offset) * srv_sort_buf_size; > > DBUG_ENTER("row_merge_read"); > - DBUG_PRINT("ib_merge_sort", ("fd=%d ofs=" UINT64PF, fd, ofs)); > + DBUG_LOG("ib_merge_sort", "fd=" << fd << " ofs=" << ofs); > DBUG_EXECUTE_IF("row_merge_read_failure", DBUG_RETURN(FALSE);); > > IORequest request; > @@ -1224,7 +1219,7 @@ row_merge_write( > void* out_buf = (void *)buf; > > DBUG_ENTER("row_merge_write"); > - DBUG_PRINT("ib_merge_sort", ("fd=%d ofs=" UINT64PF, fd, ofs)); > + DBUG_LOG("ib_merge_sort", "fd=" << fd << " ofs=" << ofs); > DBUG_EXECUTE_IF("row_merge_write_failure", DBUG_RETURN(FALSE);); > > IORequest request(IORequest::WRITE); > @@ -1296,11 +1291,10 @@ row_merge_read_rec( > if (UNIV_UNLIKELY(!extra_size)) { > /* End of list */ > *mrec = NULL; > - DBUG_PRINT("ib_merge_sort", > - ("read %p,%p,%d,%lu EOF\n", > - reinterpret_cast<const void*>(b), > - reinterpret_cast<const void*>(block), > - fd, ulint(*foffs))); > + DBUG_LOG("ib_merge_sort", > + "read " << reinterpret_cast<const void*>(b) << > ',' << > + reinterpret_cast<const void*>(block) << ',' << > + fd << ',' << *foffs << " EOF"); > DBUG_RETURN(NULL); > } > > @@ -1413,18 +1407,11 @@ row_merge_read_rec( > b += extra_size + data_size - avail_size; > > func_exit: > - > -#ifdef UNIV_DEBUG > - { > - rec_printer p(*mrec, 0, offsets); > - DBUG_PRINT("ib_merge_sort", > - ("%p,%p,fd=%d,%lu: %s", > - reinterpret_cast<const void*>(b), > - reinterpret_cast<const void*>(block), > - fd, ulint(*foffs), > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_merge_sort", > + reinterpret_cast<const void*>(b) << ',' << > + reinterpret_cast<const void*>(block) > + << ",fd=" << fd << ',' << *foffs << ": " > + << rec_printer(*mrec, 0, offsets).str()); > DBUG_RETURN(b); > } > > @@ -1455,15 +1442,9 @@ row_merge_write_rec_low( > #endif /* DBUG_OFF */ > DBUG_ASSERT(e == rec_offs_extra_size(offsets) + 1); > > -#ifdef UNIV_DEBUG > - { > - rec_printer p(mrec, 0, offsets); > - DBUG_PRINT("ib_merge_sort", > - ("%p,fd=%d,%lu: %s", > - reinterpret_cast<const void*>(b), fd, > ulint(foffs), > - p.str().c_str())); > - } > -#endif > + DBUG_LOG("ib_merge_sort", > + reinterpret_cast<const void*>(b) << ",fd=" << fd << ',' > + << foffs << ": " << rec_printer(mrec, 0, offsets).str()); > > if (e < 0x80) { > *b++ = (byte) e; > @@ -1573,11 +1554,10 @@ row_merge_write_eof( > ut_ad(foffs); > > DBUG_ENTER("row_merge_write_eof"); > - DBUG_PRINT("ib_merge_sort", > - ("%p,%p,fd=%d,%lu", > - reinterpret_cast<const void*>(b), > - reinterpret_cast<const void*>(block), > - fd, ulint(*foffs))); > + DBUG_LOG("ib_merge_sort", > + reinterpret_cast<const void*>(b) << ',' << > + reinterpret_cast<const void*>(block) << > + ",fd=" << fd << ',' << *foffs); > > if (b == &block[0]) { > b+= ROW_MERGE_RESERVE_SIZE; > @@ -2937,10 +2917,9 @@ row_merge_blocks( > ulint* offsets1;/* offsets of mrec1 */ > > DBUG_ENTER("row_merge_blocks"); > - DBUG_PRINT("ib_merge_sort", > - ("fd=%d,%lu+%lu to fd=%d,%lu", > - file->fd, ulint(*foffs0), ulint(*foffs1), > - of->fd, ulint(of->offset))); > + DBUG_LOG("ib_merge_sort", > + "fd=" << file->fd << ',' << *foffs0 << '+' << *foffs1 > + << " to fd=" << of->fd << ',' << of->offset); > > heap = row_merge_heap_create(dup->index, &buf, &offsets0, > &offsets1); > > @@ -3050,10 +3029,9 @@ row_merge_blocks_copy( > ulint* offsets1;/* dummy offsets */ > > DBUG_ENTER("row_merge_blocks_copy"); > - DBUG_PRINT("ib_merge_sort", > - ("fd=%d,%lu to fd=%d,%lu", > - file->fd, ulint(foffs0), > - of->fd, ulint(of->offset))); > + DBUG_LOG("ib_merge_sort", > + "fd=" << file->fd << ',' << foffs0 > + << " to fd=" << of->fd << ',' << of->offset); > > heap = row_merge_heap_create(index, &buf, &offsets0, &offsets1); > > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

1 0

[Maria-developers] Blackhole, Archive and others only as loadable plugins in 10.1 - intentional change?
by Hartmut Holzgraefe 15 Feb '17

15 Feb '17

In MariaDB 10.1 binary packages several storage engines, e.g BLACKHOLE and ARCHIVE, are no longer compiled in by default but only come as loadable plugins that need to be loaded into the server explicitly. This is mentioned on the knowledge base pages for these engines, e.g. https://mariadb.com/kb/en/mariadb/blackhole/ and https://mariadb.com/kb/en/mariadb/archive/ but is not mentioned in the release notes and the detailed changelog pages for 10.1.x releases at all. I also searched my "maria-developers" and "maria-discuss" mail archives but could not find any discussion or announcement on this topic either. Same for Jira, can't find any ticket that would describe that sort of change there either. Problems with the current situation: * comes as a surprise for installation using these storage engines when upgraded to 10.1 * mysql_upgrade should handle this, but doesn't: https://jira.mariadb.org/browse/MDEV-11942 * it is not even easily possible to identify affected tables after an upgrade to 10.1. For BLACKHOLE tables I_S.TABLES only shows these under specific conditions, and for ARCHIVE it seems to be impossible to identify affected tables at all if the plugin is not loaded https://jira.mariadb.org/browse/MDEV-11943 I have submitted a a fix for MDEV-11942, but without having a fix for MDEV-11943 it only works for BLACKHOLE so far, not for ARCHIVE (haven't tested FEDERATED or other potentially affected engines yet). https://github.com/MariaDB/server/pull/304 -- Hartmut Holzgraefe, Principal Support Engineer (EMEA) MariaDB Corporation | http://www.mariadb.com/

2 1

[Maria-developers] Fwd: 41f885f64fe: MDEV-10859: Wrong result of aggregate window function in query with HAVING and no ORDER BY
by Vicențiu Ciorbaru 14 Feb '17

14 Feb '17

Hi Igor, Sergey! I need a review on this patch that fixes a problem with Window Functions being computed over too many rows. The details are in the commit message. Thank you! Vicențiu ---------- Forwarded message --------- From: vicentiu <vicentiu(a)mariadb.org> Date: Tue, 14 Feb 2017 at 14:08 Subject: 41f885f64fe: MDEV-10859: Wrong result of aggregate window function in query with HAVING and no ORDER BY To: <commits(a)mariadb.org> revision-id: 41f885f64fe4c02e03ec655f225b1dc01e9d5376 (mariadb-10.2.3-252-g41f885f64fe) parent(s): d731ce21a7bc25a49958789d24b3f0eec5845aea author: Vicențiu Ciorbaru committer: Vicențiu Ciorbaru timestamp: 2017-02-14 14:02:29 +0200 message: MDEV-10859: Wrong result of aggregate window function in query with HAVING and no ORDER BY Window functions need to be computed after applying the HAVING clause. An optimization that we have for regular, non-window function, cases is to apply having only during sending of the rows to the client. This allows rows that should be filtered from the temporary table used to store aggregation results to be stored there. This behaviour is undesireable for window functions, as we have to compute window functions on the result-set after HAVING is applied. Storing extra rows in the table leads to wrong values as the frame bounds might capture those -to be filtered afterwards- rows. --- mysql-test/r/win.result | 33 +++++++++++++++++++++++++++++++++ mysql-test/t/win.test | 23 +++++++++++++++++++++++ sql/sql_select.cc | 11 ++++++++--- sql/sql_window.cc | 6 ++++++ 4 files changed, 70 insertions(+), 3 deletions(-) diff --git a/mysql-test/r/win.result b/mysql-test/r/win.result index fd3aea80083..b3adf358be6 100644 --- a/mysql-test/r/win.result +++ b/mysql-test/r/win.result @@ -2925,3 +2925,36 @@ WHERE EXISTS ( SELECT * FROM t2 WHERE c IN ( SELECT MAX(d) FROM t3 ) ); a MAX(a) AVG(a) OVER (PARTITION BY b) NULL NULL NULL DROP TABLE t1,t2,t3; +# +# MDEV-MDEV-10859: Wrong result of aggregate window function in query +# with HAVING and no ORDER BY +# +create table empsalary (depname varchar(32), empno smallint primary key, salary int); +insert into empsalary values +('develop', 1, 5000), ('develop', 2, 4000),('sales', 3, '6000'),('sales', 4, 5000); +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary; +depname empno salary avg(salary) OVER (PARTITION BY depname) +develop 1 5000 4500.0000 +develop 2 4000 4500.0000 +sales 3 6000 5500.0000 +sales 4 5000 5500.0000 +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary ORDER BY depname; +depname empno salary avg(salary) OVER (PARTITION BY depname) +develop 1 5000 4500.0000 +develop 2 4000 4500.0000 +sales 3 6000 5500.0000 +sales 4 5000 5500.0000 +# +# These last 2 should have the same row results, ignoring order. +# +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary HAVING empno > 1; +depname empno salary avg(salary) OVER (PARTITION BY depname) +develop 2 4000 4000.0000 +sales 3 6000 5500.0000 +sales 4 5000 5500.0000 +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary HAVING empno > 1 ORDER BY depname; +depname empno salary avg(salary) OVER (PARTITION BY depname) +develop 2 4000 4000.0000 +sales 3 6000 5500.0000 +sales 4 5000 5500.0000 +drop table empsalary; diff --git a/mysql-test/t/win.test b/mysql-test/t/win.test index aa636f7a294..90bbf41688f 100644 --- a/mysql-test/t/win.test +++ b/mysql-test/t/win.test @@ -1711,3 +1711,26 @@ SELECT a, MAX(a), AVG(a) OVER (PARTITION BY b) FROM t1 WHERE EXISTS ( SELECT * FROM t2 WHERE c IN ( SELECT MAX(d) FROM t3 ) ); DROP TABLE t1,t2,t3; + +--echo # +--echo # MDEV-MDEV-10859: Wrong result of aggregate window function in query +--echo # with HAVING and no ORDER BY +--echo # + +create table empsalary (depname varchar(32), empno smallint primary key, salary int); +insert into empsalary values + ('develop', 1, 5000), ('develop', 2, 4000),('sales', 3, '6000'),('sales', 4, 5000); + +--sorted_result +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary; +--sorted_result +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary ORDER BY depname; +--echo # +--echo # These last 2 should have the same row results, ignoring order. +--echo # +--sorted_result +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary HAVING empno > 1; +--sorted_result +SELECT depname, empno, salary, avg(salary) OVER (PARTITION BY depname) FROM empsalary HAVING empno > 1 ORDER BY depname; + +drop table empsalary; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 5c7ae1e88c1..473270f8a69 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -2411,13 +2411,18 @@ bool JOIN::make_aggr_tables_info() If having is not handled here, it will be checked before the row is sent to the client. + + In the case of window functions however, we *must* make sure to not + store any rows which don't match HAVING within the temp table, + as rows will end up being used during their computation. */ if (having && - (sort_and_group || (exec_tmp_table->distinct && !group_list))) + (sort_and_group || (exec_tmp_table->distinct && !group_list) || + select_lex->have_window_funcs())) { - // Attach HAVING to tmp table's condition + /* Attach HAVING to tmp table's condition */ curr_tab->having= having; - having= NULL; // Already done + having= NULL; /* Already done */ } /* Change sum_fields reference to calculated fields in tmp_table */ diff --git a/sql/sql_window.cc b/sql/sql_window.cc index 37095ad78ca..2bdac89f293 100644 --- a/sql/sql_window.cc +++ b/sql/sql_window.cc @@ -2840,6 +2840,12 @@ bool Window_funcs_computation::setup(THD *thd, order_window_funcs_by_window_specs(window_funcs); SQL_SELECT *sel= NULL; + /* + If the tmp table is filtered during sorting + (ex: SELECT with HAVING && ORDER BY), we must make sure to keep the + filtering conditions when we perform sorting for window function + computation. + */ if (tab->filesort && tab->filesort->select) { sel= tab->filesort->select;

1 0

[Maria-developers] bb-10.2-compatibility - MDEV 10585 or 10866
by jerome brauge 14 Feb '17

14 Feb '17

Hello Alexander, Attached script fail on my local bb-10.2-compatibility branch on the following assert : /* If called from a stored procedure, ensure that we won't rollback external changes when cleaning up after validation. */ DBUG_ASSERT(thd->change_list.is_empty()); Stack trace : mysqld.exe!my_sigabrt_handler(int sig) Line 477 C mysqld.exe!raise(int signum) Line 516 C++ mysqld.exe!abort() Line 64 C++ mysqld.exe!common_assert_to_stderr_direct(const wchar_t * const expression, const wchar_t * const file_name, const unsigned int line_number) Line 124 C++ mysqld.exe!common_assert_to_stderr<wchar_t>(const wchar_t * const expression, const wchar_t * const file_name, const unsigned int line_number) Line 138 C++ mysqld.exe!common_assert<wchar_t>(const wchar_t * const expression, const wchar_t * const file_name, const unsigned int line_number, void * const return_address) Line 383 C++ mysqld.exe!_wassert(const wchar_t * expression, const wchar_t * file_name, unsigned int line_number) Line 404 C++ > mysqld.exe!Prepared_statement::prepare(const char * packet, unsigned int packet_len) Line 3848 C++ mysqld.exe!Prepared_statement::execute_immediate(const char * query, unsigned int query_len) Line 4740 C++ mysqld.exe!mysql_sql_stmt_execute_immediate(THD * thd) Line 2863 C++ mysqld.exe!mysql_execute_command(THD * thd) Line 3722 C++ mysqld.exe!sp_instr_stmt::exec_core(THD * thd, unsigned int * nextp) Line 3335 C++ mysqld.exe!sp_lex_keeper::reset_lex_and_exec_core(THD * thd, unsigned int * nextp, bool open_tables, sp_instr * instr) Line 3098 C++ mysqld.exe!sp_instr_stmt::execute(THD * thd, unsigned int * nextp) Line 3251 C++ mysqld.exe!sp_head::execute(THD * thd, bool merge_da_on_success) Line 1265 C++ mysqld.exe!sp_head::execute_procedure(THD * thd, List<Item> * args) Line 2090 C++ mysqld.exe!do_execute_sp(THD * thd, sp_head * sp) Line 2871 C++ mysqld.exe!mysql_execute_command(THD * thd) Line 5900 C++ mysqld.exe!mysql_parse(THD * thd, char * rawbuf, unsigned int length, Parser_state * parser_state, bool is_com_multi, bool is_next_command) Line 7990 C++ mysqld.exe!dispatch_command(enum_server_command command, THD * thd, char * packet, unsigned int packet_length, bool is_com_multi, bool is_next_command) Line 1809 C++ mysqld.exe!do_command(THD * thd) Line 1367 C++ mysqld.exe!threadpool_process_request(THD * thd) Line 346 C++ mysqld.exe!tp_callback(TP_connection * c) Line 192 C++ mysqld.exe!tp_callback(_TP_CALLBACK_INSTANCE * instance, void * context) Line 377 C++ mysqld.exe!work_callback(_TP_CALLBACK_INSTANCE * instance, void * context, _TP_WORK * work) Line 451 C++ Can you reproduce this error ? Regards, Jérôme.

2 1

Re: [Maria-developers] [Commits] 822aa21672b: MDEV-11170: MariaDB 10.2 cannot start on MySQL 5.7 datadir
by Sergei Golubchik 13 Feb '17

13 Feb '17

Hi, Vicentiu! Looks fine, thanks. Few comments below. On Feb 13, vicentiu wrote: > revision-id: 822aa21672ba6650a31a3ec05ab5c1bce7a80c1e (mariadb-10.2.3-198-g822aa21672b) > parent(s): b745ea489f0698c328383d9b8ec20d2f9777b1b8 > author: Vicențiu Ciorbaru > committer: Vicențiu Ciorbaru > timestamp: 2017-02-13 14:44:00 +0200 > message: > > MDEV-11170: MariaDB 10.2 cannot start on MySQL 5.7 datadir > > PART 2 of the fix adds the logic of not using password column, unless it > exists. If password column is missing we attempt to use plugin && > authentication_string columns. > > diff --git a/mysql-test/r/no_password_column-mdev-11170.result b/mysql-test/r/no_password_column-mdev-11170.result > new file mode 100644 > index 00000000000..c3920a1b530 > --- /dev/null > +++ b/mysql-test/r/no_password_column-mdev-11170.result > @@ -0,0 +1,170 @@ ... > +# Reset to final original state. > +# > +drop table mysql.user; > +create table mysql.user like backup_user; > +insert into mysql.user select * from backup_user; you could RENAME TABLE backup_user to mysql.user. > +drop table backup_user; > +flush privileges; > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index 568aff9ac92..7e03253ca98 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -2071,7 +2103,7 @@ static bool acl_load(THD *thd, const Grant_tables& tables) > 8, 8, MYF(0)); > > /* check default role, if any */ > - if (!is_role && user_table.num_fields() > DEFAULT_ROLE_COLUMN_IDX) > + if (!is_role && user_table.default_role()) all these changes could've went into the first patch... > { > user.default_rolename.str= > get_field(&acl_memroot, user_table.default_role()); > @@ -4030,11 +4092,16 @@ static int replace_user_table(THD *thd, const User_table &user_table, > > rights= user_table.get_access(); > > - DBUG_PRINT("info",("table fields: %d",user_table.num_fields())); > - if (combo.pwhash.str[0]) > + DBUG_PRINT("info",("table fields: %d", user_table.num_fields())); > + /* If we don't have a password column, we'll use the authentication_string > + column later. */ > + if (combo.pwhash.str[0] && user_table.password()) > user_table.password()->store(combo.pwhash.str, combo.pwhash.length, > system_charset_info); > - if (user_table.num_fields() >= 31) /* From 4.0.0 we have more fields */ > + /* We either have the password column, the plugin column, or both. Otherwise > + we have a corrupt user table. */ > + DBUG_ASSERT(user_table.password() || user_table.plugin()); you cannot assert the table structure. ALTER TABLE shouldn't crash the server. > + if (user_table.ssl_type()) /* From 4.0.0 we have more fields */ > { > /* We write down SSL related ACL stuff */ > switch (lex->ssl_type) { Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1