maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

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@xxxxxxxxxxx
> 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