launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17987
Re: [Merge] lp:~wgrant/launchpad/pofile-filter-stormify into lp:launchpad
Review: Approve code
I believe that you can remove the _getClausesForPOFileMessages method now too.
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))),
You seem to have lost a DISTINCT here. Does that matter?
> + 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
