From 52176fe78556cab07b3c823e9454029a1e15271c Mon Sep 17 00:00:00 2001
From: "James E. Blair" <jeblair@hp.com>
Date: Mon, 12 Jan 2015 11:45:14 +1300
Subject: [PATCH] Search: join tables when necessary
Some searches (eg, "owner:foo" or "project:bar") were performing
an implicit join on the primary key that joins them with the
change table in a 1:1 relationship. If a user negated those,
then the result would be "not (a.key=b.key and a.value=foo)"
which is equal to "a.key!=b.key or a.value!=foo". The partial
negation of the join criteria is definitely not what we want.
Instead, for the known 1:1 relationships that can be produced,
add the join criteria outside of the main expression iff those
tables are referenced. Then the remaining part of the
condition (a.value == foo) can be optionally negated as normal.
Expressions for relationships other than 1:1 are handled by
subselects and so should not be impacted by this.
Also, I'm leaving a simple bit of debug code in the search
module for future use.
Change-Id: I92bb0a01f1768cb4f7cc9b3b28d54ebd309fa406
---
gertty/search/__init__.py | 39 ++++++++++++++++++++++++++++++++++++++-
gertty/search/parser.py | 13 +++++--------
2 files changed, 43 insertions(+), 9 deletions(-)
diff --git a/gertty/search/__init__.py b/gertty/search/__init__.py
index 5837a05..7902970 100644
--- a/gertty/search/__init__.py
+++ b/gertty/search/__init__.py
@@ -13,6 +13,7 @@
# under the License.
import sqlalchemy.sql.expression
+from sqlalchemy.sql.expression import and_
from gertty.search import tokenizer, parser
import gertty.db
@@ -28,6 +29,42 @@ class SearchCompiler(object):
self.lexer = tokenizer.SearchTokenizer()
self.parser = parser.SearchParser()
+ def findTables(self, expression):
+ tables = set()
+ stack = [expression]
+ while stack:
+ x = stack.pop()
+ if hasattr(x, 'table'):
+ if (x.table != gertty.db.change_table
+ and hasattr(x.table, 'name')):
+ tables.add(x.table)
+ for child in x.get_children():
+ if not isinstance(child, sqlalchemy.sql.selectable.Select):
+ stack.append(child)
+ return tables
+
def parse(self, data):
self.parser.username = self.app.config.username
- return self.parser.parse(data, lexer=self.lexer)
+ result = self.parser.parse(data, lexer=self.lexer)
+ tables = self.findTables(result)
+ if gertty.db.project_table in tables:
+ result = and_(gertty.db.change_table.c.project_key == gertty.db.project_table.c.key,
+ result)
+ tables.remove(gertty.db.project_table)
+ if gertty.db.account_table in tables:
+ result = and_(gertty.db.change_table.c.account_key == gertty.db.account_table.c.key,
+ result)
+ tables.remove(gertty.db.account_table)
+ if tables:
+ raise Exception("Unknown table in search: %s" % tables)
+ return result
+
+if __name__ == '__main__':
+ class Dummy(object):
+ pass
+ app = Dummy()
+ app.config = Dummy()
+ app.config.username = 'bob'
+ search = SearchCompiler(app)
+ x = search.parse('owner:self')
+ print x
diff --git a/gertty/search/parser.py b/gertty/search/parser.py
index 0b747a2..5f680fa 100644
--- a/gertty/search/parser.py
+++ b/gertty/search/parser.py
@@ -122,13 +122,11 @@ def SearchParser():
'''owner_term : OP_OWNER string'''
if p[2] == 'self':
username = p.parser.username
- p[0] = and_(gertty.db.change_table.c.account_key == gertty.db.account_table.c.key,
- gertty.db.account_table.c.username == username)
+ p[0] = gertty.db.account_table.c.username == username
else:
- p[0] = and_(gertty.db.change_table.c.account_key == gertty.db.account_table.c.key,
- or_(gertty.db.account_table.c.username == p[2],
- gertty.db.account_table.c.email == p[2],
- gertty.db.account_table.c.name == p[2]))
+ p[0] = or_(gertty.db.account_table.c.username == p[2],
+ gertty.db.account_table.c.email == p[2],
+ gertty.db.account_table.c.name == p[2])
def p_reviewer_term(p):
'''reviewer_term : OP_REVIEWER string'''
@@ -266,8 +264,7 @@ def SearchParser():
elif p[2] == 'abandoned':
p[0] = gertty.db.change_table.c.status == 'ABANDONED'
elif p[2] == 'owner':
- p[0] = and_(gertty.db.change_table.c.account_key == gertty.db.account_table.c.key,
- gertty.db.account_table.c.username == username)
+ p[0] = gertty.db.account_table.c.username == username
elif p[2] == 'reviewer':
filters = []
filters.append(gertty.db.approval_table.c.change_key == gertty.db.change_table.c.key)