Merge lp:~wgrant/launchpad/pofile-filter-stormify into lp:launchpad

[William Grant]

Stormify the remaining POTMsgSet filter methods in POFile.

Stormify the remaining POTMsgSet filter methods in POFile. There are no non-cosmetic query changes, but it's much easier to rework the queries to use new denormalised columns.

findPOTMsgSetsContaining is handled separately in https://code.launchpad.net/~wgrant/launchpad/bug-736005-trivialise/+merge/250447.

[Colin Watson (cjwatson)]

I believe that you can remove the _getClausesForPOFileMessages method now too.

review: Approve (code)

=== modified file 'lib/lp/translations/model/pofile.py'
--- lib/lp/translations/model/pofile.py	2013-06-20 05:50:00 +0000
+++ lib/lp/translations/model/pofile.py	2015-02-27 22:28:33 +0000
@@ -23,6 +23,7 @@
     )
 from storm.expr import (
     And,
+    Cast,
     Coalesce,
     Desc,
     Exists,
@@ -267,8 +268,10 @@
         Orders the result by TranslationTemplateItem.sequence which must
         be among `origin_tables`.
         """
+        if isinstance(query, basestring):
+            query = SQL(query)
         results = IMasterStore(POTMsgSet).using(origin_tables).find(
-            POTMsgSet, SQL(query))
+            POTMsgSet, query)
         return results.order_by(TranslationTemplateItem.sequence)
 
     def findPOTMsgSetsContaining(self, text):
@@ -581,50 +584,42 @@
         """
         flag_name = getUtility(ITranslationSideTraitsSet).getForTemplate(
             self.potemplate).flag_name
-        clause_tables = ['TranslationTemplateItem', 'TranslationMessage']
-        clauses = self._getClausesForPOFileMessages()
-        clauses.append('TranslationMessage.%s IS TRUE' % flag_name)
-        self._appendCompletePluralFormsConditions(clauses)
+        clause_tables = [TranslationTemplateItem, TranslationMessage]
+        clauses = self._getStormClausesForPOFileMessages()
+        clauses.append(getattr(TranslationMessage, flag_name))
+        clauses.extend(
+            SQL(clause) for clause in self._getCompletePluralFormsConditions())
 
         # A message is current in this pofile if:
         #  * it's current (above) AND
         #  * (it's diverged AND non-empty)
         #     OR (it's shared AND non-empty AND no diverged one exists)
-        diverged_translation_clauses = [
-            'TranslationMessage.potemplate = %s' % sqlvalues(self.potemplate),
-        ]
-        diverged_translation_query = ' AND '.join(
-            diverged_translation_clauses)
-
-        shared_translation_clauses = [
-            'TranslationMessage.potemplate IS NULL',
-            '''NOT EXISTS (
-                 SELECT * FROM TranslationMessage AS diverged
-                   WHERE
-                     diverged.potemplate=%(potemplate)s AND
-                     diverged.%(flag_name)s IS TRUE AND
-                     diverged.language = %(language)s AND
-                     diverged.potmsgset=TranslationMessage.potmsgset)''' % (
-                dict(
-                     flag_name=flag_name,
-                     language=quote(self.language),
-                     potemplate=quote(self.potemplate),
-                     )
-                ),
-        ]
-        shared_translation_query = ' AND '.join(shared_translation_clauses)
-
-        translated_query = ('( (' + diverged_translation_query + ') OR ('
-                            + shared_translation_query + ') )')
-        clauses.append(translated_query)
+        diverged_translation_clause = (
+            TranslationMessage.potemplateID == self.potemplate.id,
+            )
+
+        Diverged = ClassAlias(TranslationMessage, 'Diverged')
+        shared_translation_clause = And(
+            TranslationMessage.potemplateID == None,
+            Not(Exists(Select(
+                1,
+                tables=[Diverged],
+                where=And(
+                    Diverged.potmsgsetID == TranslationMessage.potmsgsetID,
+                    Diverged.languageID == self.language.id,
+                    getattr(Diverged, flag_name),
+                    Diverged.potemplateID == self.potemplate.id)))))
+
+        clauses.append(
+            Or(diverged_translation_clause, shared_translation_clause))
         return (clauses, clause_tables)
 
     def getPOTMsgSetTranslated(self):
         """See `IPOFile`."""
         clauses, clause_tables = self._getTranslatedMessagesQuery()
-        clauses.append('TranslationTemplateItem.potmsgset = POTMsgSet.id')
-
-        query = ' AND '.join(clauses)
+        query = And(
+            TranslationTemplateItem.potmsgsetID == POTMsgSet.id,
+            *clauses)
         clause_tables.insert(0, POTMsgSet)
         return self._getOrderedPOTMsgSets(clause_tables, query)
 
@@ -633,86 +628,82 @@
         # We get all POTMsgSet.ids with translations, and later
         # exclude them using a NOT IN subselect.
         translated_clauses, clause_tables = self._getTranslatedMessagesQuery()
-        translated_clauses.append(
-            'POTMsgSet.id=TranslationTemplateItem.potmsgset')
-        # Even though this seems silly, Postgres prefers
-        # TranslationTemplateItem index if we add it (and on staging we
-        # get more than a 10x speed improvement: from 8s to 0.7s).  We
-        # also need to put it before any other clauses to be actually useful.
-        translated_clauses.insert(0,
-            'TranslationTemplateItem.potmsgset ='
-            ' TranslationTemplateItem.potmsgset')
-        translated_query = (
-            "(SELECT POTMsgSet.id"
-            "   FROM TranslationTemplateItem, TranslationMessage, POTMsgSet"
-            "   WHERE " + " AND ".join(translated_clauses) + ")")
+        translated_query = Select(
+            POTMsgSet.id,
+            tables=[TranslationTemplateItem, TranslationMessage, POTMsgSet],
+            where=And(
+                # Even though this seems silly, Postgres prefers
+                # TranslationTemplateItem index if we add it (and on
+                # staging we get more than a 10x speed improvement: from
+                # 8s to 0.7s).  We also need to put it before any other
+                # clauses to be actually useful.
+                TranslationTemplateItem.potmsgsetID ==
+                    TranslationTemplateItem.potmsgsetID,
+                POTMsgSet.id == TranslationTemplateItem.potmsgsetID,
+                *translated_clauses))
         clauses = [
-            'TranslationTemplateItem.potemplate = %s' % sqlvalues(
-                self.potemplate),
-            'TranslationTemplateItem.potmsgset = POTMsgSet.id',
-            'TranslationTemplateItem.sequence > 0',
+            TranslationTemplateItem.potemplateID == self.potemplate.id,
+            TranslationTemplateItem.potmsgsetID == POTMsgSet.id,
+            TranslationTemplateItem.sequence > 0,
+            Not(TranslationTemplateItem.potmsgsetID.is_in(translated_query)),
             ]
-        clauses.append(
-            'TranslationTemplateItem.potmsgset NOT IN (%s)' % (
-                translated_query))
-
-        query = ' AND '.join(clauses)
         return self._getOrderedPOTMsgSets(
-            [POTMsgSet, TranslationTemplateItem], query)
+            [POTMsgSet, TranslationTemplateItem], And(*clauses))
 
     def getPOTMsgSetWithNewSuggestions(self):
         """See `IPOFile`."""
         flag_name = getUtility(ITranslationSideTraitsSet).getForTemplate(
             self.potemplate).flag_name
