limitation)
Bug#24854 (Mixing Searched Case with Simple Case inside Stored Procedure
  crashes Mysqld)

Implemented code review (19194) comments


mysql-test/r/sp_stress_case.result:
  Implemented code review comments : use SQL instead of a shell script to
  generate the code
mysql-test/t/sp_stress_case.test:
  Adjusted
sql/sql_yacc.yy:
  Added more explicit comments
BitKeeper/deleted/.del-sp_stress_case.sh:
  Delete: mysql-test/t/sp_stress_case.sh

  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.


mysql-test/r/sp-code.result:
  Test cases for nested CASE statements.
mysql-test/t/sp-code.test:
  Test cases for nested CASE statements.
sql/sp_head.cc:
  Re factored opt_mark() to avoid recursion, clean up.
sql/sp_head.h:
  Re factored opt_mark() to avoid recursion, clean up.
sql/sql_lex.cc:
  Removed when_list.
sql/sql_lex.h:
  Removed when_list.
sql/sql_yacc.yy:
  Minor clean up for case expressions,
  Major re write for case statements (Bug#19194).
mysql-test/r/sp_stress_case.result:
  New test for massive CASE statements.
mysql-test/t/sp_stress_case.sh:
  New test for massive CASE statements.
mysql-test/t/sp_stress_case.test:
  New test for massive CASE statements.

mysql-test/t/sp-error.test:
  Move test for bug#20953 to the end of file.

into  moonlight.intranet:/home/tomash/src/mysql_ab/mysql-5.0-bug20953


mysql-test/r/view.result:
  Auto merged
mysql-test/t/sp-error.test:
  Auto merged
mysql-test/t/view.test:
  Auto merged
sql/sql_lex.cc:
  Auto merged
sql/sql_lex.h:
  Auto merged
sql/sql_view.cc:
  Auto merged
sql/sql_yacc.yy:
  Auto merged
mysql-test/r/sp-error.result:
  Manual merge.

           should fail to create

The problem was that this type of errors was checked during view
creation, which doesn't happen when CREATE VIEW is a statement of
a created stored routine.

The solution is to perform the checks at parse time.  The idea of the
fix is that the parser checks if a construction just parsed is allowed
in current circumstances by testing certain flags, and this flags are
reset for VIEWs.

The side effect of this change is that if the user already have
such bogus routines, it will now get a error when trying to do

  SHOW CREATE PROCEDURE proc;

(and some other) and when trying to execute such routine he will get

  ERROR 1457 (HY000): Failed to load routine test.p5. The table mysql.proc is missing, corrupt, or contains bad data (internal code -6)

However there should be very few such users (if any), and they may
(and should) drop these bogus routines.


mysql-test/r/sp-error.result:
  Add result for bug#20953: create proc with a create view that uses
  local vars/params should fail to create.
mysql-test/r/view.result:
  Update results.
mysql-test/t/sp-error.test:
  Add test case for bug#20953: create proc with a create view that uses
  local vars/params should fail to create.
mysql-test/t/view.test:
  Add second test for variable in a view.
  Remove SP variable in a view test, as it tests wrong behaviour.
  Add test for derived table in a view.
sql/sql_lex.cc:
  Remove LEX::variables_used.
sql/sql_lex.h:
  Remove LEX::variables_used and add st_parsing_options structure and
  LEX::parsing_options member.
sql/sql_view.cc:
  Move some error checking to sql/sql_yacc.yy.
sql/sql_yacc.yy:
  Check for disallowed syntax in a CREATE VIEW at parse time to rise a
  error when it is used inside CREATE PROCEDURE and CREATE FUNCTION, as
  well as by itself.

into  moonlight.intranet:/home/tomash/src/mysql_ab/mysql-5.0-bug21354

into  moonlight.intranet:/home/tomash/src/mysql_ab/mysql-5.0-bug19111

into  moonlight.intranet:/home/tomash/src/mysql_ab/mysql-5.0-bug21354


mysql-test/t/func_gconcat.test:
  Auto merged
sql/item_sum.cc:
  Auto merged
mysql-test/r/ps.result:
  Manual merge.
mysql-test/t/ps.test:
  Manual merge.

into  moonlight.intranet:/home/tomash/src/mysql_ab/mysql-5.0-bug21354


mysql-test/t/func_gconcat.test:
  Auto merged
sql/item_sum.cc:
  Auto merged
mysql-test/r/ps.result:
  Manual merge.
mysql-test/t/ps.test:
  Manual merge.
sql/item_sum.h:
  Manual merge.

           statement.

The problem was that during statement re-execution if the result was
empty the old result could be returned for group functions.

The solution is to implement proper cleanup() method in group
functions.


mysql-test/r/ps.result:
  Add result for bug#21354: (COUNT(*) = 1) not working in SELECT inside
  prepared statement.
mysql-test/t/func_gconcat.test:
  Add a comment that the test case is from bug#836.
mysql-test/t/ps.test:
  Add test case for bug#21354: (COUNT(*) = 1) not working in SELECT inside
  prepared statement.
sql/item_sum.cc:
  Call clear() in Item_sum_count::cleanup().
sql/item_sum.h:
  Add comments.
  Add proper cleanup() methods.
  Change Item_sum::no_rows_in_result() to call clear() instead of reset(),
  as the latter also issues add(), and there is nothing to add when there
  are no rows in result.

into  mysql.com:/home/cps/mysql/trees/mysql-5.0-virgin

into  moonlight.intranet:/home/tomash/src/mysql_ab/mysql-5.0-bug19111


sql/sql_base.cc:
  Auto merged
mysql-test/r/view.result:
  Manual merge.
mysql-test/t/view.test:
  Manual merge.

In a trigger or a function used in a statement it is possible to do
SELECT from a table being modified by the statement.  However,
encapsulation of such SELECT into a view and selecting from a view
instead of direct SELECT was not possible.

This happened because tables used by views (which in their turn
were used from functions/triggers) were not excluded from checks
in unique_table() routine as it happens for the rest of tables
added to the statement table list for prelocking.

With this fix we ignore all such tables in unique_table(), thus
providing consistency: inside a trigger or a functions SELECT from
a view may be used where plain SELECT is allowed.  Modification of
the same table from function or trigger is still disallowed.  Also,
this patch doesn't affect the case where SELECT from the table being
modified is done outside of function of trigger, such SELECTs are
still disallowed (this limitation and visibility problem when function
select from a table being modified are subjects of bug 21326).  See
also bug 22427.


mysql-test/r/view.result:
  Add result for bug#19111: TRIGGERs selecting from a VIEW on the
  firing base table fail.
mysql-test/t/view.test:
  Add test case for bug#19111: TRIGGERs selecting from a VIEW on the
  firing base table fail.
sql/sql_base.cc:
  In unique_table() do not check tables that are used in a stored
  function or a trigger ('prelocking_placeholder' is set).  If such
  function or a trigger will attempt to modify a table, the error will
  be given, however select is allowed there.

into  bodhi.local:/opt/local/work/mysql-5.0-runtime


mysql-test/mysql-test-run.pl:
  Auto merged

The syntax of the CALL statement, to invoke a stored procedure, has been
changed to make the use of parenthesis optional in the argument list.
With this change, "CALL p;" is equivalent to "CALL p();".

While the SQL spec does not explicitely mandate this syntax, supporting it
is needed for practical reasons, for integration with JDBC / ODBC connectors.

Also, warnings in the sql/sql_yacc.yy file, which were not reported by Bison 2.1
but are now reported by Bison 2.2, have been fixed.

The warning found were:
bison -y -p MYSQL  -d --debug --verbose sql_yacc.yy
sql_yacc.yy:653.9-18: warning: symbol UNLOCK_SYM redeclared
sql_yacc.yy:656.9-17: warning: symbol UNTIL_SYM redeclared
sql_yacc.yy:658.9-18: warning: symbol UPDATE_SYM redeclared
sql_yacc.yy:5169.11-5174.11: warning: unused value: $2
sql_yacc.yy:5208.11-5220.11: warning: unused value: $5
sql_yacc.yy:5221.11-5234.11: warning: unused value: $5
conflicts: 249 shift/reduce

"unused value: $2" correspond to the $$=$1 assignment in the 1st {} block
in table_ref -> join_table {} {},
which does not procude a result ($$) for the rule but an intermediate $2
value for the action instead.
"unused value: $5" are similar, with $$ assignments in {} actions blocks
which are not for the final reduce.


mysql-test/r/sp.result:
  New test case for Bug#21462
mysql-test/t/sp.test:
  New test case for Bug#21462
sql/sql_yacc.yy:
  "CALL p;" syntax for calling a stored procedure
  Fixed bison 2.2 warnings.

into  mysql.com:/home/svoj/devel/mysql/engines/mysql-5.0-engines

into  may.pils.ru:/home/svoj/devel/bk/mysql-5.0-engines


sql/sql_update.cc:
  Auto merged

into  mysql.com:/home/svoj/devel/mysql/BUG22937/mysql-5.0-engines


sql/ha_myisammrg.cc:
  Manual merge.

into  mysql.com:/home/svoj/devel/mysql/BUG22937/mysql-4.1-engines

This is addition to fix for bug21617. Valgrind reports an error when
opening merge table that has underlying tables with less indexes than
in a merge table itself.

Copy at most min(file->keys, table->key_parts) elements from rec_per_key array.
This fixes problems when merge table and subtables have different number of keys.


sql/ha_myisammrg.cc:
  Copy at most min(file->keys, table->key_parts) elements from rec_per_key array.
  This fixes problems when merge table and subtables have different number of keys.

into  mysql.com:/home/svoj/devel/mysql/BUG21381/mysql-5.0-engines


mysql-test/r/ndb_update.result:
  Auto merged
mysql-test/t/ndb_update.test:
  Auto merged

into  mysql.com:/home/svoj/devel/mysql/BUG10974/mysql-5.0-engines


BitKeeper/deleted/.del-errmsg.txt~f96b7055cac394e:
  Auto merged
mysql-test/r/merge.result:
  Manual merge.

Stop guardian and all the rest of threads before shutdown in case of an error


server-tools/instance-manager/instance_map.cc:
  flush_instances shouldn't reinit guardian, if it
  failed to load info about them
server-tools/instance-manager/manager.cc:
  On error we should 1) stop guardian 2) terminate all
  other threads and exit

