Skip to content

Commit afc8400

Browse files
committed
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.
1 parent 1d8cf87 commit afc8400

File tree

4 files changed

+30
-19
lines changed

4 files changed

+30
-19
lines changed

partitionmanager/bootstrap.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,19 @@ def _generate_sql_copy_commands(
149149

150150
range_id_string = ", ".join(map_data["range_cols"])
151151

152+
if len(map_data["range_cols"]) == 1:
153+
range_cols_string = "RANGE"
154+
max_val_string = "MAXVALUE"
155+
else:
156+
num_cols = len(map_data["range_cols"])
157+
range_cols_string = "RANGE COLUMNS"
158+
max_val_string = "(" + ", ".join(["MAXVALUE"] * num_cols) + ")"
159+
152160
yield f"DROP TABLE IF EXISTS {new_table.name};"
153161
yield f"CREATE TABLE {new_table.name} LIKE {existing_table.name};"
154162
yield f"ALTER TABLE {new_table.name} REMOVE PARTITIONING;"
155-
yield f"ALTER TABLE {new_table.name} PARTITION BY RANGE({range_id_string}) ("
156-
yield f"\tPARTITION {max_val_part.name} VALUES LESS THAN MAXVALUE"
163+
yield f"ALTER TABLE {new_table.name} PARTITION BY {range_cols_string} ({range_id_string}) ("
164+
yield f"\tPARTITION {max_val_part.name} VALUES LESS THAN {max_val_string}"
157165
yield ");"
158166

159167
for command in alter_commands_iter:

partitionmanager/bootstrap_test.py

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ def test_read_state_info(self):
128128
"DROP TABLE IF EXISTS test_new_20210303;",
129129
"CREATE TABLE test_new_20210303 LIKE test;",
130130
"ALTER TABLE test_new_20210303 REMOVE PARTITIONING;",
131-
"ALTER TABLE test_new_20210303 PARTITION BY RANGE(id) (",
131+
"ALTER TABLE test_new_20210303 PARTITION BY RANGE (id) (",
132132
"\tPARTITION p_start VALUES LESS THAN MAXVALUE",
133133
");",
134134
"ALTER TABLE `test_new_20210303` REORGANIZE PARTITION `p_start` "
@@ -152,22 +152,22 @@ def test_read_state_info(self):
152152
def test_read_state_info_map_table(self):
153153
self.maxDiff = None
154154
conf = Config()
155-
conf.assume_partitioned_on = ["order", "auth"]
155+
conf.assume_partitioned_on = ["orderID", "authzID"]
156156
conf.curtime = datetime(2021, 3, 3)
157157
conf.dbcmd = MockDatabase()
158-
conf.dbcmd._select_response = [[{"auth": 22}], [{"order": 11}]]
158+
conf.dbcmd._select_response = [[{"authzID": 22}], [{"orderID": 11}]]
159159
conf.dbcmd._response = [
160160
[
161-
{"Field": "order", "Type": "bigint UNSIGNED"},
162-
{"Field": "auth", "Type": "bigint UNSIGNED"},
161+
{"Field": "orderID", "Type": "bigint UNSIGNED"},
162+
{"Field": "authzID", "Type": "bigint UNSIGNED"},
163163
]
164164
]
165165
conf.tables = [Table("map_table").set_partition_period(timedelta(days=30))]
166166

167167
state_fs = io.StringIO()
168168
yaml.dump(
169169
{
170-
"tables": {"map_table": {"order": 11, "auth": 22}},
170+
"tables": {"map_table": {"orderID": 10, "authzID": 20}},
171171
"time": (conf.curtime - timedelta(days=1)),
172172
},
173173
state_fs,
@@ -183,20 +183,21 @@ def test_read_state_info_map_table(self):
183183
"DROP TABLE IF EXISTS map_table_new_20210303;",
184184
"CREATE TABLE map_table_new_20210303 LIKE map_table;",
185185
"ALTER TABLE map_table_new_20210303 REMOVE PARTITIONING;",
186-
"ALTER TABLE map_table_new_20210303 PARTITION BY RANGE(order, auth) (",
187-
"\tPARTITION p_assumed VALUES LESS THAN MAXVALUE",
186+
"ALTER TABLE map_table_new_20210303 PARTITION BY RANGE "
187+
+ "COLUMNS (orderID, authzID) (",
188+
"\tPARTITION p_assumed VALUES LESS THAN (MAXVALUE, MAXVALUE)",
188189
");",
189190
"ALTER TABLE `map_table_new_20210303` REORGANIZE PARTITION "
190191
+ "`p_assumed` INTO (PARTITION `p_20210303` VALUES LESS THAN "
191192
+ "(11, 22), PARTITION `p_20210402` VALUES LESS THAN "
192-
+ "(11, 22), PARTITION `p_20210502` VALUES LESS THAN "
193-
+ "MAXVALUE, MAXVALUE);",
193+
+ "(41, 82), PARTITION `p_20210502` VALUES LESS THAN "
194+
+ "(MAXVALUE, MAXVALUE));",
194195
"CREATE OR REPLACE TRIGGER copy_inserts_from_map_table_"
195196
+ "to_map_table_new_20210303",
196197
"\tAFTER INSERT ON map_table FOR EACH ROW",
197198
"\t\tINSERT INTO map_table_new_20210303 SET",
198-
"\t\t\t`auth` = NEW.`auth`,",
199-
"\t\t\t`order` = NEW.`order`;",
199+
"\t\t\t`authzID` = NEW.`authzID`,",
200+
"\t\t\t`orderID` = NEW.`orderID`;",
200201
]
201202
},
202203
)
@@ -242,7 +243,7 @@ def test_generate_sql_copy_commands(self):
242243
"DROP TABLE IF EXISTS new;",
243244
"CREATE TABLE new LIKE old;",
244245
"ALTER TABLE new REMOVE PARTITIONING;",
245-
"ALTER TABLE new PARTITION BY RANGE(id) (",
246+
"ALTER TABLE new PARTITION BY RANGE (id) (",
246247
"\tPARTITION p_assumed VALUES LESS THAN MAXVALUE",
247248
");",
248249
"STRAIGHT_UP_INSERTED",

