MDEV-19464: Altering partitioned table into S3 causes an obscure error

The error occured because aria_copy_to_s3() function tried to copy .frm file
of partition, but partition does not have it's own .frm file. The same is true
for aria_rename_s3().

To fix this issue the new parameter was added to those two functions to specify
if .frm file must be copied or not. The parameter is set to 'false' for
partitions.

Also there was other issue with EXCHANGE PARTITION. Briefly, there is the
following sequence of operations(see  exchange_name_with_ddl_log() for details):
1) rename swap table to temporary table,
2) rename partition to swap table,
3) rename temporary table to partition.

On step (1) .frm file is renamed too. On step (2) the swap table does not
have .frm file, as partition does not have it. On step (3) partition will have
.frm file, because it will be renamed from temporary table. All of this causes
error on different stages of the table access. To fix it, .frm is not touched
at all for s3 during EXCHANGE PARTITION operation. This is implemented in
ha_s3::rename_table() by additional checking of
current_thd->lex->alter_info.partition_flags(see also ALTER_PARTITION_EXCHANGE
in sql_yacc.yy).

7 files changed, 284 insertions, 29 deletions

diff --git a/mysql-test/suite/s3/partitions-master.opt b/mysql-test/suite/s3/partitions-master.opt
new file mode 100644
index 00000000000..bbb6d7f9ff4
--- /dev/null
+++ b/mysql-test/suite/s3/partitions-master.opt
@@ -0,0 +1 @@
+--loose-partition
diff --git a/mysql-test/suite/s3/partitions.result b/mysql-test/suite/s3/partitions.result
new file mode 100644
index 00000000000..c7f9a9d8cc7
--- /dev/null
+++ b/mysql-test/suite/s3/partitions.result
@@ -0,0 +1,128 @@
+# Test for COALESCE PARTITION, ALTER TABLE and ADD PARTITIONS
+# for tables with HASH partitions
+CREATE TABLE t1 (
+c1 INT DEFAULT NULL
+) ENGINE=Aria
+PARTITION BY HASH (c1)
+PARTITIONS 3;
+INSERT INTO t1 VALUE (1), (2), (101), (102), (201), (202);
+ALTER TABLE t1 ENGINE=S3;
+SELECT count(*) FROM t1;
+count(*)
+6
+ALTER TABLE t1 COALESCE PARTITION 2;
+ERROR HY000: Storage engine S3 of the table `s3`.`t1` doesn't have this option
+ALTER TABLE t1 ADD PARTITION PARTITIONS 6;
+ERROR HY000: Storage engine S3 of the table `s3`.`t1` doesn't have this option
+SELECT count(*) FROM t1;
+count(*)
+6
+ALTER TABLE t1 ADD COLUMN c INT;
+SELECT count(*) FROM t1;
+count(*)
+6
+DROP TABLE t1;
+# Test for simple change engine to S3
+CREATE TABLE t1 (
+c1 int DEFAULT NULL,
+c2 int DEFAULT NULL
+) ENGINE=Aria
+PARTITION BY RANGE (c1)
+SUBPARTITION BY HASH(c2)
+SUBPARTITIONS 2
+(PARTITION p0 VALUES LESS THAN (100),
+PARTITION p1 VALUES LESS THAN (200),
+PARTITION p3 VALUES LESS THAN (300));
+INSERT INTO t1 VALUE (1,1), (2,2), (101,101), (102,102), (201,201), (202,202);
+ALTER TABLE t1 ENGINE=S3;
+SELECT count(*) FROM t1;
+count(*)
+6
+# Test for rename table
+RENAME TABLE t1 TO t2;
+SELECT count(*) FROM t2;
+count(*)
+6
+# Test for TRUNCATE, ANALYZE, CHECK, REBUILD, OPTIMIZE, REPAIR,
+# ADD, DROP, REORGANIZE partition
+ALTER TABLE t2 TRUNCATE PARTITION p3;
+ERROR HY000: Table 't2' is read only
+ALTER TABLE t2 ANALYZE PARTITION p3;
+Table	Op	Msg_type	Msg_text
+s3.t2	analyze	error	Table 's3.t2' is read only
+SELECT count(*) FROM t2;
+count(*)
+6
+ALTER TABLE t2 CHECK PARTITION p3;
+Table	Op	Msg_type	Msg_text
+s3.t2	check	error	Subpartition p3sp0 returned error
+s3.t2	check	error	Unknown - internal error 131 during operation
+SELECT count(*) FROM t2;
+count(*)
+6
+ALTER TABLE t2 REBUILD PARTITION p0, p1;
+ERROR HY000: Storage engine S3 of the table `s3`.`t2` doesn't have this option
+ALTER TABLE t2 OPTIMIZE PARTITION p0, p1;
+Table	Op	Msg_type	Msg_text
+s3.t2	optimize	Error	Table 't2' is read only
+s3.t2	optimize	status	Operation failed
+SELECT count(*) FROM t2;
+count(*)
+6
+ALTER TABLE t2 REPAIR PARTITION p0, p1;
+Table	Op	Msg_type	Msg_text
+s3.t2	repair	Error	Table 't2' is read only
+s3.t2	repair	status	Operation failed
+SELECT count(*) FROM t2;
+count(*)
+6
+ALTER TABLE t2 ADD PARTITION (PARTITION p4 VALUES LESS THAN (400));
+ERROR HY000: Storage engine S3 of the table `s3`.`t2` doesn't have this option
+ALTER TABLE t2
+REORGANIZE PARTITION p3 INTO (
+PARTITION n0 VALUES LESS THAN (500),
+PARTITION n1 VALUES LESS THAN (600)
+);
+ERROR HY000: Storage engine S3 of the table `s3`.`t2` doesn't have this option
+ALTER TABLE t2 DROP PARTITION p3;
+SELECT count(*) from t2;
+count(*)
+4
+# Test for ALTER TABLE
+ALTER TABLE t2 ADD COLUMN c INT;
+SELECT count(*) FROM t2;
+count(*)
+4
+ALTER TABLE t2 DROP COLUMN c;
+SELECT count(*) FROM t2;
+count(*)
+4
+# Test for REMOVE PARTITIONING
+ALTER TABLE t2 REMOVE PARTITIONING;
+SELECT count(*) FROM t2;
+count(*)
+4
+DROP TABLE t2;
+# Test for EXCHANGE PARTITION
+CREATE TABLE t1 (
+c1 int DEFAULT NULL
+) ENGINE=Aria
+PARTITION BY RANGE (c1)
+(PARTITION p0 VALUES LESS THAN (100),
+PARTITION p1 VALUES LESS THAN (200));
+INSERT INTO t1 VALUE (1), (2), (101), (102);
+ALTER TABLE t1 ENGINE=S3;
+CREATE TABLE t_part (
+c1 int DEFAULT NULL
+) ENGINE=Aria;
+INSERT INTO t_part VALUE (120), (130), (140);
+ALTER TABLE t_part ENGINE=S3;
+ALTER TABLE t1 EXCHANGE PARTITION p1 WITH TABLE t_part;
+SELECT count(*) FROM t_part;
+count(*)
+2
+SELECT count(*) FROM t1;
+count(*)
+5
+DROP TABLE t1;
+DROP TABLE t_part;
diff --git a/mysql-test/suite/s3/partitions.test b/mysql-test/suite/s3/partitions.test
new file mode 100644
index 00000000000..196a72e2826
--- /dev/null
+++ b/mysql-test/suite/s3/partitions.test
@@ -0,0 +1,113 @@
+--source include/have_partition.inc
+--source include/have_s3.inc
+--source create_database.inc
+
+
+--echo # Test for COALESCE PARTITION, ALTER TABLE and ADD PARTITIONS
+--echo # for tables with HASH partitions
+CREATE TABLE t1 (
+  c1 INT DEFAULT NULL
+) ENGINE=Aria
+  PARTITION BY HASH (c1)
+  PARTITIONS 3;
+INSERT INTO t1 VALUE (1), (2), (101), (102), (201), (202);
+ALTER TABLE t1 ENGINE=S3;
+SELECT count(*) FROM t1;
+--replace_result $database s3
+--error ER_ILLEGAL_HA
+ALTER TABLE t1 COALESCE PARTITION 2;
+--replace_result $database s3
+--error ER_ILLEGAL_HA
+ALTER TABLE t1 ADD PARTITION PARTITIONS 6;
+SELECT count(*) FROM t1;
+ALTER TABLE t1 ADD COLUMN c INT;
+SELECT count(*) FROM t1;
+DROP TABLE t1;
+
+--echo # Test for simple change engine to S3
+CREATE TABLE t1 (
+  c1 int DEFAULT NULL,
+  c2 int DEFAULT NULL
+) ENGINE=Aria
+  PARTITION BY RANGE (c1)
+  SUBPARTITION BY HASH(c2)
+  SUBPARTITIONS 2
+ (PARTITION p0 VALUES LESS THAN (100),
+ PARTITION p1 VALUES LESS THAN (200),
+ PARTITION p3 VALUES LESS THAN (300));
+
+INSERT INTO t1 VALUE (1,1), (2,2), (101,101), (102,102), (201,201), (202,202);
+ALTER TABLE t1 ENGINE=S3;
+SELECT count(*) FROM t1;
+
+--echo # Test for rename table
+RENAME TABLE t1 TO t2;
+SELECT count(*) FROM t2;
+
+--echo # Test for TRUNCATE, ANALYZE, CHECK, REBUILD, OPTIMIZE, REPAIR,
+--echo # ADD, DROP, REORGANIZE partition
+--error ER_OPEN_AS_READONLY
+ALTER TABLE t2 TRUNCATE PARTITION p3;
+--replace_result $database s3
+ALTER TABLE t2 ANALYZE PARTITION p3;
+SELECT count(*) FROM t2;
+--replace_result $database s3
+ALTER TABLE t2 CHECK PARTITION p3;
+SELECT count(*) FROM t2;
+--replace_result $database s3
+--error ER_ILLEGAL_HA
+ALTER TABLE t2 REBUILD PARTITION p0, p1;
+--replace_result $database s3
+ALTER TABLE t2 OPTIMIZE PARTITION p0, p1;
+SELECT count(*) FROM t2;
+--replace_result $database s3
+ALTER TABLE t2 REPAIR PARTITION p0, p1;
+SELECT count(*) FROM t2;
+--replace_result $database s3
+--error ER_ILLEGAL_HA
+ALTER TABLE t2 ADD PARTITION (PARTITION p4 VALUES LESS THAN (400));
+--replace_result $database s3
+--error ER_ILLEGAL_HA
+ALTER TABLE t2
+    REORGANIZE PARTITION p3 INTO (
+        PARTITION n0 VALUES LESS THAN (500),
+        PARTITION n1 VALUES LESS THAN (600)
+);
+ALTER TABLE t2 DROP PARTITION p3;
+SELECT count(*) from t2;
+
+--echo # Test for ALTER TABLE
+ALTER TABLE t2 ADD COLUMN c INT;
+SELECT count(*) FROM t2;
+ALTER TABLE t2 DROP COLUMN c;
+SELECT count(*) FROM t2;
+
+--echo # Test for REMOVE PARTITIONING
+ALTER TABLE t2 REMOVE PARTITIONING;
+SELECT count(*) FROM t2;
+DROP TABLE t2;
+
+--echo # Test for EXCHANGE PARTITION
+CREATE TABLE t1 (
+  c1 int DEFAULT NULL
+) ENGINE=Aria
+  PARTITION BY RANGE (c1)
+ (PARTITION p0 VALUES LESS THAN (100),
+ PARTITION p1 VALUES LESS THAN (200));
+INSERT INTO t1 VALUE (1), (2), (101), (102);
+ALTER TABLE t1 ENGINE=S3;
+CREATE TABLE t_part (
+  c1 int DEFAULT NULL
+) ENGINE=Aria;
+INSERT INTO t_part VALUE (120), (130), (140);
+ALTER TABLE t_part ENGINE=S3;
+ALTER TABLE t1 EXCHANGE PARTITION p1 WITH TABLE t_part;
+SELECT count(*) FROM t_part;
+SELECT count(*) FROM t1;
+DROP TABLE t1;
+DROP TABLE t_part;
+
+#
+# clean up
+#
+--source drop_database.inc
diff --git a/storage/maria/aria_s3_copy.cc b/storage/maria/aria_s3_copy.cc
index 9e8a83a1602..ac72242ff1c 100644
--- a/storage/maria/aria_s3_copy.cc
+++ b/storage/maria/aria_s3_copy.cc
@@ -237,9 +237,10 @@ int main(int argc, char** argv)
 
     switch (opt_operation) {
     case 0:
+      /* Don't copy .frm file for partioned table */
       if (aria_copy_to_s3(global_s3_client, opt_s3_bucket, path,
                           db, table_name, opt_block_size, opt_compression,
-                          opt_force, opt_verbose))
+                          opt_force, opt_verbose, !strstr(table_name, "#P#")))
       {
         fprintf(stderr, "Aborting copying of %s\n", path);
         my_exit(-1);
diff --git a/storage/maria/ha_s3.cc b/storage/maria/ha_s3.cc
index 89a7e6ea773..3589dcc539e 100644
--- a/storage/maria/ha_s3.cc
+++ b/storage/maria/ha_s3.cc
@@ -59,6 +59,7 @@
   at least s3_block_size * 32. The default cache is 512M.
 */
 
+#define MYSQL_SERVER 1
 #include "maria_def.h"
 #include "sql_class.h"
 #include <mysys_err.h>
@@ -333,6 +334,8 @@ int ha_s3::rename_table(const char *from, const char *to)
   ms3_st *s3_client;
   MY_STAT stat_info;
   int error;
+  bool is_partition= (strstr(from, "#P#") != NULL) ||
+                     (strstr(to, "#P#") != NULL);
   DBUG_ENTER("ha_s3::rename_table");
 
   if (s3_info_init(&to_s3_info, to, to_name, NAME_LEN))
@@ -347,7 +350,7 @@ int ha_s3::rename_table(const char *from, const char *to)
   */
   fn_format(frm_name, from, "", reg_ext, MYF(0));
   if (!strncmp(from + dirname_length(from), "#sql-", 5) &&
-      my_stat(frm_name, &stat_info, MYF(0)))
+      (is_partition || my_stat(frm_name, &stat_info, MYF(0))))
   {
     /*
       The table is a temporary table as part of ALTER TABLE.
@@ -356,7 +359,7 @@ int ha_s3::rename_table(const char *from, const char *to)
     error= aria_copy_to_s3(s3_client, to_s3_info.bucket.str, from,
                            to_s3_info.database.str,
                            to_s3_info.table.str,
-                           0, 0, 0, 0);
+                           0, 0, 0, 0, !is_partition);
     if (!error)
     {
       /* Remove original files table files, keep .frm */
@@ -377,7 +380,9 @@ int ha_s3::rename_table(const char *from, const char *to)
                           from_s3_info.database.str,
                           from_s3_info.table.str,
                           to_s3_info.database.str,
-                          to_s3_info.table.str);
+                          to_s3_info.table.str,
+                          !is_partition &&
+                          !current_thd->lex->alter_info.partition_flags);
   }
   ms3_deinit(s3_client);
   DBUG_RETURN(error);
