Skip to content

Commit 73df8cd

Browse files
fix: query count discrepancy between MySQL and SQLite
Claude explanation: In `_create_side_effects_for_change_log`, `change_log.records.all()` has no `ORDER BY`. On SQLite, this returns records in insertion order (PK order) — which is root-first, since `publish_from_drafts` inserts the initial draft first, then its dependencies layer by layer. On MySQL/InnoDB, the query uses the `oel_plr_uniq_pl_publishable` unique index on (`publish_log`, `entity`) to service the `WHERE publish_log_id = ?` filter, which returns records in `entity_id` order — i.e. leaf-first, because `child_entity2` has a lower entity PK than `parent_of_two`, `grandparent`, etc. The `processed_entity_ids` optimization on line 899–904 is order-sensitive: when you process leaf records first, `processed_entity_ids` isn't yet populated with their ancestors' IDs, so the while loop traverses all the way up the tree redundantly on each leaf
1 parent fe1b594 commit 73df8cd

3 files changed

Lines changed: 3 additions & 17 deletions

File tree

src/openedx_content/applets/publishing/api.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -810,7 +810,7 @@ def _create_side_effects_for_change_log(change_log: DraftChangeLog | PublishLog)
810810
# It also guards against infinite parent-child relationship loops, though
811811
# those aren't *supposed* to happen anyhow.
812812
processed_entity_ids: set[int] = set()
813-
for original_change in change_log.records.all():
813+
for original_change in change_log.records.order_by("pk"):
814814
affected_by_original_change = branch_cls.objects.filter(
815815
version__dependencies=original_change.entity
816816
)

tests/openedx_content/applets/containers/test_api.py

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,11 +1273,7 @@ def test_deep_publish_log(
12731273
]
12741274

12751275
# Publish great_grandparent. Should publish the whole tree.
1276-
# FIXME: this is using 144 queries on MySQL vs 132 on SQLite. The disparity only happens when publishing four levels
1277-
# of hierarchy at once (e.g. publishing a Section-->Component or Great-Grandparent-->Child). i.e. if you change this
1278-
# to publish "grandparent" instead of "great_grandparent" or you pre-publish the leaves (child entities), there is
1279-
# no disparity between the databases.
1280-
with django_assert_num_queries(133):
1276+
with django_assert_num_queries(132):
12811277
publish_log = publish_entity(great_grandparent)
12821278
assert list(publish_log.records.order_by("entity__pk").values_list("entity__key", flat=True)) == [
12831279
"child_entity2",
@@ -1286,16 +1282,6 @@ def test_deep_publish_log(
12861282
"grandparent",
12871283
"great_grandparent",
12881284
]
1289-
assert list(
1290-
PublishSideEffect.objects.filter(cause__publish_log=publish_log)
1291-
.values_list("cause__entity__key", "effect__entity__key")
1292-
.order_by("cause__entity__key") # Note: order_by("pk") is different on MySQL vs SQLite
1293-
) == [
1294-
("child_entity2", "parent_of_two"),
1295-
("grandparent", "great_grandparent"),
1296-
("parent_of_three", "grandparent"),
1297-
("parent_of_two", "grandparent"),
1298-
]
12991285

13001286

13011287
# get_entities_in_container

tests/openedx_content/applets/sections/test_api.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ def test_section_queries(self) -> None:
154154
"""
155155
with self.assertNumQueries(37):
156156
section = self.create_section_with_subsections([self.subsection_1, self.subsection_2_v1])
157-
with self.assertNumQueries(169): # TODO: this seems high? FIXME: this is 181 on MySQL but 169 on SQLite?
157+
with self.assertNumQueries(169):
158158
content_api.publish_from_drafts(
159159
self.learning_package.id,
160160
draft_qset=content_api.get_all_drafts(self.learning_package.id).filter(entity=section.pk),

0 commit comments

Comments
 (0)