launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07100
[Merge] lp:~wgrant/launchpad/stormify-more-bug-search into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/stormify-more-bug-search into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/stormify-more-bug-search/+merge/101713
This branch Stormifies most of the remaining bits of bug search. This allows the elimination of a fair bit of duplication, and paves the way for the BugTaskFlat feature flag. The only functional change should be the removal of the "if params.product" branch from _build_pending_bugwatch_elsewhere_clause, which was broken and unused anyway (pending_bugwatch is only linked for distributions).
--
https://code.launchpad.net/~wgrant/launchpad/stormify-more-bug-search/+merge/101713
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/stormify-more-bug-search into lp:launchpad.
=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py 2012-04-03 06:14:09 +0000
+++ lib/lp/bugs/model/bugtask.py 2012-04-12 10:29:36 +0000
@@ -1980,10 +1980,7 @@
See `IBugTask.getBugCountsForPackages` for more information.
"""
- from lp.bugs.model.bugtasksearch import (
- get_bug_privacy_filter,
- search_value_to_where_condition,
- )
+ from lp.bugs.model.bugtasksearch import get_bug_privacy_filter
packages = [
package for package in packages
@@ -1992,23 +1989,23 @@
package.sourcepackagename.id for package in packages]
open_bugs_cond = (
- 'BugTask.status %s' % search_value_to_where_condition(
- any(*DB_UNRESOLVED_BUGTASK_STATUSES)))
+ 'BugTask.status IN %s' %
+ sqlvalues(DB_UNRESOLVED_BUGTASK_STATUSES))
sum_template = "SUM(CASE WHEN %s THEN 1 ELSE 0 END) AS %s"
sums = [
sum_template % (open_bugs_cond, 'open_bugs'),
sum_template % (
- 'BugTask.importance %s' % search_value_to_where_condition(
- BugTaskImportance.CRITICAL), 'open_critical_bugs'),
+ 'BugTask.importance = %s' %
+ sqlvalues(BugTaskImportance.CRITICAL), 'open_critical_bugs'),
sum_template % (
'BugTask.assignee IS NULL', 'open_unassigned_bugs'),
sum_template % (
- 'BugTask.status %s' % search_value_to_where_condition(
- BugTaskStatus.INPROGRESS), 'open_inprogress_bugs'),
+ 'BugTask.status = %s' %
+ sqlvalues(BugTaskStatus.INPROGRESS), 'open_inprogress_bugs'),
sum_template % (
- 'BugTask.importance %s' % search_value_to_where_condition(
- BugTaskImportance.HIGH), 'open_high_bugs'),
+ 'BugTask.importance = %s' %
+ sqlvalues(BugTaskImportance.HIGH), 'open_high_bugs'),
]
conditions = [
=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py 2012-04-12 04:39:06 +0000
+++ lib/lp/bugs/model/bugtasksearch.py 2012-04-12 10:29:36 +0000
@@ -7,7 +7,6 @@
'get_bug_privacy_filter',
'orderby_expression',
'search_bugs',
- 'search_value_to_where_condition',
]
from operator import itemgetter
@@ -61,11 +60,13 @@
from lp.bugs.model.bugnomination import BugNomination
from lp.bugs.model.bugsubscription import BugSubscription
from lp.bugs.model.bugtask import BugTask
+from lp.bugs.model.structuralsubscription import StructuralSubscription
from lp.registry.interfaces.distribution import IDistribution
from lp.registry.interfaces.distroseries import IDistroSeries
from lp.registry.interfaces.milestone import IProjectGroupMilestone
from lp.registry.interfaces.product import IProduct
from lp.registry.interfaces.productseries import IProductSeries
+from lp.registry.model.distribution import Distribution
from lp.registry.model.milestone import Milestone
from lp.registry.model.milestonetag import MilestoneTag
from lp.registry.model.person import Person
@@ -76,6 +77,10 @@
quote,
sqlvalues,
)
+from lp.services.database.stormexpr import (
+ Array,
+ NullCount,
+ )
from lp.services.propertycache import get_property_cache
from lp.services.searchbuilder import (
all,
@@ -167,42 +172,6 @@
}
-def search_value_to_where_condition(search_value):
- """Convert a search value to a string WHERE condition.
-
- >>> search_value_to_where_condition(any(1, 2, 3))
- 'IN (1,2,3)'
- >>> search_value_to_where_condition(any()) is None
- True
- >>> search_value_to_where_condition(not_equals('foo'))
- "!= 'foo'"
- >>> search_value_to_where_condition(greater_than('foo'))
- "> 'foo'"
- >>> search_value_to_where_condition(1)
- '= 1'
- >>> search_value_to_where_condition(NULL)
- 'IS NULL'
-
- """
- if zope_isinstance(search_value, any):
- # When an any() clause is provided, the argument value
- # is a list of acceptable filter values.
- if not search_value.query_values:
- return None
- return "IN (%s)" % ",".join(sqlvalues(*search_value.query_values))
- elif zope_isinstance(search_value, not_equals):
- return "!= %s" % sqlvalues(search_value.value)
- elif zope_isinstance(search_value, greater_than):
- return "> %s" % sqlvalues(search_value.value)
- elif search_value is not NULL:
- return "= %s" % sqlvalues(search_value)
- else:
- # The argument value indicates we should match
- # only NULL values for the column named by
- # arg_name.
- return "IS NULL"
-
-
def search_value_to_storm_where_condition(comp, search_value):
"""Convert a search value to a Storm WHERE condition."""
if zope_isinstance(search_value, any):
@@ -496,59 +465,70 @@
'''ss as (SELECT * from StructuralSubscription
WHERE StructuralSubscription.subscriber = %s)'''
% sqlvalues(params.structural_subscriber))
+
+ class StructuralSubscriptionWith(StructuralSubscription):
+ __storm_table__ = 'ss'
+
join_tables.append(
(Product, LeftJoin(Product, And(
BugTask.productID == Product.id,
Product.active))))
- join_tables.append(
- (None,
- LeftJoin(
- SQL('ss ss1'),
- BugTask.product == SQL('ss1.product'))))
- join_tables.append(
- (None,
- LeftJoin(
- SQL('ss ss2'),
- BugTask.productseries == SQL('ss2.productseries'))))
- join_tables.append(
- (None,
- LeftJoin(
- SQL('ss ss3'),
- Product.project == SQL('ss3.project'))))
- join_tables.append(
- (None,
- LeftJoin(
- SQL('ss ss4'),
- And(BugTask.distribution == SQL('ss4.distribution'),
- Or(BugTask.sourcepackagename ==
- SQL('ss4.sourcepackagename'),
- SQL('ss4.sourcepackagename IS NULL'))))))
+ ProductSub = ClassAlias(StructuralSubscriptionWith)
+ join_tables.append((
+ ProductSub,
+ LeftJoin(
+ ProductSub,
+ BugTask.productID == ProductSub.productID)))
+ ProductSeriesSub = ClassAlias(StructuralSubscriptionWith)
+ join_tables.append((
+ ProductSeriesSub,
+ LeftJoin(
+ ProductSeriesSub,
+ BugTask.productseriesID == ProductSub.productseriesID)))
+ ProjectSub = ClassAlias(StructuralSubscriptionWith)
+ join_tables.append((
+ ProjectSub,
+ LeftJoin(
+ ProjectSub,
+ Product.projectID == ProjectSub.projectID)))
+ DistributionSub = ClassAlias(StructuralSubscriptionWith)
+ join_tables.append((
+ DistributionSub,
+ LeftJoin(
+ DistributionSub,
+ And(BugTask.distributionID == DistributionSub.distributionID,
+ Or(
+ DistributionSub.sourcepackagenameID ==
+ BugTask.sourcepackagenameID,
+ DistributionSub.sourcepackagenameID == None)))))
if params.distroseries is not None:
parent_distro_id = params.distroseries.distributionID
else:
parent_distro_id = 0
- join_tables.append(
- (None,
- LeftJoin(
- SQL('ss ss5'),
- Or(BugTask.distroseries == SQL('ss5.distroseries'),
+ DistroSeriesSub = ClassAlias(StructuralSubscriptionWith)
+ join_tables.append((
+ DistroSeriesSub,
+ LeftJoin(
+ DistroSeriesSub,
+ Or(BugTask.distroseriesID == 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 == SQL('ss5.distribution'),
- BugTask.sourcepackagename == SQL(
- 'ss5.sourcepackagename'))))))
- join_tables.append(
- (None,
- LeftJoin(
- SQL('ss ss6'),
- BugTask.milestone == SQL('ss6.milestone'))))
+ And(parent_distro_id == DistroSeriesSub.distributionID,
+ BugTask.sourcepackagenameID ==
+ DistroSeriesSub.sourcepackagenameID)))))
+ MilestoneSub = ClassAlias(StructuralSubscriptionWith)
+ join_tables.append((
+ MilestoneSub,
+ LeftJoin(
+ MilestoneSub,
+ BugTask.milestoneID == MilestoneSub.milestoneID)))
extra_clauses.append(
- "NULL_COUNT("
- "ARRAY[ss1.id, ss2.id, ss3.id, ss4.id, ss5.id, ss6.id]"
- ") < 6")
+ NullCount(Array(
+ ProductSub.id, ProductSeriesSub.id, ProjectSub.id,
+ DistributionSub.id, DistroSeriesSub.id, MilestoneSub.id)) < 6)
has_duplicate_results = True
# Remove bugtasks from deactivated products, if necessary.
@@ -703,7 +683,7 @@
if branches is not None:
where.append(
search_value_to_storm_where_condition(
- BugBranch.branchID, params.linked_branches))
+ BugBranch.branchID, branches))
return Exists(Select(1, tables=[BugBranch], where=And(*where)))
if zope_isinstance(params.linked_branches, BaseItem):
@@ -848,20 +828,18 @@
if fast:
assert params.searchtext is None, (
'Cannot use searchtext at the same time as fast_searchtext.')
- searchtext_quoted = quote(params.fast_searchtext)
+ searchtext = params.fast_searchtext
else:
assert params.fast_searchtext is None, (
'Cannot use fast_searchtext at the same time as searchtext.')
- searchtext_quoted = quote(params.searchtext)
+ searchtext = params.searchtext
if params.orderby is None:
# Unordered search results aren't useful, so sort by relevance
# instead.
- params.orderby = [
- SQLConstant("-rank(Bug.fti, ftq(%s))" % searchtext_quoted),
- ]
+ params.orderby = [SQL("-rank(Bug.fti, ftq(?))", params=(searchtext,))]
- return "Bug.fti @@ ftq(%s)" % searchtext_quoted
+ return SQL("Bug.fti @@ ftq(?)", params=(searchtext,))
def _build_status_clause(col, status):
@@ -911,7 +889,7 @@
# Perform a LEFT JOIN to the conjoined master bugtask. If the
# conjoined master is not null, it gets filtered out.
ConjoinedMaster = ClassAlias(BugTask, 'ConjoinedMaster')
- extra_clauses = ["ConjoinedMaster.id IS NULL"]
+ extra_clauses = [ConjoinedMaster.id == None]
if milestone.distribution is not None:
current_series = milestone.distribution.currentseries
join = LeftJoin(
@@ -1031,109 +1009,57 @@
def _build_blueprint_related_clause(params):
"""Find bugs related to Blueprints, or not."""
linked_blueprints = params.linked_blueprints
+
+ def make_clause(blueprints=None):
+ where = [SpecificationBug.bugID == Bug.id]
+ if blueprints is not None:
+ where.append(
+ search_value_to_storm_where_condition(
+ SpecificationBug.specificationID, blueprints))
+ return Exists(Select(1, tables=[SpecificationBug], where=And(*where)))
+
if linked_blueprints is None:
return None
elif zope_isinstance(linked_blueprints, BaseItem):
if linked_blueprints == BugBlueprintSearch.BUGS_WITH_BLUEPRINTS:
- return "EXISTS (%s)" % (
- "SELECT 1 FROM SpecificationBug"
- " WHERE SpecificationBug.bug = Bug.id")
+ return make_clause()
elif (linked_blueprints ==
BugBlueprintSearch.BUGS_WITHOUT_BLUEPRINTS):
- return "NOT EXISTS (%s)" % (
- "SELECT 1 FROM SpecificationBug"
- " WHERE SpecificationBug.bug = Bug.id")
+ return Not(make_clause())
else:
# A specific search term has been supplied.
- return """EXISTS (
- SELECT TRUE FROM SpecificationBug
- WHERE SpecificationBug.bug=Bug.id AND
- SpecificationBug.specification %s)
- """ % search_value_to_where_condition(linked_blueprints)
+ return make_clause(linked_blueprints)
# Upstream task restrictions
-_open_resolved_upstream = """
- EXISTS (
- SELECT TRUE FROM BugTask AS RelatedBugTask
- WHERE RelatedBugTask.bug = BugTask.bug
- AND RelatedBugTask.id != BugTask.id
- AND ((
- RelatedBugTask.bugwatch IS NOT NULL AND
- RelatedBugTask.status %s)
- OR (
- RelatedBugTask.product IS NOT NULL AND
- RelatedBugTask.bugwatch IS NULL AND
- RelatedBugTask.status %s))
- )
- """
-
-_open_resolved_upstream_with_target = """
- EXISTS (
- SELECT TRUE FROM BugTask AS RelatedBugTask
- WHERE RelatedBugTask.bug = BugTask.bug
- AND RelatedBugTask.id != BugTask.id
- AND ((
- RelatedBugTask.%(target_column)s = %(target_id)s AND
- RelatedBugTask.bugwatch IS NOT NULL AND
- RelatedBugTask.status %(status_with_watch)s)
- OR (
- RelatedBugTask.%(target_column)s = %(target_id)s AND
- RelatedBugTask.bugwatch IS NULL AND
- RelatedBugTask.status %(status_without_watch)s))
- )
- """
-
-
def _build_pending_bugwatch_elsewhere_clause(params):
"""Return a clause for BugTaskSearchParams.pending_bugwatch_elsewhere
"""
- if params.product:
- # Include only bugtasks that do no have bug watches that
- # belong to a product that does not use Malone.
- return """
- EXISTS (
- SELECT TRUE
- FROM BugTask AS RelatedBugTask
- LEFT OUTER JOIN Product AS OtherProduct
- ON RelatedBugTask.product = OtherProduct.id
- WHERE RelatedBugTask.bug = BugTask.bug
- AND RelatedBugTask.id = BugTask.id
- AND RelatedBugTask.bugwatch IS NULL
- AND OtherProduct.official_malone IS FALSE
- AND RelatedBugTask.status != %s)
- """ % sqlvalues(BugTaskStatus.INVALID)
- elif params.upstream_target is None:
- # Include only bugtasks that have other bugtasks on targets
- # not using Malone, which are not Invalid, and have no bug
- # watch.
- return """
- EXISTS (
- SELECT TRUE
- FROM BugTask AS RelatedBugTask
- LEFT OUTER JOIN Distribution AS OtherDistribution
- ON RelatedBugTask.distribution =
- OtherDistribution.id
- LEFT OUTER JOIN Product AS OtherProduct
- ON RelatedBugTask.product = OtherProduct.id
- WHERE RelatedBugTask.bug = BugTask.bug
- AND RelatedBugTask.id != BugTask.id
- AND RelatedBugTask.bugwatch IS NULL
- AND (
- OtherDistribution.official_malone IS FALSE
- OR OtherProduct.official_malone IS FALSE)
- AND RelatedBugTask.status != %s)
- """ % sqlvalues(BugTaskStatus.INVALID)
+ RelatedBugTask = ClassAlias(BugTask)
+ extra_joins = []
+ if params.upstream_target is None:
+ # Restrict the target to distributions or products which don't
+ # use Launchpad for bug tracking.
+ OtherDistribution = ClassAlias(Distribution)
+ OtherProduct = ClassAlias(Product)
+ extra_joins = [
+ LeftJoin(
+ OtherDistribution,
+ OtherDistribution.id == RelatedBugTask.distributionID),
+ LeftJoin(
+ OtherProduct,
+ OtherProduct.id == RelatedBugTask.productID),
+ ]
+ target_clause = Or(
+ OtherDistribution.official_malone == False,
+ OtherProduct.official_malone == False)
else:
- # Include only bugtasks that have other bugtasks on
- # params.upstream_target, but only if this this product
- # does not use Malone and if the bugtasks are not Invalid,
- # and have no bug watch.
+ # Restrict the target to params.upstream_target.
if IProduct.providedBy(params.upstream_target):
- target_clause = 'RelatedBugTask.product = %s'
+ target_col = RelatedBugTask.productID
elif IDistribution.providedBy(params.upstream_target):
- target_clause = 'RelatedBugTask.distribution = %s'
+ target_col = RelatedBugTask.distributionID
else:
raise AssertionError(
'params.upstream_target must be a Distribution or '
@@ -1141,51 +1067,37 @@
# There is no point to construct a real sub-select if we
# already know that the result will be empty.
if params.upstream_target.official_malone:
- return 'false'
- target_clause = target_clause % sqlvalues(
- params.upstream_target.id)
- return """
- EXISTS (
- SELECT TRUE
- FROM BugTask AS RelatedBugTask
- WHERE RelatedBugTask.bug = BugTask.bug
- AND RelatedBugTask.id != BugTask.id
- AND RelatedBugTask.bugwatch IS NULL
- AND %s
- AND RelatedBugTask.status != %s)
- """ % (target_clause, sqlvalues(BugTaskStatus.INVALID)[0])
+ return False
+ target_clause = target_col == params.upstream_target.id
+ # Include only bugtasks that have other bugtasks on matching
+ # targets which are not Invalid and have no bug watch.
+ return Exists(Select(
+ 1,
+ tables=[RelatedBugTask] + extra_joins,
+ where=And(
+ RelatedBugTask.bugID == BugTask.bugID,
+ RelatedBugTask.id != BugTask.id,
+ RelatedBugTask.bugwatchID == None,
+ RelatedBugTask.status != BugTaskStatus.INVALID,
+ target_clause)))
def _build_no_upstream_bugtask_clause(params):
"""Return a clause for BugTaskSearchParams.has_no_upstream_bugtask."""
+ OtherBugTask = ClassAlias(BugTask)
if params.upstream_target is None:
- # Find all bugs that has no product bugtask. We limit the
- # SELECT by matching against BugTask.bug to make the query
- # faster.
- return """
- NOT EXISTS (SELECT TRUE
- FROM BugTask AS OtherBugTask
- WHERE OtherBugTask.bug = BugTask.bug
- AND OtherBugTask.product IS NOT NULL)
- """
+ target = OtherBugTask.productID != None
elif IProduct.providedBy(params.upstream_target):
- return """
- NOT EXISTS (SELECT TRUE
- FROM BugTask AS OtherBugTask
- WHERE OtherBugTask.bug = BugTask.bug
- AND OtherBugTask.product=%s)
- """ % sqlvalues(params.upstream_target.id)
+ target = OtherBugTask.productID == params.upstream_target.id
elif IDistribution.providedBy(params.upstream_target):
- return """
- NOT EXISTS (SELECT TRUE
- FROM BugTask AS OtherBugTask
- WHERE OtherBugTask.bug = BugTask.bug
- AND OtherBugTask.distribution=%s)
- """ % sqlvalues(params.upstream_target.id)
+ target = OtherBugTask.distributionID == params.upstream_target.id
else:
raise AssertionError(
'params.upstream_target must be a Distribution or '
'a Product')
+ return Not(Exists(Select(
+ 1, tables=[OtherBugTask],
+ where=And(OtherBugTask.bugID == BugTask.bugID, target))))
def _build_open_or_resolved_upstream_clause(params,
@@ -1193,26 +1105,38 @@
statuses_for_upstream_tasks):
"""Return a clause for BugTaskSearchParams.open_upstream or
BugTaskSearchParams.resolved_upstream."""
+ RelatedBugTask = ClassAlias(BugTask)
+ watch_status_clause = search_value_to_storm_where_condition(
+ RelatedBugTask._status, any(*statuses_for_watch_tasks))
+ no_watch_status_clause = search_value_to_storm_where_condition(
+ RelatedBugTask._status, any(*statuses_for_upstream_tasks))
if params.upstream_target is None:
- return _open_resolved_upstream % (
- search_value_to_where_condition(
- any(*statuses_for_watch_tasks)),
- search_value_to_where_condition(
- any(*statuses_for_upstream_tasks)))
- elif IProduct.providedBy(params.upstream_target):
- query_values = {'target_column': 'product'}
- elif IDistribution.providedBy(params.upstream_target):
- query_values = {'target_column': 'distribution'}
+ watch_target_clause = True
+ no_watch_target_clause = RelatedBugTask.productID != None
else:
- raise AssertionError(
- 'params.upstream_target must be a Distribution or '
- 'a Product')
- query_values['target_id'] = sqlvalues(params.upstream_target.id)[0]
- query_values['status_with_watch'] = search_value_to_where_condition(
- any(*statuses_for_watch_tasks))
- query_values['status_without_watch'] = search_value_to_where_condition(
- any(*statuses_for_upstream_tasks))
- return _open_resolved_upstream_with_target % query_values
+ if IProduct.providedBy(params.upstream_target):
+ target_col = RelatedBugTask.productID
+ elif IDistribution.providedBy(params.upstream_target):
+ target_col = RelatedBugTask.distributionID
+ else:
+ raise AssertionError(
+ 'params.upstream_target must be a Distribution or '
+ 'a Product')
+ watch_target_clause = no_watch_target_clause = (
+ target_col == params.upstream_target.id)
+ return Exists(Select(
+ 1,
+ tables=[RelatedBugTask],
+ where=And(
+ RelatedBugTask.bugID == BugTask.bugID,
+ RelatedBugTask.id != BugTask.id,
+ Or(
+ And(watch_target_clause,
+ RelatedBugTask.bugwatchID != None,
+ watch_status_clause),
+ And(no_watch_target_clause,
+ RelatedBugTask.bugwatchID == None,
+ no_watch_status_clause)))))
def _build_open_upstream_clause(params):
@@ -1272,8 +1196,7 @@
upstream_clauses.append(_build_open_upstream_clause(params))
if upstream_clauses:
- upstream_clause = " OR ".join(upstream_clauses)
- return '(%s)' % upstream_clause
+ return Or(*upstream_clauses)
return None
=== modified file 'lib/lp/services/database/stormexpr.py'
--- lib/lp/services/database/stormexpr.py 2012-02-20 12:26:57 +0000
+++ lib/lp/services/database/stormexpr.py 2012-04-12 10:29:36 +0000
@@ -3,13 +3,16 @@
__metaclass__ = type
__all__ = [
+ 'Array',
'Concatenate',
'CountDistinct',
'Greatest',
+ 'NullCount',
]
from storm.expr import (
BinaryOper,
+ ComparableExpr,
compile,
EXPR,
Expr,
@@ -48,3 +51,23 @@
"""Storm operator for string concatenation."""
__slots__ = ()
oper = " || "
+
+
+class NullCount(NamedFunc):
+ __slots__ = ()
+ name = "NULL_COUNT"
+
+
+class Array(ComparableExpr):
+ __slots__ = ("args",)
+
+ def __init__(self, *args):
+ self.args = args
+
+
+@compile.when(Array)
+def compile_array(compile, array, state):
+ state.push("context", EXPR)
+ args = compile(array.args, state)
+ state.pop()
+ return "ARRAY[%s]" % args
