Skip to content

Commit

Permalink
Merge pull request ansible#6098 from chrismeyersfsu/fix-facts_query_p…
Browse files Browse the repository at this point in the history 
…erformance

coarse json queries to use gin index
  • Loading branch information
@chrismeyersfsu
chrismeyersfsu authored Apr 25, 2017
2 parents 7037daa + d69ae2c commit 1f99a0d
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 73 deletions.
105 changes: 50 additions & 55 deletions awx/main/fields.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ def string_to_type(t):


class DynamicFilterField(models.TextField):

SEARCHABLE_RELATIONSHIP = 'ansible_facts'

class BoolOperand(object):
def __init__(self, t):
Expand All @@ -362,6 +362,16 @@ def __init__(self, t):
kwargs[k] = v
self.result = Q(**kwargs)

def strip_quotes_traditional_logic(self, v):
if type(v) is unicode and v.startswith('"') and v.endswith('"'):
return v[1:-1]
return v

def strip_quotes_json_logic(self, v):
if type(v) is unicode and v.startswith('"') and v.endswith('"') and v != u'"null"':
return v[1:-1]
return v

'''
TODO: We should be able to express this in the grammar and let
pyparsing do the heavy lifting.
Expand All @@ -371,66 +381,50 @@ def __init__(self, t):
relationship refered to to see if it's a jsonb type.
'''
def _json_path_to_contains(self, k, v):
pieces = k.split('__')
if not k.startswith(DynamicFilterField.SEARCHABLE_RELATIONSHIP):
v = self.strip_quotes_traditional_logic(v)
return (k, v)

flag_first_arr_found = False
# Strip off leading relationship key
if k.startswith(DynamicFilterField.SEARCHABLE_RELATIONSHIP + '__'):
strip_len = len(DynamicFilterField.SEARCHABLE_RELATIONSHIP) + 2
else:
strip_len = len(DynamicFilterField.SEARCHABLE_RELATIONSHIP)
k = k[strip_len:]

assembled_k = ''
assembled_v = v
pieces = k.split(u'__')

assembled_k = u'%s__contains' % (DynamicFilterField.SEARCHABLE_RELATIONSHIP)
assembled_v = None

last_kv = None
last_v = None
last_kv = None

contains_count = 0
for i, piece in enumerate(pieces):
if flag_first_arr_found is False and piece.endswith('[]'):
assembled_k += u'%s__contains' % (piece[0:-2])
contains_count += 1
flag_first_arr_found = True
elif flag_first_arr_found is False and i == len(pieces) - 1:
assembled_k += u'%s' % piece
elif flag_first_arr_found is False:
assembled_k += u'%s__' % piece
elif flag_first_arr_found is True:
new_kv = dict()
if piece.endswith('[]'):
new_v = []
new_kv[piece[0:-2]] = new_v
else:
new_v = dict()
new_kv[piece] = new_v


if last_v is None:
last_v = []
assembled_v = last_v

if type(last_v) is list:
last_v.append(new_kv)
elif type(last_v) is dict:
last_kv[last_kv.keys()[0]] = new_kv

last_v = new_v
last_kv = new_kv
contains_count += 1

'''
Explicit quotes are kept until this point.
Note: we could have totally "ripped" them off earlier when we decided
what type to convert the token to.
'''
if type(v) is unicode and v.startswith('"') and v.endswith('"') and v != u'"null"':
v = v[1:-1]

if contains_count == 0:
assembled_v = v
elif contains_count == 1:
assembled_v = [v]
elif contains_count > 1:
if type(last_v) is list:
last_v.append(v)
if type(last_v) is dict:
last_kv[last_kv.keys()[0]] = v
new_kv = dict()
if piece.endswith(u'[]'):
new_v = []
new_kv[piece[0:-2]] = new_v
else:
new_v = dict()
new_kv[piece] = new_v

if last_kv is None:
assembled_v = new_kv
elif type(last_v) is list:
last_v.append(new_kv)
elif type(last_v) is dict:
last_kv[last_kv.keys()[0]] = new_kv

last_v = new_v
last_kv = new_kv

v = self.strip_quotes_json_logic(v)

