Merge pull request #164 from realm/al-parser-tests

compound parser tests

src/object-store/parser/parser.cpp

@@ -106,28 +106,90 @@ struct pred : seq< and_pred, star< or_ext > > {};
 // state
 struct ParserState
 {
-    std::vector<Predicate *> predicate_stack;
+    std::vector<Predicate *> group_stack;
-    Predicate &current() {
-        return *predicate_stack.back();
-    }
 
-    bool negate_next = false;
+    Predicate *current_group()
-
-    void addExpression(Expression && exp)
     {
-        if (current().type == Predicate::Type::Comparison) {
+        return group_stack.back();
-            current().cmpr.expr[1] = std::move(exp);
+    }
-            predicate_stack.pop_back();
+    
+    Predicate *last_predicate()
+    {
+        Predicate *pred = current_group();
+        while (pred->type != Predicate::Type::Comparison && pred->cpnd.sub_predicates.size()) {
+            pred = &pred->cpnd.sub_predicates.back();
+        }
+        return pred;
+    }
+    
+    void add_predicate_to_current_group(Predicate::Type type)
+    {
+        current_group()->cpnd.sub_predicates.emplace_back(type, negate_next);
+        negate_next = false;
+        
+        if (current_group()->cpnd.sub_predicates.size() > 1) {
+            if (next_type == Predicate::Type::Or) {
+                apply_or();
+            }
+            else {
+                apply_and();
+            }
+        }
+    }
+    
+    bool negate_next = false;
+    Predicate::Type next_type = Predicate::Type::And;
+    
+    void add_expression(Expression && exp)
+    {
+        Predicate *current = last_predicate();
+        if (current->type == Predicate::Type::Comparison && current->cmpr.expr[1].type == parser::Expression::Type::None) {
+            current->cmpr.expr[1] = std::move(exp);
         }
         else {
-            Predicate p(Predicate::Type::Comparison);
+            add_predicate_to_current_group(Predicate::Type::Comparison);
-            p.cmpr.expr[0] = std::move(exp);
+            last_predicate()->cmpr.expr[0] = std::move(exp);
-            if (negate_next) {
+        }
-                p.negate = true;
+    }
-                negate_next = false;
+    
-            }
+    void apply_or()
-            current().cpnd.sub_predicates.emplace_back(std::move(p));
+    {
-            predicate_stack.push_back(&current().cpnd.sub_predicates.back());
+        Predicate *group = current_group();
+        if (group->type == Predicate::Type::Or) {
+            return;
+        }
+        
+        // convert to OR
+        group->type = Predicate::Type::Or;
+        if (group->cpnd.sub_predicates.size() > 2) {
+            // split the current group into an AND group ORed with the last subpredicate
+            Predicate new_sub(Predicate::Type::And);
+            new_sub.cpnd.sub_predicates = std::move(group->cpnd.sub_predicates);
+            
+            group->cpnd.sub_predicates = { new_sub, std::move(new_sub.cpnd.sub_predicates.back()) };
+            group->cpnd.sub_predicates[0].cpnd.sub_predicates.pop_back();
+        }
+    }
+    
+    void apply_and()
+    {
+        if (current_group()->type == Predicate::Type::And) {
+            return;
+        }
+        
+        auto &sub_preds = current_group()->cpnd.sub_predicates;
+        auto second_last = sub_preds.end() - 2;
+        if (second_last->type == Predicate::Type::And && !second_last->negate) {
+            // make a new and group populated with the last two predicates
+            second_last->cpnd.sub_predicates.push_back(std::move(sub_preds.back()));
+            sub_preds.pop_back();
+        }
+        else {
+            // otherwise combine last two into a new AND group
+            Predicate pred(Predicate::Type::And);
+            pred.cpnd.sub_predicates.insert(pred.cpnd.sub_predicates.begin(), second_last, sub_preds.end());
+            sub_preds.erase(second_last, sub_preds.end());
+            sub_preds.emplace_back(std::move(pred));
         }
     }
 };
@@ -142,64 +204,21 @@ struct action : nothing< Rule > {};
     #define DEBUG_PRINT_TOKEN(string)
 #endif
 
-template<> struct action< and_ext >
+template<> struct action< and_op >
 {
     static void apply( const input & in, ParserState & state )
     {
         DEBUG_PRINT_TOKEN("<and>");
-
+        state.next_type = Predicate::Type::And;
-        // if we were put into an OR group we need to rearrange
-        auto &current = state.current();
-        if (current.type == Predicate::Type::Or) {
-            auto &sub_preds = current.cpnd.sub_predicates;
-            auto &second_last = sub_preds[sub_preds.size()-2];
-            if (second_last.type == Predicate::Type::And) {
-                // if we are in an OR group and second to last predicate group is
-                // an AND group then move the last predicate inside
-                second_last.cpnd.sub_predicates.push_back(std::move(sub_preds.back()));
-                sub_preds.pop_back();
-            }
-            else {
-                // otherwise combine last two into a new AND group
-                Predicate pred(Predicate::Type::And);
-                pred.cpnd.sub_predicates.emplace_back(std::move(second_last));
-                pred.cpnd.sub_predicates.emplace_back(std::move(sub_preds.back()));
-                sub_preds.pop_back();
-                sub_preds.pop_back();
-                sub_preds.push_back(std::move(pred));
-            }
-        }
     }
 };
 
-template<> struct action< or_ext >
+template<> struct action< or_op >
 {
     static void apply( const input & in, ParserState & state )
     {
         DEBUG_PRINT_TOKEN("<or>");
-
+        state.next_type = Predicate::Type::Or;
-        // if already an OR group do nothing
-        auto &current = state.current();
-        if (current.type == Predicate::Type::Or) {
-            return;
-        }
-
-        // if only two predicates in the group, then convert to OR
-        auto &sub_preds = state.current().cpnd.sub_predicates;
-        if (sub_preds.size()) {
-            current.type = Predicate::Type::Or;
-            return;
-        }
-
-        // split the current group into to groups which are ORed together
-        Predicate pred1(Predicate::Type::And), pred2(Predicate::Type::And);
-        pred1.cpnd.sub_predicates.insert(sub_preds.begin(), sub_preds.back());
-        pred2.cpnd.sub_predicates.push_back(std::move(sub_preds.back()));
-
-        current.type = Predicate::Type::Or;
-        sub_preds.clear();
-        sub_preds.emplace_back(std::move(pred1));
-        sub_preds.emplace_back(std::move(pred2));
     }
 };
 
@@ -208,7 +227,7 @@ template<> struct action< or_ext >
 template<> struct action< rule > {                                  \
     static void apply( const input & in, ParserState & state ) {    \
         DEBUG_PRINT_TOKEN(in.string());                             \
-        state.addExpression(Expression(type, in.string())); }};
+        state.add_expression(Expression(type, in.string())); }};
 
 EXPRESSION_ACTION(dq_string_content, Expression::Type::String)
 EXPRESSION_ACTION(sq_string_content, Expression::Type::String)
