SQL: implicit fields for IB tables + misc cleanups

sql/handler.cc

@@ -6560,7 +6560,8 @@ static bool create_string(MEM_ROOT *mem_root, String **s, const char *value)
 }
 
 static bool create_sys_trx_field_if_missing(THD *thd, const char *field_name,
-                                            Alter_info *alter_info, String **s)
+                                            Alter_info *alter_info, String **s,
+                                            bool integer_fields)
 {
   Create_field *f= new (thd->mem_root) Create_field();
   if (!f)
@@ -6568,8 +6569,17 @@ static bool create_sys_trx_field_if_missing(THD *thd, const char *field_name,
 
   f->field_name= field_name;
   f->charset= system_charset_info;
-  f->sql_type= MYSQL_TYPE_TIMESTAMP2;
-  f->length= 6;
+  if (integer_fields)
+  {
+    f->sql_type= MYSQL_TYPE_LONGLONG;
+    f->flags= UNSIGNED_FLAG;
+    f->length= MY_INT64_NUM_DECIMAL_DIGITS;
+  }
+  else
+  {
+    f->sql_type= MYSQL_TYPE_TIMESTAMP2;
+    f->length= 6;
+  }
   f->decimals= 0;
 
   if (f->check(thd))
@@ -6582,11 +6592,11 @@ static bool create_sys_trx_field_if_missing(THD *thd, const char *field_name,
   return false;
 }
 
-bool System_versioning_info::add_versioning_info(THD *thd,
-                                                 Alter_info *alter_info)
+bool Vers_parse_info::add_versioning_info(
+  THD *thd,
+  Alter_info *alter_info,
+  bool integer_fields)
 {
-  DBUG_ASSERT(versioned());
-
   if (!declared_system_versioning && !has_versioned_fields)
     return false;
 
@@ -6617,21 +6627,30 @@ bool System_versioning_info::add_versioning_info(THD *thd,
     return false;
 
   return create_sys_trx_field_if_missing(thd, "sys_trx_start", alter_info,
-                                         &generated_as_row.start) ||
+                                         &generated_as_row.start, integer_fields) ||
          create_sys_trx_field_if_missing(thd, "sys_trx_end", alter_info,
-                                         &generated_as_row.end) ||
+                                         &generated_as_row.end, integer_fields) ||
          create_string(thd->mem_root, &period_for_system_time.start,
                        "sys_trx_start") ||
          create_string(thd->mem_root, &period_for_system_time.end,
                        "sys_trx_end");
 }
 
-bool System_versioning_info::check(THD *thd, Alter_info *alter_info)
+bool Vers_parse_info::check(THD *thd, Alter_info *alter_info, bool integer_fields)
 {
-  if (!versioned())
+  if (!(
+    has_versioned_fields ||
+    has_unversioned_fields ||
+    declared_system_versioning ||
+    period_for_system_time.start ||
+    period_for_system_time.end ||
+    generated_as_row.start ||
+    generated_as_row.end))
+  {
     return false;
+  }
 
-  if (add_versioning_info(thd, alter_info))
+  if (add_versioning_info(thd, alter_info, integer_fields))
     return true;
 
   bool r= false;
@@ -6714,10 +6733,3 @@ bool System_versioning_info::check(THD *thd, Alter_info *alter_info)
   return r; // false means no error
 }
 
-bool System_versioning_info::versioned() const
-{
-  return has_versioned_fields || has_unversioned_fields ||
-         declared_system_versioning || period_for_system_time.start ||
-         period_for_system_time.end || generated_as_row.start ||
-         generated_as_row.end;
-}

sql/handler.h

@@ -1669,45 +1669,34 @@ struct Schema_specification_st
 };
 
 
-struct System_versioning_info
+struct Vers_parse_info
 {
-  System_versioning_info() :
-    period_for_system_time({NULL, NULL}),
-    generated_as_row({NULL, NULL}),
+  Vers_parse_info() :
     declared_system_versioning(false),
     has_versioned_fields(false),
     has_unversioned_fields(false)
   {}
 
-  struct
-  {
-    String *start, *end;
-  } period_for_system_time;
-
-  struct
+  struct start_end_t
   {
+    start_end_t() :
+      start(NULL),
+      end(NULL) {}
     String *start;
     String *end;
-  } generated_as_row;
+  };
+
+  start_end_t period_for_system_time;
+  start_end_t generated_as_row;
 
   void set_period_for_system_time(String *start, String *end)
   {
     period_for_system_time.start = start;
     period_for_system_time.end = end;
   }