diff --git a/storage/maria/s3_func.c b/storage/maria/s3_func.c
index b8031fea16b..29491557df9 100644
--- a/storage/maria/s3_func.c
+++ b/storage/maria/s3_func.c
@@ -282,7 +282,7 @@ int aria_copy_to_s3(ms3_st *s3_client, const char *aws_bucket,
                     const char *path,
                     const char *database, const char *table_name,
                     ulong block_size, my_bool compression,
-                    my_bool force, my_bool display)
+                    my_bool force, my_bool display, my_bool copy_frm)
 {
   ARIA_TABLE_CAPABILITIES cap;
   char aws_path[FN_REFLEN+100];
@@ -313,28 +313,31 @@ int aria_copy_to_s3(ms3_st *s3_client, const char *aws_bucket,
       DBUG_RETURN(error);
   }
 
-  /*
-    Copy frm file if it exists
-    We do this first to ensure that .frm always exists. This is needed to
-    ensure that discovery of the table will work.
-  */
-  fn_format(filename, path, "", ".frm", MY_REPLACE_EXT);
-  if (!s3_read_frm_from_disk(filename, &alloc_block, &frm_length))
+  if (copy_frm)
   {
-    if (display)
-      printf("Copying frm file %s\n", filename);
+    /*
+      Copy frm file if it exists
+      We do this first to ensure that .frm always exists. This is needed to
+      ensure that discovery of the table will work.
+    */
+    fn_format(filename, path, "", ".frm", MY_REPLACE_EXT);
+    if (!s3_read_frm_from_disk(filename, &alloc_block, &frm_length))
+    {
+      if (display)
+        printf("Copying frm file %s\n", filename);
 
-    end= strmov(aws_path_end,"/frm");
-    convert_frm_to_s3_format(alloc_block);
+      end= strmov(aws_path_end,"/frm");
+      convert_frm_to_s3_format(alloc_block);
 
-    /* Note that frm is not compressed! */
-    if (s3_put_object(s3_client, aws_bucket, aws_path, alloc_block, frm_length,
-                      0))
-      goto err;
+      /* Note that frm is not compressed! */
+      if (s3_put_object(s3_client, aws_bucket, aws_path, alloc_block, frm_length,
+                        0))
+        goto err;
 
-    frm_created= 1;
-    my_free(alloc_block);
-    alloc_block= 0;
+      frm_created= 1;
+      my_free(alloc_block);
+      alloc_block= 0;
+    }
   }
 
   if (display)
@@ -721,7 +724,8 @@ int aria_delete_from_s3(ms3_st *s3_client, const char *aws_bucket,
 
 int aria_rename_s3(ms3_st *s3_client, const char *aws_bucket,
                    const char *from_database, const char *from_table,
-                   const char *to_database, const char *to_table)
+                   const char *to_database, const char *to_table,
+                   my_bool rename_frm)
 {
   ms3_status_st status;
   char to_aws_path[FN_REFLEN+100], from_aws_path[FN_REFLEN+100];
@@ -755,10 +759,12 @@ int aria_rename_s3(ms3_st *s3_client, const char *aws_bucket,
   error|= s3_rename_directory(s3_client, aws_bucket, from_aws_path,
                               to_aws_path, 1);
 
-  strmov(from_aws_path_end, "/frm");
-  strmov(to_aws_path_end, "/frm");
+  if (rename_frm) {
+    strmov(from_aws_path_end, "/frm");
+    strmov(to_aws_path_end, "/frm");
 
-  s3_rename_object(s3_client, aws_bucket, from_aws_path, to_aws_path, 1);
+    s3_rename_object(s3_client, aws_bucket, from_aws_path, to_aws_path, 1);
+  }
 
   strmov(from_aws_path_end,"/aria");
   strmov(to_aws_path_end,"/aria");
diff --git a/storage/maria/s3_func.h b/storage/maria/s3_func.h
index d679e0790db..6f01d9ee56d 100644
--- a/storage/maria/s3_func.h
+++ b/storage/maria/s3_func.h
@@ -56,7 +56,7 @@ int aria_copy_to_s3(ms3_st *s3_client, const char *aws_bucket,
                     const char *path,
                     const char *database, const char *table_name,
                     ulong block_size, my_bool compression,
-                    my_bool force, my_bool display);
+                    my_bool force, my_bool display, my_bool copy_frm);
 int aria_copy_from_s3(ms3_st *s3_client, const char *aws_bucket,
                       const char *path,const char *database,
                       my_bool compression, my_bool force, my_bool display);
@@ -65,7 +65,8 @@ int aria_delete_from_s3(ms3_st *s3_client, const char *aws_bucket,
                         my_bool display);
 int aria_rename_s3(ms3_st *s3_client, const char *aws_bucket,
                    const char *from_database, const char *from_table,
-                   const char *to_database, const char *to_table);
+                   const char *to_database, const char *to_table,
+                   my_bool rename_frm);
 ms3_st *s3_open_connection(S3_INFO *s3);
 my_bool s3_put_object(ms3_st *s3_client, const char *aws_bucket,
                       const char *name, uchar *data, size_t length,