maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexander!

The approach is fine. Two comments below:

> The remaining heavy piece of the code is:
> 
>   Item* propagate_equal_fields(THD *thd, const Context &ctx, COND_EQUAL *cond)
>   {
>     Context cmpctx(ANY_SUBST, cmp.compare_type(), cmp.compare_collation());
>     args[0]->propagate_equal_fields_and_change_item_tree(thd, cmpctx,
>                                                          cond, &args[0]);
>     args[1]->propagate_equal_fields_and_change_item_tree(thd, cmpctx,
>                                                          cond, &args[1]);
>     args[2]->propagate_equal_fields_and_change_item_tree(thd,
>                                                          Context_identity(),
>                                                          cond, &args[2]);
>     return this;
>   }
> 
> We cannot use the same approach here, because args[0] and args[2]
> are propagated with a different context (ANY_SUBST vs IDENTITY_SUBST).

An idea:

  Item *old= args[0];
  args[0]->propagate_equal_fields_and_change_item_tree(thd, cmpctx,
                                                       cond, &args[0]);
  if (args[0] != old)
      args[2]->propagate_equal_fields_and_change_item_tree(thd, Context_identity(),
                                                           cond, &args[2]);

First substitution uses more relaxed rules, so, supposedly, if it did
not substitute anything, than a stricter IDENTITY_SUBST ssubstitution
won't substitute anything either.

> @@ -2513,7 +2530,14 @@ void Item_func_nullif::update_used_tables()
>    }
>    else
>    {
> -    Item_func::update_used_tables();
> +    /*
> +      No needs to iterate through args[2] when it's just a copy of args[0].
> +      See MDEV-9712 Performance degradation of nested NULLIF
> +    */
> +    DBUG_ASSERT(arg_count == 3);
> +    used_tables_and_const_cache_init();
> +    used_tables_and_const_cache_update_and_join(args[0] == args[2] ? 2 : 3,
> +                                                args);

here, you can unconditionally run

  used_tables_and_const_cache_update_and_join(2, args);

because even if args[2] differs from args[0], it cannot use more
tables, so for the purpose of this method, it's redundant.

>    }
>  }

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx