MDEV-16039 Crash when selecting virtual columns generated using functions with DAYNAME()

* Allocate items on thd->mem_root while refixing vcol exprs
* Make vcol tree changes register and roll them back after the statement is executed.

Explanation:
Due to collation implementation specifics an Item tree could change while fixing.
The tricky thing here is to make it on a proper arena.
It's usually not a problem when a field is deterministic, however, makes a pain vice-versa, during allocation allocating.
A non-deterministic field should be refixed on each statement, since it depends on the environment state.
Changing the tree will be temporary and therefore it should be reverted after the statement execution.

10 files changed, 195 insertions, 51 deletions

diff --git a/mysql-test/suite/gcol/r/gcol_bugfixes.result b/mysql-test/suite/gcol/r/gcol_bugfixes.result
index 9aff30aabc9..8eb7a9372b5 100644
--- a/mysql-test/suite/gcol/r/gcol_bugfixes.result
+++ b/mysql-test/suite/gcol/r/gcol_bugfixes.result
@@ -603,3 +603,69 @@ test	gcol_t1	sidea		NEVER	NULL
 test	gcol_t1	sideb		NEVER	NULL
 test	gcol_t1	sidec	VIRTUAL GENERATED	ALWAYS	sqrt(`sidea` * `sidea` + `sideb` * `sideb`)
 DROP TABLE gcol_t1;
+#
+# MDEV-16039 Crash when selecting virtual columns
+# generated using functions with DAYNAME()
+#
+CREATE TABLE t1 (
+suppliersenttoday INT NOT NULL,
+suppliercaptoday CHAR(10) AS (CONCAT('',DAYNAME('2020-02-05')))
+) COLLATE utf8_bin;
+INSERT INTO t1 (suppliersenttoday) VALUES (0);
+INSERT INTO t1 (suppliersenttoday) VALUES (0);
+SELECT * FROM t1;
+suppliersenttoday	suppliercaptoday
+0	Wednesday
+0	Wednesday
+PREPARE STMT FROM 'INSERT INTO t1 (suppliersenttoday) VALUES (1)';
+CREATE OR REPLACE TABLE t1 (
+suppliersenttoday INT NOT NULL,
+suppliercaptoday CHAR(10) AS (CONCAT('',DAYNAME('2020-02-05')))
+) COLLATE utf8_bin;
+EXECUTE STMT;
+EXECUTE STMT;
+SELECT * FROM t1;
+suppliersenttoday	suppliercaptoday
+1	Wednesday
+1	Wednesday
+DROP TABLE t1;
+# (duplicate) MDEV-20380 Server crash during update
+CREATE TABLE gafld (
+nuigafld INTEGER NOT NULL,
+ucrgafld VARCHAR(30) COLLATE UTF8_BIN NOT NULL
+DEFAULT SUBSTRING_INDEX(USER(),'@',1)
+);
+EXPLAIN UPDATE gafld SET nuigafld = 0 WHERE nuigafld = 10;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	gafld	ALL	NULL	NULL	NULL	NULL	1	Using where
+EXPLAIN UPDATE gafld SET nuigafld = 0 WHERE nuigafld = 10;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	gafld	ALL	NULL	NULL	NULL	NULL	1	Using where
+DROP TABLE gafld;
+# (duplicate) MDEV-17653 replace into generated columns is unstable
+# Some columns are snipped from the MDEV test
+CREATE TABLE t (
+c0 TIMESTAMP NOT NULL DEFAULT current_timestamp()
+ON UPDATE current_timestamp(),
+c1 DECIMAL(27,25) GENERATED ALWAYS AS (DAYOFMONTH('2020-02-05')),
+c4 TIME NOT NULL,
+c8 SMALLINT(6) GENERATED ALWAYS AS
+(CONCAT_WS(CONVERT(C1 USING CP932),
+'900') <> (c4 = 1)),
+PRIMARY KEY (c4)
+) DEFAULT CHARSET=latin1;
+REPLACE INTO t SET c0 = '2018-06-03 10:31:43', c4 = '02:58:55';
+REPLACE INTO t SET c0 = '2018-06-03 10:31:44', c4 = '02:58:55';
+REPLACE INTO t SET c0 = '2018-06-03 10:31:45', c4 = '02:58:55';
+DROP TABLE t;
+# (duplicate) MDEV-17986 crash when I insert on a table
+CREATE OR REPLACE TABLE t2 (
+number BIGINT(20) NOT NULL,
+lrn BIGINT(20) NOT NULL DEFAULT 0,
+source VARCHAR(15) NOT NULL
+DEFAULT (REVERSE(SUBSTRING_INDEX(REVERSE(user()), '@', 1))),
+PRIMARY KEY (number)
+);
+REPLACE t2(number) VALUES('1');
+REPLACE t2(number) VALUES('1');
+DROP TABLE t2;
diff --git a/mysql-test/suite/gcol/t/gcol_bugfixes.test b/mysql-test/suite/gcol/t/gcol_bugfixes.test
index 5563347a02a..033c430853d 100644
--- a/mysql-test/suite/gcol/t/gcol_bugfixes.test
+++ b/mysql-test/suite/gcol/t/gcol_bugfixes.test
@@ -564,3 +564,73 @@ SELECT table_schema,table_name,column_name,extra,is_generated,generation_express
 FROM information_schema.columns WHERE table_name='gcol_t1';
 
 DROP TABLE gcol_t1;
