MDEV-17554 history partitioning cleanups

* Fixed missed warning on condition boundary
* REORGANIZE cases
* vers_utils.h removed
* test cases cleanup

mysql-test/suite/versioning/r/delete_history.result

+call mtr.add_suppression("need more HISTORY partitions");
 create table t (a int);
 delete history from t before system_time now();
 ERROR HY000: Table `t` is not system-versioned
@@ -56,16 +57,15 @@ select * from t for system_time all;
 a
 drop procedure truncate_sp;
 # Truncate partitioned
-create or replace table t (a int)
-with system versioning
-partition by system_time limit 1 (
-partition p0 history,
-partition p1 history,
-partition pn current);
+create or replace table t (a int) with system versioning
+partition by system_time limit 1 partitions 3;
 insert into t values (1);
 update t set a= 2;
 update t set a= 3;
 delete history from t;
+Warnings:
+Warning	4114	Versioned table `test`.`t`: last HISTORY partition (`p1`) is out of LIMIT, need more HISTORY partitions
+# The above warning is one command late (MDEV-20345) ^^^
 select * from t for system_time all;
 a
 3

mysql-test/suite/versioning/t/delete_history.test

@@ -2,6 +2,8 @@
 --source include/have_partition.inc
 --source suite/versioning/engines.inc
 
+call mtr.add_suppression("need more HISTORY partitions");
+
 create table t (a int);
 --error ER_VERS_NOT_VERSIONED
 delete history from t before system_time now();
@@ -56,16 +58,13 @@ select * from t for system_time all;
 drop procedure truncate_sp;
 
 --echo # Truncate partitioned
-create or replace table t (a int)
-with system versioning
-partition by system_time limit 1 (
-    partition p0 history,
-    partition p1 history,
-    partition pn current);
+create or replace table t (a int) with system versioning
+partition by system_time limit 1 partitions 3;
 insert into t values (1);
 update t set a= 2;
 update t set a= 3;
 delete history from t;
+--echo # The above warning is one command late (MDEV-20345) ^^^
 select * from t for system_time all;
 
 --echo # VIEW

plugin/versioning/versioning.cc

@@ -22,7 +22,8 @@
 #include "sql_plugin.h"                         // st_plugin_int
 #include "sql_class.h"
 #include "item.h"
-#include "vers_utils.h"
+#include "table.h"
+#include "vers_string.h"
 
 /* System Versioning: TRT_TRX_ID(), TRT_COMMIT_ID(), TRT_BEGIN_TS(), TRT_COMMIT_TS(), TRT_ISO_LEVEL() */
 template <TR_table::field_id_t TRT_FIELD>

sql/partition_info.cc

@@ -32,9 +32,10 @@
 #include "sql_parse.h"
 #include "sql_acl.h"                          // *_ACL
 #include "sql_base.h"                         // fill_record
-#include "sql_statistics.h"                   // vers_stat_end
-#include "vers_utils.h"
 #include "lock.h"
+#include "table.h"
+#include "sql_class.h"
+#include "vers_string.h"
 
 #ifdef WITH_PARTITION_STORAGE_ENGINE
 #include "ha_partition.h"
@@ -196,49 +197,6 @@ bool partition_info::set_named_partition_bitmap(const char *part_name, size_t le
 }
 
 
