Merge ~cjwatson/launchpad:simplify-external-translation-messages into launchpad:master

[Colin Watson]

Simplify POTMsgSet._getExternalTranslationMessages

There's no obvious point to sorting the results of a subquery and then passing it straight to the IN operator; this seems to be a vestige of earlier code that fetched the result of the subquery into Python. This doesn't seem to make a massive difference (though it's a bit hard to tell due to caching), but it at least simplifies the query plan.

[William Grant (wgrant)]

It's trying to get the most recent TranslationMessage with each distinct set of translations, not sort the output as such, so this change is likely to cause the result of the subquery and parent query to become orders of magnitude larger. Have you considered that?

review: Needs Information (code)

[Colin Watson (cjwatson)]

A realisation of the query in question (from https://oops.canonical.com/oops/?oopsid=OOPS-6ce2e41d4a1fc3371ecbbfa31fafcc74) is:

WITH msgsets AS
  ( SELECT POTMsgSet.id
   FROM POTMsgSet
   JOIN TranslationTemplateItem ON TranslationTemplateItem.potmsgset = POTMsgSet.id
   JOIN SuggestivePOTemplate ON TranslationTemplateItem.potemplate = SuggestivePOTemplate.potemplate
   WHERE POTMsgSet.msgid_singular = 10087
     AND POTMsgSet.id <> 29306498 )
SELECT TranslationMessage.COMMENT, TranslationMessage.date_created,
                                   TranslationMessage.date_reviewed,
                                   TranslationMessage.id,
                                   TranslationMessage.is_current_ubuntu,
                                   TranslationMessage.is_current_upstream,
                                   TranslationMessage.LANGUAGE, TranslationMessage.msgstr0,
                                                                TranslationMessage.msgstr1,
                                                                TranslationMessage.msgstr2,
                                                                TranslationMessage.msgstr3,
                                                                TranslationMessage.msgstr4,
                                                                TranslationMessage.msgstr5,
                                                                TranslationMessage.origin,
                                                                TranslationMessage.potemplate,
                                                                TranslationMessage.potmsgset,
                                                                TranslationMessage.reviewer,
                                                                TranslationMessage.submitter,
                                                                TranslationMessage.validation_status,
                                                                TranslationMessage.was_obsolete_in_last_import
FROM TranslationMessage
WHERE TranslationMessage.id IN
    ( SELECT DISTINCT ON (COALESCE(msgstr0, -1),
                          COALESCE(msgstr1, -1),
                          COALESCE(msgstr2, -1),
                          COALESCE(msgstr3, -1),
                          COALESCE(msgstr4, -1),
                          COALESCE(msgstr5, -1)) TranslationMessage.id
     FROM TranslationMessage
     JOIN msgsets ON msgsets.id = TranslationMessage.potmsgset
     WHERE (TranslationMessage.LANGUAGE IN (129))
     ORDER BY COALESCE(msgstr0, -1),
              COALESCE(msgstr1, -1),
              COALESCE(msgstr2, -1),
              COALESCE(msgstr3, -1),
              COALESCE(msgstr4, -1),
              COALESCE(msgstr5, -1),
              date_created DESC ) LIMIT 2001;

I'm struggling to see how that fetches the most recent TranslationMessage; as far as I can see the parent query just does a set membership check on a subquery that returns all the associated TranslationMessages with some sorting that doesn't seem as though it can change the end result. What am I missing?

[William Grant (wgrant)]

The subquery's DISTINCT ON combined with its ORDER BY mean only some of the TranslationMessages will be returned, don't they?

Unmerged commits

846d40c... by Colin Watson on 2020-04-01

Simplify POTMsgSet._getExternalTranslationMessages

There's no obvious point to sorting the results of a subquery and then
passing it straight to the IN operator; this seems to be a vestige of
earlier code that fetched the result of the subquery into Python. This
doesn't seem to make a massive difference (though it's a bit hard to
tell due to caching), but it at least simplifies the query plan.

LP: #736005

diff --git a/lib/lp/translations/model/potmsgset.py b/lib/lp/translations/model/potmsgset.py
index 12f9f05..4895840 100644
--- a/lib/lp/translations/model/potmsgset.py
+++ b/lib/lp/translations/model/potmsgset.py
@@ -419,25 +419,14 @@ class POTMsgSet(SQLBase):
 
         # Subquery to find the ids of TranslationMessages that are
         # matching suggestions.
-        # We're going to get a lot of duplicates, sometimes resulting in
-        # thousands of suggestions.  Weed out most of that duplication by
-        # excluding older messages that are identical to newer ones in
-        # all translated forms.  The Python code can later sort out the
-        # distinct translations per form.
-        msgstrs = ', '.join([
-            'COALESCE(msgstr%d, -1)' % form
-            for form in range(TranslationConstants.MAX_PLURAL_FORMS)])
         ids_query_params = {
-            'msgstrs': msgstrs,
             'where': '(' + ' OR '.join(lang_used) + ')',
         }
         ids_query = '''
-            SELECT DISTINCT ON (%(msgstrs)s)
-                TranslationMessage.id
+            SELECT TranslationMessage.id
             FROM TranslationMessage
             JOIN msgsets ON msgsets.id = TranslationMessage.potmsgset
             WHERE %(where)s
-            ORDER BY %(msgstrs)s, date_created DESC
             ''' % ids_query_params
 
         result = IStore(TranslationMessage).with_(msgsets).find(