-        clauses = self._getClausesForPOFileMessages()
-        msgstr_clause = make_plurals_sql_fragment(
-            "TranslationMessage.msgstr%(form)d IS NOT NULL", "OR")
+        clauses = self._getStormClausesForPOFileMessages()
+        msgstr_clause = Or(*(
+            getattr(TranslationMessage, 'msgstr%d' % form) != None
+            for form in xrange(TranslationConstants.MAX_PLURAL_FORMS)))
         clauses.extend([
-            'TranslationTemplateItem.potmsgset = POTMsgSet.id',
-            'TranslationMessage.%s IS NOT TRUE' % flag_name,
-            "(%s)" % msgstr_clause,
+            TranslationTemplateItem.potmsgsetID == POTMsgSet.id,
+            Not(getattr(TranslationMessage, flag_name)),
+            msgstr_clause,
             ])
 
-        diverged_translation_query = (
-            '''(SELECT COALESCE(diverged.date_reviewed, diverged.date_created)
-                 FROM TranslationMessage AS diverged
-                 WHERE
-                   diverged.%(flag_name)s IS TRUE AND
-                   diverged.potemplate = %(potemplate)s AND
-                   diverged.language = %(language)s AND
-                   diverged.potmsgset=POTMsgSet.id)''' % dict(
-            flag_name=flag_name,
-            potemplate=quote(self.potemplate),
-            language=quote(self.language)))
-
-        shared_translation_query = (
-            '''(SELECT COALESCE(shared.date_reviewed, shared.date_created)
-                 FROM TranslationMessage AS shared
-                 WHERE
-                   shared.%(flag_name)s IS TRUE AND
-                   shared.potemplate IS NULL AND
-                   shared.language = %(language)s AND
-                   shared.potmsgset=POTMsgSet.id)''' % dict(
-            flag_name=flag_name,
-            language=quote(self.language)))
-        beginning_of_time = "TIMESTAMP '1970-01-01 00:00:00'"
-        newer_than_query = (
-            "TranslationMessage.date_created > COALESCE(" +
-            ",".join([diverged_translation_query,
-                      shared_translation_query,
-                      beginning_of_time]) + ")")
-        clauses.append(newer_than_query)
+        Diverged = ClassAlias(TranslationMessage, "Diverged")
+        diverged_translation_query = Select(
+            Coalesce(Diverged.date_reviewed, Diverged.date_created),
+            tables=[Diverged],
+            where=And(
+                Diverged.potmsgsetID == POTMsgSet.id,
+                Diverged.languageID == self.language.id,
+                getattr(Diverged, flag_name),
+                Diverged.potemplateID == self.potemplate.id))
+
+        Shared = ClassAlias(TranslationMessage, "Shared")
+        shared_translation_query = Select(
+            Coalesce(Shared.date_reviewed, Shared.date_created),
+            tables=[Shared],
+            where=And(
+                Shared.potmsgsetID == POTMsgSet.id,
+                Shared.languageID == self.language.id,
+                getattr(Shared, flag_name),
+                Shared.potemplateID == None))
+
+        beginning_of_time = Cast(u'1970-01-01 00:00:00', 'timestamp')
+        clauses.append(
+            TranslationMessage.date_created >
+                Coalesce(
+                    diverged_translation_query, shared_translation_query,
+                    beginning_of_time))
 
         # A POT set has "new" suggestions if there is a non current
         # TranslationMessage newer than the current reviewed one.
