From 949984beed26b2536d2f24d3d63e8a06e4dfa089 Mon Sep 17 00:00:00 2001 From: Michael Widenius Date: Sat, 3 Sep 2011 11:50:56 +0300 Subject: Fixed lp:828514 "Assertion `! is_set()' failed in Diagnostics_area::set_ok_status with derived table + subquery + concurrent DML" sql/item_subselect.cc: Added check of error condtions (safety) sql/sql_join_cache.cc: Added DBUG to some functions. Added error checking for calls to check_match(); This fixed the bug. sql/sql_select.cc: Moved variable assignment to be close to where it's used (cleanup) --- sql/sql_join_cache.cc | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) (limited to 'sql/sql_join_cache.cc') diff --git a/sql/sql_join_cache.cc b/sql/sql_join_cache.cc index f292375ee6f..c12d7dc5999 100644 --- a/sql/sql_join_cache.cc +++ b/sql/sql_join_cache.cc @@ -2021,6 +2021,7 @@ enum_nested_loop_state JOIN_CACHE::join_records(bool skip_last) JOIN_TAB *tab; enum_nested_loop_state rc= NESTED_LOOP_OK; bool outer_join_first_inner= join_tab->is_first_inner_for_outer_join(); + DBUG_ENTER("JOIN_CACHE::join_records"); if (outer_join_first_inner && !join_tab->first_unmatched) join_tab->not_null_compl= TRUE; @@ -2102,7 +2103,8 @@ enum_nested_loop_state JOIN_CACHE::join_records(bool skip_last) finish: restore_last_record(); reset(TRUE); - return rc; + DBUG_PRINT("exit", ("rc: %d", rc)); + DBUG_RETURN(rc); } @@ -2164,10 +2166,11 @@ enum_nested_loop_state JOIN_CACHE::join_matching_records(bool skip_last) join_tab->table->null_row= 0; bool check_only_first_match= join_tab->check_only_first_match(); bool outer_join_first_inner= join_tab->is_first_inner_for_outer_join(); + DBUG_ENTER("JOIN_CACHE::join_matching_records"); /* Return at once if there are no records in the join buffer */ if (!records) - return NESTED_LOOP_OK; + DBUG_RETURN(NESTED_LOOP_OK); /* When joining we read records from the join buffer back into record buffers. @@ -2241,7 +2244,7 @@ finish: rc= error < 0 ? NESTED_LOOP_NO_MORE_ROWS: NESTED_LOOP_ERROR; finish2: join_tab_scan->close(); - return rc; + DBUG_RETURN(rc); } @@ -2323,6 +2326,7 @@ bool JOIN_CACHE::set_match_flag_if_none(JOIN_TAB *first_inner, enum_nested_loop_state JOIN_CACHE::generate_full_extensions(uchar *rec_ptr) { enum_nested_loop_state rc= NESTED_LOOP_OK; + DBUG_ENTER("JOIN_CACHE::generate_full_extensions"); /* Check whether the extended partial join record meets @@ -2340,16 +2344,18 @@ enum_nested_loop_state JOIN_CACHE::generate_full_extensions(uchar *rec_ptr) if (rc != NESTED_LOOP_OK && rc != NESTED_LOOP_NO_MORE_ROWS) { reset(TRUE); - return rc; + DBUG_RETURN(rc); } } if (res == -1) { rc= NESTED_LOOP_ERROR; - return rc; + DBUG_RETURN(rc); } } - return rc; + else if (join->thd->is_error()) + rc= NESTED_LOOP_ERROR; + DBUG_RETURN(rc); } @@ -2374,16 +2380,20 @@ enum_nested_loop_state JOIN_CACHE::generate_full_extensions(uchar *rec_ptr) RETURN VALUE TRUE there is a match FALSE there is no match + In this case the caller must also check thd->is_error() to see + if there was a fatal error for the query. */ inline bool JOIN_CACHE::check_match(uchar *rec_ptr) { /* Check whether pushdown conditions are satisfied */ + DBUG_ENTER("JOIN_CACHE:check_match"); + if (join_tab->select && join_tab->select->skip_record(join->thd) <= 0) - return FALSE; + DBUG_RETURN(FALSE); if (!join_tab->is_last_inner_table()) - return TRUE; + DBUG_RETURN(TRUE); /* This is the last inner table of an outer join, @@ -2396,7 +2406,7 @@ inline bool JOIN_CACHE::check_match(uchar *rec_ptr) set_match_flag_if_none(first_inner, rec_ptr); if (first_inner->check_only_first_match() && !join_tab->first_inner) - return TRUE; + DBUG_RETURN(TRUE); /* This is the first match for the outer table row. The function set_match_flag_if_none has turned the flag @@ -2410,13 +2420,12 @@ inline bool JOIN_CACHE::check_match(uchar *rec_ptr) for (JOIN_TAB *tab= first_inner; tab <= join_tab; tab++) { if (tab->select && tab->select->skip_record(join->thd) <= 0) - return FALSE; + DBUG_RETURN(FALSE); } } while ((first_inner= first_inner->first_upper) && first_inner->last_inner == join_tab); - - return TRUE; + DBUG_RETURN(TRUE); } @@ -2451,10 +2460,11 @@ enum_nested_loop_state JOIN_CACHE::join_null_complements(bool skip_last) ulonglong cnt; enum_nested_loop_state rc= NESTED_LOOP_OK; bool is_first_inner= join_tab == join_tab->first_unmatched; + DBUG_ENTER("JOIN_CACHE::join_null_complements"); /* Return at once if there are no records in the join buffer */ if (!records) - return NESTED_LOOP_OK; + DBUG_RETURN(NESTED_LOOP_OK); cnt= records - (is_key_access() ? 0 : test(skip_last)); @@ -2484,7 +2494,7 @@ enum_nested_loop_state JOIN_CACHE::join_null_complements(bool skip_last) } finish: - return rc; + DBUG_RETURN(rc); } -- cgit v1.2.1 From 5673aa41c3e0fe909689cea70b1ca46f10c38831 Mon Sep 17 00:00:00 2001 From: Sergey Petrunya Date: Thu, 8 Sep 2011 19:48:14 +0400 Subject: BUG#830993: Crash in end_read_record with derived table - Let join buffering code correctly take into account rowids needed by DuplicateElimination when it is calculating minimum record sizes. - In JOIN_CACHE::write_record_data, added asserts that prevent us from writing beyond the end of the buffer. --- sql/sql_join_cache.cc | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) (limited to 'sql/sql_join_cache.cc') diff --git a/sql/sql_join_cache.cc b/sql/sql_join_cache.cc index f292375ee6f..bbc7e5cd26c 100644 --- a/sql/sql_join_cache.cc +++ b/sql/sql_join_cache.cc @@ -225,8 +225,6 @@ void JOIN_CACHE::calc_record_fields() flag_fields+= test(tab->table->maybe_null); fields+= tab->used_fields; blobs+= tab->used_blobs; - - fields+= tab->check_rowid_field(); } if ((with_match_flag= join_tab->use_match_flag())) flag_fields++; @@ -620,7 +618,12 @@ void JOIN_CACHE::create_remaining_fields() copy->type= CACHE_ROWID; copy->field= 0; copy->referenced_field_no= 0; - length+= copy->length; + /* + Note: this may seem odd, but at this point we have + table->file->ref==NULL while table->file->ref_length is already set + to correct value. + */ + length += table->file->ref_length; data_field_count++; copy++; } @@ -1297,6 +1300,7 @@ uint JOIN_CACHE::write_record_data(uchar * link, bool *is_full) if (with_length) { rec_len_ptr= cp; + DBUG_ASSERT(cp + size_of_rec_len <= buff + buff_size); cp+= size_of_rec_len; } @@ -1306,6 +1310,7 @@ uint JOIN_CACHE::write_record_data(uchar * link, bool *is_full) */ if (prev_cache) { + DBUG_ASSERT(cp + prev_cache->get_size_of_rec_offset() <= buff + buff_size); cp+= prev_cache->get_size_of_rec_offset(); prev_cache->store_rec_ref(cp, link); } @@ -1322,6 +1327,7 @@ uint JOIN_CACHE::write_record_data(uchar * link, bool *is_full) flags_pos= cp; for ( ; copy < copy_end; copy++) { + DBUG_ASSERT(cp + copy->length <= buff + buff_size); memcpy(cp, copy->str, copy->length); cp+= copy->length; } @@ -1348,6 +1354,7 @@ uint JOIN_CACHE::write_record_data(uchar * link, bool *is_full) { last_rec_blob_data_is_in_rec_buff= 1; /* Put down the length of the blob and the pointer to the data */ + DBUG_ASSERT(cp + copy->length + sizeof(char*) <= buff + buff_size); blob_field->get_image(cp, copy->length+sizeof(char*), blob_field->charset()); cp+= copy->length+sizeof(char*); @@ -1357,6 +1364,7 @@ uint JOIN_CACHE::write_record_data(uchar * link, bool *is_full) /* First put down the length of the blob and then copy the data */ blob_field->get_image(cp, copy->length, blob_field->charset()); + DBUG_ASSERT(cp + copy->length + copy->blob_length <= buff + buff_size); memcpy(cp+copy->length, copy->str, copy->blob_length); cp+= copy->length+copy->blob_length; } @@ -1367,12 +1375,14 @@ uint JOIN_CACHE::write_record_data(uchar * link, bool *is_full) case CACHE_VARSTR1: /* Copy the significant part of the short varstring field */ len= (uint) copy->str[0] + 1; + DBUG_ASSERT(cp + len <= buff + buff_size); memcpy(cp, copy->str, len); cp+= len; break; case CACHE_VARSTR2: /* Copy the significant part of the long varstring field */ len= uint2korr(copy->str) + 2; + DBUG_ASSERT(cp + len <= buff + buff_size); memcpy(cp, copy->str, len); cp+= len; break; @@ -1387,6 +1397,7 @@ uint JOIN_CACHE::write_record_data(uchar * link, bool *is_full) end > str && end[-1] == ' '; end--) ; len=(uint) (end-str); + DBUG_ASSERT(cp + len + 2 <= buff + buff_size); int2store(cp, len); memcpy(cp+2, str, len); cp+= len+2; @@ -1406,6 +1417,7 @@ uint JOIN_CACHE::write_record_data(uchar * link, bool *is_full) /* fall through */ default: /* Copy the entire image of the field from the record buffer */ + DBUG_ASSERT(cp + copy->length <= buff + buff_size); memcpy(cp, copy->str, copy->length); cp+= copy->length; } @@ -1425,6 +1437,7 @@ uint JOIN_CACHE::write_record_data(uchar * link, bool *is_full) cnt++; } } + DBUG_ASSERT(cp + size_of_fld_ofs*cnt <= buff + buff_size); cp+= size_of_fld_ofs*cnt; } -- cgit v1.2.1 From a8f7c03c1e46dd556aa57d45bbd078101c68ec9e Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Tue, 25 Oct 2011 14:18:19 -0700 Subject: Fixed LP bug #881318. If a materialized derived table / view is empty then for this table the value of file->ref is 0. This was not taken into account by the function JOIN_CACHE::write_record_data. As a result a query using an empty materialized derived tables as inner tables of outer joins and IN subqueries in WHERE conditions could cause server crashes when the optimizer employed join caches and duplicate elimination for semi-joins. --- sql/sql_join_cache.cc | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) (limited to 'sql/sql_join_cache.cc') diff --git a/sql/sql_join_cache.cc b/sql/sql_join_cache.cc index c8933fe69ee..f53bf738b86 100644 --- a/sql/sql_join_cache.cc +++ b/sql/sql_join_cache.cc @@ -1413,12 +1413,22 @@ uint JOIN_CACHE::write_record_data(uchar * link, bool *is_full) TABLE *table= (TABLE *) copy->str; copy->str= table->file->ref; copy->length= table->file->ref_length; + if (!copy->str) + { + /* + If table is an empty inner table of an outer join and it is + a materialized derived table then table->file->ref == NULL. + */ + cp+= copy->length; + break; + } } /* fall through */ default: /* Copy the entire image of the field from the record buffer */ DBUG_ASSERT(cp + copy->length <= buff + buff_size); - memcpy(cp, copy->str, copy->length); + if (copy->str) + memcpy(cp, copy->str, copy->length); cp+= copy->length; } } @@ -1811,6 +1821,13 @@ uint JOIN_CACHE::read_record_field(CACHE_FIELD *copy, bool blob_in_rec_buff) memset(copy->str+len, ' ', copy->length-len); len+= 2; break; + case CACHE_ROWID: + if (!copy->str) + { + len= copy->length; + break; + } + /* fall through */ default: /* Copy the entire image of the field from the record buffer */ len= copy->length; -- cgit v1.2.1