Merge lp:~wgrant/launchpad/tm-performance-4-queries into lp:launchpad

[William Grant]

[lp:~wgrant/launchpad/tm-performance-4-queries]

17375. By William Grant on 2015-02-28

Drop unused functions.

17376. By William Grant on 2015-02-28

Merge bug-736005-trivialise.

17377. By William Grant on 2015-02-28

Text searches now use TTI.msgid_*.

17378. By William Grant on 2015-02-28

POFile POTMsgSet filters now use TTI denormed columns.

Unmerged revisions

17378. By William Grant on 2015-02-28

POFile POTMsgSet filters now use TTI denormed columns.

17377. By William Grant on 2015-02-28

Text searches now use TTI.msgid_*.

17376. By William Grant on 2015-02-28

Merge bug-736005-trivialise.

17375. By William Grant on 2015-02-28

Drop unused functions.

17374. By William Grant on 2015-02-28

Merge tm-performance-2-model.

17373. By William Grant on 2015-02-27

Translation suggestion queries now use the denormed columns.

17372. By William Grant on 2015-02-27

Fix POTMsgSet.suggestive clone and tests.

17371. By William Grant on 2015-02-27

Set POTemplate.suggestive based on normal suggestiveness criteria, and add POTemplateSet.filterSuggestiveTargets with the same.

17370. By William Grant on 2015-02-27

Propagate suggestive down from POTemplate to POTMsgSet to TranslationMessage.

17369. By William Grant on 2015-02-27

submitSuggestion now uses _makeTranslationMessage.

=== modified file 'lib/lp/translations/model/pofile.py'
--- lib/lp/translations/model/pofile.py	2015-02-27 22:58:59 +0000
+++ lib/lp/translations/model/pofile.py	2015-02-28 01:27:16 +0000
@@ -13,6 +13,7 @@
     ]
 
 import datetime
+from operator import itemgetter
 
 import pytz
 from sqlobject import (
@@ -50,6 +51,7 @@
 from lp.registry.interfaces.person import validate_public_person
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.datetimecol import UtcDateTimeCol
+from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.interfaces import (
     IMasterStore,
     IStore,
@@ -57,7 +59,6 @@
 from lp.services.database.sqlbase import (
     flush_database_updates,
     quote,
-    quote_like,
     SQLBase,
     sqlvalues,
     )
@@ -97,11 +98,9 @@
     credits_message_str,
     POTMsgSet,
     )
+from lp.translations.model.potranslation import POTranslation
 from lp.translations.model.translationimportqueue import collect_import_info
-from lp.translations.model.translationmessage import (
-    make_plurals_sql_fragment,
-    TranslationMessage,
-    )
+from lp.translations.model.translationmessage import TranslationMessage
 from lp.translations.model.translationtemplateitem import (
     TranslationTemplateItem,
     )
@@ -112,17 +111,6 @@
     )
 
 
