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

@@ -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:
+                if next_token in values_map:
-                    next_token = int(next_token)
+                    next_token = values_map[next_token]
-                except ValueError:
+
-                    try:
-                        next_token = float(next_token)
-                    except ValueError:
-                        pass
                 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:
+            # Convert tokens back to real types
-                token = int(token)
+            if token in values_map:
-            except ValueError:
+                token = values_map[token]
-                try:
+
-                    token = float(token)
+            # Need to join >= <= <>
-                except ValueError:
+            if len(tokens2) > 0 and ((tokens2[-1] == '>' and token == '=') or (tokens2[-1] == '<' and token == '=') or (tokens2[-1] == '<' and token == '>')):
-                    pass
+                tokens2.append(tokens2.pop() + token)
-            tokens2.append(token)
+            else:
+                tokens2.append(token)
 
     # Start of the Shunting-Yard algorithm. <-- Proper beast algorithm!
     def is_number(val):

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