MDEV-24176 Preparations

1. moved fix_vcol_exprs() call to open_table()

mysql_alter_table() doesn't do lock_tables() so it cannot win from
fix_vcol_exprs() from there. Tests affected: main.default_session

2. Vanilla cleanups and comments.

sql/field.h

@@ -616,6 +616,10 @@ public:
   {
     in_partitioning_expr= TRUE;
   }
+  bool fix_expr(THD *thd);
+  bool fix_session_expr(THD *thd);
+  bool fix_session_expr_for_read(THD *thd, Field *field);
+  bool fix_and_check_expr(THD *thd, TABLE *table);
   inline bool is_equal(const Virtual_column_info* vcol) const;
   inline void print(String*);
 };

sql/item.cc

@@ -6376,8 +6376,9 @@ bool Item_field::fix_fields(THD *thd, Item **reference)
   }
 #endif
   fixed= 1;
-  if (field->vcol_info)
+  if (field->vcol_info &&
-    fix_session_vcol_expr_for_read(thd, field, field->vcol_info);
+      field->vcol_info->fix_session_expr_for_read(thd, field))
+    goto error;
   if (thd->variables.sql_mode & MODE_ONLY_FULL_GROUP_BY &&
       !outer_fixed && !thd->lex->in_sum_func &&
       select &&
@@ -9503,7 +9504,8 @@ bool Item_default_value::fix_fields(THD *thd, Item **items)
       Even if DEFAULT() do not read tables fields, the default value
       expression can do it.
     */
-    fix_session_vcol_expr_for_read(thd, def_field, def_field->default_value);
+    if (def_field->default_value->fix_session_expr_for_read(thd, def_field))
+      goto error;
     if (should_mark_column(thd->column_usage))
       def_field->default_value->expr->update_used_tables();
     def_field->move_field(newptr+1, def_field->maybe_null() ? newptr : 0, 1);

sql/item.h

@@ -2924,6 +2924,12 @@ public:
   const char *db_name;
   const char *table_name;
   LEX_CSTRING field_name;
+  /*
+     NOTE: came from TABLE::alias_name_used and this is only a hint! It works
+     only in need_correct_ident() condition. On other cases it is FALSE even if
+     table_name is alias! It cannot be TRUE in these cases, lots of spaghetti
+     logic depends on that.
+  */
   bool alias_name_used; /* true if item was resolved against alias */
   /* 
     Cached value of index for this field in table->field array, used by prep. 

sql/sql_base.cc

@@ -2040,6 +2040,9 @@ retry_share:
   table_list->updatable= 1; // It is not derived table nor non-updatable VIEW
   table_list->table= table;
 
+  if (table->vcol_fix_exprs(thd))
+    goto err_lock;
+
 #ifdef WITH_PARTITION_STORAGE_ENGINE
   if (unlikely(table->part_info))
   {
@@ -5290,52 +5293,44 @@ static void mark_real_tables_as_free_for_reuse(TABLE_LIST *table_list)
     }
 }
 
-int TABLE::fix_vcol_exprs(THD *thd)
+bool TABLE::vcol_fix_exprs(THD *thd)
 {
+  if (pos_in_table_list->placeholder() || !s->vcols_need_refixing ||
+      pos_in_table_list->lock_type < TL_WRITE_ALLOW_WRITE)
+    return false;
+
+  DBUG_ASSERT(pos_in_table_list != thd->lex->first_not_own_table());
+
+  bool result= true;
+  Security_context *save_security_ctx= thd->security_ctx;
+  Query_arena *stmt_backup= thd->stmt_arena;
+  if (thd->stmt_arena->is_conventional())
+    thd->stmt_arena= expr_arena;
+
+  if (pos_in_table_list->security_ctx)
+    thd->security_ctx= pos_in_table_list->security_ctx;
+
+
   for (Field **vf= vfield; vf && *vf; vf++)
-    if (fix_session_vcol_expr(thd, (*vf)->vcol_info))
+    if ((*vf)->vcol_info->fix_session_expr(thd))
-      return 1;
+      goto end;
 
   for (Field **df= default_field; df && *df; df++)
     if ((*df)->default_value &&
-        fix_session_vcol_expr(thd, (*df)->default_value))
+        (*df)->default_value->fix_session_expr(thd))
-      return 1;
+      goto end;
 
   for (Virtual_column_info **cc= check_constraints; cc && *cc; cc++)
-    if (fix_session_vcol_expr(thd, (*cc)))
+    if ((*cc)->fix_session_expr(thd))
-      return 1;
+      goto end;
 
-  return 0;
+  result= false;
-}
-
-
-static bool fix_all_session_vcol_exprs(THD *thd, TABLE_LIST *tables)
-{
-  Security_context *save_security_ctx= thd->security_ctx;
-  TABLE_LIST *first_not_own= thd->lex->first_not_own_table();
-  DBUG_ENTER("fix_session_vcol_expr");
-
-  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;
-
-      error= t->fix_vcol_exprs(thd);
 
+end:
   thd->security_ctx= save_security_ctx;
   thd->stmt_arena= stmt_backup;
-    }
+
-  }
+  return result;
-  DBUG_RETURN(error);
 }
 
 
@@ -5500,9 +5495,7 @@ bool lock_tables(THD *thd, TABLE_LIST *tables, uint count,
     }
   }
 
-  bool res= fix_all_session_vcol_exprs(thd, tables);
+  const bool res= thd->decide_logging_format(tables);
-  if (!res)
-    res= thd->decide_logging_format(tables);
 
   DBUG_RETURN(res);
 }

sql/sql_partition.cc

@@ -5548,7 +5548,6 @@ that are reorganised.
         my_error(ER_ROW_IS_REFERENCED, MYF(0));
         goto err;
       }
-      tab_part_info->num_parts-= num_parts_dropped;
     }
     else if (alter_info->partition_flags & ALTER_PARTITION_REBUILD)
     {
@@ -6214,8 +6213,6 @@ static bool mysql_drop_partitions(ALTER_PARTITION_PARAM_TYPE *lpt)
   char path[FN_REFLEN+1];
   partition_info *part_info= lpt->table->part_info;
   List_iterator<partition_element> part_it(part_info->partitions);
-  uint i= 0;
-  uint remove_count= 0;
   int error;
   DBUG_ENTER("mysql_drop_partitions");
 
@@ -6230,16 +6227,6 @@ static bool mysql_drop_partitions(ALTER_PARTITION_PARAM_TYPE *lpt)
     lpt->table->file->print_error(error, MYF(0));
     DBUG_RETURN(TRUE);
   }
-  do
-  {
-    partition_element *part_elem= part_it++;
-    if (part_elem->part_state == PART_IS_DROPPED)
-    {
-      part_it.remove();
-      remove_count++;
-    }
-  } while (++i < part_info->num_parts);
-  part_info->num_parts-= remove_count;
   DBUG_RETURN(FALSE);
 }
 

sql/table.cc

@@ -60,10 +60,8 @@ public:
   }
 };
 
-static Virtual_column_info * unpack_vcol_info_from_frm(THD *, MEM_ROOT *,
+static Virtual_column_info * unpack_vcol_info_from_frm(THD *,
               TABLE *, String *, Virtual_column_info **, bool *);
-static bool check_vcol_forward_refs(Field *, Virtual_column_info *,
-                                    bool check_constraint);
 
 /* INFORMATION_SCHEMA name */
 LEX_CSTRING INFORMATION_SCHEMA_NAME= {STRING_WITH_LEN("information_schema")};
@@ -1015,6 +1013,31 @@ static void mysql57_calculate_null_position(TABLE_SHARE *share,
 bool parse_vcol_defs(THD *thd, MEM_ROOT *mem_root, TABLE *table,
                      bool *error_reported, vcol_init_mode mode)
 {
+  struct check_vcol_forward_refs
+  {
+    static bool check(Field *field, Virtual_column_info *vcol)
+    {
+      return vcol &&
+             vcol->expr->walk(&Item::check_field_expression_processor, 0, field);
+    }
+    static bool check_constraint(Field *field, Virtual_column_info *vcol)
+    {
+      uint32 flags= field->flags;
+      /* Check constraints can refer it itself */
+      field->flags|= NO_DEFAULT_VALUE_FLAG;
+      const bool res= check(field, vcol);
+      field->flags= flags;
+      return res;
+    }
+    static bool check(Field *field)
+    {
+      if (check(field, field->vcol_info) ||
+          check_constraint(field, field->check_constraint) ||
+          check(field, field->default_value))
+        return true;
+      return false;
+    }
+  };
   CHARSET_INFO *save_character_set_client= thd->variables.character_set_client;
   CHARSET_INFO *save_collation= thd->variables.collation_connection;
   Query_arena  *backup_stmt_arena_ptr= thd->stmt_arena;
@@ -1095,7 +1118,7 @@ bool parse_vcol_defs(THD *thd, MEM_ROOT *mem_root, TABLE *table,
     switch (type) {
     case VCOL_GENERATED_VIRTUAL:
     case VCOL_GENERATED_STORED:
-      vcol= unpack_vcol_info_from_frm(thd, mem_root, table, &expr_str,
+      vcol= unpack_vcol_info_from_frm(thd, table, &expr_str,
                                     &((*field_ptr)->vcol_info), error_reported);
       *(vfield_ptr++)= *field_ptr;
       if (vcol && field_ptr[0]->check_vcol_sql_mode_dependency(thd, mode))
@@ -1106,7 +1129,7 @@ bool parse_vcol_defs(THD *thd, MEM_ROOT *mem_root, TABLE *table,
       }
       break;
     case VCOL_DEFAULT:
-      vcol= unpack_vcol_info_from_frm(thd, mem_root, table, &expr_str,
+      vcol= unpack_vcol_info_from_frm(thd, table, &expr_str,
                                       &((*field_ptr)->default_value),
                                       error_reported);
       *(dfield_ptr++)= *field_ptr;
@@ -1114,13 +1137,13 @@ bool parse_vcol_defs(THD *thd, MEM_ROOT *mem_root, TABLE *table,
         table->s->non_determinstic_insert= true;
       break;
     case VCOL_CHECK_FIELD:
-      vcol= unpack_vcol_info_from_frm(thd, mem_root, table, &expr_str,
+      vcol= unpack_vcol_info_from_frm(thd, table, &expr_str,
                                       &((*field_ptr)->check_constraint),
                                       error_reported);
       *check_constraint_ptr++= (*field_ptr)->check_constraint;
       break;
     case VCOL_CHECK_TABLE:
-      vcol= unpack_vcol_info_from_frm(thd, mem_root, table, &expr_str,
+      vcol= unpack_vcol_info_from_frm(thd, table, &expr_str,
                                       check_constraint_ptr, error_reported);
       check_constraint_ptr++;
       break;
@@ -1140,7 +1163,7 @@ bool parse_vcol_defs(THD *thd, MEM_ROOT *mem_root, TABLE *table,
       expr_str.append(STRING_WITH_LEN("current_timestamp("));
       expr_str.append_ulonglong(field->decimals());
       expr_str.append(')');
-      vcol= unpack_vcol_info_from_frm(thd, mem_root, table, &expr_str,
+      vcol= unpack_vcol_info_from_frm(thd, table, &expr_str,
                                       &((*field_ptr)->default_value),
                                       error_reported);
       *(dfield_ptr++)= *field_ptr;
@@ -1162,16 +1185,11 @@ bool parse_vcol_defs(THD *thd, MEM_ROOT *mem_root, TABLE *table,
 
   /* Check that expressions aren't referring to not yet initialized fields */
   for (field_ptr= table->field; *field_ptr; field_ptr++)
-  {
+    if (check_vcol_forward_refs::check(*field_ptr))
-    Field *field= *field_ptr;
-    if (check_vcol_forward_refs(field, field->vcol_info, 0) ||
-        check_vcol_forward_refs(field, field->check_constraint, 1) ||
-        check_vcol_forward_refs(field, field->default_value, 0))
     {
       *error_reported= true;
       goto end;
     }
-  }
 
   res=0;
 end:
@@ -2943,21 +2961,21 @@ void TABLE_SHARE::free_frm_image(const uchar *frm)
 }
 
 
-static bool fix_vcol_expr(THD *thd, Virtual_column_info *vcol)
+bool Virtual_column_info::fix_expr(THD *thd)
 {
   DBUG_ENTER("fix_vcol_expr");
 
   const enum enum_column_usage saved_column_usage= thd->column_usage;
   thd->column_usage= COLUMNS_WRITE;
 
-  int error= vcol->expr->fix_fields(thd, &vcol->expr);
+  int error= expr->fix_fields(thd, &expr);
 
   thd->column_usage= saved_column_usage;
 
   if (unlikely(error))
   {
     StringBuffer<MAX_FIELD_WIDTH> str;
-    vcol->print(&str);
+    print(&str);
     my_error(ER_ERROR_EVALUATING_EXPRESSION, MYF(0), str.c_ptr_safe());
     DBUG_RETURN(1);
   }
@@ -2970,15 +2988,15 @@ static bool fix_vcol_expr(THD *thd, Virtual_column_info *vcol)
     @note this is done for all vcols for INSERT/UPDATE/DELETE,
     and only as needed for SELECTs.
 */
-bool fix_session_vcol_expr(THD *thd, Virtual_column_info *vcol)
+bool Virtual_column_info::fix_session_expr(THD *thd)
 {
   DBUG_ENTER("fix_session_vcol_expr");
-  if (!(vcol->flags & (VCOL_TIME_FUNC|VCOL_SESSION_FUNC)))
+  if (!(flags & (VCOL_TIME_FUNC|VCOL_SESSION_FUNC)))
     DBUG_RETURN(0);
 
-  vcol->expr->walk(&Item::cleanup_excluding_fields_processor, 0, 0);
+  expr->walk(&Item::cleanup_excluding_fields_processor, 0, 0);
-  DBUG_ASSERT(!vcol->expr->fixed);
+  DBUG_ASSERT(!expr->fixed);
-  DBUG_RETURN(fix_vcol_expr(thd, vcol));
+  DBUG_RETURN(fix_expr(thd));
 }
 
 
@@ -2987,8 +3005,7 @@ bool fix_session_vcol_expr(THD *thd, Virtual_column_info *vcol)
     @note this is called for generated column or a DEFAULT expression from
     their corresponding fix_fields on SELECT.
 */
-bool fix_session_vcol_expr_for_read(THD *thd, Field *field,
+bool Virtual_column_info::fix_session_expr_for_read(THD *thd, Field *field)
-                                    Virtual_column_info *vcol)
 {
   DBUG_ENTER("fix_session_vcol_expr_for_read");
   TABLE_LIST *tl= field->table->pos_in_table_list;
@@ -2997,7 +3014,7 @@ bool fix_session_vcol_expr_for_read(THD *thd, Field *field,
   Security_context *save_security_ctx= thd->security_ctx;
   if (tl->security_ctx)
     thd->security_ctx= tl->security_ctx;
-  bool res= fix_session_vcol_expr(thd, vcol);
+  bool res= fix_session_expr(thd);
   thd->security_ctx= save_security_ctx;
   DBUG_RETURN(res);
 }
@@ -3024,28 +3041,24 @@ bool fix_session_vcol_expr_for_read(THD *thd, Field *field,
     FALSE          Otherwise
 */
 
-static bool fix_and_check_vcol_expr(THD *thd, TABLE *table,
+bool Virtual_column_info::fix_and_check_expr(THD *thd, TABLE *table)
-                                    Virtual_column_info *vcol)
 {
-  Item* func_expr= vcol->expr;
   DBUG_ENTER("fix_and_check_vcol_expr");
-  DBUG_PRINT("info", ("vcol: %p", vcol));
+  DBUG_PRINT("info", ("vcol: %p", this));
-  DBUG_ASSERT(func_expr);
+  DBUG_ASSERT(expr);
 
-  if (func_expr->fixed)
+  if (expr->fixed)
     DBUG_RETURN(0); // nothing to do
 
-  if (fix_vcol_expr(thd, vcol))
+  if (fix_expr(thd))
     DBUG_RETURN(1);
 
-  if (vcol->flags)
+  if (flags)
     DBUG_RETURN(0); // already checked, no need to do it again
 
-  /* fix_fields could've changed the expression */
-  func_expr= vcol->expr;
 
   /* this was checked in check_expression(), but the frm could be mangled... */
-  if (unlikely(func_expr->result_type() == ROW_RESULT))
+  if (unlikely(expr->result_type() == ROW_RESULT))
   {
     my_error(ER_OPERAND_COLUMNS, MYF(0), 1);
     DBUG_RETURN(1);
@@ -3058,12 +3071,12 @@ static bool fix_and_check_vcol_expr(THD *thd, TABLE *table,
   Item::vcol_func_processor_result res;
   res.errors= 0;
 
-  int error= func_expr->walk(&Item::check_vcol_func_processor, 0, &res);
+  int error= expr->walk(&Item::check_vcol_func_processor, 0, &res);
   if (unlikely(error || (res.errors & VCOL_IMPOSSIBLE)))
   {
     // this can only happen if the frm was corrupted
     my_error(ER_VIRTUAL_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(0), res.name,
-             vcol->get_vcol_type_name(), vcol->name.str);
+             get_vcol_type_name(), name.str);
     DBUG_RETURN(1);
   }
   else if (unlikely(res.errors & VCOL_AUTO_INC))
@@ -3078,13 +3091,13 @@ static bool fix_and_check_vcol_expr(THD *thd, TABLE *table,
     */
     myf warn= table->s->frm_version < FRM_VER_EXPRESSSIONS ? ME_JUST_WARNING : 0;
     my_error(ER_VIRTUAL_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(warn),
-             "AUTO_INCREMENT", vcol->get_vcol_type_name(), res.name);
+             "AUTO_INCREMENT", get_vcol_type_name(), res.name);
     if (!warn)
       DBUG_RETURN(1);
   }
-  vcol->flags= res.errors;
+  flags= res.errors;
 
-  if (vcol->flags & VCOL_SESSION_FUNC)
+  if (flags & VCOL_SESSION_FUNC)
     table->s->vcols_need_refixing= true;
 
   DBUG_RETURN(0);
@@ -3123,7 +3136,7 @@ static bool fix_and_check_vcol_expr(THD *thd, TABLE *table,
 */
 
 static Virtual_column_info *
-unpack_vcol_info_from_frm(THD *thd, MEM_ROOT *mem_root, TABLE *table,
+unpack_vcol_info_from_frm(THD *thd, TABLE *table,
                           String *expr_str, Virtual_column_info **vcol_ptr,
                           bool *error_reported)
 {
@@ -3162,7 +3175,7 @@ unpack_vcol_info_from_frm(THD *thd, MEM_ROOT *mem_root, TABLE *table,
   vcol_storage.vcol_info->stored_in_db=      vcol->stored_in_db;
   vcol_storage.vcol_info->name=              vcol->name;
   vcol_storage.vcol_info->utf8=              vcol->utf8;
-  if (!fix_and_check_vcol_expr(thd, table, vcol_storage.vcol_info))
+  if (!vcol_storage.vcol_info->fix_and_check_expr(thd, table))
   {
     *vcol_ptr= vcol_info= vcol_storage.vcol_info;   // Expression ok
     DBUG_ASSERT(vcol_info->expr);
@@ -3176,22 +3189,6 @@ end:
   DBUG_RETURN(vcol_info);
 }
 
-static bool check_vcol_forward_refs(Field *field, Virtual_column_info *vcol,
-                                    bool check_constraint)
-{
-  bool res;
-  uint32 flags= field->flags;
-  if (check_constraint)
-  {
-    /* Check constraints can refer it itself */
-    field->flags|= NO_DEFAULT_VALUE_FLAG;
-  }
-  res= (vcol &&
-        vcol->expr->walk(&Item::check_field_expression_processor, 0, field));
-  field->flags= flags;
-  return res;
-}
-
 /*
   Open a table based on a TABLE_SHARE
 

sql/table.h

@@ -1404,6 +1404,12 @@ public:
   */
   bool auto_increment_field_not_null;
   bool insert_or_update;             /* Can be used by the handler */
+  /*
+     NOTE: alias_name_used is only a hint! It works only in need_correct_ident()
+     condition. On other cases it is FALSE even if table_name is alias!
+     It cannot be TRUE in these cases, lots of spaghetti logic depends on that
+     (TODO).
+  */
   bool alias_name_used;              /* true if table_name is alias */
   bool get_fields_in_item_tree;      /* Signal to fix_field */
 private:
@@ -1607,7 +1613,7 @@ public:
                                       TABLE *tmp_table,
                                       TMP_TABLE_PARAM *tmp_table_param,
                                       bool with_cleanup);
-  int fix_vcol_exprs(THD *thd);
+  bool vcol_fix_exprs(THD *thd);
   Field *find_field_by_name(LEX_CSTRING *str) const;
   bool export_structure(THD *thd, class Row_definition_list *defs);
   bool is_splittable() { return spl_opt_info != NULL; }