@@ -224,7 +243,7 @@ template<> struct action< true_pred >
     static void apply( const input & in, ParserState & state )
     {
         DEBUG_PRINT_TOKEN(in.string());
-        state.current().cpnd.sub_predicates.emplace_back(Predicate::Type::True);
+        state.current_group()->cpnd.sub_predicates.emplace_back(Predicate::Type::True);
     }
 };
 
@@ -233,7 +252,7 @@ template<> struct action< false_pred >
     static void apply( const input & in, ParserState & state )
     {
         DEBUG_PRINT_TOKEN(in.string());
-        state.current().cpnd.sub_predicates.emplace_back(Predicate::Type::False);
+        state.current_group()->cpnd.sub_predicates.emplace_back(Predicate::Type::False);
     }
 };
 
@@ -241,7 +260,7 @@ template<> struct action< false_pred >
 template<> struct action< rule > {                                  \
     static void apply( const input & in, ParserState & state ) {    \
         DEBUG_PRINT_TOKEN(in.string());                             \
-        state.current().cmpr.op = oper; }};
+        state.last_predicate()->cmpr.op = oper; }};
 
 OPERATOR_ACTION(eq, Predicate::Operator::Equal)
 OPERATOR_ACTION(noteq, Predicate::Operator::NotEqual)
@@ -258,15 +277,8 @@ template<> struct action< one< '(' > >
     static void apply( const input & in, ParserState & state )
     {
         DEBUG_PRINT_TOKEN("<begin_group>");
-
+        state.add_predicate_to_current_group(Predicate::Type::And);
-        Predicate group(Predicate::Type::And);
+        state.group_stack.push_back(state.last_predicate());
-        if (state.negate_next) {
-            group.negate = true;
-            state.negate_next = false;
-        }
-
-        state.current().cpnd.sub_predicates.emplace_back(std::move(group));
-        state.predicate_stack.push_back(&state.current().cpnd.sub_predicates.back());
     }
 };
 
@@ -275,7 +287,7 @@ template<> struct action< group_pred >
     static void apply( const input & in, ParserState & state )
     {
         DEBUG_PRINT_TOKEN("<end_group>");
-        state.predicate_stack.pop_back();
+        state.group_stack.pop_back();
     }
 };
 
@@ -308,10 +320,12 @@ const std::string error_message_control< Rule >::error_message = "Invalid predic
 
 Predicate parse(const std::string &query)
 {
+    DEBUG_PRINT_TOKEN(query);
+
     Predicate out_predicate(Predicate::Type::And);
 
     ParserState state;
-    state.predicate_stack.push_back(&out_predicate);
+    state.group_stack.push_back(&out_predicate);
 
     pegtl::parse< must< pred, eof >, action, error_message_control >(query, query, state);
     if (out_predicate.type == Predicate::Type::And && out_predicate.cpnd.sub_predicates.size() == 1) {

src/object-store/parser/parser.hpp

@@ -28,7 +28,7 @@ namespace realm {
     namespace parser {
         struct Expression
         {
-            enum class Type { Number, String, KeyPath, Argument, True, False } type;
+            enum class Type { None, Number, String, KeyPath, Argument, True, False } type = Type::None;
             std::string s;
             Expression() {}
             Expression(Type t, std::string s) : type(t), s(s) {}
@@ -76,7 +76,7 @@ namespace realm {
 
             bool negate = false;
 
-            Predicate(Type t) : type(t) {}
+            Predicate(Type t, bool n = false) : type(t), negate(n) {}
         };
 
         Predicate parse(const std::string &query);

src/object-store/parser/queryTests.json

@@ -245,6 +245,37 @@
         ["QueryThrows", "LinkObject", "linkCol > $0", [0, "linkCol"]],
         ["QueryThrows", "LinkObject", "intCol = $0", [0, "linkCol"]]
     ]
+},
+
+"compoundTests" : {
+    "schema" : [
+        { "name": "IntObject",  
+          "properties": [{ "name": "intCol",  "type": "int" }],
+          "primaryKey" : "intCol" }
+    ],
+    "objects": [
+        { "type": "IntObject", "value": [0] },
+        { "type": "IntObject", "value": [1] },
+        { "type": "IntObject", "value": [2] },    
+        { "type": "IntObject", "value": [3] }
+    ],
+    "tests": [
+        ["ObjectSet", [],       "IntObject", "intCol == 0 && intCol == 1"],
+        ["ObjectSet", [0, 1],   "IntObject", "intCol == 0 || intCol == 1"],
+        ["ObjectSet", [0],      "IntObject", "intCol == 0 && intCol != 1"],
+        ["ObjectSet", [2, 3],   "IntObject", "intCol >= 2 && intCol < 4"],
+        ["ObjectSet", [0],      "IntObject", "intCol == 0 && NOT intCol != 0"],
+        ["ObjectSet", [0, 3],   "IntObject", "intCol == 0 || NOT (intCol == 1 || intCol == 2)"],
+        ["ObjectSet", [1],      "IntObject", "(intCol == 0 || intCol == 1) && intCol >= 1"],
+        ["ObjectSet", [1],      "IntObject", "intCol >= 1 && (intCol == 0 || intCol == 1)"],
+        ["ObjectSet", [0, 1],   "IntObject", "intCol == 0 || (intCol == 1 && intCol >= 1)"],
+        ["ObjectSet", [0, 1],   "IntObject", "(intCol == 1 && intCol >= 1) || intCol == 0"],
+        ["ObjectSet", [0, 1],   "IntObject", "intCol == 0 || intCol == 1 && intCol >= 1"],
+        ["ObjectSet", [0, 1, 2],"IntObject", "intCol == 0 || intCol == 1 || intCol <= 2"],
+        ["ObjectSet", [0, 1],   "IntObject", "intCol == 1 && intCol >= 1 || intCol == 0"],
+        ["ObjectSet", [0, 1],   "IntObject", "intCol == 1 || intCol == 0 && intCol <= 0 && intCol >= 0"],
+        ["ObjectSet", [0, 1],   "IntObject", "intCol == 0 || NOT (intCol == 3 && intCol >= 0) && intCol == 1"]   
+    ]
 }
 
 }

tests/QueryTests.js

@@ -37,27 +37,44 @@ function runQuerySuite(suite) {
         }
     });
 
