Skip to content

Commit

Permalink
Correct multi-column key partition ordering (#61)
Browse files Browse the repository at this point in the history
To quote from the documentation:

> With RANGE COLUMNS, a row matches a partition if all row values are less
> than specified values. The first partition that matches row values will be
> used.

Previously, with `RANGE COLUMNS`, partition-manager got this logic backward,
which led to partitions being considered "unfilled" and candidates for rename
which actually had data in them.

Fixes #60
  • Loading branch information
jcjones authored Apr 12, 2023
1 parent 98deada commit ba35884
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 8 deletions.
6 changes: 5 additions & 1 deletion partitionmanager/table_append_partition_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,11 @@ def test_split(self):
[mkPPart("a", 10, 10), mkPPart("b", 20, 20), mkTailPart("z", count=2)],
mkPos(19, 500),
),
([mkPPart("a", 10, 10)], mkPPart("b", 20, 20), [mkTailPart("z", count=2)]),
(
[mkPPart("a", 10, 10), mkPPart("b", 20, 20)],
mkTailPart("z", count=2),
[],
),
)

def test_get_position_increase_per_day(self):
Expand Down
8 changes: 5 additions & 3 deletions partitionmanager/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,10 +343,12 @@ def __lt__(self, other):
f"Expected {len(self._position)} columns but partition has {other_position_list}."
)

# If ALL of v_mine >= v_other, then self is greater than other
# If ANY of v_mine < v_other, then self is less than other
for v_mine, v_other in zip(self._position.as_list(), other_position_list):
if v_mine >= v_other:
return False
return True
if v_mine < v_other:
return True
return False

def __eq__(self, other):
if isinstance(other, PositionPartition):
Expand Down
20 changes: 16 additions & 4 deletions partitionmanager/types_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,18 +345,30 @@ def test_partition_timestamps(self):
with self.assertRaises(UnexpectedPartitionException):
mkPPart("a", 10, 10) < mkTailPart("b", count=1)

self.assertFalse(mkPPart("a", 10, 10) < mkPPart("b", 11, 10))
self.assertFalse(mkPPart("a", 10, 10) < mkPPart("b", 10, 11))
self.assertTrue(mkPPart("a", 10, 10) < mkPPart("b", 11, 10))
self.assertTrue(mkPPart("a", 10, 10) < mkPPart("b", 10, 11))
self.assertLess(mkPPart("a", 10, 10), mkPPart("b", 11, 11))
self.assertFalse(mkPPart("a", 10, 10) < [10, 11])
self.assertFalse(mkPPart("a", 10, 10) < [11, 10])
self.assertTrue(mkPPart("a", 10, 10) < [10, 11])
self.assertTrue(mkPPart("a", 10, 10) < [11, 10])
self.assertLess(mkPPart("a", 10, 10), [11, 11])

with self.assertRaises(UnexpectedPartitionException):
mkPPart("a", 10, 10) < mkPPart("b", 11, 11, 11)
with self.assertRaises(UnexpectedPartitionException):
mkPPart("a", 10, 10, 10) < mkPPart("b", 11, 11)

def test_partition_tuple_ordering(self):
cur_pos = mkPPart("current_pos", 8236476764, 6096376984)
p_20220525 = mkPPart("p_20220525", 2805308158, 2682458996)
p_20220611 = mkPPart("p_20220611", 7882495694, 7856340600)
p_20230519 = mkPPart("p_20230519", 10790547177, 11048018089)
p_20230724 = mkPPart("p_20230724", 95233456870, 97348306298)

self.assertGreater(cur_pos, p_20220525)
self.assertGreater(cur_pos, p_20220611)
self.assertLess(cur_pos, p_20230519)
self.assertLess(cur_pos, p_20230724)

def test_instant_partition(self):
now = datetime.utcnow()

Expand Down

0 comments on commit ba35884

Please sign in to comment.