if type(last_v) is list:
last_v.append(v)
elif type(last_v) is dict:
last_kv[last_kv.keys()[0]] = v

return (assembled_k, assembled_v)

Expand Down Expand Up @@ -526,6 +520,7 @@ def filter_string_to_q(cls, filter_string):

try:
res = boolExpr.parseString('(' + filter_string + ')')
#except ParseException as e:
except Exception:
raise RuntimeError(u"Invalid query %s" % filter_string_raw)

Expand Down
38 changes: 20 additions & 18 deletions awx/main/tests/unit/test_fields.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from django.db.models import Q



class TestDynamicFilterFieldFilterStringToQ():
@pytest.mark.parametrize("filter_string,q_expected", [
('facts__facts__blank=""', Q(**{u"facts__facts__blank": u""})),
Expand All @@ -18,7 +19,7 @@ class TestDynamicFilterFieldFilterStringToQ():
('a__b__c=3.14', Q(**{u"a__b__c": 3.14})),
('a__b__c=true', Q(**{u"a__b__c": True})),
('a__b__c=false', Q(**{u"a__b__c": False})),
('a__b__c="true"', Q(**{u"a__b__c": u"true"})),
('ansible_facts__a="true"', Q(**{u"ansible_facts__contains": {u"a": u"true"}})),
#('"a__b\"__c"="true"', Q(**{u"a__b\"__c": "true"})),
#('a__b\"__c="true"', Q(**{u"a__b\"__c": "true"})),
])
Expand All @@ -27,8 +28,8 @@ def test_query_generated(self, filter_string, q_expected):
assert unicode(q) == unicode(q_expected)

@pytest.mark.parametrize("filter_string", [
'facts__facts__blank='
'a__b__c__ space =ggg',
'ansible_facts__facts__facts__blank='
'ansible_facts__a__b__c__ space =ggg',
])
def test_invalid_filter_strings(self, filter_string):
with pytest.raises(RuntimeError) as e:
Expand All @@ -37,6 +38,7 @@ def test_invalid_filter_strings(self, filter_string):

@pytest.mark.parametrize("filter_string,q_expected", [
(u'(a=abc\u1F5E3def)', Q(**{u"a": u"abc\u1F5E3def"})),
(u'(ansible_facts__a=abc\u1F5E3def)', Q(**{u"ansible_facts__contains": {u"a": u"abc\u1F5E3def"}})),
])
def test_unicode(self, filter_string, q_expected):
q = DynamicFilterField.filter_string_to_q(filter_string)
Expand All @@ -57,18 +59,18 @@ def test_boolean_parenthesis(self, filter_string, q_expected):
assert unicode(q) == unicode(q_expected)

@pytest.mark.parametrize("filter_string,q_expected", [
('a__b__c[]=3', Q(**{u"a__b__c__contains": [3]})),
('a__b__c[]=3.14', Q(**{u"a__b__c__contains": [3.14]})),
('a__b__c[]=true', Q(**{u"a__b__c__contains": [True]})),
('a__b__c[]=false', Q(**{u"a__b__c__contains": [False]})),
('a__b__c[]="true"', Q(**{u"a__b__c__contains": [u"true"]})),
('a__b__c[]="hello world"', Q(**{u"a__b__c__contains": [u"hello world"]})),
('a__b__c[]__d[]="foobar"', Q(**{u"a__b__c__contains": [{u"d": [u"foobar"]}]})),
('a__b__c[]__d="foobar"', Q(**{u"a__b__c__contains": [{u"d": u"foobar"}]})),
('a__b__c[]__d__e="foobar"', Q(**{u"a__b__c__contains": [{u"d": {u"e": u"foobar"}}]})),
('a__b__c[]__d__e[]="foobar"', Q(**{u"a__b__c__contains": [{u"d": {u"e": [u"foobar"]}}]})),
('a__b__c[]__d__e__f[]="foobar"', Q(**{u"a__b__c__contains": [{u"d": {u"e": {u"f": [u"foobar"]}}}]})),
('(a__b__c[]__d__e__f[]="foobar") and (a__b__c[]__d__e[]="foobar")', Q(**{ u"a__b__c__contains": [{u"d": {u"e": {u"f": [u"foobar"]}}}]}) & Q(**{u"a__b__c__contains": [{u"d": {u"e": [u"foobar"]}}]})),
('ansible_facts__a__b__c[]=3', Q(**{u"ansible_facts__contains": {u"a": {u"b": {u"c": [3]}}}})),
('ansible_facts__a__b__c[]=3.14', Q(**{u"ansible_facts__contains": {u"a": {u"b": {u"c": [3.14]}}}})),
('ansible_facts__a__b__c[]=true', Q(**{u"ansible_facts__contains": {u"a": {u"b": {u"c": [True]}}}})),
('ansible_facts__a__b__c[]=false', Q(**{u"ansible_facts__contains": {u"a": {u"b": {u"c": [False]}}}})),
('ansible_facts__a__b__c[]="true"', Q(**{u"ansible_facts__contains": {u"a": {u"b": {u"c": [u"true"]}}}})),
('ansible_facts__a__b__c[]="hello world"', Q(**{u"ansible_facts__contains": {u"a": {u"b": {u"c": [u"hello world"]}}}})),
('ansible_facts__a__b__c[]__d[]="foobar"', Q(**{u"ansible_facts__contains": {u"a": {u"b": {u"c": [{u"d": [u"foobar"]}]}}}})),
('ansible_facts__a__b__c[]__d="foobar"', Q(**{u"ansible_facts__contains": {u"a": {u"b": {u"c": [{u"d": u"foobar"}]}}}})),
('ansible_facts__a__b__c[]__d__e="foobar"', Q(**{u"ansible_facts__contains": {u"a": {u"b": {u"c": [{u"d": {u"e": u"foobar"}}]}}}})),
('ansible_facts__a__b__c[]__d__e[]="foobar"', Q(**{u"ansible_facts__contains": {u"a": {u"b": {u"c": [{u"d": {u"e": [u"foobar"]}}]}}}})),
('ansible_facts__a__b__c[]__d__e__f[]="foobar"', Q(**{u"ansible_facts__contains": {u"a": {u"b": {u"c": [{u"d": {u"e": {u"f": [u"foobar"]}}}]}}}})),
('(ansible_facts__a__b__c[]__d__e__f[]="foobar") and (ansible_facts__a__b__c[]__d__e[]="foobar")', Q(**{ u"ansible_facts__contains": {u"a": {u"b": {u"c": [{u"d": {u"e": {u"f": [u"foobar"]}}}]}}}}) & Q(**{u"ansible_facts__contains": {u"a": {u"b": {u"c": [{u"d": {u"e": [u"foobar"]}}]}}}})),
#('"a__b\"__c"="true"', Q(**{u"a__b\"__c": "true"})),
#('a__b\"__c="true"', Q(**{u"a__b\"__c": "true"})),
])
Expand All @@ -78,7 +80,7 @@ def test_contains_query_generated(self, filter_string, q_expected):

@pytest.mark.parametrize("filter_string,q_expected", [
#('a__b__c[]="true"', Q(**{u"a__b__c__contains": u"\"true\""})),
('a__b__c="true"', Q(**{u"a__b__c": u"true"})),
('ansible_facts__a="true"', Q(**{u"ansible_facts__contains": {u"a": u"true"}})),
#('"a__b\"__c"="true"', Q(**{u"a__b\"__c": "true"})),
#('a__b\"__c="true"', Q(**{u"a__b\"__c": "true"})),
])
Expand All @@ -87,8 +89,8 @@ def test_contains_query_generated_unicode(self, filter_string, q_expected):
assert unicode(q) == unicode(q_expected)

@pytest.mark.parametrize("filter_string,q_expected", [
('a__b__c=null', Q(**{u"a__b__c": u"null"})),
('a__b__c="null"', Q(**{u"a__b__c": u"\"null\""})),
('ansible_facts__a=null', Q(**{u"ansible_facts__contains": {u"a": u"null"}})),
('ansible_facts__c="null"', Q(**{u"ansible_facts__contains": {u"c": u"\"null\""}})),
])
def test_contains_query_generated_null(self, filter_string, q_expected):
q = DynamicFilterField.filter_string_to_q(filter_string)
Expand Down

0 comments on commit 1f99a0d

Please sign in to comment.