maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

On Sun, Jul 22, 2018 at 03:56:37PM +0530, Varun wrote:
> revision-id: d277c4682e4c6bb0e708296e6ae520c149dd78f4 (mariadb-5.5.60-47-gd277c4682e4)
> parent(s): 9cea4ccf12cb6e8746b9b440d9c62408a9ef04af
> author: Varun Gupta
> committer: Varun Gupta
> timestamp: 2018-07-22 15:56:25 +0530
> message:
> 
> MDEV-16751: Server crashes in st_join_table::cleanup or TABLE_LIST::is_with_table_recursive_reference
>             with join_cache_level>2
> 
> During muliple equality propagation for a query in which we have an IN subquery, the items in the select list of the
> subquery may not be part of the multiple equality because there might be another occurence of the same field in the
> where clause of the subquery.
> So we keyuse_is_valid_for_access_in_chosen_plan function which expects the items in the select list of the subquery to
> be same to the ones in the multiple equality (through these multiple equalities we create keyuse array).
> The solution would be that we expect the same field not the same Item because when we have SEMI JOIN MATERIALIZATION SCAN,
> we use copy back technique to copies back the materialised table fields to the original fields of the base tables.
> 
> ---
>  mysql-test/r/subselect_mat.result    | 35 +++++++++++++++++++++++++++++++++++
>  mysql-test/r/subselect_sj_mat.result | 35 +++++++++++++++++++++++++++++++++++
>  mysql-test/t/subselect_sj_mat.test   | 25 +++++++++++++++++++++++++
>  sql/sql_select.cc                    |  4 +++-
>  4 files changed, 98 insertions(+), 1 deletion(-)
...
> diff --git a/sql/sql_select.cc b/sql/sql_select.cc
> index 6a64a0e9952..af6f63a55f1 100644
> --- a/sql/sql_select.cc
> +++ b/sql/sql_select.cc
> @@ -7438,7 +7438,9 @@ bool JOIN_TAB::keyuse_is_valid_for_access_in_chosen_plan(JOIN *join,
>    st_select_lex *sjm_sel= emb_sj_nest->sj_subq_pred->unit->first_select(); 
>    for (uint i= 0; i < sjm_sel->item_list.elements; i++)
>    {
> -    if (sjm_sel->ref_pointer_array[i] == keyuse->val)
> +    DBUG_ASSERT(keyuse->val->type() == Item::FIELD_ITEM);
^^(1)
> +    Field *field = ((Item_field*)sjm_sel->ref_pointer_array[i])->field;
^^(2)
> +    if (field->eq(((Item_field*)keyuse->val)->field))
>        return true;
>    }
>    return false; 

As for typecast (2):
As far as I understand, it is valid because of the following:
- We get to this point in this function only when
  Semi-Join-Materialization-Scan is used.
- SJM-Scan is only used when the subquery's select list has Item_field objects
  (This is checked in subquery_types_allow_materialization()).

* Please add a comment in subquery_types_allow_materialization() about this
  function relying on that logic.
* Please add here an assert that  sjm_sel->ref_pointer_array[i] is an
  Item_field. The logic is complex and can be potentially broken.


As for typecast (1):
What if keyuse->val is not an Item_field?  If we get to this point in this 
function, it means it's an expression that only depends on the SJ-Inner table. 

This cannot just happen due to limitations in
subquery_types_allow_materialization(), and I wasn't able to construct an
example where it would happen (tried playing with multiple-equalities, but 
substitute_for_best_equal_item is called after get_best_combination(), 
tried playing with constant tables but then SJ-Materialization is not used,
etc. )

I think we will be safer by changing the assert to be this check:

  if (keyuse->val->type() == Item::FIELD_ITEM)
  {
    ... everything else
  }

The idea is that a KEYUSE that satisfies the requirement will still be
available, and we just want to filter out any extra KEYUSE that might pop up
due to some optimization.

what do you think?


Ok to push after the above is addressed.

BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog