Bug #85238 Dropping non-existent partition throws unexpected error under certain conditions
Submitted: 1 Mar 2017 2:58 Modified: 1 Mar 2017 10:35
Reporter: Sean Davidson Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: Partitions Severity:S3 (Non-critical)
Version:5.6, 5.7, 8.0 OS:Any
Assigned to: CPU Architecture:Any
Tags: ALTER TABLE, partitions

[1 Mar 2017 2:58] Sean Davidson
Description:
Dropping a non-existent partition returns unexpected error if the num of partition arguments >= number of partitions a table has.
When issuing an 'ALTER TABLE xyz DROP PARTITION pX' statement
 , if the table has only 1 partition remaining (say p0)
 , and the partition to be dropped does not exist (say p1)
 , MySQL returns 'ERROR 1508 (HY000): Cannot remove all partitions, use DROP TABLE instead' 
 instead of the expected: 'ERROR 1507 (HY000): Error in list of partitions to DROP'

It appears that MySQL checks the partition count of the table being altered, and if that count is <= the count of partitions to be dropped, signals a 1508 error condition. 
However, MySQL is doing this check *before* checking if the partition(s) to be dropped is/are in fact in the target table.  This is a nice optimization to quickly abort a command destined to fail, however this leads to the signaling of an incorrect error.

This is an issue in its own right, but may seem merely clerical.  However, this became a data loss risk in my use case. 
Take a stored routine that is trapping for MYSQL_ERRNO 1507 attempts to drop a non-existent partition, and there is only 1 partition left.  
A workaround is to also trap for MYSQL_ERRNO 1508, but this has the undesirable effect of requiring extra code to differentiate between a legitimate 1507 from a 1508.

The following source code seems to contain the logic at fault:

      if (num_parts_dropped >= tab_part_info->num_parts)   <-- this is the if block that "jumps to conclusions too early"
      {
        my_error(ER_DROP_LAST_PARTITION, MYF(0));
        goto err;
      }
      do
      {
        partition_element *part_elem= part_it++;
        if (is_name_in_list(part_elem->partition_name,
                            alter_info->partition_names))
        {
          /*
            Set state to indicate that the partition is to be dropped.
          */
          num_parts_found++;
          part_elem->part_state= PART_TO_BE_DROPPED;
        }
      } while (++part_count < tab_part_info->num_parts);

It is found in the currently public 5.6, 5.7 and 8.0 branches at these locations:

   https://github.com/mysql/mysql-server/blob/5.6/sql/sql_partition.cc#L5291
   https://github.com/mysql/mysql-server/blob/5.7/sql/sql_partition.cc#L5459
   https://github.com/mysql/mysql-server/blob/8.0/sql/sql_partition.cc#L5437

How to repeat:
(using a test case simplified from our actual use case):

mysql> CREATE TABLE batch_records (
         job_id   INT NOT NULL DEFAULT 0,
         map_key  BIGINT UNSIGNED NOT NULL DEFAULT 0,
         sometext TEXT,
         PRIMARY KEY ( job_id, map_key )
       ) PARTITION BY LIST ( job_id )
       ( PARTITION jobNULL VALUES IN ( NULL ) 
           COMMENT='Empty partition to maintain partitioning structure when table is not in active use' 
       ) ;
Query OK, 0 rows affected (0.01 sec)

mysql> ALTER TABLE batch_records  ADD PARTITION ( PARTITION job1 VALUES IN ( 1 )) ;
Query OK, 0 rows affected (0.10 sec)
Records: 0  Duplicates: 0  Warnings: 0

mysql> ALTER TABLE batch_records  ADD PARTITION ( PARTITION job2 VALUES IN ( 2 )) ;
Query OK, 0 rows affected (0.08 sec)
Records: 0  Duplicates: 0  Warnings: 0

mysql> SHOW CREATE TABLE `batch_records` \G
*************************** 1. row ***************************
       Table: batch_records
