Skip to content

Commit

Permalink
[#1725] Custom filters should be added inside parenthesis
Browse files Browse the repository at this point in the history
The extensions can get a single filter (e.g. "age_not_between") and create two
WHERE clauses (e.g. "age < %s OR age > %s"). To avoid hard-to-track bugs
related to the operator precedence in Postgres, we add every custom filter
inside parenthesis.
  • Loading branch information
vitorbaptista committed May 22, 2014
1 parent 08e8ee1 commit f70476c
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 4 deletions.
2 changes: 1 addition & 1 deletion ckanext/datastore/db.py
Expand Up @@ -762,7 +762,7 @@ def _where(field_ids, data_dict):
for plugin in p.PluginImplementations(interfaces.IDataStore):
filters, clauses = plugin.where(filters)
for clause in clauses:
where_clauses.append(clause[0])
where_clauses.append('(' + clause[0] + ')')
values += clause[1:]

for field, value in filters.iteritems():
Expand Down
37 changes: 34 additions & 3 deletions ckanext/datastore/tests/test_interface.py
Expand Up @@ -21,6 +21,13 @@ def where(self, filters):
clause = ('"age" >= %s AND "age" <= %s',
age_between[0], age_between[1])
clauses.append(clause)
if 'age_not_between' in filters:
age_not_between = filters['age_not_between']
del filters['age_not_between']

clause = ('"age" < %s OR "age" > %s',
age_not_between[0], age_not_between[1])
clauses.append(clause)
return filters, clauses


Expand All @@ -43,17 +50,41 @@ def teardown(self):

def test_can_create_custom_filters(self):
records = [
{'age': 20}, {'age': 27}, {'age': 33}
{'age': 20}, {'age': 30}, {'age': 40}
]
resource = self._create_datastore_resource(records)
filters = {'age_between': [25, 35]}

result = helpers.call_action('datastore_search',
resource_id=resource['id'],
filters=filters)

assert result['total'] == 1, result
assert result['records'][0]['age'] == 30, result
assert_equals(result['filters'], filters)

def test_custom_filters_have_the_correct_operator_precedence(self):
'''
We're testing that the WHERE clause becomes:
(age < 50 OR age > 60) AND age = 30
And not:
age < 50 OR age > 60 AND age = 30
'''
records = [
{'age': 20}, {'age': 30}, {'age': 40}
]
resource = self._create_datastore_resource(records)
filters = {'age_between': [25, 30]}
filters = {
'age_not_between': [50, 60],
'age': 30
}

result = helpers.call_action('datastore_search',
resource_id=resource['id'],
filters=filters)

assert result['total'] == 1, result
assert result['records'][0]['age'] == 27, result
assert result['records'][0]['age'] == 30, result
assert_equals(result['filters'], filters)

def _create_datastore_resource(self, records):
Expand Down

0 comments on commit f70476c

Please sign in to comment.