The segfault in wsrep_check_sequence is due to a
null pointer deference on:
db_type= thd->lex->create_info.db_type->db_type;
Where create_info.db_type is null. This occured under
a used_engine==true condition which is set in the calling
function based on create_info.used_fields==HA_CREATE_USED_ENGINE.
However the create_info.used_fields was a left over
from the parsing of the previous failed CREATE TABLE where
because of its failure, db_type wasn't populated.
This is corrected by cleaning the create_info when we start
to parse ALTER SEQUENCE statements.
Other paths to wsrep_check_sequence is via CREATE SEQUENCE
and CREATE TABLE LIKE which both initialize the create_info
correctly.
Search conditions were evaluated using val_int(), which was wrong.
Fixing the code to use val_bool() instead.
Details:
- Adding a new item_base_t::IS_COND flag which marks Items used
as <search condition> in WHERE, HAVING, JOIN ON, CASE WHEN clauses.
The flag is at the parse time.
These expressions must be evaluated using val_bool() rather than val_int().
Note, the optimizer creates more Items which are used as search conditions.
Most of these items are not marked with IS_COND yet. This is OK for now,
but eventually these Items can also be fixed to have the flag.
- Adding a method Item::is_cond() which tests if the Item has the IS_COND flag.
- Implementing Item_cache_bool. It evaluates the cached expression using
val_bool() rather than val_int().
Overriding Type_handler_bool::Item_get_cache() to create Item_cache_bool.
- Implementing Item::save_bool_in_field(). It uses val_bool() rather than
val_int() to evaluate the expression.
- Implementing Type_handler_bool::Item_save_in_field()
using Item::save_bool_in_field().
- Fixing all Item_bool_func descendants to implement a virtual val_bool()
rather than a virtual val_int().
- To find places where val_int() should be fixed to val_bool(), a few
DBUG_ASSERT(!is_cond()) where added into val_int() implementations
of selected (most frequent) classes:
Item_field
Item_str_func
Item_datefunc
Item_timefunc
Item_datetimefunc
Item_cache_bool
Item_bool_func
Item_func_hybrid_field_type
Item_basic_constant descendants
- Fixing all places where DBUG_ASSERT() happened during an "mtr" run
to use val_bool() instead of val_int().
(Polished initial patch by Alexey Botchkov)
Make the code handle DEFAULT values of any datatype
- Make Json_table_column::On_response::m_default be Item*, not LEX_STRING.
- Change the parser to use string literal non-terminals for producing
the DEFAULT value
-- Also, stop updating json_table->m_text_literal_cs for the DEFAULT
value literals as it is not used.
Improve performance of queries like
SELECT * FROM t1 WHERE field = NAME_CONST('a', 4);
by, in this example, replacing the WHERE clause with field = 4
in the case of ref access.
The rewrite is done during fix_fields and we disambiguate this
case from other cases of NAME_CONST by inspecting where we are
in parsing. We rely on THD::where to accomplish this. To
improve performance there, we change the type of THD::where to
be an enumeration, so we can avoid string comparisons during
Item_name_const::fix_fields. Consequently, this patch also
changes all usages of THD::where to conform likewise.
I checked all stack overflow potential problems found with
gcc -Wstack-usage=16384
and
clang -Wframe-larger-than=16384 -no-inline
Fixes:
Added '#pragma clang diagnostic ignored "-Wframe-larger-than="'
to a lot of function to where stack usage large but resonable.
- Added stack check warnings to BUILD scrips when using clang and debug.
Function changed to use malloc instead allocating things on stack:
- read_bootstrap_query() now allocates line_buffer (20000 bytes) with
malloc() instead of using stack. This has a small performance impact
but this is not releant for bootstrap.
- mroonga grn_select() used 65856 bytes on stack. Changed it to use
malloc().
- Wsrep_schema::replay_transaction() and
Wsrep_schema::recover_sr_transactions().
- Connect zipOpen3()
Not fixed:
- mroonga/vendor/groonga/lib/expr.c grn_proc_call() uses
43712 byte on stack. However this is not easy to fix as the stack
used is caused by a lot of code generated by defines.
- Most changes in mroonga/groonga where only adding of pragmas to disable
stack warnings.
- rocksdb/options/options_helper.cc uses 20288 of stack space.
(no reason to fix except to get rid of the compiler warning)
- Causes using alloca() where the allocation size is resonable.
- An issue in libmariadb (reported to connectors).
The IDENT_sys doesn't include keywords, so the function with the
keyword name can be created, but cannot be called.
Moving keywords to new rules keyword_func_sp_var_and_label and
keyword_func_sp_var_not_label so the functions with these
names are allowed.
Like all IF NOT EXISTS syntax, a Note should be generated.
The original commit of Seqeuences cleared the IF NOT EXISTS part
in the sql/sql_yacc.yy with lex->create_info.init(). Without this
bit set there was no way it could do anything other than error.
To remedy this removal, the sql_yacc.yy components have been
minimised as they where all set at the beginning of the ALTER.
This way the opt_if_not_exists correctly set the IF_EXISTS flag.
In MDEV-13005 (bb4dd70e7c) the error code changed, requiring
ER_UNKNOWN_SEQUENCES to be handled in the function
No_such_table_error_handler::handle_condition.
The crash happened with an indexed virtual column whose
value is evaluated using a function that has a different meaning
in sql_mode='' vs sql_mode=ORACLE:
- DECODE()
- LTRIM()
- RTRIM()
- LPAD()
- RPAD()
- REPLACE()
- SUBSTR()
For example:
CREATE TABLE t1 (
b VARCHAR(1),
g CHAR(1) GENERATED ALWAYS AS (SUBSTR(b,0,0)) VIRTUAL,
KEY g(g)
);
So far we had replacement XXX_ORACLE() functions for all mentioned function,
e.g. SUBSTR_ORACLE() for SUBSTR(). So it was possible to correctly re-parse
SUBSTR_ORACLE() even in sql_mode=''.
But it was not possible to re-parse the MariaDB version of SUBSTR()
after switching to sql_mode=ORACLE. It was erroneously mis-interpreted
as SUBSTR_ORACLE().
As a result, this combination worked fine:
SET sql_mode=ORACLE;
CREATE TABLE t1 ... g CHAR(1) GENERATED ALWAYS AS (SUBSTR(b,0,0)) VIRTUAL, ...;
INSERT ...
FLUSH TABLES;
SET sql_mode='';
INSERT ...
But the other way around it crashed:
SET sql_mode='';
CREATE TABLE t1 ... g CHAR(1) GENERATED ALWAYS AS (SUBSTR(b,0,0)) VIRTUAL, ...;
INSERT ...
FLUSH TABLES;
SET sql_mode=ORACLE;
INSERT ...
At CREATE time, SUBSTR was instantiated as Item_func_substr and printed
in the FRM file as substr(). At re-open time with sql_mode=ORACLE, "substr()"
was erroneously instantiated as Item_func_substr_oracle.
Fix:
The fix proposes a symmetric solution. It provides a way to re-parse reliably
all sql_mode dependent functions to their original CREATE TABLE time meaning,
no matter what the open-time sql_mode is.
We take advantage of the same idea we previously used to resolve sql_mode
dependent data types.
Now all sql_mode dependent functions are printed by SHOW using a schema
qualifier when the current sql_mode differs from the function sql_mode:
SET sql_mode='';
CREATE TABLE t1 ... SUBSTR(a,b,c) ..;
SET sql_mode=ORACLE;
SHOW CREATE TABLE t1; -> mariadb_schema.substr(a,b,c)
SET sql_mode=ORACLE;
CREATE TABLE t2 ... SUBSTR(a,b,c) ..;
SET sql_mode='';
SHOW CREATE TABLE t1; -> oracle_schema.substr(a,b,c)
Old replacement names like substr_oracle() are still understood for
backward compatibility and used in FRM files (for downgrade compatibility),
but they are not printed by SHOW any more.
Changing the code handling sql_mode-dependent function DECODE():
- removing parser tokens DECODE_MARIADB_SYM and DECODE_ORACLE_SYM
- removing the DECODE() related code from sql_yacc.yy/sql_yacc_ora.yy
- adding handling of DECODE() with help of a new Create_func_func_decode
There was a memory leak under these conditions:
- YYABORT was called in the end-of-rule action of a rule containing expr_lex
- This expr_lex was not bound to any sp_lex_keeper
Bison did not call %destructor <expr_lex> in this case, because its stack
already contained a reduced upper-level rule.
Fixing rules starting with RETURN, CONTINUE, EXIT keywords:
Turning end-of-rule actions with YYABORT into mid-rule actions
by adding an empty trailing { } block. This prevents the upper level
rule from being reduced without calling %destructor <expr_lex>.
In other rules expr_lex is used not immediately before the last
end-of-rule { } block, so they don't need changes.
- Moving the code from a public function trim_whitespaces()
to the class Lex_cstring as methods. This code may
be useful in other contexts, and also this code becomes
visible inside sql_class.h
- Adding a helper method THD::strmake_lex_cstring_trim_whitespaces()
- Unifying the way how CREATE PROCEDURE/CREATE FUNCTION and
CREATE PACKAGE/CREATE PACKAGE BODY work:
a) Now CREATE PACKAGE/CREATE PACKAGE BODY also calls
Lex->sphead->set_body_start() to remember the cpp body start inside
an sp_head member.
b) adding a "const char *cpp_body_end" parameter to
sp_head::set_stmt_end().
These changes made it possible to reuse sp_head::set_stmt_end() inside
LEX::create_package_finalize() and remove the duplucate code.
- Renaming sp_head::m_body_begin to m_cpp_body_begin and adding a comment
to make it clear that this member is used only during parsing, and
points to a fragment inside the cpp buffer.
- Changed sp_head::set_body_start() and sp_head::set_stmt_end()
to skip the calls related to "body_utf8" in cases when m_parent is not NULL.
A non-NULL m_parent means that we're inside a package routine.
"body_utf8" in such case belongs not to the current sphead itself,
but to parent (the package) sphead.
So an sphead instance of a package routine should neither initialize,
nor finalize, nor change in any other ways the "body_utf8" related
members of Lex_input_stream, and should not take over or copy "body_utf8"
data from Lex_input_stream to "this".
When CURSOR parameters get parsed, their sp_assignment_lex instances
(one instance per parameter) get collected to List<sp_assignment_lex>.
These instances get linked to sphead only in the end of the list.
If a syntax error happened in the middle of the parameter list,
these instances were not deleted, which caused memory leaks.
Fix:
using a Bison %destructor to free rules of the <sp_assignment_lex_list>
type (on syntax errors).
Afte the fix these sp_assignment_lex instances from CURSOR parameters
deleted as follows:
- If the CURSOR statement was fully parsed, then these instances
get properly linked to sp_head structures, so they are deleted
during ~sp_head (this did not change)
- If the CURSOR statement failed on a syntax error, then by Bison's
%destructor (this is being added in the current patch).
The parser works as follows:
The rule expr_lex returns a pointer to a newly created sp_expr_lex
instance which is not linked to any MariaDB structures yet - it is
pointed only from a Bison stack variable. The sp_expr_lex instance
gets linked to other structures (such as sp_instr_jump_if_not) later,
after scanning some following grammar.
Problem before the fix:
If a parse error happened immediately after expr_lex (before it got linked),
the created sp_expr_lex value got lost causing a memory leak.
Fix:
- Using Bison's "destructor" directive to free the results of expr_lex
on parse/oom errors.
- Moving the call for LEX::cleanup_lex_after_parse_error() from
MYSQL_YYABORT and yyerror inside parse_sql().
This is needed because Bison calls destructors after yyerror(),
while it's important to delete the sp_expr_lex instance before
LEX::cleanup_lex_after_parse_error().
The latter frees the memory root containing the sp_expr_lex instance.
After this change the code block are executed in the following order:
- yyerror() -- now only raises the error to DA (no cleanup done any more)
- %destructor { delete $$; } <expr_lex> -- destructs the sp_expr_lex instance
- LEX::cleanup_lex_after_parse_error() -- frees the memory root containing
the sp_expr_lex instance
- Removing the "delete sublex" related code from restore_lex():
- restore_lex() is called in most cases on success, when delete is not needed.
- There is one place when restore_lex() is called on error:
In sp_create_assignment_instr(). But in this case LEX::sp_lex_in_use
is true anyway.
The patch adds a new DBUG_ASSERT(lex->sp_lex_in_use) to guard this.
- Moved view checks after privilege tables are fixed. This is to avoid
warnings about wrongly defined mysql.proc when checking views.
- Don't use stat tables before they have been fixed.
- Don't run mysql_fix_view() if 'FOR MYSQL' is used if the view is
already a MariaDB view.
- Added 'FOR UPGRADE' as an option for 'REPAIR VIEW' to be able to
detect if the REPAIR command comes from mariadb_upgrade. In this
case we get a warning, instead of an error, if a definer of a view
does not exists.
Adding virtual methods to class Schema:
make_item_func_replace()
make_item_func_substr()
make_item_func_trim()
This is a non-functional preparatory change for MDEV-27744.
The parser code for single-table DELETE missed the call of the function
LEX::check_main_unit_semantics(). As a result the the field nested level
of SELECT_LEX structures remained set 0 for all non-top level selects.
This could lead to different kind of problems. In particular this did not
allow to determine properly the selects where set functions had to be
aggregated when they were used in inner subqueries.
Approved by Oleksandr Byelkin <sanja@mariadb.com>
This patch fixes the problem by adding a new rule booleat_test.
This makes the grammar clearer and less conflicting.
Additionally, fixing %prec in this grammar branch:
- | boolean_test IS NULL_SYM %prec PREC_BELOW_NOT
+ | boolean_test IS NULL_SYM %prec IS
to have consistently "%prec IS" in all grammar branches starting
with "boolean_test IS ...".
It's not clear why these three rules needed different %prec before the fix:
- boolean_test IS TRUE
- boolean_test IS UNKNOWN
- boolean_test IS NULL
the parser couldn't parse `1=2 not between 3 and 5`
after `2` it expected only NOT2_SYM, but not NOT_SYM
(visible from the sql_yacc.output file), which resulted in
Syntax error ... near 'not between 3 and 4'
The parser was confused by a rather low NOT_SYM precedence and
%prec BETWEEN_SYM didn't resolve this confusion.
As a fix, let's remove any %precedence from NOT_SYM and
specify %prec explicitly in the only place where it matters for NOT_SYM.
In other places, such as for NOT BETWEEN, NOT_SYM won't have a
precedence, so bison won't be confused about it.