Create Table: CREATE TABLE `batch_records` (
  `job_id` int(11) NOT NULL DEFAULT '0',
  `map_key` bigint(20) unsigned NOT NULL DEFAULT '0',
  `sometext` text COLLATE latin1_bin,
  PRIMARY KEY (`job_id`,`map_key`)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_bin
/*!50100 PARTITION BY LIST ( job_id)
(PARTITION jobNULL VALUES IN (NULL) COMMENT = 'Empty partition to maintain partitioning structure when table is not in active use' ENGINE = MyISAM,
 PARTITION job1 VALUES IN (1) ENGINE = MyISAM,
 PARTITION job2 VALUES IN (2) ENGINE = MyISAM) */
1 row in set (0.01 sec)

mysql> ALTER TABLE batch_records DROP PARTITION job1 ;
Query OK, 0 rows affected (0.12 sec)
Records: 0  Duplicates: 0  Warnings: 0

mysql> ALTER TABLE batch_records DROP PARTITION job1 ;
ERROR 1507 (HY000): Error in list of partitions to DROP

# This is expected - the partition was already dropped

mysql> ALTER TABLE batch_records DROP PARTITION job2 ;
Query OK, 0 rows affected (0.13 sec)
Records: 0  Duplicates: 0  Warnings: 0

mysql> ALTER TABLE batch_records DROP PARTITION job2 ;
ERROR 1508 (HY000): Cannot remove all partitions, use DROP TABLE instead

# This is *not* expected - as before, when attempting to drop a non-existent partition we expect to throw 'ERROR 1507 (HY000): Error in list of partitions to DROP'. Instead 1508 is thrown.
# We would expect 1508 if we tried to drop PARTITION jobNULL

# Just to be clear, this is what the table looks like this at this point:

mysql> SHOW CREATE TABLE `batch_records` \G
*************************** 1. row ***************************
       Table: batch_records
Create Table: CREATE TABLE `batch_records` (
  `job_id` int(11) NOT NULL DEFAULT '0',
  `map_key` bigint(20) unsigned NOT NULL DEFAULT '0',
  `sometext` text COLLATE latin1_bin,
  PRIMARY KEY (`job_id`,`map_key`)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_bin
/*!50100 PARTITION BY LIST ( job_id)
(PARTITION jobNULL VALUES IN (NULL) COMMENT = 'Empty partition to maintain partitioning structure when table is not in active use' ENGINE = MyISAM) */
1 row in set (0.00 sec)

# To support the notion that the problem stems from 5.6/sql/sql_partition.cc#L5291, the same anomaly should surface if we simply try to drop more partitions than there are, regardless of the partition names:

mysql> ALTER TABLE batch_records  ADD PARTITION ( PARTITION j1 VALUES IN (1), PARTITION j2 VALUES IN (2), PARTITION j3 VALUES IN (3), PARTITION j4 VALUES IN (4)) ;
Query OK, 0 rows affected (0.12 sec)
Records: 0  Duplicates: 0  Warnings: 0

mysql> ALTER TABLE batch_records DROP PARTITION a1, b2, c3, d4 ;
ERROR 1507 (HY000): Error in list of partitions to DROP
mysql> ALTER TABLE batch_records DROP PARTITION a1, b2, c3, d4, e5 ;
ERROR 1508 (HY000): Cannot remove all partitions, use DROP TABLE instead

Suggested fix:
Rather than checking if argument count is >= current partition count, instead check if found partition count equals total partition count.

Based on the code here: https://github.com/mysql/mysql-server/blob/5.6/sql/sql_partition.cc#L5291

Something like,
replace:

      if (num_parts_dropped >= tab_part_info->num_parts)
      {
        my_error(ER_DROP_LAST_PARTITION, MYF(0));
        goto err;
      }
      do
      {
        partition_element *part_elem= part_it++;
        if (is_name_in_list(part_elem->partition_name,
                            alter_info->partition_names))
        {
          /*
            Set state to indicate that the partition is to be dropped.
          */
          num_parts_found++;
          part_elem->part_state= PART_TO_BE_DROPPED;
        }
      } while (++part_count < tab_part_info->num_parts);
      if (num_parts_found != num_parts_dropped)
      {
        my_error(ER_DROP_PARTITION_NON_EXISTENT, MYF(0), "DROP");
        goto err;
      }

with:

      do
      {
        partition_element *part_elem= part_it++;
        if (is_name_in_list(part_elem->partition_name,
                            alter_info->partition_names))
        {
          /*
            Set state to indicate that the partition is to be dropped.
          */
          num_parts_found++;
          part_elem->part_state= PART_TO_BE_DROPPED;
        }
      } while (++part_count < tab_part_info->num_parts);

      if (num_parts_found != num_parts_dropped)
      {
        my_error(ER_DROP_PARTITION_NON_EXISTENT, MYF(0), "DROP");
        goto err;
      }
      if (num_parts_found == tab_part_info->num_parts)
      {
        my_error(ER_DROP_LAST_PARTITION, MYF(0));
        goto err;
      }
[1 Mar 2017 10:35] MySQL Verification Team
Hello Sean Davidson,

Thank you for the report and test case.
Verified as described with 5.7.17 build.

Thanks,
Umesh