+
+--echo #
+--echo # MDEV-16039 Crash when selecting virtual columns
+--echo # generated using functions with DAYNAME()
+--echo #
+
+CREATE TABLE t1 (
+  suppliersenttoday INT NOT NULL,
+  suppliercaptoday CHAR(10) AS (CONCAT('',DAYNAME('2020-02-05')))
+) COLLATE utf8_bin;
+
+INSERT INTO t1 (suppliersenttoday) VALUES (0);
+INSERT INTO t1 (suppliersenttoday) VALUES (0);
+SELECT * FROM t1;
+
+PREPARE STMT FROM 'INSERT INTO t1 (suppliersenttoday) VALUES (1)';
+
+CREATE OR REPLACE TABLE t1 (
+  suppliersenttoday INT NOT NULL,
+  suppliercaptoday CHAR(10) AS (CONCAT('',DAYNAME('2020-02-05')))
+) COLLATE utf8_bin;
+
+EXECUTE STMT;
+EXECUTE STMT;
+SELECT * FROM t1;
+
+DROP TABLE t1;
+
+--echo # (duplicate) MDEV-20380 Server crash during update
+CREATE TABLE gafld (
+  nuigafld INTEGER NOT NULL,
+  ucrgafld VARCHAR(30) COLLATE UTF8_BIN NOT NULL
+           DEFAULT SUBSTRING_INDEX(USER(),'@',1)
+);
+EXPLAIN UPDATE gafld SET nuigafld = 0 WHERE nuigafld = 10;
+EXPLAIN UPDATE gafld SET nuigafld = 0 WHERE nuigafld = 10;
+DROP TABLE gafld;
+
+--echo # (duplicate) MDEV-17653 replace into generated columns is unstable
+--echo # Some columns are snipped from the MDEV test
+CREATE TABLE t (
+  c0 TIMESTAMP NOT NULL DEFAULT current_timestamp()
+                        ON UPDATE current_timestamp(),
+  c1 DECIMAL(27,25) GENERATED ALWAYS AS (DAYOFMONTH('2020-02-05')),
+  c4 TIME NOT NULL,
+  c8 SMALLINT(6) GENERATED ALWAYS AS
+                 (CONCAT_WS(CONVERT(C1 USING CP932),
+                            '900') <> (c4 = 1)),
+  PRIMARY KEY (c4)
+) DEFAULT CHARSET=latin1;
+
+REPLACE INTO t SET c0 = '2018-06-03 10:31:43', c4 = '02:58:55';
+REPLACE INTO t SET c0 = '2018-06-03 10:31:44', c4 = '02:58:55';
+REPLACE INTO t SET c0 = '2018-06-03 10:31:45', c4 = '02:58:55';
+
+DROP TABLE t;
+
+--echo # (duplicate) MDEV-17986 crash when I insert on a table
+CREATE OR REPLACE TABLE t2 (
+  number BIGINT(20) NOT NULL,
+  lrn BIGINT(20) NOT NULL DEFAULT 0,
+  source VARCHAR(15) NOT NULL
+                    DEFAULT (REVERSE(SUBSTRING_INDEX(REVERSE(user()), '@', 1))),
+  PRIMARY KEY (number)
+);
+
+REPLACE t2(number) VALUES('1');
+REPLACE t2(number) VALUES('1');
+
+DROP TABLE t2;
diff --git a/sql/item.cc b/sql/item.cc
index 9451d4203ca..644bef7524a 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -2325,14 +2325,7 @@ bool Item_func_or_sum::agg_item_set_converter(const DTCollation &coll,
   bool res= FALSE;
   uint i;
 
-  /*
-    In case we're in statement prepare, create conversion item
-    in its memory: it will be reused on each execute.
-  */
-  Query_arena backup;
-  Query_arena *arena= thd->stmt_arena->is_stmt_prepare() ?
-                      thd->activate_stmt_arena_if_needed(&backup) :
-                      NULL;
+  DBUG_ASSERT(!thd->stmt_arena->is_stmt_prepare());
 
   for (i= 0, arg= args; i < nargs; i++, arg+= item_sep)
   {
@@ -2354,20 +2347,8 @@ bool Item_func_or_sum::agg_item_set_converter(const DTCollation &coll,
       res= TRUE;
       break; // we cannot return here, we need to restore "arena".
     }
-    /*
-      If in statement prepare, then we create a converter for two
-      constant items, do it once and then reuse it.
-      If we're in execution of a prepared statement, arena is NULL,
-      and the conv was created in runtime memory. This can be
-      the case only if the argument is a parameter marker ('?'),
-      because for all true constants the charset converter has already
-      been created in prepare. In this case register the change for
-      rollback.
-    */
-    if (thd->stmt_arena->is_stmt_prepare())
-      *arg= conv;
-    else
-      thd->change_item_tree(arg, conv);
+
+    thd->change_item_tree(arg, conv);
 
     if (conv->fix_fields(thd, arg))
     {
@@ -2375,8 +2356,6 @@ bool Item_func_or_sum::agg_item_set_converter(const DTCollation &coll,
       break; // we cannot return here, we need to restore "arena".
     }
   }
-  if (arena)
-    thd->restore_active_arena(arena, &backup);
   return res;
 }
 
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 436f753557e..674f6db8358 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -4961,6 +4961,24 @@ static void mark_real_tables_as_free_for_reuse(TABLE_LIST *table_list)
     }
 }
 
