Thanks for the excellent review :)

> I am a little weirded out by it living alone in models; the logic is a file in
> models should have a model in it--this one is just utilities for another model
> file. Since it doesn't have use outside of structuralsubscription.py, I think
> it might be better added to that file. I don't think that hurts the
> readability of that file, since the logic is still nicely contained in the
> class you created.

Yeah, fair point. I moved it out because it helped me concentrate
while working on it, but it was a lot messier then. I've moved it
back.

...
> > +        # Set up common filter conditions.
> > +        if len(self.tags) == 0:
> > +            self.filter_conditions = And(
> > +                BugSubscriptionFilter.include_any_tags == False,
> > +                self.base_conditions)
> > +        else:
> > +            self.filter_conditions = And(
> > +                BugSubscriptionFilter.exclude_any_tags == False,
> > +                self.base_conditions)
> > +
> 
> A quick comment here explaining the logic of setting up exclude_any vs
> include_any would be good.

Done.

...
> > +    def _filters_matching_x(self, join, where_condition, **extra):
> > +        # The expressions returned by this function are used in set (union,
> > +        # intersect, except) operations at the *filter* level. However, the
> > +        # interesting result of these set operations is the structural
> > +        # subscription, hence both columns are included in the expressions
> > +        # generated. Since a structural subscription can have zero or more
> > +        # filters, and a filter can never be associated with more than one
> > +        # subscription, the set operations are unaffected.
> > +        return Select(
> > +            columns=(
> > +                # Alias this column so it can be selected in
> > +                # subscriptions_matching.
> > +                Alias(
> > +                    BugSubscriptionFilter.structural_subscription_id,
> > +                    "structural_subscription_id"),
> > +                BugSubscriptionFilter.id),
> > +            tables=(
> > +                StructuralSubscription, BugSubscriptionFilter, join),
> > +            where=And(
> > +                BugSubscriptionFilter.structural_subscription_id == (
> > +                    StructuralSubscription.id),
> > +                self.filter_conditions,
> > +                where_condition),
> > +            **extra)
> > +
> 
> I would prefer some sort of docstring for this in place of in addition to the
> comment block at the top.

Done.

...
> > +    def _filters_matching_all_x_tags(self, where_condition):
> > +        tags_array = "ARRAY[%s]::TEXT[]" % ",".join(
> > +            quote(tag) for tag in self.tags)
> > +        return self._filters_matching_x(
> > +            BugSubscriptionFilterTag,
> > +            And(
> > +                BugSubscriptionFilterTag.filter_id == (
> > +                    BugSubscriptionFilter.id),
> > +                BugSubscriptionFilter.find_all_tags,
> > +                self.filter_conditions,
> > +                where_condition),
> > +            group_by=(
> > +                BugSubscriptionFilter.structural_subscription_id,
> > +                BugSubscriptionFilter.id),
> > +            having=ArrayContains(
> > +                SQL(tags_array), ArrayAgg(
> > +                    BugSubscriptionFilterTag.tag)))
> > +
> 
> Given how much this is used in the following methods, a docstring explaining
> it as a convenience for the next methods would be nice.

Done.