← Back to team overview

launchpad-reviewers team mailing list archive

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

 


Diff comments:

> === 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 13:00:56 +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,81 @@
>          # 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))),

Shouldn't, since an IN subselect is functionally distinct anyway. But I'll verify the performance.

> +            POTMsgSet.id == TranslationTemplateItem.potmsgsetID,
> +            TranslationTemplateItem.potemplateID == self.potemplate.id)
>          return self._getOrderedPOTMsgSets(
>              [POTMsgSet, TranslationTemplateItem], query)
>  
> @@ -727,44 +717,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 +779,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 +811,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),
> 


-- 
https://code.launchpad.net/~wgrant/launchpad/pofile-filter-stormify/+merge/251249
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.


References