-def compose_sql_translationmessage_has_translations(tm_sql_identifier):
-    """Compose SQL for "`TranslationMessage` is nonempty.".
-
-    :param tm_sql_identifier: The SQL identifier for the
-        `TranslationMessage` in the query that's to be tested.
-    """
-    return "COALESCE(%s) IS NOT NULL" % ", ".join([
-        "%s.msgstr%d" % (tm_sql_identifier, form)
-        for form in xrange(TranslationConstants.MAX_PLURAL_FORMS)])
-
-
 class POFileMixIn(RosettaStats):
     """Base class for `POFile` and `DummyPOFile`.
 
@@ -166,102 +154,6 @@
         header.has_plural_forms = self.potemplate.hasPluralMessage()
         return header
 
-    def _getTranslationSearchQuery(self, pofile, plural_form, text):
-        """Query to find `text` in `plural_form` translations of a `pofile`.
-
-        This produces a list of clauses that can be used to search for
-        TranslationMessages containing `text` in their msgstr[`plural_form`].
-        Returned values are POTMsgSet ids containing them, expected to be
-        used in a UNION across all plural forms.
-        """
-        translation_match = """
-        -- Find translations containing `text`.
-        -- Like in findPOTMsgSetsContaining(), to avoid seqscans on
-        -- POTranslation table, we do ILIKE comparison on them in
-        -- a subselect which is first filtered by the POFile.
-          SELECT TranslationMessage.potmsgset
-            FROM TranslationMessage
-            JOIN TranslationTemplateItem
-              ON TranslationMessage.potmsgset
-                   = TranslationTemplateItem.potmsgset
-            WHERE
-              TranslationTemplateItem.potemplate = %(potemplate)s AND
-              TranslationMessage.language = %(language)s AND
-              TranslationMessage.msgstr%(plural_form)d IN (
-                SELECT POTranslation.id FROM POTranslation WHERE
-                  POTranslation.id IN (
-                    SELECT DISTINCT(msgstr%(plural_form)d)
-                      FROM TranslationMessage AS tm_ids
-                      JOIN TranslationTemplateItem
-                        ON tm_ids.potmsgset=TranslationTemplateItem.potmsgset
-                      WHERE
-                        TranslationTemplateItem.potemplate
-                          = %(potemplate)s AND
-                        TranslationTemplateItem.sequence > 0 AND
-                        tm_ids.language=%(language)s
-                  ) AND
-                  POTranslation.translation
-                    ILIKE '%%' || %(text)s || '%%')
-                    """ % dict(potemplate=quote(pofile.potemplate),
-                               language=quote(pofile.language),
-                               plural_form=plural_form,
-                               text=quote_like(text))
-        return translation_match
-
-    def _getTemplateSearchQuery(self, text):
-        """Query for finding `text` in msgids of this POFile.
-        """
-        english_match = """
-        -- Step 1a: get POTMsgSets where msgid_singular contains `text`
-        -- To avoid seqscans on POMsgID table (what LIKE usually
-        -- does), we do ILIKE comparison on them in a subselect first
-        -- filtered by this POTemplate.
-          SELECT POTMsgSet.id
-            FROM POTMsgSet
-            JOIN TranslationTemplateItem
-              ON TranslationTemplateItem.potmsgset=POTMsgSet.id AND
-                 TranslationTemplateItem.potemplate=%s
-            WHERE
-              (POTMsgSet.msgid_singular IS NOT NULL AND
-               POTMsgSet.msgid_singular IN (
-                 SELECT POMsgID.id FROM POMsgID
-                   WHERE id IN (
-                     SELECT DISTINCT(POTMsgSet.msgid_singular)
-                       FROM POTMsgSet
-                       JOIN TranslationTemplateItem
-                         ON TranslationTemplateItem.potmsgset = POTMsgSet.id
-                       WHERE
-                         TranslationTemplateItem.potemplate=%s AND
-                         TranslationTemplateItem.sequence > 0
-                   ) AND
-                   msgid ILIKE '%%' || %s || '%%'))
-          UNION
-        -- Step 1b: like above, just on msgid_plural.
-          SELECT POTMsgSet.id
-            FROM POTMsgSet
-            JOIN TranslationTemplateItem
-              ON TranslationTemplateItem.potmsgset=POTMsgSet.id AND
-                 TranslationTemplateItem.potemplate=%s
-            WHERE
-              (POTMsgSet.msgid_plural IS NOT NULL AND
-               POTMsgSet.msgid_plural IN (
-                 SELECT POMsgID.id FROM POMsgID
-                   WHERE id IN (
-                     SELECT DISTINCT(POTMsgSet.msgid_plural)
-                       FROM POTMsgSet
-                       JOIN TranslationTemplateItem
-                         ON TranslationTemplateItem.potmsgset = POTMsgSet.id
-                       WHERE
-                         TranslationTemplateItem.potemplate=%s AND
-                         TranslationTemplateItem.sequence > 0
-                   ) AND
-                   msgid ILIKE '%%' || %s || '%%'))
-            """ % (quote(self.potemplate), quote(self.potemplate),
-                   quote_like(text),
-                   quote(self.potemplate), quote(self.potemplate),
-                   quote_like(text))
-        return english_match
-
     def _getOrderedPOTMsgSets(self, origin_tables, query):
         """Find all POTMsgSets matching `query` from `origin_tables`.
 
@@ -274,41 +166,79 @@
             POTMsgSet, query)
         return results.order_by(TranslationTemplateItem.sequence)
 
