repair it
Multi-table delete that is optimized with QUICK_RANGE reports table
corruption.
DELETE statement must not use KEYREAD optimization, and sets
table->no_keyread to 1. This was ignored in QUICK_RANGE optimization.
With this fix QUICK_RANGE optimization honors table->no_keyread
value and does not enable KEYREAD when it is requested.
When only one row was present, the subtraction of nearly the same number
resulted in catastropic cancellation, introducing an error in the
VARIANCE calculation near 1e-15. That was sqrt()ed to get STDDEV, the
error was escallated to near 1e-8.
The simple fix of testing for a row count of 1 and forcing that to yield
0.0 is insufficient, as two rows of the same value should also have a
variance of 0.0, yet the error would be about the same.
So, this patch changes the formula that computes the VARIANCE to be one
that is not subject to catastrophic cancellation.
In addition, it now uses only (faster-than-decimal) floating point numbers
to calculate, and renders that to other types on demand.
An update that used a join of a table to itself and modified the
table on one side of the join reported the table as crashed or
updated wrong rows.
Fixed by creating temporary table for self-joined multi update statement.
Handling of large signed/unsigned values was not consistent, so some string functions could return bogus results.
The current fix is to simply patch up the val_str() methods for those string items.
It would be good clean this code up in general, to make similar problems much harder to make. This is left as an exercise for the reader.
spatial index
While executing OPTIMIZE TABLE on MyISAM tables the server re-creates the
index file(s) in order to sort them physically by the key. This cannot be
done for R-tree indexes as it makes no sense.
The server was not checking the type of the index and was accessing an
R-tree index as if it was a B-tree.
Fixed by preventing sorting the index file if it contains an R-tree index.
If SELECT-part of CREATE VIEW statement contains '\Z',
it is not handled correctly.
The problem was in String::print().
Symbol with code 032 (26) is replaced with '\z',
which is not supported by the lexer.
The fix is to replace the symbol with '\Z'.
the UDF
When deleting a user defined function MySQL must remove it from both the
in-memory hash table and the mysql.proc system table.
Finding (and removal therefore) from the internal hash table is case
insensitive (or whatever the default charset is), whereas finding and
removal from the system table is case sensitive.
As a result if you supply a function name that is not in the same character
case to DROP FUNCTION the server will remove the function only from the
in-memory hash table and will keep the row in mysql.proc system table.
This will cause inconsistency between the two structures (that is fixed
only by restarting the server).
Fixed by using the name in the precise case (from the in-memory hash table)
to delete the row in the mysql.proc system table.
Problem:
When creating a temporary field for a temporary table in create_tmp_field_from_field(), a resulting field is created as an exact copy of an original one (in Field::new_field()). However, Field_enum and Field_set contain a pointer (typelib) to memory allocated in the parent table's MEM_ROOT, which under some circumstances may be deallocated later by the time a temporary table is used.
Solution:
Override the new_field() method for Field_enum and Field_set and create a separate copy of the typelib structure in there.
- When this bug was corrected it changed the behavior
for data/index directory in the myisam test case.
- This patch moves the OS depending tests to a non-windows
test file.
Blocked evaluation of constant objects of the classes
Item_func_is_null and Item_is_not_null_test at the
prepare phase in the cases when the objects used subqueries.
Removed an assertion that was not valid for the cases where the query
in a prepared statement contained a single-row non-correlated
subquery that was used as an argument of the IS NULL predicate.
Bug#4968 "Stored procedure crash if cursor opened on altered table"
Bug#19733 "Repeated alter, or repeated create/drop, fails"
Bug#19182 "CREATE TABLE bar (m INT) SELECT n FROM foo; doesn't work from
stored procedure."
Bug#6895 "Prepared Statements: ALTER TABLE DROP COLUMN does nothing"
Bug#22060 "ALTER TABLE x AUTO_INCREMENT=y in SP crashes server"
Test cases for bugs 4968, 19733, 6895 will be added in 5.0.
Re-execution of CREATE DATABASE, CREATE TABLE and ALTER TABLE
statements in stored routines or as prepared statements caused
incorrect results (and crashes in versions prior to 5.0.25).
In 5.1 the problem occured only for CREATE DATABASE, CREATE TABLE
SELECT and CREATE TABLE with INDEX/DATA DIRECTOY options).
The problem of bugs 4968, 19733, 19282 and 6895 was that functions
mysql_prepare_table, mysql_create_table and mysql_alter_table were not
re-execution friendly: during their operation they used to modify contents
of LEX (members create_info, alter_info, key_list, create_list),
thus making the LEX unusable for the next execution.
In particular, these functions removed processed columns and keys from
create_list, key_list and drop_list. Search the code in sql_table.cc
for drop_it.remove() and similar patterns to find evidence.
The fix is to supply to these functions a usable copy of each of the
above structures at every re-execution of an SQL statement.
To simplify memory management, LEX::key_list and LEX::create_list
were added to LEX::alter_info, a fresh copy of which is created for
every execution.
The problem of crashing bug 22060 stemmed from the fact that the above
metnioned functions were not only modifying HA_CREATE_INFO structure in
LEX, but also were changing it to point to areas in volatile memory of
the execution memory root.
The patch solves this problem by creating and using an on-stack
copy of HA_CREATE_INFO (note that code in 5.1 already creates and
uses a copy of this structure in mysql_create_table()/alter_table(),
but this approach didn't work well for CREATE TABLE SELECT statement).
- Using DATA/INDEX DIRECTORY option on Windows put data/index file into
default directory because the OS doesn't support readlink().
- The procedure for changing data/index file directory is
different under Windows.
- With this fix we report a warning if DATA/INDEX option is used,
but OS doesn't support readlink().
table
ROW_FORMAT option is lost during CREATE/DROP INDEX.
This fix forces CREATE/DROP INDEX to retain ROW_FORMAT by instructing
mysql_alter_table() that ROW_FORMAT is not used during creating/dropping
indexes.
The problem is that the GEOMETRY NOT NULL can't automatically set
any value as a default one. We always tried to complete LOAD DATA
command even if there's not enough data in file. That doesn't work
for GEOMETRY NOT NULL. Now Field_*::reset() returns an error sign
and it's checked in mysql_load()
Problem: replication of LC_TIME_NAMES didn't work.
Thus, INSERTS or UPDATES using date_format() always
worked with en_US on the slave side.
Fix: adding ONE_SHOT implementation for LC_TIME_NAMES.
Problem: ``SET PASSWORD FOR foo@localhost'' was written into
binary log using double quites: ``SET PASSWORD FOR "foo"@"localhost"...''.
If sql_mode was set to ANSI_QUOTES, parser on slave considered
"foo" and "localhost" as identifiers instead of strigns constants,
so it failed to parse, generated syntax error and slave then stopped.
Fix: changing binary log entries to use single quotes:
``SET PASSWORD FOR 'foo'@'localhost'...'' not to depend on ANSI_QUOTES.
prepared statement and subquery.
When a field of a view from an outer select is resolved the find_field_in_view
function creates an Item_direct_view_ref object that references the
corresponding view underlying field. After that the view_ref is marked
as a dependent one. While resolving view underlying field it also get
marked as a dependent one due to current_select still points to the subselect.
Marking the view underlying field is wrong and lead to attaching conditions
to a wrong table and thus to the wrong result of the whole statement.
Now mark_select_range_as_dependent() function isn't called for fields from a
view underlying table.
- The table_priv column of table_privs table was altered to a enum type
with fewer enums causing the SHOW/CREATE VIEW grants to be truncated.
- Improved comments and moved all declarations for table_privs, column_privs
and proc_privs to one section for each table making it easy to see hat alterations
are performed on each table
- Reduced the number of ALTER's slightly, but as this is an upgrade script we need
to take all possibilites into account.
This error is displayed anytime the SELECT statement needs a temp table to
return correct results because the object (select_dumpvar) that represents
variables named in the INTO clause stored the results before the temp
table was considered. The problem was fixed by creating the necessary
Item_func_set_user_var objects once the correct data is ready.
ALTER TABLE DISABLE KEYS doesn't work when modifying the table
ENABLE|DISABLE KEYS combined with another ALTER TABLE option, different
than RENAME TO did nothing. Also, if the table had disabled keys
and was ALTER-ed then the end table was with enabled keys.
Fixed by checking whether the table had disabled keys and enabling them
in the copied table.
statements
Currently the optimizer evaluates loose index scan only for top-level SELECT
statements
Extend loose index scan applicability by :
- Test the applicability of loose scan for each sub-select, instead of the
whole query. This change enables loose index scan for sub-queries.
- allow non-select statements with SELECT parts (like, e.g.
CREATE TABLE .. SELECT ...) to use loose index scan.
- CREATE PROCEDURE stores database name based on query context instead
of 'current database' as set by 'USE' according to manual.
The bug reporter interpret the filtering statements as bug for
DROP PROCEDURE based on this behavior.
- Removed the code which changes db context.
- Added code to check that a valid db was supplied.
When implicitly converting string fields to numbers the
string-to-number conversion error was not sent to the client.
Added code to send the conversion error as warning.
We also need to prevent generation of warnings from the places
where val_xxx() methods are called for the sole purpose of updating
the Item::null_value flag.
To achieve that a special function is added (and called) :
update_null_value(). This function will set the no_errors flag and
will call val_xxx(). The warning generation in Field_string::val_xxx()
will use the flag when generating the conversion warnings.
Problem: when loading mysqlbinlog dumps, CREATE PROCEDURE having semicolons
in their bodies failed.
Fix: Using safe delimiter "/*!*/;" to dump log entries.
master_port after a "change master" will be set to the compiled in default value
i.e not always the same as what the master report as it's port number.
If a view was created with the DEFINER security and later the definer user
was dropped then a SELECT from the view throws the error message saying that
there is no definer user is registered. This is ok for a root but too much
for a mere user.
Now the st_table_list::prepare_view_securety_context() function reveals
the absence of the definer only to a superuser and throws the 'access denied'
error to others.
fails randomly.
The problem was that the test case used command line tool (mysql)
without specifying connect_timeout argument. In some cases,
this lead to hanging of the test case.
The fix is to specify --connect_timeout=1 when starting mysql.
Also, the patch contains polishing and various cleanups to simplify
analyzing of the problems further.
The patch affects only test suite, no server codebase has been
touched.
Load shared libraries from zlib (fixed that mysql-test-run.pl didn't work on some Solaris boxes)
Added connect timeout to test to make im_daemon_life_cycle more predictable
- Client side readline functions unconditionally search for Unix '\n' line
endings. In this case, the delimiter statement was set to '//\r' instead
of the intended '//'. When removing the '\n' check for and remove
preceeding '\r' character as well.
mysql_fix_privilege_tables.s's ability to convert the system tables as of
3.20 to current system table format
Add similar test for 4.1.23 tables - but use "mysql < mysql_fix_privilege_tables.sql"
so it can be run on any platform.
This is the 5.0 part of the fix.
Currently TRUNCATE command will not call
delete_all_rows() in the handler (that implements
the "fast" TRUNCATE for InnoDB) when there are
triggers on the table.
As decided by the architecture team TRUNCATE must
use "fast" TRUNCATE even when there are triggers.
Thus it must ignore the triggers.
Made TRUNCATE to ignore the triggers and call
delete_all_rows() for all storage engines
to maintain engine consistency.
(Mostly in DBUG_PRINT() and unused arguments)
Fixed bug in query cache when used with traceing (--with-debug)
Fixed memory leak in mysqldump
Removed warnings from mysqltest scripts (replaced -- with #)
Do not issue a 'read-only' error in case of DROP TEMPORARY TABLE on a non-existing temporary table.
Instead produce the correct "Unknown table" error or warning (in cases when the IF EXISTS clause was specified).
To a documentor: the part of the manual describing the 'read_only' system variable should be clarified to state the following:
"When the read_only variable is set to ON, all operations which create/update/drop tables are rejected with the exceptions for:
1. Any operation performed by the replication thread on a slave server
2. Any operation performed by a user that have the SUPER privilege
3. Any operation that creates/updates/drops only temporary tables"
limitation)
Note to the reviewer
====================
Warning: reviewing this patch is somewhat involved.
Due to the nature of several issues all affecting the same area,
fixing separately each issue is not practical, since each fix can not be
implemented and tested independently.
In particular, the issues with
- rule recursion
- nested case statements
- forward jump resolution (backpatch list)
are tightly coupled (see below).
Definitions
===========
The expression
CASE expr
WHEN expr THEN expr
WHEN expr THEN expr
...
END
is a "Simple Case Expression".
The expression
CASE
WHEN expr THEN expr
WHEN expr THEN expr
...
END
is a "Searched Case Expression".
The statement
CASE expr
WHEN expr THEN stmts
WHEN expr THEN stmts
...
END CASE
is a "Simple Case Statement".
The statement
CASE
WHEN expr THEN stmts
WHEN expr THEN stmts
...
END CASE
is a "Searched Case Statement".
A "Left Recursive" rule is like
list:
element
| list element
;
A "Right Recursive" rule is like
list:
element
| element list
;
Left and right recursion produces the same language, the difference only
affects the *order* in which the text is parsed.
In a descendant parser (usually written manually), right recursion works
very well, and is typically implemented with a while loop.
In an ascendant parser (yacc/bison) left recursion works very well,
and is implemented naturally by the parser stack.
In both cases, using the wrong type or recursion is very bad and should be
avoided, as it causes technical issues with the parser implementation.
Before this change
==================
The "Simple Case Expression" and "Searched Case Expression" were both
implemented by the "when_list" and "when_list2" rules, which are left
recursive (ok).
These rules, however, used lex->when_list instead of using the parser stack,
which is more complex that necessary, and potentially dangerous because
of other rules using THD::reset_lex.
The "Simple Case Statement" and "Searched Case Statements" were implemented
by the "sp_case", "sp_whens" and in part by "sp_proc_stmt" rules.
Both cases were right recursive (bad).
The grammar involved was convoluted, and is assumed to be the results of
tweaks to get the code generation to work, but is not what someone would
naturally write.
In addition, using a common rule for both "Simple" and "Searched" case
statements was implemented with sp_head::m_flags |= IN_SIMPLE_CASE,
which is a flag and not a stack, and therefore does not take into account
*nested* case statements. This leads to incorrect generated code, and either
a server crash or an incorrect result.
With regards to the backpatch mechanism, a *different* backpatch list was
created for each jump from "WHEN expr THEN stmt" to "END CASE", which
relied on the grammar to be right recursive.
This is a mis-use of the backpatch list, since this list can resolve
multiple references to the same target at once.
The optimizer algorithm used to detect dead code in the "assembly" SQL
instructions, implemented by sp_head::opt_mark(uint ip), was recursive
in some cases (a conditional jump pointing forward to another conditional
jump).
In case of specially crafted code, like
- a long list of "IF expr THEN stmt END IF"
- a long CASE statement
this would actually cause a server crash with a stack overflow.
In general, having a stack that grows proportionally with user data (the
SQL code given by the client in a CREATE PROCEDURE) is to be avoided.
In debug builds only, creating a SP / SF / Trigger which had a significant
amount of code would spend --literally-- several minutes in sp_head::create,
because of the debug code involved with DBUG_PRINT("info", ("Code %s ...
There are several issues with this code:
- in a CASE with 5 000 WHEN, there are 15 000 instructions generated,
which create a sting representation of the code which is 500 000 bytes
long,
- using a String instead of an io stream causes performances to degrade
to a total server freeze, as time is spent doing realloc of a buffer
always too short,
- Printing a 500 000 long string in the debug log is too verbose,
- Generating this string even when DBUG_PRINT is off is useless,
- Having code that potentially can affect the server behavior, used with
#ifdef / #endif is useful in some cases, but is also a bad practice.
After this change
=================
"Case Expressions" (both simple and searched) have been simplified to
not use LEX::when_list, which has been removed.
Considering all the issues affecting case statements, the grammar for these
has been totally re written.
The existing actions, used to generate "assembly" sp_inst* code, have been
preserved but moved in the new grammar, with the following changes:
a) Bison rules are no longer shared between "Simple" and "Searched" case
statements, because a stack instead of a flag is required to handle them.
Nested statements are handled naturally by the parser stack, which by
definition uses the correct rule in the correct context.
Nested statements of the opposite type (simple vs searched) works correctly.
The flag sp_head::IN_SIMPLE_CASE is no longer used.
This is a step towards resolution of WL#2999, which correctly identified
that temporary parsing flags do not belong to sp_head.
The code in the action is shared by mean of the case_stmt_action_xxx()
helpers.
b) The backpatch mechanism, used to resolve forward jumps in the generated
code, has been changed to:
- create a label for the instruction following 'END CASE',
- register each jump at the end of a "WHEN expr THEN stmt" in a *unique*
backpatch list associated with the 'END CASE' label
- resolve all the forward jumps for this label at once.
In addition, the code involving backpatch has been commented, so that a
reader can now understand by reading matching "Registering" and "Resolving"
comments how the forward jumps are resolved and what target they resolve to,
as this is far from evident when reading the code alone.
The implementation of sp_head::opt_mark() has been revised to avoid
recursive calls from jump instructions, and instead add the jump location
to the list of paths to explore during the flow analysis of the instruction
graph, with a call to sp_head::add_mark_lead().
In addition, the flow analysis will stop if an instruction has already
been marked as reachable, which the previous code failed to do in the
recursive case.
sp_head::opt_mark() is now private, to prevent new calls to this method from
being introduced.
The debug code present in sp_head::create() has been removed.
Considering that SHOW PROCEDURE CODE is also available in debug builds,
and can be used anytime regardless of the trace level, as opposed to
"CREATE PROCEDURE" time and only if the trace was on,
removing the code actually makes debugging easier (usable trace).
Tests have been written to cover the parser overflow (big CASE),
and to cover nested CASE statements.
There was an improper order of doing chained operations.
To the documentor: ENABLE|DISABLE KEYS combined with RENAME TO, and no other
ALTER TABLE clause, leads to server crash independent of the presence of
indices and data in the table.
The problem was that some functions (namely IN() starting with 4.1, and
CHAR() starting with 5.0) were returning NULL in certain conditions,
while they didn't set their maybe_null flag. Because of that there could
be some problems with 'IS NULL' check, and statements that depend on the
function value domain, like CREATE TABLE t1 SELECT 1 IN (2, NULL);.
The fix is to set maybe_null correctly.
- Added 'SET NAMES <charset>" upon ::open
- Added test and results for simple UTF test
federated.test:
BUG #17044 Federated Storage Engine not UTF8 clean
New test. Using hex - pasting various charsets in the terminal doesn't work.
federated.result:
BUG# 17044 Federated Storage Engine not UTF8 clean
New test results
ha_federated.cc:
BUG# 17044 Federated Storage Engine not UTF8 clean
Upon ::open, set names to table's charset
Problem: When we have a really large number (between 2^63 and 2^64)
as the left side of the mod operator, it gets improperly corerced
into a signed value.
Solution: Added check to see if the "negative" number is really
positive, and if so, cast it.
The problem was that THD::row_count_func was zeroed too. It was zeroed
as a fix for bug 4905 "Stored procedure doesn't clear for "Rows affected"
However, the proper solution is not to zero, because THD::row_count_func has
been set to -1 already in mysql_execute_command(), a later fix, which obsoletes
the incorrect fix of #4095
The regression is caused by the fix for bug 14767. When INSERT ... SELECT
used a view in the SELECT list that was not inlined, and there was an
active transaction, the server could crash in Query_cache::invalidate.
On INSERT ... SELECT only the table being inserted into is invalidated.
Thus views that can't be inlined are skipped from invalidation.
The bug manifests itself in two ways so there is 2 test cases.
One checks that the only the table being inserted into is invalidated.
And the second one checks that there is no crash on INSERT ... SELECT.
This change set implements the DROP TRIGGER IF EXISTS functionality.
This fix is considered a bug and not a feature, because without it,
there is no known method to write a database creation script that can create
a trigger without failing, when executed on a database that may or may not
contain already a trigger of the same name.
Implementing this functionality closes an orthogonality gap between triggers
and stored procedures / stored functions (which do support the DROP IF
EXISTS syntax).
In sql_trigger.cc, in mysql_create_or_drop_trigger,
the code has been reordered to:
- perform the tests that do not depend on the file system (access()),
- get the locks (wait_if_global_read_lock, LOCK_open)
- call access()
- perform the operation
- write to the binlog
- unlock (LOCK_open, start_waiting_global_read_lock)
This is to ensure that all the code that depends on the presence of the
trigger file is executed in the same critical section,
and prevents race conditions similar to the case fixed by Bug 14262 :
- thread 1 executes DROP TRIGGER IF EXISTS, access() returns a failure
- thread 2 executes CREATE TRIGGER
- thread 2 logs CREATE TRIGGER
- thread 1 logs DROP TRIGGER IF EXISTS
The patch itself is based on code contributed by the MySQL community,
under the terms of the Contributor License Agreement (See Bug 18161).
The code that set up data to be passed to user-defined functions was very
old and analyzed the "Type" of the data that was passed into the UDF, when
it really should analyze the "return_type", which is hard-coded for simple
Items and works correctly for complex ones like functions.
---
Added test at Sergei's behest.
