MDEV-39916: Crash with having filter when being pushed down

[bsrikanth-mariadb]

MDEV-39916: Crash when pushing "NOT col" from HAVING into WHERE

"NOT column" rewrites itself into "column = 0" in fix_fields().
However the rewrite is not done when the "column" is wrapped in an Item_ref.

Later, pushdown_from_having_into_where() will strip the Item_ref object
and will call
Item_func_not(Item_field())->fix_fields(thd, ref=NULL);
This call will attempt the rewrite and crash when writing into NULL pointer.

Fixed this in two places:

• Make Item_func_not::fix_fields() use real_item() and so recognize fields
  wrapped in Item_ref-like objects.

• Make pushdown_from_having_into_where() pass the right pointer as
  fix_fields() argument.

[gemini-code-assist]

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

[spetrunia]

Review input from the optimizer call: We should avoid making calls to item->fix_fields(..., NULL) unless we definitely know that item is the kind of item that will not try to substitute itself (which is not the case here).

[spetrunia]

Please also add this change as a separate commit:

--- a/sql/sql_lex.h
+++ b/sql/sql_lex.h
@@ -1181,7 +1181,13 @@ class st_select_lex: public st_select_lex_node
 
   /* List of references to fields referenced from inner selects */
   List<Item_outer_ref> inner_refs_list;
+
+  /*
+    Pushdown from HAVING into WHERE optimization: conditions from the HAVING
+    clause that should be added into the WHERE.
+  */
   List<Item> attach_to_conds;
+
   /* Saved values of the WHERE and HAVING clauses*/
   Item::cond_result cond_value, having_value;
   /* 

[spetrunia]

An experimental diff to side-step the problem:
allow Item_func_not(X) rewrite itself to "X=0" even when X is an Item_ref.

For some reason, Item_ref->ref points to the Item_ref itself before it is fixed so we need to call fix_fields() for it to be able to use item_ref->real_item().

--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -6829,7 +6829,11 @@ Item *Item_func_not::neg_transformer(THD *thd)   /* NOT(x)  ->  x */
 bool Item_func_not::fix_fields(THD *thd, Item **ref)
 {
   args[0]->under_not(this);
-  if (args[0]->type() == FIELD_ITEM)
+  // psergey:
+  if (args[0]->fix_fields(thd, &args[0]))
+    return true;
+ 
+  if (args[0]->real_item()->type() == FIELD_ITEM)
   {
     /* replace  "NOT <field>" with "<field> == 0" */
     Query_arena backup, *arena;

[spetrunia]

use fix_fields_if_needed() as Claude has suggested.

[spetrunia]

Commit comment suggestion.

MDEV-39916: Crash when pushing "NOT col" from HAVING into WHERE

"NOT column" rewrites itself into "column = 0" in fix_fields().
However the rewrite is not done when the "column" is wrapped in an Item_ref.

Later, pushdown_from_having_into_where() will strip the Item_ref object 
and will call
  Item_func_not(Item_field())->fix_fields(thd, ref=NULL);
This call will attempt the rewrite and crash when writing into NULL pointer.

Fixed this in two places:
* Make Ittem_func_not::fix_fields() use real_item() and so recognize fields 
  wrapped in Item_ref-like objects.

* Make pushdown_from_having_into_where() pass the right pointer as 
  fix_fields() argument.

[spetrunia]

• Please fix the typo in the commit comment : Ittem_func_not
• In the testcase , use the new MDEV description:

MDEV-39916: Crash when pushing "NOT col" from HAVING into WHERE

[spetrunia]

Ok to push after the above is addressed.