process compound operators in the correct order

parser/parser.cpp

@@ -106,38 +106,91 @@ struct pred : seq< and_pred, star< or_ext > > {};
 // state
 struct ParserState
 {
-    std::vector<Predicate *> predicate_stack;
+    std::vector<Predicate *> group_stack;
-    Predicate &current()
+
+    Predicate *current_group()
     {
-        return *predicate_stack.back();
+        return group_stack.back();
     }
     
-    bool current_is_compound()
+    Predicate *last_predicate()
     {
-        return current().type == Predicate::Type::And || current().type == Predicate::Type::Or;
+        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 addExpression(Expression && exp)
+    void add_expression(Expression && exp)
     {
-        Predicate &cur = current();
+        Predicate *current = last_predicate();
-        if (cur.type == Predicate::Type::Comparison) {
+        if (current->type == Predicate::Type::Comparison && current->cmpr.expr[1].type == parser::Expression::Type::None) {
-            cur.cmpr.expr[1] = std::move(exp);
+            current->cmpr.expr[1] = std::move(exp);
-            predicate_stack.pop_back();
         }
         else {
-            assert(current_is_compound());
+            add_predicate_to_current_group(Predicate::Type::Comparison);
-
+            last_predicate()->cmpr.expr[0] = std::move(exp);
-            Predicate p(Predicate::Type::Comparison);
+        }
-            p.cmpr.expr[0] = std::move(exp);
-            if (negate_next) {
-                p.negate = true;
-                negate_next = false;
     }
     
-            cur.cpnd.sub_predicates.emplace_back(std::move(p));
+    void apply_or()
-            predicate_stack.push_back(&cur.cpnd.sub_predicates.back());
+    {
+        Predicate &group = *group_stack.back();
+        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) {
+            // 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.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));
         }
     }
 };
@@ -152,68 +205,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>");
-        assert(state.current_is_compound());
+        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>");
-        assert(state.current_is_compound());
+        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;
-        assert(sub_preds.size() > 1);
-        if (sub_preds.size() == 2) {
-            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);
-        std::vector<Predicate>::iterator second_last = sub_preds.end() - 2;
-        pred1.cpnd.sub_predicates.insert(pred1.cpnd.sub_predicates.begin(), sub_preds.begin(), second_last);
-        pred2.cpnd.sub_predicates.insert(pred2.cpnd.sub_predicates.begin(), second_last, sub_preds.end());
-
-        current.type = Predicate::Type::Or;
-        sub_preds.clear();
-        sub_preds.emplace_back(std::move(pred1));
-        sub_preds.emplace_back(std::move(pred2));
     }
 };
 
@@ -222,7 +228,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)
@@ -238,7 +244,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);
     }
 };
 
@@ -247,7 +253,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);
     }
 };
 
@@ -255,7 +261,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)
@@ -272,15 +278,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());
     }
 };
 
@@ -289,7 +288,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();
     }
 };
 
@@ -322,10 +321,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) {

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 { Number, String, KeyPath, Argument, True, False, None } 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);

parser/queryTests.json

@@ -267,7 +267,8 @@
         ["ObjectSet", [0],      "IntObject", "intCol == 0 && NOT intCol != 0"],
         ["ObjectSet", [1],      "IntObject", "(intCol == 0 || intCol == 1) && intCol >= 1"],
         ["ObjectSet", [0, 1],   "IntObject", "intCol == 0 || (intCol == 1 && intCol >= 1)"],
-        ["ObjectSet", [0, 1],   "IntObject", "intCol == 0 || intCol == 1 && intCol >= 1"]
+        ["ObjectSet", [0, 1],   "IntObject", "intCol == 0 || intCol == 1 && intCol >= 1"],
+        ["ObjectSet", [0, 1],   "IntObject", "intCol == 1 && intCol >= 1 || intCol == 0"]
     ]
 }