Varun Gupta (Inactive) added a comment - 2021-01-13 16:05 Patch http://lists.askmonty.org/pipermail/commits/2021-January/014439.html

Igor Babaev (Inactive) added a comment - 2021-01-15 06:44 It looks like that Item_cache_row::bring_value() has lines that can be removed without causing any problem: --- a/sql/item.cc +++ b/sql/item.cc @@ -10298,8 +10298,6 @@ void Item_cache_row::bring_value() return; example->bring_value(); null_value= example->null_value; - for (uint i= 0; i < item_count; i++) - values[i]->bring_value(); } They became unneeded after the following commit: commit da7646b64c7e76930e05c0235c724872416ba1dc Author: Alexey Kopytov <Alexey.Kopytov@Sun.com> Thu Sep 9 07:44:53 2010 Committer: Alexey Kopytov <Alexey.Kopytov@Sun.com> Thu Sep 9 07:44:53 2010

Igor Babaev (Inactive) added a comment - 2021-01-15 06:54 Interesting that the following patch also fixes the problem: --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -1308,8 +1308,18 @@ bool Item_singlerow_subselect::null_inside() void Item_singlerow_subselect::bring_value() { + if (!unit->uncacheable && engine->is_executed()) + return; if (!exec() && assigned()) + { null_value= 0; + if (!unit->uncacheable) + { + null_value= 1; + for (uint i= 0; i < max_columns ;i++) + null_value&= row[i]->null_value; + } + } else reset(); } This patch does not call exec() for a cacheable subquery if it has been already executed. It looks like we still have some inconsistency in how we set null_value flags for Item_row objects and Item_cache_row objects

Varun Gupta (Inactive) added a comment - 2021-01-19 12:49 Made a couple of patch With the help of monty . Patch1: https://github.com/MariaDB/server/commit/d6782fcded7cfe5f561359124153d5deaba28ec9 Patch2: https://github.com/MariaDB/server/commit/a87320e335b6345f1ce04eea8b790a741c1ec23e

Igor Babaev (Inactive) added a comment - 2021-01-23 01:43 I continued investigating the problem and came to the conclusion that it is a result of different interpretations of the null_value flag for row values. Some code assumes that the flag null_value for a row is set to true if every component of the row has null_value flag set to true (see, for example, the code that compares two row values). In other pieces of code the flag is set to true if there is a component with its null_value flag set to true: bool Item_cache_row::cache_value() { if (!example) return FALSE; value_cached= TRUE; null_value= 0; example->bring_value(); for (uint i= 0; i < item_count; i++) { values[i]->cache_value(); null_value|= values[i]->null_value; } return TRUE; } Another function that uses such interpretation of the null_value flag in row items is Item_in_optimizer::val_int(). In the following patch I tried to make sure that we use only the first interpretation of the null_value flag in row item (it seems to me that was the original one). diff --git a/sql/item.cc b/sql/item.cc index e633964..36b7a3e 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -10218,12 +10218,13 @@ bool Item_cache_row::cache_value() if (!example) return FALSE; value_cached= TRUE; - null_value= 0; example->bring_value(); + null_value= 1; for (uint i= 0; i < item_count; i++) { + values[i]->bring_value(); values[i]->cache_value(); - null_value|= values[i]->null_value; + null_value&= values[i]->null_value; } return TRUE; } diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index d16c741..ecaeb39 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -1596,7 +1596,7 @@ longlong Item_in_optimizer::val_int() DBUG_RETURN(res); } - if (cache->null_value) + if (cache->null_value || cache->null_inside()) { DBUG_PRINT("info", ("Left NULL...")); /* diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index 802bfca..56b3b1b 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -831,7 +831,8 @@ bool Item_subselect::expr_cache_is_needed(THD *thd) inline bool Item_in_subselect::left_expr_has_null() { - return (*(optimizer->get_cache()))->null_value; + return (*(optimizer->get_cache()))->null_value || + (*(optimizer->get_cache()))->null_inside(); } @@ -1299,7 +1300,17 @@ bool Item_singlerow_subselect::null_inside() void Item_singlerow_subselect::bring_value() { if (!exec() && assigned()) - null_value= 0; + { + null_value= 1; + for (uint i= 0; i < max_columns ; i++) + { + if (!row[i]->null_value) + { + null_value= 0; + return; + } + } + } else reset(); } All tests passed with this patch (as well as the reported test case of course). Yet I'm not sure that I covered all pieces of code where the interpretations of the null_value flag in rows differed.