Merge ~cjwatson/launchpad:fix-miscompiled-pofile-query into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 49064791591257da0a00980472d12e8f5cf21ad4
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:fix-miscompiled-pofile-query
Merge into: launchpad:master
Diff against target: 20 lines (+2/-2)
1 file modified
lib/lp/translations/model/pofile.py (+2/-2)
Reviewer Review Type Date Requested Status
Thiago F. Pappacena (community) Approve
Review via email: mp+395934@code.launchpad.net

Commit message

Fix miscompiled query in POFileMixin._getTranslationSearchQuery

Description of the change

Converting POFile queries to Storm (https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/394702) introduced a serious performance regression. It turns out that using Reference objects via ClassAlias doesn't work as expected. The following:

    tm_ids = ClassAlias(TranslationMessage, "tm_ids")
    tm_ids.language == pofile.language

... is compiled to "TranslationMessage.language = $ID" rather than "tm_ids.language = $ID". Using the Reference's Int partner instead works around this.

I haven't worked out how to test this automatically (or even whether it results in a different set of rows rather than merely a much slower query), but I've manually tested on staging that the modified query performs reasonably again.

To post a comment you must log in.
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

LGTM. Good catch!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/lib/lp/translations/model/pofile.py b/lib/lp/translations/model/pofile.py
index b6af3e7..799eb18 100644
--- a/lib/lp/translations/model/pofile.py
+++ b/lib/lp/translations/model/pofile.py
@@ -189,13 +189,13 @@ class POFileMixIn(RosettaStats):
189 tm_ids,189 tm_ids,
190 Join(190 Join(
191 TranslationTemplateItem,191 TranslationTemplateItem,
192 tm_ids.potmsgset ==192 tm_ids.potmsgsetID ==
193 TranslationTemplateItem.potmsgsetID)),193 TranslationTemplateItem.potmsgsetID)),
194 where=And(194 where=And(
195 TranslationTemplateItem.potemplate ==195 TranslationTemplateItem.potemplate ==
196 pofile.potemplate,196 pofile.potemplate,
197 TranslationTemplateItem.sequence > 0,197 TranslationTemplateItem.sequence > 0,
198 tm_ids.language == pofile.language),198 tm_ids.languageID == pofile.languageID),
199 distinct=True)),199 distinct=True)),
200 Like(200 Like(
201 POTranslation.translation,201 POTranslation.translation,

Subscribers

People subscribed via source and target branches

to status/vote changes: