cleanup: ha_partition::update_row/delete_row

implement log-term TODO item, convert redundant if()-s into asserts.

sql/ha_partition.cc

@@ -4303,29 +4303,15 @@ int ha_partition::write_row(uchar * buf)
 int ha_partition::update_row(const uchar *old_data, const uchar *new_data)
 {
   THD *thd= ha_thd();
-  uint32 new_part_id, old_part_id;
+  uint32 new_part_id, old_part_id= m_last_part;
   int error= 0;
-  longlong func_value;
   DBUG_ENTER("ha_partition::update_row");
   m_err_rec= NULL;
 
   // Need to read partition-related columns, to locate the row's partition:
   DBUG_ASSERT(bitmap_is_subset(&m_part_info->full_part_field_set,
                                table->read_set));
-  if ((error= get_parts_for_update(old_data, new_data, table->record[0],
-                                   m_part_info, &old_part_id, &new_part_id,
-                                   &func_value)))
-  {
-    m_part_info->err_value= func_value;
-    goto exit;
-  }
-  DBUG_ASSERT(bitmap_is_set(&(m_part_info->read_partitions), old_part_id));
-  if (!bitmap_is_set(&(m_part_info->lock_partitions), new_part_id))
-  {
-    error= HA_ERR_NOT_IN_LOCK_PARTITIONS;
-    goto exit;
-  }
-
+#ifndef DBUG_OFF
   /*
     The protocol for updating a row is:
     1) position the handler (cursor) on the row to be updated,
@@ -4343,12 +4329,21 @@ int ha_partition::update_row(const uchar *old_data, const uchar *new_data)
     Notice that HA_READ_BEFORE_WRITE_REMOVAL does not require this protocol,
     so this is not supported for this engine.
   */
-  if (old_part_id != m_last_part)
+  error= get_part_for_buf(old_data, m_rec0, m_part_info, &old_part_id);
+  DBUG_ASSERT(!error);
+  DBUG_ASSERT(old_part_id == m_last_part);
+  DBUG_ASSERT(bitmap_is_set(&(m_part_info->read_partitions), old_part_id));
+#endif
+
+  if ((error= get_part_for_buf(new_data, m_rec0, m_part_info, &new_part_id)))
+    goto exit;
+  if (!bitmap_is_set(&(m_part_info->lock_partitions), new_part_id))
   {
-    m_err_rec= old_data;
-    DBUG_RETURN(HA_ERR_ROW_IN_WRONG_PARTITION);
+    error= HA_ERR_NOT_IN_LOCK_PARTITIONS;
+    goto exit;
   }
 
+
   m_last_part= new_part_id;
   start_part_bulk_insert(thd, new_part_id);
   if (new_part_id == old_part_id)
@@ -4395,13 +4390,8 @@ int ha_partition::update_row(const uchar *old_data, const uchar *new_data)
     error= m_file[old_part_id]->ha_delete_row(old_data);
     reenable_binlog(thd);
     if (error)
-    {
-#ifdef IN_THE_FUTURE
-      (void) m_file[new_part_id]->delete_last_inserted_row(new_data);
-#endif
       goto exit;
   }
-  }
 
 exit:
   /*
@@ -4460,7 +4450,6 @@ int ha_partition::update_row(const uchar *old_data, const uchar *new_data)
 
 int ha_partition::delete_row(const uchar *buf)
 {
-  uint32 part_id;
   int error;
   THD *thd= ha_thd();
   DBUG_ENTER("ha_partition::delete_row");
@@ -4468,16 +4457,7 @@ int ha_partition::delete_row(const uchar *buf)
 
   DBUG_ASSERT(bitmap_is_subset(&m_part_info->full_part_field_set,
                                table->read_set));
-  if ((error= get_part_for_delete(buf, m_rec0, m_part_info, &part_id)))
-  {
-    DBUG_RETURN(error);
-  }
-  /* Should never call delete_row on a partition which is not read */
-  DBUG_ASSERT(bitmap_is_set(&(m_part_info->read_partitions), part_id));
-  DBUG_ASSERT(bitmap_is_set(&(m_part_info->lock_partitions), part_id));
-  if (!bitmap_is_set(&(m_part_info->lock_partitions), part_id))
-    DBUG_RETURN(HA_ERR_NOT_IN_LOCK_PARTITIONS);
-
+#ifndef DBUG_OFF
   /*
     The protocol for deleting a row is:
     1) position the handler (cursor) on the row to be deleted,
@@ -4494,19 +4474,20 @@ int ha_partition::delete_row(const uchar *buf)
 
     Notice that HA_READ_BEFORE_WRITE_REMOVAL does not require this protocol,
     so this is not supported for this engine.
-
-    TODO: change the assert in InnoDB into an error instead and make this one
-    an assert instead and remove the get_part_for_delete()!
   */