-
-/**
-  Prune away partitions not mentioned in the PARTITION () clause,
-  if used.
-
-    @param table_list  Table list pointing to table to prune.
-
-  @return Operation status
-    @retval false Success
-    @retval true  Failure
-*/
-bool partition_info::set_read_partitions(List<char> *partition_names)
-{
-  DBUG_ENTER("partition_info::set_read_partitions");
-  if (!partition_names || !partition_names->elements)
-  {
-    DBUG_RETURN(true);
-  }
-
-  uint num_names= partition_names->elements;
-  List_iterator<char> partition_names_it(*partition_names);
-  uint i= 0;
-  /*
-    TODO: When adding support for FK in partitioned tables, the referenced
-    table must probably lock all partitions for read, and also write depending
-    of ON DELETE/UPDATE.
-  */
-  bitmap_clear_all(&read_partitions);
-
-  /* No check for duplicate names or overlapping partitions/subpartitions. */
-
-  DBUG_PRINT("info", ("Searching through partition_name_hash"));
-  do
-  {
-    char *part_name= partition_names_it++;
-    if (add_named_partition(part_name, strlen(part_name)))
-      DBUG_RETURN(true);
-  } while (++i < num_names);
-  DBUG_RETURN(false);
-}
-
-
-
 /**
   Prune away partitions not mentioned in the PARTITION () clause,
   if used.
@@ -849,6 +807,15 @@ bool partition_info::has_unique_name(partition_element *element)
   DBUG_RETURN(TRUE);
 }
 
+
+/**
+  @brief Switch history partition according limit or interval
+
+  @note
+    vers_info->limit      Limit by number of partition records
+    vers_info->interval   Limit by fixed time interval
+    vers_info->hist_part  (out) Working history partition
+*/
 void partition_info::vers_set_hist_part(THD *thd)
 {
   if (vers_info->limit)
@@ -867,7 +834,7 @@ void partition_info::vers_set_hist_part(THD *thd)
       vers_info->hist_part= next;
       records= next_records;
     }