partitionmanager/cli_test.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ def test_bootstrap_cmd_in(self):
490490
"ALTER TABLE partitioned_yesterday_new_20210421 "
491491
+ "REMOVE PARTITIONING;",
492492
"ALTER TABLE partitioned_yesterday_new_20210421 "
493-
+ "PARTITION BY RANGE(id) (",
493+
+ "PARTITION BY RANGE (id) (",
494494
"\tPARTITION p_assumed VALUES LESS THAN MAXVALUE",
495495
");",
496496
"ALTER TABLE `partitioned_yesterday_new_20210421` "
@@ -515,7 +515,7 @@ def test_bootstrap_cmd_in(self):
515515
"DROP TABLE IF EXISTS two_new_20210421;",
516516
"CREATE TABLE two_new_20210421 LIKE two;",
517517
"ALTER TABLE two_new_20210421 REMOVE PARTITIONING;",
518-
"ALTER TABLE two_new_20210421 PARTITION BY RANGE(id) (",
518+
"ALTER TABLE two_new_20210421 PARTITION BY RANGE (id) (",
519519
"\tPARTITION p_assumed VALUES LESS THAN MAXVALUE",
520520
");",
521521
"ALTER TABLE `two_new_20210421` REORGANIZE PARTITION "
@@ -575,7 +575,7 @@ def test_bootstrap_cmd_in_unpartitioned_with_override(self):
575575
"DROP TABLE IF EXISTS unpartitioned_new_20210421;",
576576
"CREATE TABLE unpartitioned_new_20210421 LIKE unpartitioned;",
577577
"ALTER TABLE unpartitioned_new_20210421 REMOVE PARTITIONING;",
578-
"ALTER TABLE unpartitioned_new_20210421 PARTITION BY RANGE(id) (",
578+
"ALTER TABLE unpartitioned_new_20210421 PARTITION BY RANGE (id) (",
579579
"\tPARTITION p_assumed VALUES LESS THAN MAXVALUE",
580580
");",
581581
"ALTER TABLE `unpartitioned_new_20210421` REORGANIZE "

partitionmanager/types.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,9 @@ def num_columns(self):
313313
return self._count
314314

315315
def values(self):
316-
return ", ".join(["MAXVALUE"] * self._count)
316+
if self._count == 1:
317+
return "MAXVALUE"
318+
return "(" + ", ".join(["MAXVALUE"] * self._count) + ")"
317319

318320
def __lt__(self, other):
319321
"""MaxValuePartitions are always greater than every other partition."""

0 commit comments

Comments
 (0)