database/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2025-07-29 05:21:33 +03:00
Code Activity

MDEV-6768 Wrong result with aggregate with join with no result set

Browse Source 
When a query does implicit grouping and join operation produces an empty
result set, a NULL-complemented row combination is generated.
However, constant table fields still show non-NULL values.

What happens in the is that end_send_group() is called with a
const row but without any rows matching the WHERE clause.
This last part is shown by 'join->first_record' not being set.

This causes item->no_rows_in_result() to be called for all items to reset
all sum functions to their initial state. However fields are not set
to NULL.

The used fix is to produce NULL-complemented records for constant tables
as well. Also, reset the constant table's records back in case we're
in a subquery which may get re-executed.
An alternative fix would have item->no_rows_in_result() also work
with Item_field objects.

There is some other issues with the code:
- join->no_rows_in_result_called is used but never set.
- Tables that are used with group functions are not properly marked as
  maybe_null, which is required if the table rows should be regarded as
  null-complemented (not existing).
- The code that tries to detect if mixed_implicit_grouping should be set
  didn't take into account all usage of fields and sum functions.
- Item_func::restore_to_before_no_rows_in_result() called the wrong
  function.
- join->clear() does not use a table_map argument to clear_tables(),
  which caused it to ignore constant tables.
- unclear_tables() does not correctly restore status to what is
  was before clear_tables().

Main bug fix was to always use a table_map argument to clear_tables() and
always use join->clear() and clear_tables() together with unclear_tables().

Other fixes:
- Fixed Item_func::restore_to_before_no_rows_in_result()
- Set 'join->no_rows_in_result_called' when no_rows_in_result_set()
  is called.
- Removed not used argument from setup_end_select_func().
- More code comments
- Ensure that end_send_group() modifies the same fields as are in the
  result set.
- Changed return_zero_rows() to use pointers instead of references,
  similar to the rest of the code.

Reviewer: Sergei Petrunia <sergey@mariadb.com>
This commit is contained in:
Monty
2023-03-31 19:35:04 +03:00
parent c0adb05b30
commit 16258677b3
8 changed files with 397 additions and 107 deletions
Download Patch File Download Diff File Expand all files Collapse all files

264
sql/sql_select.cc
View File

