Fix review comments from Tim

This corrects a bug in MaxValuePartition where its value string wasn't
paren-surrounded if there are actually multiple columns.

Author: jcjones

partitionmanager/bootstrap.py

@@ -149,11 +149,19 @@ def _generate_sql_copy_commands(
 
     range_id_string = ", ".join(map_data["range_cols"])
 
+    if len(map_data["range_cols"]) == 1:
+        range_cols_string = "RANGE"
+        max_val_string = "MAXVALUE"
+    else:
+        num_cols = len(map_data["range_cols"])
+        range_cols_string = "RANGE COLUMNS"
+        max_val_string = "(" + ", ".join(["MAXVALUE"] * num_cols) + ")"
+
     yield f"DROP TABLE IF EXISTS {new_table.name};"
     yield f"CREATE TABLE {new_table.name} LIKE {existing_table.name};"
     yield f"ALTER TABLE {new_table.name} REMOVE PARTITIONING;"
-    yield f"ALTER TABLE {new_table.name} PARTITION BY RANGE({range_id_string}) ("
-    yield f"\tPARTITION {max_val_part.name} VALUES LESS THAN MAXVALUE"
+    yield f"ALTER TABLE {new_table.name} PARTITION BY {range_cols_string} ({range_id_string}) ("
+    yield f"\tPARTITION {max_val_part.name} VALUES LESS THAN {max_val_string}"
     yield ");"
 
     for command in alter_commands_iter:

partitionmanager/bootstrap_test.py

@@ -128,7 +128,7 @@ def test_read_state_info(self):
                     "DROP TABLE IF EXISTS test_new_20210303;",
                     "CREATE TABLE test_new_20210303 LIKE test;",
                     "ALTER TABLE test_new_20210303 REMOVE PARTITIONING;",
-                    "ALTER TABLE test_new_20210303 PARTITION BY RANGE(id) (",
+                    "ALTER TABLE test_new_20210303 PARTITION BY RANGE (id) (",
                     "\tPARTITION p_start VALUES LESS THAN MAXVALUE",
                     ");",
                     "ALTER TABLE `test_new_20210303` REORGANIZE PARTITION `p_start` "
@@ -152,22 +152,22 @@ def test_read_state_info(self):
     def test_read_state_info_map_table(self):
         self.maxDiff = None
         conf = Config()
-        conf.assume_partitioned_on = ["order", "auth"]
+        conf.assume_partitioned_on = ["orderID", "authzID"]
         conf.curtime = datetime(2021, 3, 3)
         conf.dbcmd = MockDatabase()
-        conf.dbcmd._select_response = [[{"auth": 22}], [{"order": 11}]]
+        conf.dbcmd._select_response = [[{"authzID": 22}], [{"orderID": 11}]]
         conf.dbcmd._response = [
             [
-                {"Field": "order", "Type": "bigint UNSIGNED"},
-                {"Field": "auth", "Type": "bigint UNSIGNED"},
+                {"Field": "orderID", "Type": "bigint UNSIGNED"},
+                {"Field": "authzID", "Type": "bigint UNSIGNED"},
             ]
         ]
         conf.tables = [Table("map_table").set_partition_period(timedelta(days=30))]
 
         state_fs = io.StringIO()
         yaml.dump(
             {
-                "tables": {"map_table": {"order": 11, "auth": 22}},
+                "tables": {"map_table": {"orderID": 10, "authzID": 20}},
                 "time": (conf.curtime - timedelta(days=1)),
             },
             state_fs,
@@ -183,20 +183,21 @@ def test_read_state_info_map_table(self):
                     "DROP TABLE IF EXISTS map_table_new_20210303;",
                     "CREATE TABLE map_table_new_20210303 LIKE map_table;",
                     "ALTER TABLE map_table_new_20210303 REMOVE PARTITIONING;",
-                    "ALTER TABLE map_table_new_20210303 PARTITION BY RANGE(order, auth) (",
-                    "\tPARTITION p_assumed VALUES LESS THAN MAXVALUE",
+                    "ALTER TABLE map_table_new_20210303 PARTITION BY RANGE "
+                    + "COLUMNS (orderID, authzID) (",
+                    "\tPARTITION p_assumed VALUES LESS THAN (MAXVALUE, MAXVALUE)",
                     ");",
                     "ALTER TABLE `map_table_new_20210303` REORGANIZE PARTITION "
                     + "`p_assumed` INTO (PARTITION `p_20210303` VALUES LESS THAN "
                     + "(11, 22), PARTITION `p_20210402` VALUES LESS THAN "
-                    + "(11, 22), PARTITION `p_20210502` VALUES LESS THAN "
-                    + "MAXVALUE, MAXVALUE);",
+                    + "(41, 82), PARTITION `p_20210502` VALUES LESS THAN "
+                    + "(MAXVALUE, MAXVALUE));",
                     "CREATE OR REPLACE TRIGGER copy_inserts_from_map_table_"
                     + "to_map_table_new_20210303",
                     "\tAFTER INSERT ON map_table FOR EACH ROW",
                     "\t\tINSERT INTO map_table_new_20210303 SET",
-                    "\t\t\t`auth` = NEW.`auth`,",
-                    "\t\t\t`order` = NEW.`order`;",
+                    "\t\t\t`authzID` = NEW.`authzID`,",
+                    "\t\t\t`orderID` = NEW.`orderID`;",
                 ]
             },
         )
