This is backport of 25ad623d64 for 10.2.
The issue is similar to the one from MDEV-13240. Item::save_in_field()
returns an error during tmp table population in a create table from select query
as we try to save an empty string as a date value when
force_return_blank is set to true for window functions.
MDEV-13240 Wrong warning with MAX(datetime_field) OVER (...)
The problem resided in Item_window_func implementation,
and it was revealed by bb-10.2-ext specific changes:
Item_window_func::save_in_field() works differently in bb-10.2-ext vs 10.2:
- 10.2 goes through val_str()
- bb-10.2-ext goes through get_date(), due to Type_handler related changes.
get_date() tries to convert empty string to DATETIME, hence the warning.
During a discussion with Vicentiu, it was decided to fix
Item_window_func::val_xxx() to return NULL
(instead of an "empty" value, such as 0 for numbers and '' for strings)
when force_return_blank is set.
- Added sql/mariadb.h file that should be included first by files in sql
directory, if sql_plugin.h is not used (sql_plugin.h adds SHOW variables
that must be done before my_global.h is included)
- Removed a lot of include my_global.h from include files
- Removed include's of some files that my_global.h automatically includes
- Removed duplicated include's of my_sys.h
- Replaced include my_config.h with my_global.h
The problem resided in Item_window_func implementation,
and it was revealed by bb-10.2-ext specific changes:
Item_window_func::save_in_field() works differently in bb-10.2-ext vs 10.2:
- 10.2 goes through val_str()
- bb-10.2-ext goes through get_date(), due to Type_handler related changes.
get_date() tries to convert empty string to DATETIME, hence the warning.
During a discussion with Vicentiu, it was decided to fix
Item_window_func::val_xxx() to return NULL
(instead of an "empty" value, such as 0 for numbers and '' for strings)
when force_return_blank is set.
The "is null" function performs one operation which no other Item_func
does, which is to update used tables during fix_length_and_dec().
This however can not be performed before window functions have had a
chance to resolve their order by and partition by definitions, which
happens after the initial setup_fields call. Consequently, do not call
Item_func_isnull update_used_tables during fix_length_and_dec().
There was another issue detected once the crash was resolved.
Because window functions did not implement is_null() method, we would
end up returning bad results for "is null" and "is not null" functions.
Implemented is_null() method for Item_windowfunc.
Benefits of this patch:
- Removed a lot of calls to strlen(), especially for field_string
- Strings generated by parser are now const strings, less chance of
accidently changing a string
- Removed a lot of calls with LEX_STRING as parameter (changed to pointer)
- More uniform code
- Item::name_length was not kept up to date. Now fixed
- Several bugs found and fixed (Access to null pointers,
access of freed memory, wrong arguments to printf like functions)
- Removed a lot of casts from (const char*) to (char*)
Changes:
- This caused some ABI changes
- lex_string_set now uses LEX_CSTRING
- Some fucntions are now taking const char* instead of char*
- Create_field::change and after changed to LEX_CSTRING
- handler::connect_string, comment and engine_name() changed to LEX_CSTRING
- Checked printf() related calls to find bugs. Found and fixed several
errors in old code.
- A lot of changes from LEX_STRING to LEX_CSTRING, especially related to
parsing and events.
- Some changes from LEX_STRING and LEX_STRING & to LEX_CSTRING*
- Some changes for char* to const char*
- Added printf argument checking for my_snprintf()
- Introduced null_clex_str, star_clex_string, temp_lex_str to simplify
code
- Added item_empty_name and item_used_name to be able to distingush between
items that was given an empty name and items that was not given a name
This is used in sql_yacc.yy to know when to give an item a name.
- select table_name."*' is not anymore same as table_name.*
- removed not used function Item::rename()
- Added comparision of item->name_length before some calls to
my_strcasecmp() to speed up comparison
- Moved Item_sp_variable::make_field() from item.h to item.cc
- Some minimal code changes to avoid copying to const char *
- Fixed wrong error message in wsrep_mysql_parse()
- Fixed wrong code in find_field_in_natural_join() where real_item() was
set when it shouldn't
- ER_ERROR_ON_RENAME was used with extra arguments.
- Removed some (wrong) ER_OUTOFMEMORY, as alloc_root will already
give the error.
TODO:
- Check possible unsafe casts in plugin/auth_examples/qa_auth_interface.c
- Change code to not modify LEX_CSTRING for database name
(as part of lower_case_table_names)
The method Item_sum::print did not print opening '(' after the name
of simple window functions (like rank, dense_rank etc).
As a result the view definitions with such window functions
were formed invalid in .frm files.
There was no implementation of the virtual method print()
for the Item_window_func class. As a result for a view
containing window function an invalid view definition could
be written in the frm file. When a query that refers to
this view was executed a syntax error was reported.
Refactour out (into a copy for now) the logic of Item_sum_hybrid, to
allow for multiple arguments. It does not contain the comparator
members. The result is the class Item_sum_hybrid_simple.
LEAD and LAG make use of this Item to store previous rows in a chache.
It also helps in specifying the field type. Currently LEAD/LAG do not
support default values.
NTH_VALUE behaves identical to LEAD and LAG, except that the starting
position cursor is placed on the top of the frame instead of the current
row.
With clever use of partition bounds, we only need to add one row to the
items at a time. This way we remove the need to "reset" the item and run
through the full partition again.
Fix window function expressions such as win_func() <operator> expr.
The problem was found in 2 places.
First, when we have complex expressions containing window functions, we
can only compute their final value _after_ we have computed the window
function's values. These values must be stored within the temporary
table that we are using, before sending them off.
This is done by performing an extra copy_funcs call before the final
end_send() call.
Second, such expressions need to have their inner arguments,
changed such that the references within those arguments point to fields within
the temporary table.
Ex: sum(t.a) over (order by t.b) + sum(t.a) over (order by t.b)
Before this fix, t.a pointed to the original table's a field. In order
to compute the sum function's value correctly, it needs to point to the
copy of this field inside the temp table.
This is done by calling split_sum_func for each argument in the
expression in turn.
The win.test results have also been updated as they contained wrong
values for such a use case.
When running with --ps protocol, window functions cleanup method may
be called twice. Enforce proper cleanup by setting pointers to NULL
after deletion. This prevents double deletion on second cleanup.
Perform only one table scan for each window function present. We do this
by keeping keeping cursors for each window function frame bound and
running them for each function for every row.
When ordering by a column and partitioning by another, we must reset the
peer_tracker for dense_rank, regardless if the value for the order
column changes or not.
Example:
select a, b, dense_rank() over (partition by b order by a)
a | b | dense_rank
----------------------
1 | p1 | 1
2 | p1 | 2
2 | p2 | 1 // Here, without this fix we returned 0.
2 | p2 | 2 // And 1 here.
The bug was caused by a weird behaviour in test_if_group_changed, not
returning true when testing for the first time after initializing
the Cached_item list.
- When window functions are present, JOIN::simple_order should be set
to FALSE. (Otherwise, the optimizer may attempt to do a "pre-sorting"
on the first join_tab. Which can work in some cases, but generally
isn't)
- filesort tries to only read table fields that it requires. Window
function requires its temp.table field. In order to pass this info
to filesort, added an implementation of Item_window_func::
register_field_in_read_map.
