Skip to content

Commit

Permalink
Bug#35201901: Server failure of MySQL #3
Browse files Browse the repository at this point in the history
Two problems were identified for this bug. The first is seen by looking
at the reduced query:

select subq_1.c1 as c1
from (select subq_0.c0 as c0,
             subq_0.c0 as c1,
             90 as c2,
             subq_0.c1 as c3
      from (select (select v2 from table1) as c0,
                   ref_0.v4 as c1
            from table1 as ref_0
           ) as subq_0
      ) as subq_1
where EXISTS (select subq_1.c0 as c2,
                     case
                     when EXISTS (select (select v0 from table1) as c1
                                          from table1 as ref_8
                                          where EXISTS (select subq_1.c2 as c7
                                                        from table1 as ref_9
                                                       )
                                         )
                     then subq_1.c3
                     end as c5
              from table1 as ref_7);

In the innermost EXISTS predicate, a column subq_1.c2 is looked up.
It is erroneously found as the column subq_1.c0 with alias c2 in the
query block of the outermost EXISTS predicate. But this resolving is not
according to SQL standard: A table name cannot be part of a column alias,
it has to be a simple identifier, and any referencing column must also
be a simple identifier. By changing item_ref->item_name to
item_ref->field_name in a test in find_item_in_list, we ensure that the
match is against a table (view) name and column name and not an alias.

But there is also another problem. The EXISTS predicate contains a few
selected columns that are resolved and then immediately deleted since
they are redundant in EXISTS processing. But if these columns are
outer references and defined in a derived table, we may actually
de-reference them before the initial reference increment. Thus, those
columns are removed before they are possibly used later. This happens
to subq_1.c2 which is resolved in the outer-most query block and
coming from a derived table. We prevent this problem by incrementing
the reference count of selected expressions from derived tables earlier,
and we try to prevent this problem from re-occuring by adding an
"m_abandoned" field in class Item, which is set to true when the
reference count is decremented to zero and prevents the reference count
from ever be incremented after that.

Change-Id: Idda48ae726a580c1abdc000371b49a753e197bc6
  • Loading branch information
roylyseng committed Apr 25, 2023
1 parent 2d44cc3 commit 98cd109
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 8 deletions.
9 changes: 7 additions & 2 deletions sql/item.h
Original file line number Diff line number Diff line change
Expand Up @@ -3210,12 +3210,16 @@ class Item : public Parse_tree_node {
bool is_blob_field() const;

/// Increment reference count
void increment_ref_count() { ++m_ref_count; }
void increment_ref_count() {
assert(!m_abandoned);
++m_ref_count;
}

/// Decrement reference count
uint decrement_ref_count() {
assert(m_ref_count > 0);
return --m_ref_count;
if (--m_ref_count == 0) m_abandoned = true;
return m_ref_count;
}

protected:
Expand Down Expand Up @@ -3435,6 +3439,7 @@ class Item : public Parse_tree_node {
be unused and can be removed.
*/
uint m_ref_count{0};
bool m_abandoned{false}; ///< true if item has been fully de-referenced
const bool is_parser_item; ///< true if allocated directly by parser
int8 is_expensive_cache; ///< Cache of result of is_expensive()
uint8 m_data_type; ///< Data type assigned to Item
Expand Down
8 changes: 7 additions & 1 deletion sql/sql_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8268,7 +8268,8 @@ bool find_item_in_list(THD *thd, Item *find, mem_root_deque<Item *> *items,
Item_field for tables.
*/
Item_ident *item_ref = down_cast<Item_ident *>(item);
if (item_ref->item_name.eq_safe(find_ident->field_name) &&
if (!my_strcasecmp(system_charset_info, item_ref->field_name,
find_ident->field_name) &&
item_ref->table_name != nullptr &&
!my_strcasecmp(table_alias_charset, item_ref->table_name,
find_ident->table_name) &&
Expand Down Expand Up @@ -9061,6 +9062,11 @@ bool setup_fields(THD *thd, ulong want_privilege, bool allow_sum_func,
if (!ref.is_null()) {
ref[0] = item;
ref.pop_front();
/*
Items present in ref_array have a positive reference count since
removal of unused columns from derived tables depends on this.
*/
item->increment_ref_count();
}
Item *typed_item = nullptr;
if (typed_items != nullptr && typed_it != typed_items->end()) {
Expand Down
5 changes: 0 additions & 5 deletions sql/sql_resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -281,11 +281,6 @@ bool Query_block::prepare(THD *thd, mem_root_deque<Item *> *insert_field_list) {
insert_field_list, &fields, base_ref_items))
return true;

// Ensure that all selected expressions have a positive reference count
for (auto it : fields) {
it->increment_ref_count();
}

resolve_place = RESOLVE_NONE;

const nesting_map save_allow_sum_func = thd->lex->allow_sum_func;
Expand Down

0 comments on commit 98cd109

Please sign in to comment.