+    def _getTranslationSearchBits(self, pofile):
+        ThisTM = ClassAlias(TranslationMessage)
+        ThisPT = ClassAlias(POTranslation)
+        str_id_cols = [
+            getattr(ThisTM, 'msgstr%dID' % plural_form)
+            for plural_form in range(pofile.plural_forms)]
+        origin = [
+            LeftJoin(
+                ThisTM,
+                And(
+                    ThisTM.potmsgsetID ==
+                        TranslationTemplateItem.potmsgsetID,
+                    ThisTM.languageID == pofile.languageID)),
+            LeftJoin(
+                ThisPT,
+                ThisPT.id.is_in(str_id_cols)),
+            ]
+        search_options = [ThisPT.translation]
+        return origin, search_options
+
     def findPOTMsgSetsContaining(self, text):
         """See `IPOFile`."""
+        origin = [
+            POTMsgSet,
+            Join(
+                TranslationTemplateItem,
+                TranslationTemplateItem.potmsgsetID == POTMsgSet.id),
+            ]
         clauses = [
-            'TranslationTemplateItem.potemplate = %s' % sqlvalues(
-                self.potemplate),
-            'TranslationTemplateItem.potmsgset = POTMsgSet.id',
-            'TranslationTemplateItem.sequence > 0',
+            TranslationTemplateItem.potemplateID == self.potemplate.id,
+            TranslationTemplateItem.sequence > 0,
             ]
 
         if text is not None:
             assert len(text) > 1, (
                 "You can not search for strings shorter than 2 characters.")
 
+            # A list of string columns, of which at least one must match
+            # the search term.
+            search_options = []
+
+            # Search the English strings.
             if self.potemplate.uses_english_msgids:
-                english_match = self._getTemplateSearchQuery(text)
+                origin.append(Join(
+                    POMsgID,
+                    POMsgID.id.is_in((
+                        TranslationTemplateItem.msgid_singularID,
+                        TranslationTemplateItem.msgid_pluralID))))
+                search_options.extend([POMsgID.msgid])
             else:
-                # If msgids are not in English, use English PO file
-                # to fetch original strings instead.
-                en_pofile = self.potemplate.getPOFileByLang('en')
-                english_match = self._getTranslationSearchQuery(
-                    en_pofile, 0, text)
+                en_po = self.potemplate.getPOFileByLang('en')
+                en_origin, en_options = self._getTranslationSearchBits(en_po)
+                origin.extend(en_origin)
+                search_options.extend(en_options)
 
-            # Do not look for translations in a DummyPOFile.
-            search_clauses = [english_match]
+            # Search the translations themselves, unless this is a
+            # DummyPOFile.
             if self.id is not None:
-                for plural_form in range(self.plural_forms):
-                    translation_match = self._getTranslationSearchQuery(
-                        self, plural_form, text)
-                    search_clauses.append(translation_match)
-
-            clauses.append(
-                "POTMsgSet.id IN (" + " UNION ".join(search_clauses) + ")")
-
-        return self._getOrderedPOTMsgSets(
-            [POTMsgSet, TranslationTemplateItem], ' AND '.join(clauses))
+                po_origin, po_options = self._getTranslationSearchBits(self)
+                origin.extend(po_origin)
+                search_options.extend(po_options)
+
+            # Case-insensitively substring-match the search term against
+            # each searchable column.
+            clauses.append(Or(*(
+                str.like('%%%s%%' % text, case_sensitive=False)
+                for str in search_options)))
+
+        result = IMasterStore(POTMsgSet).using(*origin).find(
+            (POTMsgSet, TranslationTemplateItem),
+            *clauses).config(distinct=True).order_by(
+            TranslationTemplateItem.sequence)
+        return DecoratedResultSet(result, itemgetter(0))
 
     def getFullLanguageCode(self):
         """See `IPOFile`."""
@@ -529,25 +459,6 @@
                 "Calling prepareTranslationCredits on a message with "
                 "unknown credits type '%s'." % credits_type.title)
 
-    def _getClausesForPOFileMessages(self, current=True):
-        """Get TranslationMessages for the POFile via TranslationTemplateItem.
-
-        Call-site will have to have appropriate clauseTables.
-        """
-        # When all the code that uses this method is moved to Storm,
-        # we can replace it with _getStormClausesForPOFileMessages
-        # and then remove it.
-        clauses = [
-            'TranslationTemplateItem.potemplate = %s' % sqlvalues(
-                self.potemplate),
-            ('TranslationTemplateItem.potmsgset'
-             ' = TranslationMessage.potmsgset'),
-            'TranslationMessage.language = %s' % sqlvalues(self.language)]
-        if current:
-            clauses.append('TranslationTemplateItem.sequence > 0')
-
-        return clauses
-
     def _getStormClausesForPOFileMessages(self, current=True):
         """Get TranslationMessages for the POFile via TranslationTemplateItem.
         """
