From 2bb3e841d12c5f614971492d241629794f35fe85 Mon Sep 17 00:00:00 2001 From: Terry Cain Date: Mon, 16 Oct 2017 21:56:03 +0100 Subject: [PATCH] Fixed #1261 dynamodb FilterExpression bugs (#1262) * Fixed #1261 dynamodb FilterExpression bugs FilterExpression was incorrectly handling numbers, stupid typo there. Also >= <= and <> was not being parsed correctly. * Switched up logic a bit for better end result. Fixes #1263 * Fixed another bug --- moto/dynamodb2/comparisons.py | 53 +++++++++++++-------- tests/test_dynamodb2/test_dynamodb.py | 68 ++++++++++++++++++++++++--- 2 files changed, 95 insertions(+), 26 deletions(-) diff --git a/moto/dynamodb2/comparisons.py b/moto/dynamodb2/comparisons.py index 8462c2de..faeffbaa 100644 --- a/moto/dynamodb2/comparisons.py +++ b/moto/dynamodb2/comparisons.py @@ -61,15 +61,27 @@ def get_filter_expression(expr, names, values): # Do substitutions for key, value in names.items(): expr = expr.replace(key, value) + + # Store correct types of values for use later + values_map = {} for key, value in values.items(): if 'N' in value: - expr.replace(key, float(value['N'])) + values_map[key] = float(value['N']) + elif 'BOOL' in value: + values_map[key] = value['BOOL'] + elif 'S' in value: + values_map[key] = value['S'] + elif 'NS' in value: + values_map[key] = tuple(value['NS']) + elif 'SS' in value: + values_map[key] = tuple(value['SS']) + elif 'L' in value: + values_map[key] = tuple(value['L']) else: - expr = expr.replace(key, value['S']) + raise NotImplementedError() # Remove all spaces, tbf we could just skip them in the next step. # The number of known options is really small so we can do a fair bit of cheating - #expr = list(re.sub('\s', '', expr)) # 'Id>5ANDattribute_exists(test)ORNOTlength<6' expr = list(expr) # DodgyTokenisation stage 1 @@ -130,13 +142,9 @@ def get_filter_expression(expr, names, values): next_token = six.next(token_iterator) while next_token != ')': - try: - next_token = int(next_token) - except ValueError: - try: - next_token = float(next_token) - except ValueError: - pass + if next_token in values_map: + next_token = values_map[next_token] + tuple_list.append(next_token) next_token = six.next(token_iterator) @@ -149,10 +157,14 @@ def get_filter_expression(expr, names, values): tokens2.append(tuple(tuple_list)) elif token == 'BETWEEN': field = tokens2.pop() - op1 = int(six.next(token_iterator)) + # if values map contains a number, it would be a float + # so we need to int() it anyway + op1 = six.next(token_iterator) + op1 = int(values_map.get(op1, op1)) and_op = six.next(token_iterator) assert and_op == 'AND' - op2 = int(six.next(token_iterator)) + op2 = six.next(token_iterator) + op2 = int(values_map.get(op2, op2)) tokens2.append(['between', field, op1, op2]) elif is_function(token): @@ -169,14 +181,15 @@ def get_filter_expression(expr, names, values): tokens2.append(function_list) else: - try: - token = int(token) - except ValueError: - try: - token = float(token) - except ValueError: - pass - tokens2.append(token) + # Convert tokens back to real types + if token in values_map: + token = values_map[token] + + # Need to join >= <= <> + if len(tokens2) > 0 and ((tokens2[-1] == '>' and token == '=') or (tokens2[-1] == '<' and token == '=') or (tokens2[-1] == '<' and token == '>')): + tokens2.append(tokens2.pop() + token) + else: + tokens2.append(token) # Start of the Shunting-Yard algorithm. <-- Proper beast algorithm! def is_number(val): diff --git a/tests/test_dynamodb2/test_dynamodb.py b/tests/test_dynamodb2/test_dynamodb.py index 85d8feb3..26d38062 100644 --- a/tests/test_dynamodb2/test_dynamodb.py +++ b/tests/test_dynamodb2/test_dynamodb.py @@ -581,24 +581,24 @@ def test_filter_expression(): row2 = moto.dynamodb2.models.Item(None, None, None, None, {'Id': {'N': '8'}, 'Subs': {'N': '10'}, 'Desc': {'S': 'A description'}, 'KV': {'SS': ['test3', 'test4']}}) # AND test - filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id > 5 AND Subs < 7', {}, {}) + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id > :v0 AND Subs < :v1', {}, {':v0': {'N': 5}, ':v1': {'N': 7}}) filter_expr.expr(row1).should.be(True) filter_expr.expr(row2).should.be(False) # OR test - filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id = 5 OR Id=8', {}, {}) + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id = :v0 OR Id=:v1', {}, {':v0': {'N': 5}, ':v1': {'N': 8}}) filter_expr.expr(row1).should.be(True) # BETWEEN test - filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id BETWEEN 5 AND 10', {}, {}) + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id BETWEEN :v0 AND :v1', {}, {':v0': {'N': 5}, ':v1': {'N': 10}}) filter_expr.expr(row1).should.be(True) # PAREN test - filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id = 8 AND (Subs = 8 OR Subs = 5)', {}, {}) + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id = :v0 AND (Subs = :v0 OR Subs = :v1)', {}, {':v0': {'N': 8}, ':v1': {'N': 5}}) filter_expr.expr(row1).should.be(True) # IN test - filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id IN (7,8, 9)', {}, {}) + filter_expr = moto.dynamodb2.comparisons.get_filter_expression('Id IN :v0', {}, {':v0': {'NS': [7, 8, 9]}}) filter_expr.expr(row1).should.be(True) # attribute function tests @@ -655,6 +655,63 @@ def test_scan_filter(): assert response['Count'] == 1 +@mock_dynamodb2 +def test_scan_filter2(): + client = boto3.client('dynamodb', region_name='us-east-1') + + # Create the DynamoDB table. + client.create_table( + TableName='test1', + AttributeDefinitions=[{'AttributeName': 'client', 'AttributeType': 'S'}, {'AttributeName': 'app', 'AttributeType': 'N'}], + KeySchema=[{'AttributeName': 'client', 'KeyType': 'HASH'}, {'AttributeName': 'app', 'KeyType': 'RANGE'}], + ProvisionedThroughput={'ReadCapacityUnits': 123, 'WriteCapacityUnits': 123} + ) + client.put_item( + TableName='test1', + Item={ + 'client': {'S': 'client1'}, + 'app': {'N': '1'} + } + ) + + response = client.scan( + TableName='test1', + Select='ALL_ATTRIBUTES', + FilterExpression='#tb >= :dt', + ExpressionAttributeNames={"#tb": "app"}, + ExpressionAttributeValues={":dt": {"N": str(1)}} + ) + assert response['Count'] == 1 + + +@mock_dynamodb2 +def test_scan_filter3(): + client = boto3.client('dynamodb', region_name='us-east-1') + dynamodb = boto3.resource('dynamodb', region_name='us-east-1') + + # Create the DynamoDB table. + client.create_table( + TableName='test1', + AttributeDefinitions=[{'AttributeName': 'client', 'AttributeType': 'S'}, {'AttributeName': 'app', 'AttributeType': 'N'}], + KeySchema=[{'AttributeName': 'client', 'KeyType': 'HASH'}, {'AttributeName': 'app', 'KeyType': 'RANGE'}], + ProvisionedThroughput={'ReadCapacityUnits': 123, 'WriteCapacityUnits': 123} + ) + client.put_item( + TableName='test1', + Item={ + 'client': {'S': 'client1'}, + 'app': {'N': '1'}, + 'active': {'BOOL': True} + } + ) + + table = dynamodb.Table('test1') + response = table.scan( + FilterExpression=Attr('active').eq(True) + ) + assert response['Count'] == 1 + + @mock_dynamodb2 def test_bad_scan_filter(): client = boto3.client('dynamodb', region_name='us-east-1') @@ -680,7 +737,6 @@ def test_bad_scan_filter(): raise RuntimeError('Should of raised ResourceInUseException') - @mock_dynamodb2 def test_duplicate_create(): client = boto3.client('dynamodb', region_name='us-east-1')