-  void set_period_for_system_time()
-  {
-    set_period_for_system_time(NULL, NULL);
-  }
-
-  /** Returns true on failure */
-  bool add_versioning_info(THD *thd, Alter_info *alter_info);
 
-  /** Returns true on failure */
-  bool check(THD *thd, Alter_info *alter_info);
-
-  /** Returns true if table is versioned */
-  bool versioned() const;
+  bool add_versioning_info(THD *thd, Alter_info *alter_info, bool integer_fields);
+  bool check(THD *thd, Alter_info *alter_info, bool integer_fields);
 
   /** User has added 'WITH SYSTEM VERSIONING' to table definition */
   bool declared_system_versioning : 1;
@@ -1799,7 +1788,7 @@ struct Table_scope_and_contents_source_st
   bool table_was_deleted;
   sequence_definition *seq_create_info;
 
-  System_versioning_info system_versioning_info;
+  Vers_parse_info vers_info;
 
   void init()
   {
@@ -1814,19 +1803,7 @@ struct Table_scope_and_contents_source_st
 
   bool versioned() const
   {
-    return system_versioning_info.versioned();
-  }
-  const System_versioning_info *get_system_versioning_info() const
-  {
-    if (!versioned())
-      return NULL;
-    return &system_versioning_info;
-  }
-  System_versioning_info *get_system_versioning_info()
-  {
-    if (!versioned())
-      return NULL;
-    return &system_versioning_info;
+    return options & HA_VERSIONED_TABLE;
   }
 };
 

sql/sql_lex.h

@@ -3565,9 +3565,9 @@ struct LEX: public Query_tables_list
   bool add_unit_in_brackets(SELECT_LEX *nselect);
   void check_automatic_up(enum sub_select_type type);
 
-  System_versioning_info &vers_get_info()
+  Vers_parse_info &vers_get_info()
   {
-    return create_info.system_versioning_info;
+    return create_info.vers_info;
   }
 };
 

sql/sql_parse.cc

@@ -3855,9 +3855,6 @@ mysql_execute_command(THD *thd)
       goto end_with_restore_list;
     }
 
-    if (create_info.system_versioning_info.check(thd, &alter_info))
-      goto end_with_restore_list;
-
     /* Check privileges */
     if ((res= create_table_precheck(thd, select_tables, create_table)))
       goto end_with_restore_list;
@@ -3878,6 +3875,11 @@ mysql_execute_command(THD *thd)
     */
     if (!(create_info.used_fields & HA_CREATE_USED_ENGINE))
       create_info.use_default_db_type(thd);