@ -141,10 +141,10 @@ static void update_depend_map_for_order(JOIN *join, ORDER *order);
static ORDER *remove_const(JOIN *join,ORDER *first_order,COND *cond,
bool change_list, bool *simple_order);
static int return_zero_rows(JOIN *join, select_result *res,
List<TABLE_LIST> &tables,
List<Item> &fields, bool send_row,
List<TABLE_LIST> *tables,
List<Item> *fields, bool send_row,
ulonglong select_options, const char *info,
Item *having, List<Item> &all_fields);
Item *having, List<Item> *all_fields);
static COND *build_equal_items(JOIN *join, COND *cond,
COND_EQUAL *inherited,
List<TABLE_LIST> *join_list,
@ -1165,11 +1165,40 @@ int SELECT_LEX::vers_setup_conds(THD *thd, TABLE_LIST *tables)
DBUG_RETURN(0);
}
/*****************************************************************************
Check fields, find best join, do the select and output fields.
mysql_select assumes that all tables are already opened
*****************************************************************************/
/*
Check if we have a field reference. If yes, we have to use
mixed_implicit_grouping.
*/
static bool check_list_for_field(List<Item> *items)
{
List_iterator_fast <Item> select_it(*items);
Item *select_el;
while ((select_el= select_it++))
{
if (select_el->with_field)
return true;
}
return false;
}
static bool check_list_for_field(ORDER *order)
{
for (; order; order= order->next)
{
if (order->item[0]->with_field)
return true;
}
return false;
}
/**
Prepare of whole select (including sub queries in future).
@ -1248,53 +1277,45 @@ JOIN::prepare(TABLE_LIST *tables_init, COND *conds_init, uint og_num,
DBUG_RETURN(-1);
/*
TRUE if the SELECT list mixes elements with and without grouping,
and there is no GROUP BY clause. Mixing non-aggregated fields with
aggregate functions in the SELECT list is a MySQL extenstion that
is allowed only if the ONLY_FULL_GROUP_BY sql mode is not set.
mixed_implicit_grouping will be set to TRUE if the SELECT list
mixes elements with and without grouping, and there is no GROUP BY
clause.
Mixing non-aggregated fields with aggregate functions in the
SELECT list or HAVING is a MySQL extension that is allowed only if
the ONLY_FULL_GROUP_BY sql mode is not set.
*/
mixed_implicit_grouping= false;
if ((~thd->variables.sql_mode & MODE_ONLY_FULL_GROUP_BY) &&
select_lex->with_sum_func && !group_list)
{
List_iterator_fast <Item> select_it(fields_list);
Item *select_el; /* Element of the SELECT clause, can be an expression. */
bool found_field_elem= false;
bool found_sum_func_elem= false;
while ((select_el= select_it++))
if (check_list_for_field(&fields_list) ||
check_list_for_field(order))
{
if (select_el->with_sum_func())
found_sum_func_elem= true;
if (select_el->with_field)
found_field_elem= true;
if (found_sum_func_elem && found_field_elem)
TABLE_LIST *tbl;
List_iterator_fast<TABLE_LIST> li(select_lex->leaf_tables);
mixed_implicit_grouping= true; // mark for future
while ((tbl= li++))
{
mixed_implicit_grouping= true;
break;
/*
If the query uses implicit grouping where the select list
contains both aggregate functions and non-aggregate fields,
any non-aggregated field may produce a NULL value. Set all
fields of each table as nullable before semantic analysis to
take into account this change of nullability.
Note: this loop doesn't touch tables inside merged
semi-joins, because subquery-to-semijoin conversion has not
been done yet. This is intended.
*/
if (tbl->table)
tbl->table->maybe_null= 1;
}
}
}
table_count= select_lex->leaf_tables.elements;
TABLE_LIST *tbl;
List_iterator_fast<TABLE_LIST> li(select_lex->leaf_tables);
while ((tbl= li++))
{
/*
If the query uses implicit grouping where the select list contains both
aggregate functions and non-aggregate fields, any non-aggregated field
may produce a NULL value. Set all fields of each table as nullable before
semantic analysis to take into account this change of nullability.
Note: this loop doesn't touch tables inside merged semi-joins, because
subquery-to-semijoin conversion has not been done yet. This is intended.
*/
if (mixed_implicit_grouping && tbl->table)
tbl->table->maybe_null= 1;
}
uint real_og_num= og_num;
if (skip_order_by &&
select_lex != select_lex->master_unit()->global_parameters())
@ -3838,7 +3859,7 @@ bool JOIN::make_aggr_tables_info()
set_items_ref_array(items0);
if (join_tab)
join_tab[exec_join_tab_cnt() + aggr_tables - 1].next_select=
setup_end_select_func(this, NULL);
setup_end_select_func(this);
group= has_group_by;
DBUG_RETURN(false);
@ -4216,13 +4237,7 @@ JOIN::reinit()
}
}
/* Reset of sum functions */
if (sum_funcs)
{
Item_sum *func, **func_ptr= sum_funcs;
while ((func= *(func_ptr++)))
func->clear();
}
clear_sum_funcs();
if (no_rows_in_result_called)
{
@ -4507,12 +4522,12 @@ void JOIN::exec_inner()
}
else
{
(void) return_zero_rows(this, result, select_lex->leaf_tables,
*columns_list,
(void) return_zero_rows(this, result, &select_lex->leaf_tables,
columns_list,
send_row_on_empty_set(),
select_options,
zero_result_cause,
having ? having : tmp_having, all_fields);
having ? having : tmp_having, &all_fields);
DBUG_VOID_RETURN;
}
}
@ -14768,10 +14783,36 @@ ORDER *simple_remove_const(ORDER *order, COND *where)
}
/*
Set all fields in the table to have a null value
@param tables Table list
*/
static void make_tables_null_complemented(List<TABLE_LIST> *tables)
{
List_iterator<TABLE_LIST> ti(*tables);
TABLE_LIST *table;
while ((table= ti++))
{
/*
Don't touch semi-join materialization tables, as the a join_free()
call may have freed them (and HAVING clause can't have references to
them anyway).
*/
if (!table->is_jtbm())
{
TABLE *tbl= table->table;
mark_as_null_row(tbl); // Set fields to NULL
}
}
}
static int
return_zero_rows(JOIN *join, select_result *result, List<TABLE_LIST> &tables,
List<Item> &fields, bool send_row, ulonglong select_options,
const char *info, Item *having, List<Item> &all_fields)
return_zero_rows(JOIN *join, select_result *result, List<TABLE_LIST> *tables,
List<Item> *fields, bool send_row, ulonglong select_options,
const char *info, Item *having, List<Item> *all_fields)
{
DBUG_ENTER("return_zero_rows");
@ -14787,24 +14828,15 @@ return_zero_rows(JOIN *join, select_result *result, List<TABLE_LIST> &tables,
Set all tables to have NULL row. This is needed as we will be evaluating
HAVING condition.
*/
List_iterator<TABLE_LIST> ti(tables);
TABLE_LIST *table;
while ((table= ti++))
{
/*
Don't touch semi-join materialization tables, as the above join_free()
call has freed them (and HAVING clause can't have references to them
anyway).
*/
if (!table->is_jtbm())
mark_as_null_row(table->table); // All fields are NULL
}
List_iterator_fast<Item> it(all_fields);
make_tables_null_complemented(tables);
List_iterator_fast<Item> it(*all_fields);
Item *item;
/*
Inform all items (especially aggregating) to calculate HAVING correctly,
also we will need it for sending results.
*/
join->no_rows_in_result_called= 1;
while ((item= it++))
item->no_rows_in_result();
if (having && having->val_int() == 0)
@ -14818,12 +14850,12 @@ return_zero_rows(JOIN *join, select_result *result, List<TABLE_LIST> &tables,
join->thd->limit_found_rows= 0;
}
if (!(result->send_result_set_metadata(fields,
if (!(result->send_result_set_metadata(*fields,
Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF)))
{
bool send_error= FALSE;
if (send_row)
send_error= result->send_data_with_check(fields, join->unit, 0) > 0;
send_error= result->send_data_with_check(*fields, join->unit, 0) > 0;
if (likely(!send_error))
result->send_eof(); // Should be safe
}
@ -14839,49 +14871,42 @@ return_zero_rows(JOIN *join, select_result *result, List<TABLE_LIST> &tables,
}
/**
used only in JOIN::clear (always) and in do_select()
(if there where no matching rows)
Reset table rows to contain a null-complement row (all fields are null)
Used only in JOIN::clear() and in do_select() if there where no matching rows.
@param join JOIN
@param cleared_tables If not null, clear also const tables and mark all
cleared tables in the map. cleared_tables is only
set when called from do_select() when there is a
group function and there where no matching rows.
@param cleared_tables Used to mark all cleared tables in the map. Needed for
unclear_tables() to know which tables to restore to
their original state.
*/
static void clear_tables(JOIN *join, table_map *cleared_tables)
{
/*
must clear only the non-const tables as const tables are not re-calculated.
*/
DBUG_ASSERT(cleared_tables);
for (uint i= 0 ; i < join->table_count ; i++)
{
TABLE *table= join->table[i];
if (table->null_row)
continue; // Nothing more to do
if (!(table->map & join->const_table_map) || cleared_tables)
(*cleared_tables)|= (((table_map) 1) << i);
if (table->s->null_bytes)
{
if (cleared_tables)
{
(*cleared_tables)|= (((table_map) 1) << i);
if (table->s->null_bytes)
{
/*
Remember null bits for the record so that we can restore the
original const record in unclear_tables()
*/
memcpy(table->record[1], table->null_flags, table->s->null_bytes);
}
}
mark_as_null_row(table); // All fields are NULL
/*
Remember null bits for the record so that we can restore the
original const record in unclear_tables()
*/
memcpy(table->record[1], table->null_flags, table->s->null_bytes);
}
mark_as_null_row(table); // All fields are NULL
}
}
/**
Reverse null marking for tables and restore null bits.
This return the tables to the state of before clear_tables().
We have to do this because the tables may be re-used in a sub query
and the subquery will assume that the const tables contains the original
@ -20519,9 +20544,9 @@ void set_postjoin_aggr_write_func(JOIN_TAB *tab)
end_select function to use. This function can't fail.
*/
Next_select_func setup_end_select_func(JOIN *join, JOIN_TAB *tab)
Next_select_func setup_end_select_func(JOIN *join)
{
TMP_TABLE_PARAM *tmp_tbl= tab ? tab->tmp_table_param : &join->tmp_table_param;
TMP_TABLE_PARAM *tmp_tbl= &join->tmp_table_param;
/*
Choose method for presenting result to user. Use end_send_group
@ -20591,7 +20616,7 @@ do_select(JOIN *join, Procedure *procedure)
join->duplicate_rows= join->send_records=0;
if (join->only_const_tables() && !join->need_tmp)
{
Next_select_func end_select= setup_end_select_func(join, NULL);
Next_select_func end_select= setup_end_select_func(join);
/*
HAVING will be checked after processing aggregate functions,
@ -21077,6 +21102,7 @@ sub_select(JOIN *join,JOIN_TAB *join_tab,bool end_of_records)
}
}
/* Restore state if mark_as_null_row() have been called */
if (join_tab->last_inner)
{
JOIN_TAB *last_inner_tab= join_tab->last_inner;
@ -22461,11 +22487,18 @@ end_send_group(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)),
{
int idx= -1;
enum_nested_loop_state ok_code= NESTED_LOOP_OK;
/*
join_tab can be 0 in the case all tables are const tables and we did not
need a temporary table to store the result.
In this case we use the original given fields, which is stored in
join->fields.
*/
List<Item> *fields= join_tab ? (join_tab-1)->fields : join->fields;
DBUG_ENTER("end_send_group");
if (!join->items3.is_null() && !join->set_group_rpa)
{
/* Move ref_pointer_array to points to items3 */
join->set_group_rpa= true;
join->set_items_ref_array(join->items3);
}
@ -22473,10 +22506,12 @@ end_send_group(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)),
if (!join->first_record || end_of_records ||
(idx=test_if_group_changed(join->group_fields)) >= 0)
{
if (!join->group_sent &&
(join->first_record ||
(end_of_records && !join->group && !join->group_optimized_away)))
{
table_map cleared_tables= (table_map) 0;
if (join->procedure)
join->procedure->end_group();
if (idx < (int) join->send_group_parts)
@ -22499,11 +22534,13 @@ end_send_group(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)),
{
if (!join->first_record)
{
List_iterator_fast<Item> it(*join->fields);
Item *item;
/* No matching rows for group function */
join->clear();
List_iterator_fast<Item> it(*fields);
Item *item;
join->no_rows_in_result_called= 1;
join->clear(&cleared_tables);
while ((item= it++))
item->no_rows_in_result();
}
@ -22531,7 +22568,14 @@ end_send_group(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)),
if (join->rollup_send_data((uint) (idx+1)))
error= 1;
}
}
if (join->no_rows_in_result_called)
{
/* Restore null tables to original state */
join->no_rows_in_result_called= 0;
if (cleared_tables)
unclear_tables(join, &cleared_tables);
}
}
if (unlikely(error > 0))
DBUG_RETURN(NESTED_LOOP_ERROR); /* purecov: inspected */
if (end_of_records)
@ -22835,6 +22879,7 @@ end_write_group(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)),
{
if (join->first_record || (end_of_records && !join->group))
{
table_map cleared_tables= (table_map) 0;
if (join->procedure)
join->procedure->end_group();
int send_group_parts= join->send_group_parts;
@ -22843,7 +22888,7 @@ end_write_group(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)),
if (!join->first_record)
{
/* No matching rows for group function */
join->clear();
join->clear(&cleared_tables);
}
copy_sum_funcs(join->sum_funcs,
join->sum_funcs_end[send_group_parts]);
@ -22866,6 +22911,8 @@ end_write_group(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)),
DBUG_RETURN(NESTED_LOOP_ERROR);
}
}
if (cleared_tables)
unclear_tables(join, &cleared_tables);
if (end_of_records)
goto end;
}
@ -26965,11 +27012,8 @@ int JOIN::rollup_write_data(uint idx, TMP_TABLE_PARAM *tmp_table_param_arg, TABL
(end_send_group/end_write_group)
*/
void JOIN::clear()
void inline JOIN::clear_sum_funcs()
{
clear_tables(this, 0);
copy_fields(&tmp_table_param);
if (sum_funcs)
{
Item_sum *func, **func_ptr= sum_funcs;
@ -26979,6 +27023,22 @@ void JOIN::clear()
}
/*
Prepare for returning 'empty row' when there is no matching row.
- Mark all tables with mark_as_null_row()
- Make a copy of of all simple SELECT items
- Reset all sum functions to NULL or 0.
*/
void JOIN::clear(table_map *cleared_tables)
{
clear_tables(this, cleared_tables);
copy_fields(&tmp_table_param);
clear_sum_funcs();
}
/**
Print an EXPLAIN line with all NULLs and given message in the 'Extra' column