into  mysql.com:/home/svoj/devel/mysql/BUG21381/mysql-5.0-engines


mysql-test/r/ndb_update.result:
  Auto merged
mysql-test/t/ndb_update.test:
  Auto merged
sql/sql_update.cc:
  Manual merge.

into  mysql.com:/home/svoj/devel/mysql/BUG21381/mysql-4.1-engines

Though this is not storage engine specific problem, I was able to
repeat this problem with BDB and NDB engines only. That was the
reason to add a test case into ndb_update.test. As a result
different bad things could happen.

BDB has removed duplicate rows which is not expected.
NDB returns an error.

For multi table update notify storage engine about UPDATE IGNORE
as it is done in single table UPDATE.


mysql-test/r/ndb_update.result:
  A test case for bug#21381.
mysql-test/t/ndb_update.test:
  A test case for bug#21381.
sql/sql_update.cc:
  For multi table update notify storage engine about UPDATE IGNORE
  as it is done in single table UPDATE.

into  mysql.com:/home/gluh/MySQL/Merge/5.0-kt

the option is useless on windows. It was removed from listing of
mysqlmanager --help on Windows


server-tools/instance-manager/options.cc:
  move socket_file_name under unix-specific define
server-tools/instance-manager/options.h:
  move socket_file_name under unix-specific define

