Skip to content

Commit

Permalink
MDEV-22805: SIGSEGV in check_fields on UPDATE
Browse files Browse the repository at this point in the history
For debug build of MariaDB server running of the following test case
will hit the assert `thd->lex->sql_command == SQLCOM_UPDATE' in the function
check_fields() on attempt to execute the UPDATE statement.

  CREATE TABLE t1 (a INT);
  UPDATE t1 FOR PORTION OF APPTIME FROM (SELECT 1 FROM t1) TO 2 SET a = 1;

Stack trace to the fired assert statement
  DBUG_ASSERT(thd->lex->sql_command == SQLCOM_UPDATE)
listed below:
  mysql_execute_command() ->
    mysql_multi_update_prepare() -->
      Multiupdate_prelocking_strategy::handle_end() -->
        check_fiels()

It's worth to note that this stack trace looks like a multi update
statement is being executed. The fired assert is checked inside the
function check_fields() in case table->has_period() returns the value
true that in turns happens when temporal period specified in the UPDATE
statement. Condition specified in the DEBUG_ASSERT statement returns
the false value since the data member thd->lex->sql_command have the
value SQLCOM_UPDATE_MULTI. So, the main question is why a program control
flow go to the path prescribed for handling MULTI update statement
despite of the fact that the ordinary UPDATE statement being executed.

The answer is a way that SQL grammar rules written.

When the statement
  UPDATE t1 FOR PORTION OF APPTIME FROM (SELECT 1 FROM t1) TO 2 SET a = 1;
being parsed an action for the rule 'table_primary_ident' (part of this action
is listed below to simplify description) is  invoked to handle the table
name 't1' specified in the clause 'SELECT 1 FROM t1'.

table_primary_ident:
  table_ident opt_use_partition opt_for_system_time_clause
  opt_table_alias_clause opt_key_definition
  {
    SELECT_LEX *sel= Select;
    sel->table_join_options= 0;
    if (!($$= Select->add_table_to_list(thd, $1, $4,

This action calls the method st_select_lex::add_table_to_list()
to add the table name 't1' to the list of tables being used by the statement.

Later, an action for the following grammar rule
update_table_list:
  table_ident opt_use_partition for_portion_of_time_clause
  opt_table_alias_clause opt_key_definition
  {
    SELECT_LEX *sel= Select;
    sel->table_join_options= 0;
    if (!($$= Select->add_table_to_list(thd, $1, $4,

is invoked to handle the clause 't1 FOR PORTION OF APPTIME FROM ... TO 2'.
This action also calls the method st_select_lex::add_table_to_list()
to add the table name 't1' to the list of tables being used by the statement.

In result the table name 't1' contained twice in this list.

Presence of duplicate names for the table 't1' in a list of table used by
a statement leads to the fact that the function unique_table() called
from the function mysql_update() returns the value true that forces
implementation of the function mysql_update() to return the value 2 as
a signal to fall through the case boundary of the switch statement placed
in the function mysql_execute_statement() and start handling of the case
for sql_command SQLCOM_UPDATE_MULTI. The compound statement block for the
case SQLCOM_UPDATE_MULTI invokes the function mysql_multi_update_prepare()
that executes the statement
  set thd->lex->sql_command= SQLCOM_UPDATE_MULTI;
and after that calls the method
  Multiupdate_prelocking_strategy::handle_end(). Finally, this method
invokes the check_field() function and assert is fired.

The above analysis shows that update for a table that simultaneously specified
both as a destination table of UPDATE statement and as a table taking part in
subquery is actually treated by MariaDB server as multi-update statement.
Taking into account that multi-update statement for temporal period
table is not supported yet by MariaDB, correct way to fix the bug is to return
the error ER_NOT_SUPPORTED_YET for this case.
  • Loading branch information
dmitryshulga committed Oct 27, 2020
1 parent 31cde27 commit 97b10b7
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 0 deletions.
12 changes: 12 additions & 0 deletions mysql-test/suite/period/r/update.result
Original file line number Diff line number Diff line change
Expand Up @@ -292,3 +292,15 @@ Warning 1264 Out of range value for column 'id' at row 1
Warning 1264 Out of range value for column 'id' at row 2
update ignore t1 for portion of p from '1995-07-06' to '2009-01-12' set f = 1;
drop table t1;
#
# MDEV-22805 SIGSEGV in check_fields on UPDATE (optimized builds) | Assertion `thd->lex->sql_command == SQLCOM_UPDATE' failed.
#
CREATE TABLE t1 (a INT, b DATE, c DATE, PERIOD FOR APPTIME(b, c));
INSERT INTO t1 VALUES(1, '1999-01-01', '2018-12-12');
UPDATE t1 FOR PORTION OF APPTIME FROM (SELECT '1999-01-01' FROM t1 WHERE a=2) TO '2018-01-01' SET a = 100;
ERROR 42000: This version of MariaDB doesn't yet support 'updating and querying the same temporal periods table'
CREATE VIEW v1 AS SELECT * FROM t1;
UPDATE v1 FOR PORTION OF APPTIME FROM (SELECT '1999-01-01' FROM t1 WHERE a=2) TO '2018-01-01' SET a = 100;
ERROR 42S02: 'v1' is a view
DROP VIEW v1;
DROP TABLE t1;
18 changes: 18 additions & 0 deletions mysql-test/suite/period/t/update.test
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,21 @@ update ignore t1 set id = 2429681664;
update ignore t1 for portion of p from '1995-07-06' to '2009-01-12' set f = 1;

drop table t1;

--echo #
--echo # MDEV-22805 SIGSEGV in check_fields on UPDATE (optimized builds) | Assertion `thd->lex->sql_command == SQLCOM_UPDATE' failed.
--echo #
CREATE TABLE t1 (a INT, b DATE, c DATE, PERIOD FOR APPTIME(b, c));

INSERT INTO t1 VALUES(1, '1999-01-01', '2018-12-12');

# Without a patch the following statement crashs a server built in debug mode
--error ER_NOT_SUPPORTED_YET
UPDATE t1 FOR PORTION OF APPTIME FROM (SELECT '1999-01-01' FROM t1 WHERE a=2) TO '2018-01-01' SET a = 100;

CREATE VIEW v1 AS SELECT * FROM t1;
--error ER_IT_IS_A_VIEW
UPDATE v1 FOR PORTION OF APPTIME FROM (SELECT '1999-01-01' FROM t1 WHERE a=2) TO '2018-01-01' SET a = 100;

DROP VIEW v1;
DROP TABLE t1;
5 changes: 5 additions & 0 deletions sql/sql_parse.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4373,6 +4373,11 @@ mysql_execute_command(THD *thd)
/* mysql_update return 2 if we need to switch to multi-update */
if (up_result != 2)
break;
if (thd->lex->period_conditions.is_set())
{
DBUG_ASSERT(0); // Should never happen
goto error;
}
}
/* fall through */
case SQLCOM_UPDATE_MULTI:
Expand Down
8 changes: 8 additions & 0 deletions sql/sql_update.cc
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,14 @@ int mysql_update(THD *thd,
DBUG_PRINT("info", ("Switch to multi-update"));
/* pass counter value */
thd->lex->table_count= table_count;
if (thd->lex->period_conditions.is_set())
{
my_error(ER_NOT_SUPPORTED_YET, MYF(0),
"updating and querying the same temporal periods table");

DBUG_RETURN(1);
}

/* convert to multiupdate */
DBUG_RETURN(2);
}
Expand Down

0 comments on commit 97b10b7

Please sign in to comment.