The problem is that the first execution of the prepared statement makes
a permanent optimization of converting the LEFT JOIN to an INNER JOIN.
This is based on the assumption that all the user parameters (?) are
always constants and that parameters to Item_cond() will not change value
from true and false between different executions.
(The example was using IS NULL, which will change value if parameter
depending on if the parameter is NULL or not).
The fix is to change Item_cond::fix_fields() and
Item_cond::eval_not_null_tables() to not threat user parameters as
constants. This will ensure that we don't do the LEFT_JOIN -> INNER
JOIN conversion that causes problems.
There is also some things that needs to be improved regarding
calculations of not_null_tables_cache as we get a different value for
WHERE 1 or t1.a=1
compared to
WHERE t1.a= or 1
Changes done:
- Mark Item_param with the PARAM flag to be able to quickly check
in Item_cond::eval_not_null_tables() if an item contains a
prepared statement parameter (just like we check for stored procedure
parameters).
- Fixed that Item_cond::not_null_tables_cache is not depending on
order of arguments.
- Don't call item->eval_const_cond() for items that are NOT on the top
level of the WHERE clause. This removed a lot of unnecessary
warnings in the test suite!
- Do not reset not_null_tables_cache for not top level items.
- Simplified Item_cond::fix_fields by calling eval_not_null_tables()
instead of having duplication of all the code in
eval_not_null_tables().
- Return an error if Item_cond::fix_field() generates an error
The old code did generate an error in some cases, but not in all
cases.
- Fixed all handling of the above error in make_cond_for_tables().
The error handling by the callers did not exists before which
could lead to asserts in many different places in the old code).
- All changes in sql_select.cc are just checking the return value of
fix_fields() and make_cond_for_tables() and returning an error
value if fix_fields() returns true or make_cond_for_tables()
returns NULL and is_error() is set.
- Mark Item_cond as const_item if all arguments returns true for
can_eval_in_optimize().
Reviewer: Sergei Petrunia <sergey@mariadb.com>
* invoke check_expression() for all vcol_info's in
mysql_prepare_create_table() to check for FK CASCADE
* also check for SET NULL and SET DEFAULT
* to check against existing FKs when a vcol is added in ALTER TABLE,
old FKs must be added to the new_key_list just like other indexes are
* check columns recursively, if vcol1 references vcol2,
flags of vcol2 must be taken into account
* remove check_table_name_processor(), put that logic under
check_vcol_func_processor() to avoid walking the tree twice
This bug could manifest itself at the first execution of prepared statement
created for queries using a materialized view defined as union. A crash
could happen for sure if the query contained a condition pushable into
the view and this condition was over the column defined via a complex string
expression requiring implicit conversion from one charset to another for
some of its sub-expressions. The bug could cause crashes when executing
PS for some other queries whose optimization needed building clones for
such expressions.
This bug was introduced in the patch for MDEV-29988 where the class
Item_direct_ref_to_item was added. The implementations of the virtual
methods get_copy() and build_clone() were invalid for the class and this
could cause crashes after the method build_clone() was called for
expressions containing objects of the Item_direct_ref_to_item type.
Approved by Sergei Golubchik <serg@mariadb.com>
This patch is the result of running
run-clang-tidy -fix -header-filter=.* -checks='-*,modernize-use-equals-default' .
Code style changes have been done on top. The result of this change
leads to the following improvements:
1. Binary size reduction.
* For a -DBUILD_CONFIG=mysql_release build, the binary size is reduced by
~400kb.
* A raw -DCMAKE_BUILD_TYPE=Release reduces the binary size by ~1.4kb.
2. Compiler can better understand the intent of the code, thus it leads
to more optimization possibilities. Additionally it enabled detecting
unused variables that had an empty default constructor but not marked
so explicitly.
Particular change required following this patch in sql/opt_range.cc
result_keys, an unused template class Bitmap now correctly issues
unused variable warnings.
Setting Bitmap template class constructor to default allows the compiler
to identify that there are no side-effects when instantiating the class.
Previously the compiler could not issue the warning as it assumed Bitmap
class (being a template) would not be performing a NO-OP for its default
constructor. This prevented the "unused variable warning".
(Variant 3, initial variant was by Rex Jonston)
A LEFT JOIN with a constant as a column of the inner table produced wrong
query result if the optimizer had to write the inner table column into a
temp table. Query pattern:
SELECT ...
FROM (SELECT /*non-mergeable select*/
FROM t1 LEFT JOIN (SELECT 'Y' as Val) t2 ON ...) as tbl
Fixed this by adding Item_direct_view_ref::save_in_field() which follows
the pattern of Item_direct_view_ref's save_org_in_field(),
save_in_result_field() and val_XXX() functions:
* call check_null_ref() and handle NULL value
* if we didn't get a NULL-complemented row, call Item_direct_ref's function.
it's incorrect to use change_item_tree() to replace arguments
of top-level AND/OR, because they (arguments) are stored in a List,
so a pointer to an argument is in the list_node, and individual
list_node's of top-level AND/OR can be deleted in Item_cond::build_equal_items().
In that case rollback_item_tree_changes() will modify the deleted object.
Luckily, it's not needed to use change_item_tree() for top-level
AND/OR, because the whole top-level item is copied and preserved
in prep_where and prep_on, and restored from there.
So, just don't.
Additionally to the test case in the commit it fixes
* ASAN failure of main.opt_tvc --ps
* ASAN failure of main.having_cond_pushdown --ps
The idea is to put Item_direct_ref_to_item as a transparent and
permanent wrapper before a string which require conversion.
So that Item_direct_ref_to_item would be the only place where
the pointer to the string item is stored, this pointer can be changed
and restored during PS execution as needed. And if any permanent
(subquery) optimization would need a pointer to the item,
it'll use a pointer to the Item_direct_ref_to_item - which is
a permanent item and won't go away.
The incorrect type handler caused an incorrect result_type() for
Item_cache_row (STRING_RESULT rather than ROW_RESULT). By updating the
constructor of Item_cache_row with the correct type handler, it fixes
this problem.
Signed-off-by: Yuchen Pei <yuchen.pei@mariadb.com>
Reviewed-by: Sergei Golubchik <serg@mariadb.com>
it's incorrect to use change_item_tree() to replace arguments
of top-level AND/OR, because they (arguments) are stored in a List,
so a pointer to an argument is in the list_node, and individual
list_node's of top-level AND/OR can be deleted in Item_cond::build_equal_items().
In that case rollback_item_tree_changes() will modify the deleted object.
Luckily, it's not needed to use change_item_tree() for top-level
AND/OR, because the whole top-level item is copied and preserved
in prep_where and prep_on, and restored from there.
So, just don't.
If all elements in the list of 'IN' or 'NOT IN' clause are equal
and there are no NULLs then clause
- "a IN (e1,..,en)" can be converted to "a = e1"
- "a NOT IN (e1,..,en)" can be converted to "a <> e1".
This means an object of Item_func_in can be replaced with an object
of Item_func_eq for IN (e1,..,en) clause and Item_func_ne for
NOT IN (e1,...,en). Such a replacement allows the optimizer to choose
a better execution plan
To prevent ASAN heap-use-after-poison in the MDEV-16549 part of
./mtr --repeat=6 main.derived
the initialization of Name_resolution_context was cleaned up.
SELECT FROM JSON_TABLE
Analysis: When fix_fields_if_needed() is called, it doesnt check if operands
are valid because check_cols() is not called. So it doesn't error out and
eventually crashes.
Fix: Use fix_fields_if_needed_for_scalar() instead of
fix_fields_if_needed(). It filters the scalar and returns the error if
it occurs.
Nowdays subquery in a UNION's ORDER BY placed correctly in fake select,
the only problem was incorrect Name_resolution_contect is fixed by this
patch in parsing, so we do not need scanning/reseting of ORDER BY of
a union.
Errors where:
/buildbot/amd64-ubuntu-2004-msan/build/sql/item.h:6478:12: error: 'val_datetime_packed' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
longlong val_datetime_packed(THD *thd)
^
/buildbot/amd64-ubuntu-2004-msan/build/sql/item.h:3501:12: note: overridden virtual function is here
longlong val_datetime_packed(THD *thd) override;
^
/buildbot/amd64-ubuntu-2004-msan/build/sql/item.h:6480:12: error: 'val_time_packed' overrides a member function but is not marked 'override' [-Werror,-Winconsistent-missing-override]
longlong val_time_packed(THD *thd)
^
/buildbot/amd64-ubuntu-2004-msan/build/sql/item.h:3502:12: note: overridden virtual function is here
longlong val_time_packed(THD *thd) override;
^
