Skip to content

Commit 179df4f

Browse files
committed
fix(#64): cartesian product in reln reflection to non-versioned classes
In many-to-many relationships when one of the classes is not versioned, we were not including the secondaryjoin in the filter criteria leading to a cartesian product.
1 parent b6d50b0 commit 179df4f

2 files changed

Lines changed: 25 additions & 1 deletion

File tree

sqlalchemy_history/relationship_builder.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,12 @@ def criteria(self, obj):
8383
return self.many_to_one_criteria(obj)
8484
else:
8585
reflector = VersionExpressionReflector(obj, self.property)
86-
return reflector(self.property.primaryjoin)
86+
criteria = reflector(self.property.primaryjoin)
87+
88+
# For many-to-many relationships, we also need to include the secondary join
89+
if direction.name == "MANYTOMANY" and self.property.secondaryjoin is not None:
90+
criteria = sa.and_(criteria, reflector(self.property.secondaryjoin))
91+
return criteria
8792

8893
def many_to_many_criteria(self, obj):
8994
"""Returns the many-to-many query.

tests/relationships/test_non_versioned_classes.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,22 @@ def test_single_insert(self):
101101
self.session.commit()
102102
assert len(article.versions[0].tags) == 1
103103
assert isinstance(article.versions[0].tags[0], self.Tag)
104+
105+
def test_no_cartesian_product_with_multiple_unrelated_tags(self):
106+
# Create an article with one tag
107+
article = self.Article(name="Some article")
108+
tag1 = self.Tag(name="tag1")
109+
article.tags.append(tag1)
110+
self.session.add(article)
111+
self.session.commit()
112+
113+
# Create another article with a different tag
114+
article2 = self.Article(name="Another article")
115+
tag2 = self.Tag(name="tag2")
116+
article2.tags.append(tag2)
117+
self.session.add(article2)
118+
self.session.commit()
119+
120+
# Ensure the first article's version only has its own tag, not all tags
121+
assert len(article.versions[0].tags) == 1
122+
assert article.versions[0].tags[0] == tag1

0 commit comments

Comments
 (0)