From 462808f3b65b60958eaf8013b80857e341cc9f6c Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Wed, 20 Dec 2017 13:52:27 +0100 Subject: MDEV-10657: incorrect result returned with binary protocol (prepared statements) If translation table present when we materialize the derived table then change it to point to the materialized table. Added debug info to see really what happens with what derived. --- mysql-test/r/ps.result | 18 ++++++++++++++++++ mysql-test/t/ps.test | 15 +++++++++++++++ sql/sql_class.cc | 9 ++++++++- sql/sql_derived.cc | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- sql/table.cc | 3 +++ sql/table.h | 6 ++++++ 6 files changed, 96 insertions(+), 3 deletions(-) diff --git a/mysql-test/r/ps.result b/mysql-test/r/ps.result index 3fed0a5b0d1..209d3d85108 100644 --- a/mysql-test/r/ps.result +++ b/mysql-test/r/ps.result @@ -4316,4 +4316,22 @@ set join_cache_level=@join_cache_level_save; deallocate prepare stmt; drop view v1,v2,v3; drop table t1,t2,t3; +# +# MDEV-10657: incorrect result returned with binary protocol +# (prepared statements) +# +create table t1 (code varchar(10) primary key); +INSERT INTO t1(code) VALUES ('LINE1'), ('LINE2'), ('LINE3'); +SELECT X.* +FROM +(SELECT CODE, RN +FROM +(SELECT A.CODE, @cnt := @cnt + 1 AS RN +FROM t1 A, (SELECT @cnt := 0) C) T +) X; +CODE RN +LINE1 1 +LINE2 2 +LINE3 3 +drop table t1; # End of 5.5 tests diff --git a/mysql-test/t/ps.test b/mysql-test/t/ps.test index 4431f722ae0..ea67e1074f9 100644 --- a/mysql-test/t/ps.test +++ b/mysql-test/t/ps.test @@ -3843,4 +3843,19 @@ deallocate prepare stmt; drop view v1,v2,v3; drop table t1,t2,t3; +--echo # +--echo # MDEV-10657: incorrect result returned with binary protocol +--echo # (prepared statements) +--echo # + +create table t1 (code varchar(10) primary key); +INSERT INTO t1(code) VALUES ('LINE1'), ('LINE2'), ('LINE3'); +SELECT X.* +FROM + (SELECT CODE, RN + FROM + (SELECT A.CODE, @cnt := @cnt + 1 AS RN + FROM t1 A, (SELECT @cnt := 0) C) T + ) X; +drop table t1; --echo # End of 5.5 tests diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 00d860e7887..b007729494e 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -2192,6 +2192,8 @@ void THD::nocheck_register_item_tree_change(Item **place, Item *old_value, MEM_ROOT *runtime_memroot) { Item_change_record *change; + DBUG_ENTER("THD::nocheck_register_item_tree_change"); + DBUG_PRINT("enter", ("Register %p <- %p", old_value, (*place))); /* Now we use one node per change, which adds some memory overhead, but still is rather fast as we use alloc_root for allocations. @@ -2204,12 +2206,13 @@ void THD::nocheck_register_item_tree_change(Item **place, Item *old_value, OOM, thd->fatal_error() is called by the error handler of the memroot. Just return. */ - return; + DBUG_VOID_RETURN; } change= new (change_mem) Item_change_record; change->place= place; change->old_value= old_value; change_list.append(change); + DBUG_VOID_RETURN; } /** @@ -2250,7 +2253,11 @@ void THD::rollback_item_tree_changes() DBUG_ENTER("rollback_item_tree_changes"); while ((change= it++)) + { + DBUG_PRINT("info", ("revert %p -> %p", + change->old_value, (*change->place))); *change->place= change->old_value; + } /* We can forget about changes memory: it's allocated in runtime memroot */ change_list.empty(); DBUG_VOID_RETURN; diff --git a/sql/sql_derived.cc b/sql/sql_derived.cc index 6cd4547aaf9..2a6d82c161a 100644 --- a/sql/sql_derived.cc +++ b/sql/sql_derived.cc @@ -361,6 +361,9 @@ bool mysql_derived_merge(THD *thd, LEX *lex, TABLE_LIST *derived) SELECT_LEX *parent_lex= derived->select_lex; Query_arena *arena, backup; DBUG_ENTER("mysql_derived_merge"); + DBUG_PRINT("enter", ("Alias: '%s' Unit: %p", + (derived->alias ? derived->alias : ""), + derived->get_unit())); if (derived->merged) DBUG_RETURN(FALSE); @@ -508,6 +511,9 @@ unconditional_materialization: bool mysql_derived_merge_for_insert(THD *thd, LEX *lex, TABLE_LIST *derived) { DBUG_ENTER("mysql_derived_merge_for_insert"); + DBUG_PRINT("enter", ("Alias: '%s' Unit: %p", + (derived->alias ? derived->alias : ""), + derived->get_unit())); if (derived->merged_for_insert) DBUG_RETURN(FALSE); if (derived->init_derived(thd, FALSE)) @@ -554,6 +560,9 @@ bool mysql_derived_init(THD *thd, LEX *lex, TABLE_LIST *derived) { SELECT_LEX_UNIT *unit= derived->get_unit(); DBUG_ENTER("mysql_derived_init"); + DBUG_PRINT("enter", ("Alias: '%s' Unit: %p", + (derived->alias ? derived->alias : ""), + derived->get_unit())); // Skip already prepared views/DT if (!unit || unit->prepared) @@ -624,7 +633,9 @@ bool mysql_derived_prepare(THD *thd, LEX *lex, TABLE_LIST *derived) SELECT_LEX_UNIT *unit= derived->get_unit(); DBUG_ENTER("mysql_derived_prepare"); bool res= FALSE; - DBUG_PRINT("enter", ("unit 0x%lx", (ulong) unit)); + DBUG_PRINT("enter", ("Alias: '%s' Unit: %p", + (derived->alias ? derived->alias : ""), + derived->get_unit())); // Skip already prepared views/DT if (!unit || unit->prepared || @@ -781,6 +792,9 @@ bool mysql_derived_optimize(THD *thd, LEX *lex, TABLE_LIST *derived) bool res= FALSE; DBUG_ENTER("mysql_derived_optimize"); + DBUG_PRINT("enter", ("Alias: '%s' Unit: %p", + (derived->alias ? derived->alias : ""), + derived->get_unit())); if (unit->optimized) DBUG_RETURN(FALSE); @@ -846,6 +860,9 @@ err: bool mysql_derived_create(THD *thd, LEX *lex, TABLE_LIST *derived) { DBUG_ENTER("mysql_derived_create"); + DBUG_PRINT("enter", ("Alias: '%s' Unit: %p", + (derived->alias ? derived->alias : ""), + derived->get_unit())); TABLE *table= derived->table; SELECT_LEX_UNIT *unit= derived->get_unit(); @@ -895,9 +912,13 @@ bool mysql_derived_create(THD *thd, LEX *lex, TABLE_LIST *derived) bool mysql_derived_fill(THD *thd, LEX *lex, TABLE_LIST *derived) { - DBUG_ENTER("mysql_derived_fill"); + Field_iterator_table field_iterator; SELECT_LEX_UNIT *unit= derived->get_unit(); bool res= FALSE; + DBUG_ENTER("mysql_derived_fill"); + DBUG_PRINT("enter", ("Alias: '%s' Unit: %p", + (derived->alias ? derived->alias : ""), + derived->get_unit())); if (unit->executed && !unit->uncacheable && !unit->describe) DBUG_RETURN(FALSE); @@ -937,7 +958,27 @@ bool mysql_derived_fill(THD *thd, LEX *lex, TABLE_LIST *derived) if (derived_result->flush()) res= TRUE; unit->executed= TRUE; + + if (derived->field_translation) + { + /* reset translation table to materialized table */ + field_iterator.set_table(derived->table); + for (uint i= 0; + !field_iterator.end_of_fields(); + field_iterator.next(), i= i + 1) + { + Item *item; + + if (!(item= field_iterator.create_item(thd))) + { + res= TRUE; + break; + } + thd->change_item_tree(&derived->field_translation[i].item, item); + } + } } + if (res || !lex->describe) unit->cleanup(); lex->current_select= save_current_select; @@ -966,6 +1007,9 @@ bool mysql_derived_fill(THD *thd, LEX *lex, TABLE_LIST *derived) bool mysql_derived_reinit(THD *thd, LEX *lex, TABLE_LIST *derived) { DBUG_ENTER("mysql_derived_reinit"); + DBUG_PRINT("enter", ("Alias: '%s' Unit: %p", + (derived->alias ? derived->alias : ""), + derived->get_unit())); st_select_lex_unit *unit= derived->get_unit(); if (derived->table) diff --git a/sql/table.cc b/sql/table.cc index fbcd91f5326..9cade76cb78 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -4108,6 +4108,9 @@ bool TABLE_LIST::create_field_translation(THD *thd) Query_arena *arena, backup; bool res= FALSE; DBUG_ENTER("TABLE_LIST::create_field_translation"); + DBUG_PRINT("enter", ("Alias: '%s' Unit: %p", + (alias ? alias : ""), + get_unit())); if (thd->stmt_arena->is_conventional() || thd->stmt_arena->is_stmt_prepare_or_first_sp_execute()) diff --git a/sql/table.h b/sql/table.h index c981243f28c..98f8c7ad73f 100644 --- a/sql/table.h +++ b/sql/table.h @@ -2107,6 +2107,9 @@ struct TABLE_LIST inline void set_merged_derived() { DBUG_ENTER("set_merged_derived"); + DBUG_PRINT("enter", ("Alias: '%s' Unit: %p", + (alias ? alias : ""), + get_unit())); derived_type= ((derived_type & DTYPE_MASK) | DTYPE_TABLE | DTYPE_MERGE); set_check_merged(); @@ -2119,6 +2122,9 @@ struct TABLE_LIST void set_materialized_derived() { DBUG_ENTER("set_materialized_derived"); + DBUG_PRINT("enter", ("Alias: '%s' Unit: %p", + (alias ? alias : ""), + get_unit())); derived_type= ((derived_type & (derived ? DTYPE_MASK : DTYPE_VIEW)) | DTYPE_TABLE | DTYPE_MATERIALIZE); set_check_materialized(); -- cgit v1.2.1 From 1300627a5dcffdb005e2c31e0ba88f47fa519742 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Wed, 27 Dec 2017 22:10:17 +0100 Subject: MDEV-14309 MTR tests require perl-Env which is not always in the default installation * don't use Env module in tests, use $ENV{xxx} instead * collateral changes: ** $file in the error message was unset ** $file in the other error message was unset too :) ** source file arguments are conventionally upper-cased ** abort the test (die) on error, don't just echo/exit --- mysql-test/include/truncate_file.inc | 11 +++-------- mysql-test/suite/rpl/t/rpl_manual_change_index_file.test | 2 +- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/mysql-test/include/truncate_file.inc b/mysql-test/include/truncate_file.inc index 2326d6c0b94..fe88cb05bd9 100644 --- a/mysql-test/include/truncate_file.inc +++ b/mysql-test/include/truncate_file.inc @@ -1,16 +1,11 @@ # truncate a giving file, all contents of the file are be cleared -if (!$file) +if (!$TRUNCATE_FILE) { - --echo Please assign a file name to $file!! - exit; + die TRUNCATE_FILE is not set; } -let TRUNCATE_FILE= $file; - perl; -use Env; -Env::import('TRUNCATE_FILE'); -open FILE, '>', $TRUNCATE_FILE || die "Can not open file $file"; +open FILE, '>', $ENV{TRUNCATE_FILE} or die "open(>$ENV{TRUNCATE_FILE}): $!"; close FILE; EOF diff --git a/mysql-test/suite/rpl/t/rpl_manual_change_index_file.test b/mysql-test/suite/rpl/t/rpl_manual_change_index_file.test index 981cecb66ad..1c087c550d0 100644 --- a/mysql-test/suite/rpl/t/rpl_manual_change_index_file.test +++ b/mysql-test/suite/rpl/t/rpl_manual_change_index_file.test @@ -25,7 +25,7 @@ sync_slave_with_master; connection master; # Delete './master-bin.000001' from index file. let $MYSQLD_DATADIR= `SELECT @@DATADIR`; -let $file= $MYSQLD_DATADIR/master-bin.index; +let TRUNCATE_FILE= $MYSQLD_DATADIR/master-bin.index; source include/truncate_file.inc; if (`SELECT CONVERT(@@VERSION_COMPILE_OS USING latin1) NOT IN ('Win32', 'Win64', 'Windows')`) -- cgit v1.2.1 From d384ead0f024995787b1f29bc672c33b0d3d40a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 2 Jan 2018 19:11:10 +0200 Subject: MDEV-14799 After UPDATE of indexed columns, old values will not be purged from secondary indexes This is a regression caused by MDEV-14051 'Undo log record is too big.' Purge in the secondary index is wrongly skipped in row_purge_upd_exist_or_extern() because node->row only does not contain all indexed columns. trx_undo_rec_get_partial_row(): Add the parameter for node->update so that the updated columns will be copied from the initial part of the undo log record. --- storage/innobase/include/trx0rec.h | 2 ++ storage/innobase/row/row0purge.c | 3 ++- storage/innobase/trx/trx0rec.c | 14 +++++++++++++- storage/xtradb/include/trx0rec.h | 2 ++ storage/xtradb/row/row0purge.c | 3 ++- storage/xtradb/trx/trx0rec.c | 14 +++++++++++++- 6 files changed, 34 insertions(+), 4 deletions(-) diff --git a/storage/innobase/include/trx0rec.h b/storage/innobase/include/trx0rec.h index a6e54d6dfd1..6e3233c685b 100644 --- a/storage/innobase/include/trx0rec.h +++ b/storage/innobase/include/trx0rec.h @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2009, Innobase Oy. All Rights Reserved. +Copyright (c) 2017, 2018, 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 @@ -193,6 +194,7 @@ trx_undo_rec_get_partial_row( used, as we do NOT copy the data in the record! */ dict_index_t* index, /*!< in: clustered index */ + const upd_t* update, /*!< in: updated columns */ dtuple_t** row, /*!< out, own: partial row */ ibool ignore_prefix, /*!< in: flag to indicate if we expect blob prefixes in undo. Used diff --git a/storage/innobase/row/row0purge.c b/storage/innobase/row/row0purge.c index 7b25612ba4b..f099eeb9aa9 100644 --- a/storage/innobase/row/row0purge.c +++ b/storage/innobase/row/row0purge.c @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1997, 2015, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, 2018, 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 @@ -730,7 +731,7 @@ err_exit: if (!(node->cmpl_info & UPD_NODE_NO_ORD_CHANGE)) { ptr = trx_undo_rec_get_partial_row( - ptr, clust_index, &node->row, + ptr, clust_index, node->update, &node->row, type == TRX_UNDO_UPD_DEL_REC, node->heap); } diff --git a/storage/innobase/trx/trx0rec.c b/storage/innobase/trx/trx0rec.c index 94e22d688ca..19e934fc667 100644 --- a/storage/innobase/trx/trx0rec.c +++ b/storage/innobase/trx/trx0rec.c @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2012, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2017, MariaDB Corporation. +Copyright (c) 2017, 2018, 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 @@ -1072,6 +1072,7 @@ trx_undo_rec_get_partial_row( used, as we do NOT copy the data in the record! */ dict_index_t* index, /*!< in: clustered index */ + const upd_t* update, /*!< in: updated columns */ dtuple_t** row, /*!< out, own: partial row */ ibool ignore_prefix, /*!< in: flag to indicate if we expect blob prefixes in undo. Used @@ -1081,6 +1082,8 @@ trx_undo_rec_get_partial_row( { const byte* end_ptr; ulint row_len; + const upd_field_t* uf = update->fields; + const upd_field_t* const ue = update->fields + update->n_fields; ut_ad(index); ut_ad(ptr); @@ -1094,6 +1097,15 @@ trx_undo_rec_get_partial_row( dict_table_copy_types(*row, index->table); + + for (; uf != ue; uf++) { + ulint c = dict_index_get_nth_col(index, uf->field_no)->ind; + ut_ad(uf->orig_len == UNIV_SQL_NULL + || uf->orig_len < UNIV_EXTERN_STORAGE_FIELD); + ut_ad(!dfield_is_ext(&uf->new_val)); + *dtuple_get_nth_field(*row, c) = uf->new_val; + } + end_ptr = ptr + mach_read_from_2(ptr); ptr += 2; diff --git a/storage/xtradb/include/trx0rec.h b/storage/xtradb/include/trx0rec.h index a6e54d6dfd1..6e3233c685b 100644 --- a/storage/xtradb/include/trx0rec.h +++ b/storage/xtradb/include/trx0rec.h @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2009, Innobase Oy. All Rights Reserved. +Copyright (c) 2017, 2018, 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 @@ -193,6 +194,7 @@ trx_undo_rec_get_partial_row( used, as we do NOT copy the data in the record! */ dict_index_t* index, /*!< in: clustered index */ + const upd_t* update, /*!< in: updated columns */ dtuple_t** row, /*!< out, own: partial row */ ibool ignore_prefix, /*!< in: flag to indicate if we expect blob prefixes in undo. Used diff --git a/storage/xtradb/row/row0purge.c b/storage/xtradb/row/row0purge.c index 77d60edb71f..07ed57f55dd 100644 --- a/storage/xtradb/row/row0purge.c +++ b/storage/xtradb/row/row0purge.c @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1997, 2015, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2017, 2018, 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 @@ -730,7 +731,7 @@ err_exit: if (!(node->cmpl_info & UPD_NODE_NO_ORD_CHANGE)) { ptr = trx_undo_rec_get_partial_row( - ptr, clust_index, &node->row, + ptr, clust_index, node->update, &node->row, type == TRX_UNDO_UPD_DEL_REC, node->heap); } diff --git a/storage/xtradb/trx/trx0rec.c b/storage/xtradb/trx/trx0rec.c index f3f1d9a2d96..17d51ecf8d2 100644 --- a/storage/xtradb/trx/trx0rec.c +++ b/storage/xtradb/trx/trx0rec.c @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2012, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2017, MariaDB Corporation. +Copyright (c) 2017, 2018, 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 @@ -1085,6 +1085,7 @@ trx_undo_rec_get_partial_row( used, as we do NOT copy the data in the record! */ dict_index_t* index, /*!< in: clustered index */ + const upd_t* update, /*!< in: updated columns */ dtuple_t** row, /*!< out, own: partial row */ ibool ignore_prefix, /*!< in: flag to indicate if we expect blob prefixes in undo. Used @@ -1094,6 +1095,8 @@ trx_undo_rec_get_partial_row( { const byte* end_ptr; ulint row_len; + const upd_field_t* uf = update->fields; + const upd_field_t* const ue = update->fields + update->n_fields; ut_ad(index); ut_ad(ptr); @@ -1107,6 +1110,15 @@ trx_undo_rec_get_partial_row( dict_table_copy_types(*row, index->table); + + for (; uf != ue; uf++) { + ulint c = dict_index_get_nth_col(index, uf->field_no)->ind; + ut_ad(uf->orig_len == UNIV_SQL_NULL + || uf->orig_len < UNIV_EXTERN_STORAGE_FIELD); + ut_ad(!dfield_is_ext(&uf->new_val)); + *dtuple_get_nth_field(*row, c) = uf->new_val; + } + end_ptr = ptr + mach_read_from_2(ptr); ptr += 2; -- cgit v1.2.1 From 20fab71b144f85be9e2ccc145d24d257b0e9df7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 2 Jan 2018 21:41:39 +0200 Subject: Follow-up to MDEV-14799: Remove bogus debug assertions trx_undo_rec_get_partial_row(): When the PRIMARY KEY includes a column prefix of an externally stored column, the already parsed part of the undo log record may contain a reference to an off-page column. This is the case in the bug58912 test in innodb.innodb. --- storage/innobase/trx/trx0rec.c | 3 --- storage/xtradb/trx/trx0rec.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/storage/innobase/trx/trx0rec.c b/storage/innobase/trx/trx0rec.c index 19e934fc667..aba5e707c4f 100644 --- a/storage/innobase/trx/trx0rec.c +++ b/storage/innobase/trx/trx0rec.c @@ -1100,9 +1100,6 @@ trx_undo_rec_get_partial_row( for (; uf != ue; uf++) { ulint c = dict_index_get_nth_col(index, uf->field_no)->ind; - ut_ad(uf->orig_len == UNIV_SQL_NULL - || uf->orig_len < UNIV_EXTERN_STORAGE_FIELD); - ut_ad(!dfield_is_ext(&uf->new_val)); *dtuple_get_nth_field(*row, c) = uf->new_val; } diff --git a/storage/xtradb/trx/trx0rec.c b/storage/xtradb/trx/trx0rec.c index 17d51ecf8d2..167e6b356d3 100644 --- a/storage/xtradb/trx/trx0rec.c +++ b/storage/xtradb/trx/trx0rec.c @@ -1113,9 +1113,6 @@ trx_undo_rec_get_partial_row( for (; uf != ue; uf++) { ulint c = dict_index_get_nth_col(index, uf->field_no)->ind; - ut_ad(uf->orig_len == UNIV_SQL_NULL - || uf->orig_len < UNIV_EXTERN_STORAGE_FIELD); - ut_ad(!dfield_is_ext(&uf->new_val)); *dtuple_get_nth_field(*row, c) = uf->new_val; } -- cgit v1.2.1