[MDEV-26402] A SEGV in Item_field::used_tables/update_depend_map_for_order or Assertion `fixed == 1' Created: 2021-08-19 Updated: 2023-10-05 Resolved: 2022-04-22 |
|
| Status: | Closed |
| Project: | MariaDB Server |
| Component/s: | Optimizer |
| Affects Version/s: | 10.4, 10.5, 10.6, 10.7 |
| Fix Version/s: | 10.4.25, 10.5.16, 10.6.8, 10.7.4, 10.8.3, 10.9.1 |
| Type: | Bug | Priority: | Blocker |
| Reporter: | Zhiyong Wu | Assignee: | Sergei Petrunia |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | regression | ||
| Environment: |
Linux version 5.13.0-1-MANJARO (builduser@LEGION) (gcc (GCC) 11.1.0, GNU ld (GNU Binutils) 2.36.1) #1 SMP PREEMPT Mon Jun 7 06:16:10 UTC 2021 x86_64 |
||
| Issue Links: |
|
||||||||||||||||||||||||||||||||
| Description |
|
PoC:
Log:
Coredump
|
| Comments |
| Comment by Alice Sherepa [ 2021-08-25 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Thank you! I repeated as described on 10.4-10.6, no crash on 10.2-10.3:
on 10.6 assertion was renamed : 10.6/src/sql/item_cmpfunc.cc:5509: virtual longlong Item_func_isnull::val_int(): Assertion `fixed()' failed. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Sergei Petrunia [ 2022-04-21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Debugging this example: SELECT * FROM t1 GROUP BY i HAVING i IN ( i IS NULL AND 'x' = 0); The WHERE clause is:
Item_func_isnull becomes un-fixed here:
And then we hit an assert when trying to compute the value of this item. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Sergei Petrunia [ 2022-04-21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Debugging st_select_lex::pushdown_from_having_into_where, I can see that we're trying to move the entire HAVING clause into the WHERE. This seems to be correct. The problem occurs in this call at the very end:
The item here is:
(ignore the "always not null" comment, it is incorrect. the "1" actually represents "t1.i" there) The issue is that walk(Item::cleanup_excluding_immutables_processor) call has left Item_cond_and in a state where the Item_cond_and itself is fixed while its members are not:
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Sergei Petrunia [ 2022-04-21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Look at this code:
this item:
has marker=IMMUTABLE_FL (and so is not cleaned up). Its child items have either marker=0 or marker=-1, and so are cleaned up. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Sergei Petrunia [ 2022-04-21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Analysis-1What is the point of Item::cleanup_excluding_immutables_processor, anyway? I think the logic was as follows: Pushdown from HAVING into WHERE requires one to change post-grouping-context Items into pre-grouping-context items. Because of this, each Item object from the original HAVING expression must be either in the WHERE or in the HAVING. It cannot be in both (which is common in join condition pushdown done in make_join_select()). The exception to this are constant (aka immutable) Item objects. There can be a constant item object which is present in both pushed WHERE condition and the unpushed HAVING condition. Such object has a flag IMMUTABLE_FL. What the code does not take into account is that there can be Items which are constants BUT still have column references in them. In the testcase for this bug, the HAVING clause is
and the right-hand side (i IS NULL AND 'x' = 0) is a constant item which still has column references. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Sergei Petrunia [ 2022-04-21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Can we get rid of such complex-constant Items? I mean if I run
I can see that the value of the constant is cached:
This is done by this code:
Can we do the same in st_select_lex::pushdown_from_having_into_where to get all complex constants replaced with their values? A: no, it doesn't seem to work for Item_equal object. When I added this code, the compilation didn't do anything. (Attempting to get it to compile seems like a hard problem: suppose a multiple equality member, an Item_field get compiled to something that isn't an Item_field? Also, multiple equality code has Item-pointer spaghetti architecture so it's hard to add any local changes. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Sergei Petrunia [ 2022-04-21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Another approach: if we are marking an item with IMMUTABLE_FL, mark the entire item [sub]tree with this. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Sergei Petrunia [ 2022-04-21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
https://github.com/MariaDB/server/commit/ba1c6577a36b078a41289f6259880ce77fe28711 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Sergei Petrunia [ 2022-04-21 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
sanja, please review. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Oleksandr Byelkin [ 2022-04-22 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
OK to push |