+    var args;
+    function getArgs(startArg) {
+        args = test.slice(startArg, startArg + 2);
+        for (var i = startArg + 2; i < test.length; i++) {
+            var arg = test[i];
+            if (Array.isArray(arg)) {
+                // aray arguments correspond to [objectAtIndex, propertyName]
+                args.push(objects[arg[0]][arg[1]]);
+            }
+            else {
+                args.push(arg);
+            }
+        }
+        return args;
+    }
+
     for (var test of suite.tests) {
         if (test[0] == "QueryCount") {
-            var args = test.slice(2, 4);
+            var length = realm.objects.apply(realm, getArgs(2)).length;
-            for (var i = 4; i < test.length; i++) {
+            TestCase.assertEqual(test[1], length, "Query '" + args[1] + "' on type '" + args[0] + "' expected " + test[1] + " results, got " + length);
-                var arg = test[i];
+        }
-                if (Array.isArray(arg)) {
+        else if (test[0] == "ObjectSet") {
-                    // aray arguments correspond to [objectAtIndex, propertyName]
+            var results = realm.objects.apply(realm, getArgs(2));           
-                    args.push(objects[arg[0]][arg[1]]);
+            TestCase.assertEqual(test[1].length, results.length, "Query '" + args[1] + "' on type '" + args[0] + "' expected " + test[1].length + " results, got " + results.length);
-                }
+
-                else {
+            var objSchema = suite.schema.find(function(el) { return el.name == args[0] });
-                    args.push(arg);
+            var primary = objSchema.primaryKey;
-                }
+            if (!primary) {
+                throw "Primary key required for object comparison";
             }
 
-            var results = realm.objects.apply(realm, args);
+            TestCase.assertArraysEqual(test[1], Array.prototype.map.call(results,  function(el) { 
-            TestCase.assertEqual(test[1], results.length, "Query '" + args[1] + "' on type '" + args[0] + "' expected " + test[1] + " results, got " + results.length);
+                return el[primary] 
+            }));
         }
         else if (test[0] == "QueryThrows") {
-            var args = test.slice(1);
             TestCase.assertThrows(function() {
-                realm.objects.apply(realm, args);
+                realm.objects.apply(realm, getArgs(1));
             }, "Expected exception not thrown for query: " + JSON.stringify(args));
         }
         else if (test[0] != "Disabled") {
@@ -91,6 +108,9 @@ module.exports = BaseTest.extend({
     },
     testObjectQueries: function() {
         runQuerySuite(testCases.objectTests);
+    },
+    testCompoundQueries: function() {
+        runQuerySuite(testCases.compoundTests);
     }
 });
 
@@ -922,30 +942,6 @@ module.exports = BaseTest.extend({
     XCTAssertThrows([CircleArrayObject objectsInRealm:realm where:@"NONE data.circles = '2'"]);
 }
 
-- (void)testCompoundOrQuery {
-    RLMRealm *realm = [RLMRealm defaultRealm];
-
-    [realm beginWriteTransaction];
-    [PersonObject createInRealm:realm withValue:@[@"Tim", @29]];
-    [PersonObject createInRealm:realm withValue:@[@"Ari", @33]];
-    [realm commitWriteTransaction];
-
-    XCTAssertEqual(2U, [[PersonObject objectsWhere:@"name == 'Ari' or age < 30"] count]);
-    XCTAssertEqual(1U, [[PersonObject objectsWhere:@"name == 'Ari' or age > 40"] count]);
-}
-
-- (void)testCompoundAndQuery {
-    RLMRealm *realm = [RLMRealm defaultRealm];
-
-    [realm beginWriteTransaction];
-    [PersonObject createInRealm:realm withValue:@[@"Tim", @29]];
-    [PersonObject createInRealm:realm withValue:@[@"Ari", @33]];
-    [realm commitWriteTransaction];
-
-    XCTAssertEqual(1U, [[PersonObject objectsWhere:@"name == 'Ari' and age > 30"] count]);
-    XCTAssertEqual(0U, [[PersonObject objectsWhere:@"name == 'Ari' and age > 40"] count]);
-}
-
 - (void)testClass:(Class)class
   withNormalCount:(NSUInteger)normalCount
          notCount:(NSUInteger)notCount