MDEV-25292 Refactoring: moved select_field_count into Alter_info.

There is a need in MDEV-25292 to have both C_ALTER_TABLE and
select_field_count in one call. Semantically creation mode and field
count are two different things. Making creation mode negative
constants and field count positive variable into one parameter seems
to be a lazy hack for not making the second parameter.

select_count does not make sense without alter_info->create_list, so
the natural way is to hold it in Alter_info too. select_count is now
stored in member select_field_count.

Merged and updated by: Monty

sql/handler.cc

@@ -8856,11 +8856,10 @@ bool Table_scope_and_contents_source_st::vers_fix_system_fields(
 }
 
 
-int get_select_field_pos(Alter_info *alter_info, int select_field_count,
+int get_select_field_pos(Alter_info *alter_info, bool versioned)
-                         bool versioned)
 {
-  int select_field_pos= alter_info->create_list.elements - select_field_count;
+  int select_field_pos= alter_info->field_count();
-  if (select_field_count && versioned &&
+  if (alter_info->select_field_count && versioned &&
       /*
         ALTER_PARSER_ADD_COLUMN indicates system fields was created implicitly,
         select_field_count guarantees it's not ALTER TABLE
@@ -8873,7 +8872,7 @@ int get_select_field_pos(Alter_info *alter_info, int select_field_count,
 
 bool Table_scope_and_contents_source_st::vers_check_system_fields(
         THD *thd, Alter_info *alter_info, const Lex_ident_table &table_name,
-        const Lex_ident_db &db, int select_count)
+        const Lex_ident_db &db)
 {
   if (!(options & HA_VERSIONED_TABLE))
     return false;
@@ -8884,8 +8883,7 @@ bool Table_scope_and_contents_source_st::vers_check_system_fields(
   {
     uint fieldnr= 0;
     List_iterator<Create_field> field_it(alter_info->create_list);
-    uint select_field_pos= (uint) get_select_field_pos(alter_info, select_count,
+    uint select_field_pos= (uint) get_select_field_pos(alter_info, true);
-                                                       true);
     while (Create_field *f= field_it++)
     {
       /*
@@ -9322,10 +9320,9 @@ bool Table_period_info::check_field(const Create_field* f,
 
 bool Table_scope_and_contents_source_st::check_fields(
   THD *thd, Alter_info *alter_info,
-  const Lex_ident_table &table_name, const Lex_ident_db &db, int select_count)
+  const Lex_ident_table &table_name, const Lex_ident_db &db)
 {
-  return vers_check_system_fields(thd, alter_info,
+  return vers_check_system_fields(thd, alter_info, table_name, db) ||
-                                  table_name, db, select_count) ||
     check_period_fields(thd, alter_info);
 }
 

sql/handler.h

@@ -2336,8 +2336,7 @@ struct Table_scope_and_contents_source_st:
   bool fix_period_fields(THD *thd, Alter_info *alter_info);
   bool check_fields(THD *thd, Alter_info *alter_info,
                     const Lex_ident_table &table_name,
-                    const Lex_ident_db &db,
+                    const Lex_ident_db &db);
-                    int select_count= 0);
   bool check_period_fields(THD *thd, Alter_info *alter_info);
 
   void vers_check_native();
@@ -2346,8 +2345,7 @@ struct Table_scope_and_contents_source_st:
 
   bool vers_check_system_fields(THD *thd, Alter_info *alter_info,
                                 const Lex_ident_table &table_name,
-                                const Lex_ident_db &db,
+                                const Lex_ident_db &db);
-                                int select_count= 0);
 };
 
 
@@ -5852,8 +5850,7 @@ inline void Cost_estimate::reset(handler *file)
   avg_io_cost= file->DISK_READ_COST * file->DISK_READ_RATIO;
 }
 
-int get_select_field_pos(Alter_info *alter_info, int select_field_count,
+int get_select_field_pos(Alter_info *alter_info, bool versioned);
-                         bool versioned);
 
 #ifndef DBUG_OFF
 const char* dbug_print_row(TABLE *table, const uchar *rec, bool print_names= true);

sql/sql_alter.cc

@@ -31,6 +31,7 @@ Alter_info::Alter_info(const Alter_info &rhs, MEM_ROOT *mem_root)
   key_list(rhs.key_list, mem_root),
   alter_rename_key_list(rhs.alter_rename_key_list, mem_root),
   create_list(rhs.create_list, mem_root),
+  select_field_count(rhs.select_field_count),
   alter_index_ignorability_list(rhs.alter_index_ignorability_list, mem_root),
   check_constraint_list(rhs.check_constraint_list, mem_root),
   flags(rhs.flags), partition_flags(rhs.partition_flags),

sql/sql_alter.h

@@ -98,6 +98,7 @@ public:
   List<Alter_rename_key>        alter_rename_key_list;
   // List of columns, used by both CREATE and ALTER TABLE.
   List<Create_field>            create_list;
+  uint                          select_field_count;
   // Indexes whose ignorability needs to be changed.
   List<Alter_index_ignorability>  alter_index_ignorability_list;
   List<Virtual_column_info>     check_constraint_list;
@@ -198,6 +199,7 @@ public:
 
 
   Alter_info() :
+  select_field_count(0),
   flags(0), partition_flags(0),
     keys_onoff(LEAVE_AS_IS),
     original_table(0),
@@ -219,6 +221,7 @@ public:
     drop_stat_indexes.empty();
     rename_stat_fields.empty();
     rename_stat_indexes.empty();
+    select_field_count= 0;
     flags= 0;
     partition_flags= 0;
     keys_onoff= LEAVE_AS_IS;
@@ -328,6 +331,11 @@ public:
   bool add_alter_list(THD *thd, LEX_CSTRING name, LEX_CSTRING new_name,
                       bool exists);
 
+  uint field_count() const
+  {
+    return create_list.elements - select_field_count;
+  }
+
 private:
   Alter_info &operator=(const Alter_info &rhs); // not implemented
   Alter_info(const Alter_info &rhs);            // not implemented

sql/sql_insert.cc

@@ -4720,7 +4720,7 @@ TABLE *select_create::create_table_from_items(THD *thd, List<Item> *items,
   TABLE tmp_table;		// Used during 'Create_field()'
   TABLE_SHARE share;
   TABLE *table= 0;
-  uint select_field_count= items->elements;
+  alter_info->select_field_count= items->elements;
   /* Add selected items to field list */
   List_iterator_fast<Item> it(*items);
   Item *item;
@@ -4787,8 +4787,7 @@ TABLE *select_create::create_table_from_items(THD *thd, List<Item> *items,
 
   if (create_info->check_fields(thd, alter_info,
                                 table_list->table_name,
-                                table_list->db,
+                                table_list->db))
-                                select_field_count))
     DBUG_RETURN(NULL);
 
   DEBUG_SYNC(thd,"create_table_select_before_create");
@@ -4822,7 +4821,7 @@ TABLE *select_create::create_table_from_items(THD *thd, List<Item> *items,
 
   if (!mysql_create_table_no_lock(thd, &ddl_log_state_create, &ddl_log_state_rm,
                                   create_info, alter_info, NULL,
-                                  select_field_count, table_list))
+                                  C_ORDINARY_CREATE, table_list))
   {
     DEBUG_SYNC(thd,"create_table_select_before_open");
 

sql/sql_table.cc

@@ -3225,7 +3225,6 @@ mysql_prepare_create_table_finalize(THD *thd, HA_CREATE_INFO *create_info,
   List_iterator_fast<Create_field> it(alter_info->create_list);
   List_iterator<Create_field> it2(alter_info->create_list);
   uint total_uneven_bit_length= 0;
-  int select_field_count= C_CREATE_SELECT(create_table_mode);
   bool tmp_table= create_table_mode == C_ALTER_TABLE;
   const bool create_simple= thd->lex->create_simple();
   const CHARSET_INFO *scs= system_charset_info;
@@ -3241,8 +3240,7 @@ mysql_prepare_create_table_finalize(THD *thd, HA_CREATE_INFO *create_info,
     DBUG_RETURN(TRUE);
   }
 
-  select_field_pos= get_select_field_pos(alter_info, select_field_count,
+  select_field_pos= get_select_field_pos(alter_info, create_info->versioned());
-                                         create_info->versioned());
   null_fields= 0;
   create_info->varchar= 0;
 
@@ -3401,7 +3399,7 @@ mysql_prepare_create_table_finalize(THD *thd, HA_CREATE_INFO *create_info,
    from the select tables. This order may differ on master and slave. We
    therefore mark it as unsafe.
   */
-  if (select_field_count > 0 && auto_increment)
+  if (alter_info->select_field_count > 0 && auto_increment)
     thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_CREATE_SELECT_AUTOINC);
 
   /* Create keys */

sql/sql_table.h

@@ -128,7 +128,6 @@ bool add_keyword_to_query(THD *thd, String *result, const LEX_CSTRING *keyword,
   (which should be the number of fields in the SELECT ... part), and other
   cases use constants as defined below.
 */
-#define C_CREATE_SELECT(X)        ((X) > 0 ? (X) : 0)
 #define C_ORDINARY_CREATE         0
 #define C_ASSISTED_DISCOVERY     -1
 #define C_ALTER_TABLE            -2