@@ -627,20 +538,10 @@
         """See `IPOFile`."""
         # We get all POTMsgSet.ids with translations, and later
         # exclude them using a NOT IN subselect.
-        translated_clauses, clause_tables = self._getTranslatedMessagesQuery()
+        trans_clauses, trans_tables = self._getTranslatedMessagesQuery()
         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))
+            TranslationTemplateItem.potmsgsetID,
+            tables=trans_tables, where=And(*trans_clauses))
         clauses = [
             TranslationTemplateItem.potemplateID == self.potemplate.id,
             TranslationTemplateItem.potmsgsetID == POTMsgSet.id,
@@ -669,7 +570,7 @@
             Coalesce(Diverged.date_reviewed, Diverged.date_created),
             tables=[Diverged],
             where=And(
-                Diverged.potmsgsetID == POTMsgSet.id,
+                Diverged.potmsgsetID == TranslationTemplateItem.potmsgsetID,
                 Diverged.languageID == self.language.id,
                 getattr(Diverged, flag_name),
                 Diverged.potemplateID == self.potemplate.id))
@@ -679,7 +580,7 @@
             Coalesce(Shared.date_reviewed, Shared.date_created),
             tables=[Shared],
             where=And(
-                Shared.potmsgsetID == POTMsgSet.id,
+                Shared.potmsgsetID == TranslationTemplateItem.potmsgsetID,
                 Shared.languageID == self.language.id,
                 getattr(Shared, flag_name),
                 Shared.potemplateID == None))
@@ -694,7 +595,7 @@
         # A POT set has "new" suggestions if there is a non current
         # TranslationMessage newer than the current reviewed one.
         query = And(
-            POTMsgSet.id.is_in(
+            TranslationTemplateItem.potmsgsetID.is_in(
                 Select(
                     TranslationMessage.potmsgsetID,
                     tables=[
@@ -736,7 +637,7 @@
                     Diverged.potemplateID == self.potemplate.id))))
         imported_clauses = [
             Imported.id != TranslationMessage.id,
-            Imported.potmsgsetID == POTMsgSet.id,
+            Imported.potmsgsetID == TranslationTemplateItem.potmsgsetID,
             Imported.languageID == self.language.id,
             getattr(Imported, other_side_flag_name),
             Or(
@@ -798,7 +699,8 @@
                   'table_name': table_name,
                 } for plural_form in range(1, self.plural_forms))
             query.append(
-                '(POTMsgSet.msgid_plural IS NULL OR (%s))' % plurals_query)
+                '(%(table_name)s.msgid_plural IS NULL OR (%(plurals_query)s))'
+                % {'plurals_query': plurals_query, 'table_name': table_name})
         return query
 
     def _countTranslations(self):
@@ -1531,7 +1433,7 @@
         clauses = [
             TranslationTemplateItem.potemplateID == POFile.potemplateID,
             POTMsgSet.id == TranslationTemplateItem.potmsgsetID,
-            POTMsgSet.msgid_singular == POMsgID.id,
+            TranslationTemplateItem.msgid_singular == POMsgID.id,
             POMsgID.msgid.is_in(POTMsgSet.credits_message_ids),
             ]
         if untranslated:
 
