launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #12523
[Merge] lp:~sinzui/launchpad/bug-subselect-timeout into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/bug-subselect-timeout into lp:launchpad.
Commit message:
Only match the context when searching for structures a person is subscribed to.
Requested reviews:
William Grant (wgrant)
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #904339 in Launchpad itself: "timeout querying a list of bugs for packages a team is subscribed to"
https://bugs.launchpad.net/launchpad/+bug/904339
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/bug-subselect-timeout/+merge/126566
The bug query clause for structural subscribers times out. This is
possibly caused by the superfluous joins against bugtaskflat.
--------------------------------------------------------------------
RULES
Pre-implementation: wgrant
* Structure the query to be fast. Try In() and Row() to match a CTE table
of just the relevant structural subscriptions.
* Do not match impossible conditions...when looking for distro or
distro packages, do not match on product or product series.
QA
* Visit https://bugs.qastaging.launchpad.net/ubuntu/+bugs?field.structural_subscriber=foundations-bugs
* Verify it does not timeout
LINT
lib/lp/bugs/model/bugtasksearch.py
TEST
./bin/test -vvc -t structural lp.bugs.model.tests.test_bugtasksearch
IMPLEMENTATION
Replaced the LEFT JOINS with subselects using In() and Row() as needed. I
tried a single Row() that matched the SS columns to BugTaskFlat columns,
but the tests failed -- I suspect that NULLs in Row() did not match the NULLs
in the SELECT. NILL != NULL. Maybe I botched this. Note the odd rule that
matches DSP SSs to DS SP bugs. This looks historical, but may also be
convenience because SPs can be short-lived and probably causes users to
not ask why they stopped getting email when a new series started. After I
got the tests to pass, I changed the code to build only the subqueries that
make sense for the bug params being used.
lib/lp/bugs/model/bugtasksearch.py
--
https://code.launchpad.net/~sinzui/launchpad/bug-subselect-timeout/+merge/126566
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/bug-subselect-timeout into lp:launchpad.
=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py 2012-09-24 05:17:00 +0000
+++ lib/lp/bugs/model/bugtasksearch.py 2012-09-26 22:58:23 +0000
@@ -87,11 +87,9 @@
from lp.services.database.lpstorm import IStore
from lp.services.database.sqlbase import sqlvalues
from lp.services.database.stormexpr import (
- Array,
ArrayAgg,
ArrayIntersects,
get_where_for_reference,
- NullCount,
Unnest,
)
from lp.services.propertycache import get_property_cache
@@ -477,9 +475,13 @@
BugTaskFlat.bug_id == BugSubscription.bug_id,
BugSubscription.person == params.subscriber))
+ is_person_search = (
+ params.product is None and
+ params.distribution is None and
+ params.productseries is None and
+ params.distroseries is None)
+
if params.structural_subscriber is not None:
- # See bug 787294 for the story that led to the query elements
- # below. Please change with care.
with_clauses.append(
'''ss as (SELECT * from StructuralSubscription
WHERE StructuralSubscription.subscriber = %s)'''
@@ -488,83 +490,80 @@
class StructuralSubscriptionCTE(StructuralSubscription):
__storm_table__ = 'ss'
- join_tables.append(
- (Product, LeftJoin(Product, And(
- BugTaskFlat.product_id == Product.id,
- Product.active))))
- ProductSub = ClassAlias(StructuralSubscriptionCTE)
- join_tables.append((
- ProductSub,
- LeftJoin(
- ProductSub,
- BugTaskFlat.product_id == ProductSub.productID)))
- ProductSeriesSub = ClassAlias(StructuralSubscriptionCTE)
- join_tables.append((
- ProductSeriesSub,
- LeftJoin(
- ProductSeriesSub,
- BugTaskFlat.productseries_id ==
- ProductSeriesSub.productseriesID)))
- ProjectSub = ClassAlias(StructuralSubscriptionCTE)
- join_tables.append((
- ProjectSub,
- LeftJoin(
- ProjectSub,
- Product.projectID == ProjectSub.projectID)))
- DistributionSub = ClassAlias(StructuralSubscriptionCTE)
- join_tables.append((
- DistributionSub,
- LeftJoin(
- DistributionSub,
- And(BugTaskFlat.distribution_id ==
- DistributionSub.distributionID,
- Or(
- DistributionSub.sourcepackagenameID ==
- BugTaskFlat.sourcepackagename_id,
- DistributionSub.sourcepackagenameID == None)))))
- if params.distroseries is not None:
- parent_distro_id = params.distroseries.distributionID
- else:
- parent_distro_id = 0
- DistroSeriesSub = ClassAlias(StructuralSubscriptionCTE)
- join_tables.append((
- DistroSeriesSub,
- LeftJoin(
- DistroSeriesSub,
- Or(BugTaskFlat.distroseries_id ==
- DistroSeriesSub.distroseriesID,
- # There is a mismatch between BugTask and
- # StructuralSubscription. SS does not support
- # distroseries. This clause works because other
- # joins ensure the match bugtask is the right
- # series.
- And(parent_distro_id == DistroSeriesSub.distributionID,
- BugTaskFlat.sourcepackagename_id ==
- DistroSeriesSub.sourcepackagenameID)))))
- MilestoneSub = ClassAlias(StructuralSubscriptionCTE)
- join_tables.append((
- MilestoneSub,
- LeftJoin(
- MilestoneSub,
- BugTaskFlat.milestone_id == MilestoneSub.milestoneID)))
- extra_clauses.append(
- NullCount(Array(
- ProductSub.id, ProductSeriesSub.id, ProjectSub.id,
- DistributionSub.id, DistroSeriesSub.id, MilestoneSub.id)) < 6)
- has_duplicate_results = True
+ SS = ClassAlias(StructuralSubscriptionCTE)
+ ss_clauses = []
+ if is_person_search or params.distribution is not None:
+ ss_clauses.append(In(
+ BugTaskFlat.distribution_id,
+ Select(
+ SS.distributionID,
+ tables=[SS],
+ where=(SS.sourcepackagenameID == None))))
+ ss_clauses.append(In(
+ Row(BugTaskFlat.distribution_id,
+ BugTaskFlat.sourcepackagename_id),
+ Select(
+ ((SS.distributionID,
+ SS.sourcepackagenameID),),
+ tables=[SS])))
+ if is_person_search or params.distroseries is not None:
+ ss_clauses.append(In(
+ BugTaskFlat.distroseries_id,
+ Select(
+ SS.distroseriesID,
+ tables=[SS],
+ where=(SS.sourcepackagenameID == None))))
+ # Structural subscriptions to DSPs are treated as subscriptions
+ # to Distroseries SP bugs.
+ if params.distroseries is not None:
+ distroseries_id = params.distroseries.id
+ parent_distro_id = params.distroseries.distributionID
+ else:
+ distroseries_id = 0
+ parent_distro_id = 0
+ ss_clauses.append(In(
+ Row(BugTaskFlat.distroseries_id,
+ BugTaskFlat.sourcepackagename_id),
+ Select(
+ ((distroseries_id,
+ SS.sourcepackagenameID),),
+ tables=[SS],
+ where=And(
+ SS.distributionID == parent_distro_id,
+ SS.sourcepackagenameID != None))))
+ if is_person_search or params.product is not None:
+ ss_clauses.append(In(
+ BugTaskFlat.product_id,
+ Select(
+ SS.productID,
+ tables=[SS])))
+ if is_person_search or params.productseries is not None:
+ ss_clauses.append(In(
+ BugTaskFlat.productseries_id,
+ Select(
+ SS.productseriesID,
+ tables=[SS])))
+ if is_person_search or params.project is not None:
+ ss_clauses.append(In(
+ BugTaskFlat.product_id,
+ Select(
+ Product.id,
+ tables=[SS, Product],
+ where=And(
+ SS.projectID == Product.projectID,
+ Product.project == params.project,
+ Product.active))))
+ if is_person_search or params.milestone is not None:
+ ss_clauses.append(In(
+ BugTaskFlat.milestone_id,
+ Select(
+ SS.milestoneID,
+ tables=[SS])))
+ extra_clauses.append(Or(*ss_clauses))
- # Remove bugtasks from deactivated products, if necessary.
- # We don't have to do this if
- # 1) We're searching on bugtasks for a specific product
- # 2) We're searching on bugtasks for a specific productseries
- # 3) We're searching on bugtasks for a distribution
- # 4) We're searching for bugtasks for a distroseries
- # because in those instances we don't have arbitrary products which
- # may be deactivated showing up in our search.
- if (params.product is None and
- params.distribution is None and
- params.productseries is None and
- params.distroseries is None):
+ # Remove bugtasks from deactivated products.
+ # This is needed for searches where people are the context.
+ if is_person_search:
extra_clauses.append(
Or(BugTaskFlat.product == None, Product.active == True))
join_tables.append(
@@ -618,15 +617,10 @@
if tag_clause is not None:
extra_clauses.append(tag_clause)
- # XXX Tom Berger 2008-02-14:
- # We use StructuralSubscription to determine
- # the bug supervisor relation for distribution source
- # packages, following a conversion to use this object.
- # We know that the behaviour remains the same, but we
- # should change the terminology, or re-instate
- # PackageBugSupervisor, since the use of this relation here
- # is not for subscription to notifications.
- # See bug #191809
+ # XXX sinzui 2012-09-26:
+ # This uses StructuralSubscription to assume a bug supervisor relationship
+ # for distribution source packages to preserve historical behaviour.
+ # This also duplicates params.structural_subscriber code and behaviour.
if params.bug_supervisor:
extra_clauses.append(Or(
In(
Follow ups