-        query = (
-            """POTMsgSet.id IN (SELECT DISTINCT TranslationMessage.potmsgset
-                 FROM TranslationMessage, TranslationTemplateItem, POTMsgSet
-                 WHERE (%(query)s)) AND
-               POTMsgSet.id=TranslationTemplateItem.potmsgset AND
-               TranslationTemplateItem.potemplate=%(potemplate)s
-            """ % dict(query=' AND '.join(clauses),
-                       potemplate=quote(self.potemplate)))
+        query = And(
+            POTMsgSet.id.is_in(
+                Select(
+                    TranslationMessage.potmsgsetID,
+                    tables=[
+                        TranslationMessage, TranslationTemplateItem,
+                        POTMsgSet],
+                    where=And(*clauses),
+                    distinct=True)),
+            POTMsgSet.id == TranslationTemplateItem.potmsgsetID,
+            TranslationTemplateItem.potemplateID == self.potemplate.id)
         return self._getOrderedPOTMsgSets(
             [POTMsgSet, TranslationTemplateItem], query)
 
@@ -727,44 +718,39 @@
             ITranslationSideTraitsSet).getForTemplate(
                 self.potemplate).other_side_traits.flag_name
         clauses.extend([
-            'TranslationTemplateItem.potmsgset = POTMsgSet.id',
-            'TranslationMessage.%s IS FALSE' % other_side_flag_name,
+            TranslationTemplateItem.potmsgsetID == POTMsgSet.id,
+            Not(getattr(TranslationMessage, other_side_flag_name)),
             ])
 
