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
This commit is contained in:
James E. Blair 2015-01-12 11:45:14 +13:00
parent 0189352614
commit 52176fe785
2 changed files with 43 additions and 9 deletions

View File

@ -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

View File

@ -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)