+
+    DBUG_ASSERT(create_info.db_type);
+    if (create_info.vers_info.check(thd, &alter_info, create_info.db_type->versioned()))
+      goto end_with_restore_list;
+
     /*
       If we are using SET CHARSET without DEFAULT, add an implicit
       DEFAULT to not confuse old users. (This may change).

sql/sql_table.cc

@@ -3155,12 +3155,9 @@ copy_info_about_generated_fields(Alter_info *dst_alter_info,
   if (!src_create_info->versioned())
     return;
 
-  const System_versioning_info *versioning_info =
-    src_create_info->get_system_versioning_info();
-  DBUG_ASSERT(versioning_info);
-  const char *row_start_field = versioning_info->generated_as_row.start->c_ptr();
+  const char *row_start_field = src_create_info->vers_info.generated_as_row.start->c_ptr();
   DBUG_ASSERT(row_start_field);
-  const char *row_end_field = versioning_info->generated_as_row.end->c_ptr();
+  const char *row_end_field = src_create_info->vers_info.generated_as_row.end->c_ptr();
   DBUG_ASSERT(row_end_field);
 
   List_iterator<Create_field> it(dst_alter_info->create_list);
@@ -3249,8 +3246,6 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info,
   }
 
   select_field_pos= alter_info->create_list.elements - select_field_count;
-  const System_versioning_info *versioning_info =
-    create_info->get_system_versioning_info();
 
   for (field_no=0; (sql_field=it++) ; field_no++)
   {
@@ -3478,15 +3473,15 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info,
     if (sql_field->stored_in_db())
       record_offset+= sql_field->pack_length;
 
-    if (versioning_info)
+    if (create_info->versioned())
     {
       const bool is_generated_as_row_start =
         !my_strcasecmp(system_charset_info,
-                      versioning_info->generated_as_row.start->c_ptr(),
+                      create_info->vers_info.generated_as_row.start->c_ptr(),
                       sql_field->field_name);
       const bool is_generated_as_row_end =
         !my_strcasecmp(system_charset_info,
-                      versioning_info->generated_as_row.end->c_ptr(),
+                      create_info->vers_info.generated_as_row.end->c_ptr(),
                       sql_field->field_name);
       if (is_generated_as_row_start && is_generated_as_row_end)
       {
@@ -4351,13 +4346,9 @@ vers_prepare_keys(THD *thd,
 {
   DBUG_ASSERT(create_info->versioned());
 
-  const System_versioning_info *versioning_info=
-    create_info->get_system_versioning_info();
-
-  DBUG_ASSERT(versioning_info);
-  const char *row_start_field= versioning_info->generated_as_row.start->c_ptr();
+  const char *row_start_field= create_info->vers_info.generated_as_row.start->c_ptr();
   DBUG_ASSERT(row_start_field);
-  const char *row_end_field= versioning_info->generated_as_row.end->c_ptr();
+  const char *row_end_field= create_info->vers_info.generated_as_row.end->c_ptr();
   DBUG_ASSERT(row_end_field);
 
   List_iterator<Key> key_it(alter_info->key_list);
@@ -4384,7 +4375,7 @@ vers_prepare_keys(THD *thd,
       continue; // Key already contains Sys_start or Sys_end
 
     const LEX_STRING &lex_sys_end=
-      versioning_info->generated_as_row.end->lex_string();
+      create_info->vers_info.generated_as_row.end->lex_string();
     Key_part_spec *key_part_sys_end_col=
       new(thd->mem_root) Key_part_spec(lex_sys_end, 0);
     key->columns.push_back(key_part_sys_end_col);

sql/sql_yacc.yy

@@ -5851,7 +5851,7 @@ create_table_option:
 	  }
         | WITH SYSTEM VERSIONING
           {
-            System_versioning_info &info= Lex->vers_get_info();
+            Vers_parse_info &info= Lex->vers_get_info();
             info.declared_system_versioning= true;
             Lex->create_info.options|= HA_VERSIONED_TABLE;
           }
@@ -6058,7 +6058,7 @@ period_for_system_time:
           // If FOR_SYM is followed by SYSTEM_TIME_SYM then they are merged to: FOR_SYSTEM_TIME_SYM .
           PERIOD_SYM FOR_SYSTEM_TIME_SYM '(' period_for_system_time_column_id ',' period_for_system_time_column_id ')'
           {
-            System_versioning_info &info= Lex->vers_get_info();
+            Vers_parse_info &info= Lex->vers_get_info();
             if (!my_strcasecmp(system_charset_info, $4->c_ptr(), $6->c_ptr()))
             {
               my_error(ER_SYS_START_AND_SYS_END_SAME, MYF(0), $4->c_ptr());
@@ -6164,7 +6164,7 @@ field_def:
           vcol_opt_specifier vcol_opt_attribute
         | opt_generated_always AS ROW_SYM start_or_end
           {
-            System_versioning_info &info= Lex->vers_get_info();
+            Vers_parse_info &info= Lex->vers_get_info();
             String *field_name= new (thd->mem_root)
               String((const char*)Lex->last_field->field_name, system_charset_info);
             if (!field_name)
@@ -6669,12 +6669,14 @@ serial_attribute:
         | WITH SYSTEM VERSIONING
           {
             Lex->last_field->versioning = Column_definition::WITH_VERSIONING;
-            Lex->create_info.system_versioning_info.has_versioned_fields= true;
+            Lex->create_info.vers_info.has_versioned_fields= true;
+            Lex->create_info.options|= HA_VERSIONED_TABLE;
           }
         | WITHOUT SYSTEM VERSIONING
           {
             Lex->last_field->versioning = Column_definition::WITHOUT_VERSIONING;
-            Lex->create_info.system_versioning_info.has_unversioned_fields= true;
+            Lex->create_info.vers_info.has_unversioned_fields= true;
+            Lex->create_info.options|= HA_VERSIONED_TABLE;
           }
         ;
 

sql/unireg.cc

@@ -87,31 +87,28 @@ static uchar *extra2_write(uchar *pos, enum extra2_frm_value_type type,
   return extra2_write(pos, type, reinterpret_cast<LEX_STRING *>(str));
 }
 
-static bool
-versioned(HA_CREATE_INFO *create_info)
-{
-  return create_info->versioned();
-}
+static const bool ROW_START = true;
+static const bool ROW_END = false;
 
-static uint16
-get_row_start_field(HA_CREATE_INFO *create_info, List<Create_field> &create_fields)
+inline
+uint16
+vers_get_field(HA_CREATE_INFO *create_info, List<Create_field> &create_fields, bool row_start)
 {
-  DBUG_ASSERT(versioned(create_info));
+  DBUG_ASSERT(create_info->versioned());
 
   List_iterator<Create_field> it(create_fields);
-  Create_field*sql_field = NULL;
-
-  const System_versioning_info *versioning_info =
-    create_info->get_system_versioning_info();
-  DBUG_ASSERT(versioning_info);
+  Create_field *sql_field = NULL;
 
-  const char *row_start_field = versioning_info->generated_as_row.start->c_ptr();
-  DBUG_ASSERT(row_start_field);
+  const char *row_field =
+    row_start ?
+      create_info->vers_info.generated_as_row.start->c_ptr() :
+      create_info->vers_info.generated_as_row.end->c_ptr();
+  DBUG_ASSERT(row_field);
 
   for (unsigned field_no = 0; (sql_field = it++); ++field_no)
   {
     if (!my_strcasecmp(system_charset_info,
-                       row_start_field,
+                       row_field,
                        sql_field->field_name))
     {
       DBUG_ASSERT(field_no <= uint16(~0U));
@@ -123,35 +120,6 @@ get_row_start_field(HA_CREATE_INFO *create_info, List<Create_field> &create_fiel
   return 0;
 }
 
-static uint16
-get_row_end_field(HA_CREATE_INFO *create_info, List<Create_field> &create_fields)
-{
-  DBUG_ASSERT(versioned(create_info));
-
-  List_iterator<Create_field> it(create_fields);
-  Create_field*sql_field = NULL;
-
-  const System_versioning_info *versioning_info =
-    create_info->get_system_versioning_info();
-  DBUG_ASSERT(versioning_info);
-
-  const char *row_end_field = versioning_info->generated_as_row.end->c_ptr();
-  DBUG_ASSERT(row_end_field);
-
-  for (unsigned field_no = 0; (sql_field = it++); ++field_no)
-  {
-    if (!my_strcasecmp(system_charset_info,
-                       row_end_field,
-                       sql_field->field_name))
-    {
-      DBUG_ASSERT(field_no <= uint16(~0U));
-      return uint16(field_no);
-    }
-  }
-
-  DBUG_ASSERT(0); /* Not Reachable */
-  return 0;
-}
 
 /**
   Create a frm (table definition) file
@@ -285,7 +253,7 @@ LEX_CUSTRING build_frm_image(THD *thd, const char *table,
   if (gis_extra2_len)
     extra2_size+= 1 + (gis_extra2_len > 255 ? 3 : 1) + gis_extra2_len;
 
-  if (versioned(create_info))
+  if (create_info->versioned())
   {
     extra2_size+= 1 + 1 + 2 * sizeof(uint16);
   }
@@ -345,13 +313,13 @@ LEX_CUSTRING build_frm_image(THD *thd, const char *table,
   }
 #endif /*HAVE_SPATIAL*/
 
-  if (versioned(create_info))
+  if (create_info->versioned())
   {
     *pos++= EXTRA2_PERIOD_FOR_SYSTEM_TIME;
     *pos++= 2 * sizeof(uint16);
-    int2store(pos, get_row_start_field(create_info, create_fields));
+    int2store(pos, vers_get_field(create_info, create_fields, ROW_START));
     pos+= sizeof(uint16);
-    int2store(pos, get_row_end_field(create_info, create_fields));
+    int2store(pos, vers_get_field(create_info, create_fields, ROW_END));
     pos+= sizeof(uint16);
   }