-        imported_no_diverged = (
-            '''NOT EXISTS (
-                 SELECT * FROM TranslationMessage AS diverged
-                   WHERE
-                     diverged.%(flag_name)s IS TRUE AND
-                     diverged.id <> imported.id AND
-                     diverged.potemplate = %(potemplate)s AND
-                     diverged.language = %(language)s AND
-                     diverged.potmsgset=TranslationMessage.potmsgset)''' % (
-                dict(
-                    flag_name=other_side_flag_name,
-                    potemplate=quote(self.potemplate),
-                    language=quote(self.language),
-                    )))
+        Imported = ClassAlias(TranslationMessage, 'Imported')
+        Diverged = ClassAlias(TranslationMessage, 'Diverged')
+        imported_no_diverged = Not(Exists(
+            Select(
+                1,
+                tables=[Diverged],
+                where=And(
+                    Diverged.id != Imported.id,
+                    Diverged.potmsgsetID == TranslationMessage.potmsgsetID,
+                    Diverged.languageID == self.language.id,
+                    getattr(Diverged, other_side_flag_name),
+                    Diverged.potemplateID == self.potemplate.id))))
         imported_clauses = [
-            'imported.id <> TranslationMessage.id',
-            'imported.potmsgset = POTMsgSet.id',
-            'imported.language = %s' % sqlvalues(self.language),
-            'imported.%s IS TRUE' % other_side_flag_name,
-            '(imported.potemplate=%s OR ' % sqlvalues(self.potemplate) +
-            '   (imported.potemplate IS NULL AND ' + imported_no_diverged
-            + '  ))',
+            Imported.id != TranslationMessage.id,
+            Imported.potmsgsetID == POTMsgSet.id,
+            Imported.languageID == self.language.id,
+            getattr(Imported, other_side_flag_name),
+            Or(
+                Imported.potemplateID == self.potemplate.id,
+                And(Imported.potemplateID == None, imported_no_diverged)),
             ]
-        self._appendCompletePluralFormsConditions(imported_clauses,
-                                                  'imported')
-        exists_imported_query = (
-            'EXISTS ('
-            '  SELECT * FROM TranslationMessage AS imported'
-            '      WHERE ' + ' AND '.join(imported_clauses) + ')')
-        clauses.append(exists_imported_query)
+        imported_clauses.extend(
+            SQL(clause) for clause in
+            self._getCompletePluralFormsConditions('imported'))
+        clauses.append(Exists(Select(
+            1, tables=[Imported], where=And(*imported_clauses))))
 
         clause_tables.insert(0, POTMsgSet)
-        query = ' AND '.join(clauses)
-        return self._getOrderedPOTMsgSets(clause_tables, query)
+        return self._getOrderedPOTMsgSets(clause_tables, And(*clauses))
 
     def messageCount(self):
         """See `IRosettaStats`."""
@@ -794,16 +780,17 @@
             self.rosettacount,
             self.unreviewed_count)
 
-    def _appendCompletePluralFormsConditions(self, query,
-                                             table_name='TranslationMessage'):
+    def _getCompletePluralFormsConditions(self,
+                                          table_name='TranslationMessage'):
         """Add conditions to implement ITranslationMessage.is_complete in SQL.
 
         :param query: A list of AND SQL conditions where the implementation of
             ITranslationMessage.is_complete will be appended as SQL
             conditions.
         """
-        query.append('%(table_name)s.msgstr0 IS NOT NULL' % {
-            'table_name': table_name})
+        query = [
+            '%(table_name)s.msgstr0 IS NOT NULL' % {'table_name': table_name},
+            ]
         if self.language.pluralforms > 1:
             plurals_query = ' AND '.join(
                 '%(table_name)s.msgstr%(plural_form)d IS NOT NULL' % {
@@ -825,11 +812,9 @@
         side_traits = getUtility(ITranslationSideTraitsSet).getForTemplate(
             self.potemplate)
         complete_plural_clause_this_side = ' AND '.join(
-            self._appendCompletePluralFormsConditions(
-                [], table_name='Current'))
+            self._getCompletePluralFormsConditions(table_name='Current'))
         complete_plural_clause_other_side = ' AND '.join(
-            self._appendCompletePluralFormsConditions(
-                [], table_name='Other'))
+            self._getCompletePluralFormsConditions(table_name='Other'))
         params = {
             'potemplate': quote(self.potemplate),
             'language': quote(self.language),