+int TABLE::fix_vcol_exprs(THD *thd)
+{
+  for (Field **vf= vfield; vf && *vf; vf++)
+    if (fix_session_vcol_expr(thd, (*vf)->vcol_info))
+      return 1;
+
+  for (Field **df= default_field; df && *df; df++)
+    if ((*df)->default_value &&
+        fix_session_vcol_expr(thd, (*df)->default_value))
+      return 1;
+
+  for (Virtual_column_info **cc= check_constraints; cc && *cc; cc++)
+    if (fix_session_vcol_expr(thd, (*cc)))
+      return 1;
+
+  return 0;
+}
+
 
 static bool fix_all_session_vcol_exprs(THD *thd, TABLE_LIST *tables)
 {
@@ -4968,36 +4986,27 @@ static bool fix_all_session_vcol_exprs(THD *thd, TABLE_LIST *tables)
   TABLE_LIST *first_not_own= thd->lex->first_not_own_table();
   DBUG_ENTER("fix_session_vcol_expr");
 
-  for (TABLE_LIST *table= tables; table && table != first_not_own;
+  int error= 0;
+  for (TABLE_LIST *table= tables; table && table != first_not_own && !error;
        table= table->next_global)
   {
     TABLE *t= table->table;
     if (!table->placeholder() && t->s->vcols_need_refixing &&
          table->lock_type >= TL_WRITE_ALLOW_WRITE)
     {
+      Query_arena *stmt_backup= thd->stmt_arena;
+      if (thd->stmt_arena->is_conventional())
+        thd->stmt_arena= t->expr_arena;
       if (table->security_ctx)
         thd->security_ctx= table->security_ctx;
 
-      for (Field **vf= t->vfield; vf && *vf; vf++)
-        if (fix_session_vcol_expr(thd, (*vf)->vcol_info))
-          goto err;
-
-      for (Field **df= t->default_field; df && *df; df++)
-        if ((*df)->default_value &&
-            fix_session_vcol_expr(thd, (*df)->default_value))
-          goto err;
-
-      for (Virtual_column_info **cc= t->check_constraints; cc && *cc; cc++)
-        if (fix_session_vcol_expr(thd, (*cc)))
-          goto err;
+      error= t->fix_vcol_exprs(thd);
 
       thd->security_ctx= save_security_ctx;
+      thd->stmt_arena= stmt_backup;
     }
   }
-  DBUG_RETURN(0);
-err:
-  thd->security_ctx= save_security_ctx;
-  DBUG_RETURN(1);
+  DBUG_RETURN(error);
 }
 
 
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 655824d93fe..69bfbac6920 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -3482,7 +3482,7 @@ void select_dumpvar::cleanup()
 Query_arena::Type Query_arena::type() const
 {
   DBUG_ASSERT(0); /* Should never be called */
-  return STATEMENT;
+  return Type::STATEMENT;
 }
 
 
@@ -3535,7 +3535,7 @@ Statement::Statement(LEX *lex_arg, MEM_ROOT *mem_root_arg,
 
 Query_arena::Type Statement::type() const
 {
-  return STATEMENT;
+  return Type::STATEMENT;
 }
 
 
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 838998af94f..c606e3ddca0 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -951,9 +951,9 @@ public:
   enum_state state;
 
   /* We build without RTTI, so dynamic_cast can't be used. */
-  enum Type
+  enum class Type
   {
-    STATEMENT, PREPARED_STATEMENT, STORED_PROCEDURE
+    STATEMENT, PREPARED_STATEMENT, STORED_PROCEDURE, TABLE
   };
 
   Query_arena(MEM_ROOT *mem_root_arg, enum enum_state state_arg) :
@@ -3654,13 +3654,20 @@ public:
     return 0;
   }
 
+
+  bool is_item_tree_change_register_required()
+  {
+    return !stmt_arena->is_conventional()
+           || stmt_arena->type() == Query_arena::Type::TABLE;
+  }
+
   void change_item_tree(Item **place, Item *new_value)
   {
     DBUG_ENTER("THD::change_item_tree");
     DBUG_PRINT("enter", ("Register: %p (%p) <- %p",
                        *place, place, new_value));
     /* TODO: check for OOM condition here */
-    if (!stmt_arena->is_conventional())
+    if (is_item_tree_change_register_required())
       nocheck_register_item_tree_change(place, *place, mem_root);
     *place= new_value;
     DBUG_VOID_RETURN;
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 2879e394877..543c877b7f1 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -7751,8 +7751,8 @@ void mysql_parse(THD *thd, char *rawbuf, uint length,
     sp_cache_enforce_limit(thd->sp_proc_cache, stored_program_cache_size);
     sp_cache_enforce_limit(thd->sp_func_cache, stored_program_cache_size);
     thd->end_statement();
+    thd->Item_change_list::rollback_item_tree_changes();
     thd->cleanup_after_query();
-    DBUG_ASSERT(thd->Item_change_list::is_empty());
   }
   else
   {
diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc
index f0c9f818f87..b2ee5abd8b6 100644
--- a/sql/sql_prepare.cc
+++ b/sql/sql_prepare.cc
@@ -338,7 +338,7 @@ find_prepared_statement(THD *thd, ulong id)
                     thd->last_stmt :
                     thd->stmt_map.find(id));
 
-  if (stmt == 0 || stmt->type() != Query_arena::PREPARED_STATEMENT)
+  if (stmt == 0 || stmt->type() != Query_arena::Type::PREPARED_STATEMENT)
     return NULL;
 
   return (Prepared_statement *) stmt;
@@ -3893,7 +3893,7 @@ Prepared_statement::~Prepared_statement()
 
 Query_arena::Type Prepared_statement::type() const
 {
-  return PREPARED_STATEMENT;
+  return Type::PREPARED_STATEMENT;
 }
 
 
diff --git a/sql/table.cc b/sql/table.cc
index 5ba996b746d..6e8c9aab12e 100644
--- a/sql/table.cc
+++ b/sql/table.cc
@@ -47,6 +47,17 @@
 #define MYSQL57_GENERATED_FIELD 128
 #define MYSQL57_GCOL_HEADER_SIZE 4
 
+class Table_arena: public Query_arena
+{
+public:
+  Table_arena(MEM_ROOT *mem_root, enum enum_state state_arg) :
+          Query_arena(mem_root, state_arg){}
+  virtual Type type() const
+  {
+    return Type::TABLE;
+  }
+};
+
 static Virtual_column_info * unpack_vcol_info_from_frm(THD *, MEM_ROOT *,
               TABLE *, String *, Virtual_column_info **, bool *);
 static bool check_vcol_forward_refs(Field *, Virtual_column_info *);
@@ -1020,8 +1031,9 @@ bool parse_vcol_defs(THD *thd, MEM_ROOT *mem_root, TABLE *table,
     We need to use CONVENTIONAL_EXECUTION here to ensure that
     any new items created by fix_fields() are not reverted.
   */
-  table->expr_arena= new (alloc_root(mem_root, sizeof(Query_arena)))
-                        Query_arena(mem_root, Query_arena::STMT_CONVENTIONAL_EXECUTION);
+  table->expr_arena= new (alloc_root(mem_root, sizeof(Table_arena)))
+                        Table_arena(mem_root, 
+                                    Query_arena::STMT_CONVENTIONAL_EXECUTION);
   if (!table->expr_arena)
     DBUG_RETURN(1);
 
diff --git a/sql/table.h b/sql/table.h
index 90a85b9b07e..f3a7f278604 100644
--- a/sql/table.h
+++ b/sql/table.h
@@ -1559,6 +1559,7 @@ public:
                                       TABLE *tmp_table,
                                       TMP_TABLE_PARAM *tmp_table_param,
                                       bool with_cleanup);
+  int fix_vcol_exprs(THD *thd);
 };