-  if (part_id != m_last_part)
-  {
-    m_err_rec= buf;
-    DBUG_RETURN(HA_ERR_ROW_IN_WRONG_PARTITION);
-  }
+  uint32 part_id;
+  error= get_part_for_buf(buf, m_rec0, m_part_info, &part_id);
+  DBUG_ASSERT(!error);
+  DBUG_ASSERT(part_id == m_last_part);
+  DBUG_ASSERT(bitmap_is_set(&(m_part_info->read_partitions), m_last_part));
+  DBUG_ASSERT(bitmap_is_set(&(m_part_info->lock_partitions), m_last_part));
+#endif
+
+  if (!bitmap_is_set(&(m_part_info->lock_partitions), m_last_part))
+    DBUG_RETURN(HA_ERR_NOT_IN_LOCK_PARTITIONS);
 
-  m_last_part= part_id;
   tmp_disable_binlog(thd);
-  error= m_file[part_id]->ha_delete_row(buf);
+  error= m_file[m_last_part]->ha_delete_row(buf);
   reenable_binlog(thd);
   DBUG_RETURN(error);
 }
@@ -5178,7 +5159,7 @@ int ha_partition::rnd_pos_by_record(uchar *record)
 {
   DBUG_ENTER("ha_partition::rnd_pos_by_record");
 
-  if (unlikely(get_part_for_delete(record, m_rec0, m_part_info, &m_last_part)))
+  if (unlikely(get_part_for_buf(record, m_rec0, m_part_info, &m_last_part)))
     DBUG_RETURN(1);
 
   DBUG_RETURN(handler::rnd_pos_by_record(record));
@@ -9687,7 +9668,7 @@ void ha_partition::print_error(int error, myf errflag)
       str.append("(");
       str.append_ulonglong(m_last_part);
       str.append(" != ");
-      if (get_part_for_delete(m_err_rec, m_rec0, m_part_info, &part_id))
+      if (get_part_for_buf(m_err_rec, m_rec0, m_part_info, &part_id))
         str.append("?");
       else
         str.append_ulonglong(part_id);

sql/sql_partition.cc

@@ -246,82 +246,11 @@ bool partition_default_handling(THD *thd, TABLE *table, partition_info *part_inf
 
 
 /*
-  A useful routine used by update_row for partition handlers to calculate
-  the partition ids of the old and the new record.
-
-  SYNOPSIS
-    get_part_for_update()
-    old_data                Buffer of old record
-    new_data                Buffer of new record
-    rec0                    Reference to table->record[0]
-    part_info               Reference to partition information
-    out:old_part_id         The returned partition id of old record 
-    out:new_part_id         The returned partition id of new record
-
-  RETURN VALUE
-    0                       Success
-    > 0                     Error code
-*/
-
-int get_parts_for_update(const uchar *old_data, const uchar *new_data,
-                         const uchar *rec0, partition_info *part_info,
-                         uint32 *old_part_id, uint32 *new_part_id,
-                         longlong *new_func_value)
-{
-  Field **part_field_array= part_info->full_part_field_array;
-  int error;
-  longlong old_func_value;
-  DBUG_ENTER("get_parts_for_update");
-
-  DBUG_ASSERT(new_data == rec0);             // table->record[0]
-  part_info->table->move_fields(part_field_array, old_data, rec0);
-  error= part_info->get_partition_id(part_info, old_part_id,
-                                     &old_func_value);
-  part_info->table->move_fields(part_field_array, rec0, old_data);
-  if (unlikely(error))                             // Should never happen
-  {
-    DBUG_ASSERT(0);
-    DBUG_RETURN(error);
-  }
-#ifdef NOT_NEEDED
-  if (new_data == rec0)
-#endif
-  {
-    if (unlikely(error= part_info->get_partition_id(part_info,
-                                                    new_part_id,
-                                                    new_func_value)))
-    {
-      DBUG_RETURN(error);
-    }
-  }
-#ifdef NOT_NEEDED
-  else
-  {
-    /*
-      This branch should never execute but it is written anyways for
-      future use. It will be tested by ensuring that the above
-      condition is false in one test situation before pushing the code.
-    */
-    part_info->table->move_fields(part_field_array, new_data, rec0);
-    error= part_info->get_partition_id(part_info, new_part_id,
-                                       new_func_value);
-    part_info->table->move_fields(part_field_array, rec0, new_data);
-    if (unlikely(error))
-    {
-      DBUG_RETURN(error);
-    }
-  }
-#endif
-  DBUG_RETURN(0);
-}
-
-
-/*
-  A useful routine used by delete_row for partition handlers to calculate
-  the partition id.
+  A useful routine used by update/delete_row for partition handlers to
+  calculate the partition id.
 
   SYNOPSIS
-    get_part_for_delete()
+    get_part_for_buf()
     buf                     Buffer of old record
     rec0                    Reference to table->record[0]
     part_info               Reference to partition information
@@ -337,21 +266,19 @@ int get_parts_for_update(const uchar *old_data, const uchar *new_data,
     calculate the partition id.
 */
 
-int get_part_for_delete(const uchar *buf, const uchar *rec0,
+int get_part_for_buf(const uchar *buf, const uchar *rec0,
                      partition_info *part_info, uint32 *part_id)
 {
   int error;
   longlong func_value;
-  DBUG_ENTER("get_part_for_delete");
+  DBUG_ENTER("get_part_for_buf");
 
-  if (likely(buf == rec0))
-  {
-    if (unlikely((error= part_info->get_partition_id(part_info, part_id,
-                                                     &func_value))))
+  if (buf == rec0)
   {
-      DBUG_RETURN(error);
-    }
-    DBUG_PRINT("info", ("Delete from partition %d", *part_id));
+    error= part_info->get_partition_id(part_info, part_id, &func_value);
+    if (unlikely((error)))
+      goto err;
+    DBUG_PRINT("info", ("Partition %d", *part_id));
   }
   else
   {
@@ -360,12 +287,13 @@ int get_part_for_delete(const uchar *buf, const uchar *rec0,
     error= part_info->get_partition_id(part_info, part_id, &func_value);
     part_info->table->move_fields(part_field_array, rec0, buf);
     if (unlikely(error))
-    {
-      DBUG_RETURN(error);
-    }
-    DBUG_PRINT("info", ("Delete from partition %d (path2)", *part_id));
+      goto err;
+    DBUG_PRINT("info", ("Partition %d (path2)", *part_id));
   }
   DBUG_RETURN(0);
+err:
+  part_info->err_value= func_value;
+  DBUG_RETURN(error);
 }
 
 

sql/sql_partition.h

@@ -87,11 +87,7 @@ bool check_reorganise_list(partition_info *new_part_info,
                            partition_info *old_part_info,
                            List<char> list_part_names);
 handler *get_ha_partition(partition_info *part_info);
-int get_parts_for_update(const uchar *old_data, const uchar *new_data,
-                         const uchar *rec0, partition_info *part_info,
-                         uint32 *old_part_id, uint32 *new_part_id,
-                         longlong *func_value);
-int get_part_for_delete(const uchar *buf, const uchar *rec0,
+int get_part_for_buf(const uchar *buf, const uchar *rec0,
                      partition_info *part_info, uint32 *part_id);
 void prune_partition_set(const TABLE *table, part_id_range *part_spec);
 bool check_partition_info(partition_info *part_info,handlerton **eng_type,