launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07144
[Merge] lp:~wgrant/launchpad/bugtaskflat-search-0 into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/bugtaskflat-search-0 into lp:launchpad with lp:~wgrant/launchpad/bugtaskflat-search--1 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bugtaskflat-search-0/+merge/102417
Due to the perilous performance characteristics of bug search, we want to be able to switch BugTaskFlat usage on or off quickly with a feature flag. This entails *temporarily* making lp.bugs.model.bugtasksearch a hybrid creature, able to operate on either. This will be achieved by abstracting the column references through a map which can be switched depending on the feature flag.
This branch abstracts the three main columns: the Bug and BugTask IDs.
--
https://code.launchpad.net/~wgrant/launchpad/bugtaskflat-search-0/+merge/102417
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bugtaskflat-search-0 into lp:launchpad.
=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py 2012-04-18 01:53:08 +0000
+++ lib/lp/bugs/model/bugtasksearch.py 2012-04-18 01:53:08 +0000
@@ -299,6 +299,14 @@
decorator to call on each returned row.
"""
params = _require_params(params)
+
+ # Will soon be overridden to use BugTaskFlat.bug_id instead.
+ cols = {
+ 'Bug.id': Bug.id,
+ 'BugTask.bugID': BugTask.bugID,
+ 'BugTask.id': BugTask.id,
+ }
+
extra_clauses = [Bug.id == BugTask.bugID]
clauseTables = [BugTask, Bug]
join_tables = []
@@ -376,7 +384,7 @@
if params.exclude_conjoined_tasks:
tables, clauses = _build_exclude_conjoined_clause(
- params.milestone)
+ params.milestone, cols)
join_tables += tables
extra_clauses += clauses
@@ -420,7 +428,7 @@
if params.has_cve:
extra_clauses.append(
- BugTask.bugID.is_in(
+ cols['BugTask.bugID'].is_in(
Select(BugCve.bugID, tables=[BugCve], distinct=True)))
if params.attachmenttype is not None:
@@ -428,7 +436,7 @@
extra_clauses.append(Bug.latest_patch_uploaded != None)
else:
extra_clauses.append(
- Bug.id.is_in(
+ cols['Bug.id'].is_in(
Select(
BugAttachment.bugID, tables=[BugAttachment],
where=search_value_to_storm_where_condition(
@@ -443,7 +451,7 @@
if params.subscriber is not None:
clauseTables.append(BugSubscription)
extra_clauses.append(And(
- Bug.id == BugSubscription.bug_id,
+ cols['Bug.id'] == BugSubscription.bug_id,
BugSubscription.person == params.subscriber))
if params.structural_subscriber is not None:
@@ -574,12 +582,12 @@
BugTask.sourcepackagenameID.is_in(
SQL('SELECT sourcepackagename FROM spns')))
- upstream_clause = _build_upstream_clause(params)
+ upstream_clause = _build_upstream_clause(params, cols)
if upstream_clause:
extra_clauses.append(upstream_clause)
if params.tag:
- tag_clause = _build_tag_search_clause(params.tag)
+ tag_clause = _build_tag_search_clause(params.tag, cols)
if tag_clause is not None:
extra_clauses.append(tag_clause)
@@ -621,7 +629,7 @@
if params.bug_commenter:
extra_clauses.append(
- Bug.id.is_in(Select(
+ cols['Bug.id'].is_in(Select(
BugMessage.bugID, tables=[BugMessage],
where=And(
BugMessage.index > 0,
@@ -634,7 +642,7 @@
join_tables.append(
(BugAffectsPerson, Join(
BugAffectsPerson, And(
- BugTask.bugID == BugAffectsPerson.bugID,
+ cols['BugTask.bugID'] == BugAffectsPerson.bugID,
BugAffectsPerson.affected,
BugAffectsPerson.person == params.affected_user))))
@@ -647,7 +655,7 @@
raise AssertionError(
'Unknown nomination target: %r.' % params.nominated_for)
extra_clauses.append(And(
- BugNomination.bugID == BugTask.bugID,
+ BugNomination.bugID == cols['BugTask.bugID'],
BugNomination.status == BugNominationStatus.PROPOSED,
target_col == params.nominated_for))
clauseTables.append(BugNomination)
@@ -669,12 +677,12 @@
extra_clauses.append(SQL(clause))
decorators.append(decorator)
- hw_clause = _build_hardware_related_clause(params)
+ hw_clause = _build_hardware_related_clause(params, cols)
if hw_clause is not None:
extra_clauses.append(hw_clause)
def make_branch_clause(branches=None):
- where = [BugBranch.bugID == Bug.id]
+ where = [BugBranch.bugID == cols['Bug.id']]
if branches is not None:
where.append(
search_value_to_storm_where_condition(
@@ -691,7 +699,7 @@
# A specific search term has been supplied.
extra_clauses.append(make_branch_clause(params.linked_branches))
- linked_blueprints_clause = _build_blueprint_related_clause(params)
+ linked_blueprints_clause = _build_blueprint_related_clause(params, cols)
if linked_blueprints_clause is not None:
extra_clauses.append(linked_blueprints_clause)
@@ -857,7 +865,7 @@
raise ValueError('Unrecognized status value: %r' % (status,))
-def _build_exclude_conjoined_clause(milestone):
+def _build_exclude_conjoined_clause(milestone, cols):
"""Exclude bugtasks with a conjoined master.
This search option only makes sense when searching for bugtasks
@@ -884,7 +892,7 @@
current_series = milestone.distribution.currentseries
join = LeftJoin(
ConjoinedMaster,
- And(ConjoinedMaster.bugID == BugTask.bugID,
+ And(ConjoinedMaster.bugID == cols['BugTask.bugID'],
BugTask.distributionID == milestone.distribution.id,
ConjoinedMaster.distroseriesID == current_series.id,
Not(ConjoinedMaster._status.is_in(
@@ -901,7 +909,7 @@
LeftJoin(Product, BugTask.product == Product.id),
LeftJoin(
ConjoinedMaster,
- And(ConjoinedMaster.bugID == BugTask.bugID,
+ And(ConjoinedMaster.bugID == cols['BugTask.bugID'],
ConjoinedMaster.productseriesID
== Product.development_focusID,
Not(ConjoinedMaster._status.is_in(
@@ -914,7 +922,7 @@
milestone.product.development_focusID)
join = LeftJoin(
ConjoinedMaster,
- And(ConjoinedMaster.bugID == BugTask.bugID,
+ And(ConjoinedMaster.bugID == cols['BugTask.bugID'],
BugTask.productID == milestone.product.id,
ConjoinedMaster.productseriesID == dev_focus_id,
Not(ConjoinedMaster._status.is_in(
@@ -927,7 +935,7 @@
return (join_tables, extra_clauses)
-def _build_hardware_related_clause(params):
+def _build_hardware_related_clause(params, cols):
"""Hardware related SQL expressions and tables for bugtask searches.
:return: (tables, clauses) where clauses is a list of SQL expressions
@@ -992,16 +1000,16 @@
clauses.append(Or(*bug_link_clauses))
clauses.append(_userCanAccessSubmissionStormClause(params.user))
- return Bug.id.is_in(
+ return cols['Bug.id'].is_in(
Select(Bug.id, tables=tables, where=And(*clauses), distinct=True))
-def _build_blueprint_related_clause(params):
+def _build_blueprint_related_clause(params, cols):
"""Find bugs related to Blueprints, or not."""
linked_blueprints = params.linked_blueprints
def make_clause(blueprints=None):
- where = [SpecificationBug.bugID == Bug.id]
+ where = [SpecificationBug.bugID == cols['Bug.id']]
if blueprints is not None:
where.append(
search_value_to_storm_where_condition(
@@ -1023,14 +1031,14 @@
# Upstream task restrictions
-def _build_pending_bugwatch_elsewhere_clause(params):
+def _build_pending_bugwatch_elsewhere_clause(params, cols):
"""Return a clause for BugTaskSearchParams.pending_bugwatch_elsewhere
"""
RelatedBugTask = ClassAlias(BugTask)
extra_joins = []
# Normally we want to exclude the current task from the search,
# unless we're looking at an upstream project.
- task_match_clause = RelatedBugTask.id != BugTask.id
+ task_match_clause = RelatedBugTask.id != cols['BugTask.id']
target = None
if params.product:
# Looking for pending bugwatches in a project context is
@@ -1039,7 +1047,7 @@
# does. So the task ID should match, and there is no need for a
# target clause.
target = params.product
- task_match_clause = RelatedBugTask.id == BugTask.id
+ task_match_clause = RelatedBugTask.id == cols['BugTask.id']
target_clause = True
elif params.upstream_target:
# Restrict the target to params.upstream_target.
@@ -1082,14 +1090,14 @@
1,
tables=[RelatedBugTask] + extra_joins,
where=And(
- RelatedBugTask.bugID == BugTask.bugID,
+ RelatedBugTask.bugID == cols['BugTask.bugID'],
task_match_clause,
RelatedBugTask.bugwatchID == None,
RelatedBugTask._status != BugTaskStatus.INVALID,
target_clause)))
-def _build_no_upstream_bugtask_clause(params):
+def _build_no_upstream_bugtask_clause(params, cols):
"""Return a clause for BugTaskSearchParams.has_no_upstream_bugtask."""
OtherBugTask = ClassAlias(BugTask)
if params.upstream_target is None:
@@ -1104,12 +1112,12 @@
'a Product')
return Not(Exists(Select(
1, tables=[OtherBugTask],
- where=And(OtherBugTask.bugID == BugTask.bugID, target))))
+ where=And(OtherBugTask.bugID == cols['BugTask.bugID'], target))))
def _build_open_or_resolved_upstream_clause(params,
statuses_for_watch_tasks,
- statuses_for_upstream_tasks):
+ statuses_for_upstream_tasks, cols):
"""Return a clause for BugTaskSearchParams.open_upstream or
BugTaskSearchParams.resolved_upstream."""
RelatedBugTask = ClassAlias(BugTask)
@@ -1135,8 +1143,8 @@
1,
tables=[RelatedBugTask],
where=And(
- RelatedBugTask.bugID == BugTask.bugID,
- RelatedBugTask.id != BugTask.id,
+ RelatedBugTask.bugID == cols['BugTask.bugID'],
+ RelatedBugTask.id != cols['BugTask.id'],
Or(
And(watch_target_clause,
RelatedBugTask.bugwatchID != None,
@@ -1146,7 +1154,7 @@
no_watch_status_clause)))))
-def _build_open_upstream_clause(params):
+def _build_open_upstream_clause(params, cols):
"""Return a clause for BugTaskSearchParams.open_upstream."""
statuses_for_open_tasks = [
BugTaskStatus.NEW,
@@ -1157,10 +1165,10 @@
BugTaskStatus.INPROGRESS,
BugTaskStatus.UNKNOWN]
return _build_open_or_resolved_upstream_clause(
- params, statuses_for_open_tasks, statuses_for_open_tasks)
-
-
-def _build_resolved_upstream_clause(params):
+ params, statuses_for_open_tasks, statuses_for_open_tasks, cols)
+
+
+def _build_resolved_upstream_clause(params, cols):
"""Return a clause for BugTaskSearchParams.open_upstream."""
# Our definition of "resolved upstream" means:
#
@@ -1180,10 +1188,10 @@
BugTaskStatus.FIXCOMMITTED,
BugTaskStatus.FIXRELEASED]
return _build_open_or_resolved_upstream_clause(
- params, statuses_for_watch_tasks, statuses_for_upstream_tasks)
-
-
-def _build_upstream_clause(params):
+ params, statuses_for_watch_tasks, statuses_for_upstream_tasks, cols)
+
+
+def _build_upstream_clause(params, cols):
"""Return an clause for returning upstream data if the data exists.
This method will handles BugTasks that do not have upstream BugTasks
@@ -1193,14 +1201,16 @@
upstream_clauses = []
if params.pending_bugwatch_elsewhere:
upstream_clauses.append(
- _build_pending_bugwatch_elsewhere_clause(params))
+ _build_pending_bugwatch_elsewhere_clause(params, cols))
if params.has_no_upstream_bugtask:
upstream_clauses.append(
- _build_no_upstream_bugtask_clause(params))
+ _build_no_upstream_bugtask_clause(params, cols))
if params.resolved_upstream:
- upstream_clauses.append(_build_resolved_upstream_clause(params))
+ upstream_clauses.append(
+ _build_resolved_upstream_clause(params, cols))
if params.open_upstream:
- upstream_clauses.append(_build_open_upstream_clause(params))
+ upstream_clauses.append(
+ _build_open_upstream_clause(params, cols))
if upstream_clauses:
return Or(*upstream_clauses)
@@ -1209,9 +1219,11 @@
# Tag restrictions
-def _build_tag_set_query(clauses):
+def _build_tag_set_query(clauses, cols):
subselects = [
- Select(1, tables=[BugTag], where=And(BugTag.bugID == Bug.id, clause))
+ Select(
+ 1, tables=[BugTag], where=And(BugTag.bugID == cols['Bug.id'],
+ clause))
for clause in clauses]
if len(subselects) == 1:
return Exists(subselects[0])
@@ -1219,7 +1231,7 @@
return Exists(Intersect(*subselects))
-def _build_tag_set_query_all(tags):
+def _build_tag_set_query_all(tags, cols):
"""Return a Storm expression for bugs matching all given tags.
:param tags: An iterable of valid tags without - or + and not wildcards.
@@ -1227,10 +1239,11 @@
"""
if not tags:
return None
- return _build_tag_set_query([BugTag.tag == tag for tag in sorted(tags)])
-
-
-def _build_tag_set_query_any(tags):
+ return _build_tag_set_query(
+ [BugTag.tag == tag for tag in sorted(tags)], cols)
+
+
+def _build_tag_set_query_any(tags, cols):
"""Return a Storm expression for bugs matching any given tag.
:param tags: An iterable of valid tags without - or + and not wildcards.
@@ -1238,10 +1251,10 @@
"""
if not tags:
return None
- return _build_tag_set_query([BugTag.tag.is_in(sorted(tags))])
-
-
-def _build_tag_search_clause(tags_spec):
+ return _build_tag_set_query([BugTag.tag.is_in(sorted(tags))], cols)
+
+
+def _build_tag_search_clause(tags_spec, cols):
"""Return a tag search clause.
:param tags_spec: An instance of `any` or `all` containing tag
@@ -1265,23 +1278,25 @@
combine_with = And
# The set of bugs that have *all* of the tags requested for
# *inclusion*.
- include_clause = _build_tag_set_query_all(include)
+ include_clause = _build_tag_set_query_all(include, cols)
# The set of bugs that have *any* of the tags requested for
# *exclusion*.
- exclude_clause = _build_tag_set_query_any(exclude)
+ exclude_clause = _build_tag_set_query_any(exclude, cols)
else:
# How to combine an include clause and an exclude clause when
# both are generated.
combine_with = Or
# The set of bugs that have *any* of the tags requested for
# inclusion.
- include_clause = _build_tag_set_query_any(include)
+ include_clause = _build_tag_set_query_any(include, cols)
# The set of bugs that have *all* of the tags requested for
# exclusion.
- exclude_clause = _build_tag_set_query_all(exclude)
+ exclude_clause = _build_tag_set_query_all(exclude, cols)
universal_clause = (
- Exists(Select(1, tables=[BugTag], where=BugTag.bugID == Bug.id)))
+ Exists(Select(
+ 1, tables=[BugTag], where=BugTag.bugID == cols['Bug.id'])))
+
# Search for the *presence* of any tag.
if '*' in wildcards:
# Only clobber the clause if not searching for all tags.