=== modified file 'lib/lp/translations/model/potmsgset.py'
--- lib/lp/translations/model/potmsgset.py	2015-02-28 01:27:16 +0000
+++ lib/lp/translations/model/potmsgset.py	2015-02-28 01:27:16 +0000
@@ -21,9 +21,12 @@
     StringCol,
     )
 from storm.expr import (
+    And,
     Coalesce,
     Desc,
+    Not,
     Or,
+    Select,
     SQL,
     )
 from storm.store import (
@@ -389,8 +392,9 @@
         # Watch out when changing this condition: make sure it's done in
         # a way so that indexes are indeed hit when the query is executed.
         # Also note that there is a NOT(in_use_clause) index.
-        in_use_clause = (
-            "(is_current_ubuntu IS TRUE OR is_current_upstream IS TRUE)")
+        in_use_clause = Or(
+            TranslationMessage.is_current_ubuntu,
+            TranslationMessage.is_current_upstream)
         # Present a list of language + usage constraints to sql. A language
         # can either be unconstrained, used, or suggested depending on which
         # of suggested_languages, used_languages it appears in.
@@ -401,26 +405,16 @@
         used_languages = used_languages - both_languages
         lang_used = []
         if both_languages:
-            lang_used.append('TranslationMessage.language IN %s' %
-                quote(both_languages))
+            lang_used.append(
+                TranslationMessage.languageID.is_in(both_languages))
         if used_languages:
-            lang_used.append('(TranslationMessage.language IN %s AND %s)' % (
-                quote(used_languages), in_use_clause))
+            lang_used.append(And(
+                TranslationMessage.languageID.is_in(used_languages),
+                in_use_clause))
         if suggested_languages:
-            lang_used.append(
-                '(TranslationMessage.language IN %s AND NOT %s)' % (
-                quote(suggested_languages), in_use_clause))
-
-        msgsets = SQL('''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 = %s and POTMsgSet.id <> %s
-            )''' % sqlvalues(self.msgid_singular, self))
+            lang_used.append(And(
+                TranslationMessage.languageID.is_in(suggested_languages),
+                Not(in_use_clause)))
 
         # Subquery to find the ids of TranslationMessages that are
         # matching suggestions.
@@ -429,25 +423,23 @@
         # 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([
+        msgstrs = [
             'COALESCE(msgstr%d, -1)' % form
-            for form in xrange(TranslationConstants.MAX_PLURAL_FORMS)])
-        ids_query_params = {
-            'msgstrs': msgstrs,
-            'where': '(' + ' OR '.join(lang_used) + ')',
-        }
-        ids_query = '''
-            SELECT DISTINCT ON (%(msgstrs)s)
-                TranslationMessage.id
-            FROM TranslationMessage
-            JOIN msgsets ON msgsets.id = TranslationMessage.potmsgset
-            WHERE %(where)s
-            ORDER BY %(msgstrs)s, date_created DESC
-            ''' % ids_query_params
+            for form in xrange(TranslationConstants.MAX_PLURAL_FORMS)]
+        ids_query = Select(
+            TranslationMessage.id,
+            tables=[TranslationMessage],
+            where=And(
+                TranslationMessage.msgid_singular == self.msgid_singular,
+                TranslationMessage.potmsgset != self.id,
+                TranslationMessage.suggestive,
+                Or(*lang_used)),
+            order_by=msgstrs + [Desc(TranslationMessage.date_created)],
+            distinct=msgstrs)
 
-        result = IStore(TranslationMessage).with_(msgsets).find(
+        result = IStore(TranslationMessage).find(
             TranslationMessage,
-            TranslationMessage.id.is_in(SQL(ids_query)))
+            TranslationMessage.id.is_in(ids_query))
 
         return shortlist(result, longest_expected=100, hardlimit=2000)
 
 
=== modified file 'lib/lp/translations/tests/test_suggestions.py'
--- lib/lp/translations/tests/test_suggestions.py	2012-01-01 02:58:52 +0000
+++ lib/lp/translations/tests/test_suggestions.py	2015-02-28 01:27:16 +0000
@@ -162,7 +162,8 @@
         text = "The application has exploded."
         suggested_dutch = "De applicatie is ontploft."
         now = datetime.now(timezone('UTC'))
-        before = now - timedelta(1, 1, 1)
+        before = now - timedelta(1, 1, 2)
+        after = now - timedelta(1, 1, 1)
 
         foomsg = self.factory.makePOTMsgSet(self.foo_template, text)
         barmsg = self.factory.makePOTMsgSet(self.bar_template, text)
@@ -173,7 +174,7 @@
             pofile=self.bar_nl, potmsgset=barmsg,
             translations={0: suggested_dutch})
         self.assertNotEqual(suggestion1, suggestion2)
-        removeSecurityProxy(suggestion1).date_created = before
+        removeSecurityProxy(suggestion1).date_created = after
         removeSecurityProxy(suggestion2).date_created = before
 
         # When a third project, oof, contains the same translatable