@@ -242,7 +243,7 @@ def test_generate_sql_copy_commands(self):
                 "DROP TABLE IF EXISTS new;",
                 "CREATE TABLE new LIKE old;",
                 "ALTER TABLE new REMOVE PARTITIONING;",
-                "ALTER TABLE new PARTITION BY RANGE(id) (",
+                "ALTER TABLE new PARTITION BY RANGE (id) (",
                 "\tPARTITION p_assumed VALUES LESS THAN MAXVALUE",
                 ");",
                 "STRAIGHT_UP_INSERTED",

partitionmanager/cli_test.py

@@ -490,7 +490,7 @@ def test_bootstrap_cmd_in(self):
                         "ALTER TABLE partitioned_yesterday_new_20210421 "
                         + "REMOVE PARTITIONING;",
                         "ALTER TABLE partitioned_yesterday_new_20210421 "
-                        + "PARTITION BY RANGE(id) (",
+                        + "PARTITION BY RANGE (id) (",
                         "\tPARTITION p_assumed VALUES LESS THAN MAXVALUE",
                         ");",
                         "ALTER TABLE `partitioned_yesterday_new_20210421` "
@@ -515,7 +515,7 @@ def test_bootstrap_cmd_in(self):
                         "DROP TABLE IF EXISTS two_new_20210421;",
                         "CREATE TABLE two_new_20210421 LIKE two;",
                         "ALTER TABLE two_new_20210421 REMOVE PARTITIONING;",
-                        "ALTER TABLE two_new_20210421 PARTITION BY RANGE(id) (",
+                        "ALTER TABLE two_new_20210421 PARTITION BY RANGE (id) (",
                         "\tPARTITION p_assumed VALUES LESS THAN MAXVALUE",
                         ");",
                         "ALTER TABLE `two_new_20210421` REORGANIZE PARTITION "
@@ -575,7 +575,7 @@ def test_bootstrap_cmd_in_unpartitioned_with_override(self):
                         "DROP TABLE IF EXISTS unpartitioned_new_20210421;",
                         "CREATE TABLE unpartitioned_new_20210421 LIKE unpartitioned;",
                         "ALTER TABLE unpartitioned_new_20210421 REMOVE PARTITIONING;",
-                        "ALTER TABLE unpartitioned_new_20210421 PARTITION BY RANGE(id) (",
+                        "ALTER TABLE unpartitioned_new_20210421 PARTITION BY RANGE (id) (",
                         "\tPARTITION p_assumed VALUES LESS THAN MAXVALUE",
                         ");",
                         "ALTER TABLE `unpartitioned_new_20210421` REORGANIZE "

partitionmanager/types.py

@@ -313,7 +313,9 @@ def num_columns(self):
         return self._count
 
     def values(self):
-        return ", ".join(["MAXVALUE"] * self._count)
+        if self._count == 1:
+            return "MAXVALUE"
+        return "(" + ", ".join(["MAXVALUE"] * self._count) + ")"
 
     def __lt__(self, other):
         """MaxValuePartitions are always greater than every other partition."""