into  neptunus.(none):/home/msvensson/mysql/mysql-5.0-maint


sql/item_func.cc:
  Auto merged
sql/log.cc:
  Auto merged
sql/set_var.cc:
  Auto merged
sql/sql_class.h:
  Auto merged

The problem was a race condition in a test case.

The fix eliminates the race condition by explicit
wait on UNIX socket to start accepting connections.

The patch affects only test suite (i.e. does not touch
server codebase).


mysql-test/mysql-test-run.pl:
  Expose necessary environment variables.
mysql-test/r/im_daemon_life_cycle.result:
  Update result file.
mysql-test/t/im_daemon_life_cycle.imtest:
  Wait for Instance Manager to start accepting connections
  after restart.
mysql-test/t/wait_for_socket.sh:
  Helper script: waits for UNIX socket to start accepting connections.

into  moonlight.intranet:/home/tomash/src/mysql_ab/mysql-5.0-real-bug21726-fix


sql/sql_select.cc:
  Auto merged

into  booka.:/home/alik/MySQL/devel/5.0-rt

into  moonlight.intranet:/home/tomash/src/mysql_ab/mysql-5.0-real-bug21726-fix


sql/sql_select.cc:
  Auto merged

into  neptunus.(none):/home/msvensson/mysql/mysql-5.0-maint


BitKeeper/etc/collapsed:
  auto-union
mysql-test/lib/mtr_process.pl:
  Auto merged
mysql-test/mysql-test-run.pl:
  Auto merged
mysql-test/r/ps.result:
  Auto merged
sql/mysql_priv.h:
  Auto merged
sql/opt_range.cc:
  Auto merged
sql/sql_acl.cc:
  Auto merged

into  neptunus.(none):/home/msvensson/mysql/mysql-5.0-maint


client/mysql.cc:
  Auto merged
include/m_ctype.h:
  Auto merged
mysql-test/r/ctype_utf8.result:
  Auto merged
mysql-test/r/strict.result:
  Auto merged
mysql-test/r/warnings.result:
  Auto merged
mysql-test/t/ctype_utf8.test:
  Auto merged
sql/field.cc:
  Auto merged
sql/item_func.cc:
  Auto merged