-    if (records > vers_info->limit)
+    if (records >= vers_info->limit)
     {
       if (next == vers_info->now_part)
       {
@@ -904,49 +871,6 @@ void partition_info::vers_set_hist_part(THD *thd)
 }
 
 
-bool partition_info::vers_setup_expression(THD * thd, uint32 alter_add)
-{
-  if (!table->versioned())
-  {
-    // frm must be corrupted, normally CREATE/ALTER TABLE checks for that
-    my_error(ER_FILE_CORRUPT, MYF(0), table->s->path.str);
-    return true;
-  }
-
-  DBUG_ASSERT(part_type == VERSIONING_PARTITION);
-  DBUG_ASSERT(table->versioned(VERS_TIMESTAMP));
-
-  if (!alter_add)
-  {
-    Field *row_end= table->vers_end_field();
-    // needed in handle_list_of_fields()
-    row_end->flags|= GET_FIXED_FIELDS_FLAG;
-    Name_resolution_context *context= &thd->lex->current_select->context;
-    Item *row_end_item= new (thd->mem_root) Item_field(thd, context, row_end);
-    Item *row_end_ts= new (thd->mem_root) Item_func_unix_timestamp(thd, row_end_item);
-    set_part_expr(thd, row_end_ts, false);
-  }
-
-  if (alter_add)
-  {
-    List_iterator<partition_element> it(partitions);
-    partition_element *el;
-    for(uint32 id= 0; ((el= it++)); id++)
-    {
-      DBUG_ASSERT(el->type != partition_element::CONVENTIONAL);
-      /* Newly added element is inserted before AS_OF_NOW. */
-      if (el->id == UINT_MAX32 || el->type == partition_element::CURRENT)
-      {
-        el->id= id;
-        if (el->type == partition_element::CURRENT)
-          break;
-      }
-    }
-  }
-  return false;
-}
-
-
 /*
   Check that the partition/subpartition is setup to use the correct
   storage engine
@@ -1424,9 +1348,8 @@ bool partition_info::check_partition_info(THD *thd, handlerton **eng_type,
 
   if (add_or_reorg_part)
   {
-    if (unlikely(part_type == VERSIONING_PARTITION &&
-                 vers_setup_expression(thd, add_or_reorg_part->partitions.elements)))
-      goto end;
+    if (part_type == VERSIONING_PARTITION && add_or_reorg_part->partitions.elements)
+      vers_update_el_ids();
     if (check_constants(thd, this))
       goto end;
   }

sql/partition_info.h

@@ -385,11 +385,9 @@ class partition_info : public Sql_alloc
                                        uint start_no);
   char *create_default_subpartition_name(THD *thd, uint subpart_no,
                                          const char *part_name);
-  // FIXME: prune_partition_bitmaps() is duplicate of set_read_partitions()
-  bool prune_partition_bitmaps(List<String> *partition_names);
+  bool prune_partition_bitmaps(List<String> *partition_names); // set_read_partitions() in 8.0
   bool add_named_partition(const char *part_name, size_t length);
 public:
-  bool set_read_partitions(List<char> *partition_names);
   bool has_unique_name(partition_element *element);
   bool field_in_partition_expr(Field *field) const;
 
@@ -404,7 +402,8 @@ class partition_info : public Sql_alloc
     return !limit;
   }
   void vers_set_hist_part(THD *thd);
-  bool vers_setup_expression(THD *thd, uint32 alter_add= 0); /* Stage 1. */
+  bool vers_fix_field_list(THD *thd);
+  void vers_update_el_ids();
   partition_element *get_partition(uint part_id)
   {
     List_iterator<partition_element> it(partitions);
@@ -445,4 +444,60 @@ void init_all_partitions_iterator(partition_info *part_info,
   part_iter->get_next= get_next_partition_id_range;
 }
 
+
+/**
+  @brief Update part_field_list by row_end field name
+
+  @returns true on error; false on success
+*/
+inline
+bool partition_info::vers_fix_field_list(THD * thd)
+{
+  if (!table->versioned())
+  {
+    // frm must be corrupted, normally CREATE/ALTER TABLE checks for that
+    my_error(ER_FILE_CORRUPT, MYF(0), table->s->path.str);
+    return true;
+  }
+  DBUG_ASSERT(part_type == VERSIONING_PARTITION);
+  DBUG_ASSERT(table->versioned(VERS_TIMESTAMP));
+
+  Field *row_end= table->vers_end_field();
+  // needed in handle_list_of_fields()
+  row_end->flags|= GET_FIXED_FIELDS_FLAG;
+  Name_resolution_context *context= &thd->lex->current_select->context;
+  Item *row_end_item= new (thd->mem_root) Item_field(thd, context, row_end);
+  Item *row_end_ts= new (thd->mem_root) Item_func_unix_timestamp(thd, row_end_item);
+  set_part_expr(thd, row_end_ts, false);
+
+  return false;
+}
+
+
+/**
+  @brief Update partition_element's id
+
+  @returns true on error; false on success
+*/
+inline
+void partition_info::vers_update_el_ids()
+{
+  DBUG_ASSERT(part_type == VERSIONING_PARTITION);
+  DBUG_ASSERT(table->versioned(VERS_TIMESTAMP));
+
+  List_iterator<partition_element> it(partitions);
+  partition_element *el;
+  for(uint32 id= 0; ((el= it++)); id++)
+  {
+    DBUG_ASSERT(el->type != partition_element::CONVENTIONAL);
+    /* Newly added element is inserted before AS_OF_NOW. */
+    if (el->id == UINT_MAX32 || el->type == partition_element::CURRENT)
+    {
+      el->id= id;
+      if (el->type == partition_element::CURRENT)
+        break;
+    }
+  }
+}
+
 #endif /* PARTITION_INFO_INCLUDED */

sql/sql_partition.cc

@@ -2013,7 +2013,7 @@ bool fix_partition_func(THD *thd, TABLE *table, bool is_create_table_ind)
     else
     {
       if (part_info->part_type == VERSIONING_PARTITION &&
-        part_info->vers_setup_expression(thd))
+        part_info->vers_fix_field_list(thd))
         goto end;
       if (unlikely(fix_fields_part_func(thd, part_info->part_expr,
                                         table, FALSE, is_create_table_ind)))

sql/vers_utils.h

-#ifndef VERS_UTILS_INCLUDED
-#define VERS_UTILS_INCLUDED
-
-#include "table.h"
-#include "sql_class.h"
-#include "vers_string.h"
-
-#endif // VERS_UTILS_INCLUDED

storage/innobase/srv/srv0srv.cc

@@ -2033,7 +2033,6 @@ static bool srv_task_execute()
 }
 
 
-
 /** Do the actual purge operation.
 @param[in,out]	n_total_purged	total number of purged pages
 @return length of history list before the last purge batch. */