From c3066ebf449cad294762e928b19415aa7a67a79b Mon Sep 17 00:00:00 2001
From: Tim Hinrichs <thinrichs@vmware.com>
Date: Tue, 22 Oct 2013 11:32:24 -0700
Subject: [PATCH] Fixed bug where ordering of rules and data mattered

When rule inserted after data and that rule had self-join,
failed to get right result.

This fix first computes self-joins and then ensures to evaluate
all the resulting rules before computing delta rules and inserting them.

Added test to catch this case in the future.

All tests pass

Issue: #
Change-Id: Ie09dbc82c42069e0f4925d2919ba40e88b2d1a92
---
 examples/private_public_network.script |  2 +-
 src/policy/runtime.py                  | 37 ++++++++++++++++++--------
 src/policy/tests/test_runtime.py       |  9 +++++++
 3 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/examples/private_public_network.script b/examples/private_public_network.script
index 992a617e0..2485fcb2b 100644
--- a/examples/private_public_network.script
+++ b/examples/private_public_network.script
@@ -29,7 +29,7 @@ same-group(user1, user2) :- cms:group(user1, group), cms:group(user2, group)
 --- Commands ------------------------------------
 cd congress/src/policy
 
-python
+PYTHONPATH=../../thirdparty python
 >>> import runtime
 >>> r = runtime.Runtime()
 >>> r.load_file("../../examples/private_public_network.classify")
diff --git a/src/policy/runtime.py b/src/policy/runtime.py
index bf397f1ba..719d8b151 100644
--- a/src/policy/runtime.py
+++ b/src/policy/runtime.py
@@ -851,7 +851,7 @@ class DeltaRuleTheory (object):
         self.views = {}
 
     def insert(self, rule):
-        """ Insert a compile.Rule into the theory.
+        """ Insert a self-join free compile.Rule into the theory.
             Return True iff the theory changed. """
         assert isinstance(rule, compile.Rule), \
             "DeltaRuleTheory only takes rules"
@@ -928,8 +928,10 @@ class DeltaRuleTheory (object):
         # dict from (table name, arity) to # of args for
         arities = {}
         # remove self-joins from rules
+        results = []
         for rule in theory:
             if rule.is_atom():
+                results.append(rule)
                 continue
             logging.debug("eliminating self joins from {}".format(rule))
             occurrences = {}  # for just this rule
@@ -951,6 +953,7 @@ class DeltaRuleTheory (object):
                         global_self_joins[tablearity] = \
                             max(occurrences[tablearity] - 1,
                                 global_self_joins[tablearity])
+            results.append(rule)
             logging.debug("final rule: {}".format(str(rule)))
         # add definitions for new tables
         for tablearity in global_self_joins:
@@ -961,12 +964,14 @@ class DeltaRuleTheory (object):
                 args = [compile.Variable(var) for var in n_variables(arity)]
                 head = compile.Atom(newtable, args)
                 body = [compile.Atom(table, args)]
-                theory.append(compile.Rule(head, body))
-                logging.debug("Adding rule {}".format(str(theory[-1])))
-        return theory
+                results.append(compile.Rule(head, body))
+                logging.debug("Adding rule {}".format(results[-1]))
+        return results
 
     @classmethod
     def compute_delta_rules(cls, theory):
+        """ Assuming THEORY has no self-joins, return a list of DeltaRules
+        derived from that THEORY. """
         theory = cls.eliminate_self_joins(theory)
         delta_rules = []
         for rule in theory:
@@ -1068,13 +1073,23 @@ class MaterializedRuleTheory(TopDownTheory):
             self.log(formula.table, "{}: {}".format(text, str(formula)))
             return self.modify_tables_with_atom(formula, is_insert=is_insert)
         else:
-            self.modify_tables_with_rule(
-                                formula, is_insert=is_insert)
-            self.log(formula.head.table, "{}: {}".format(text, str(formula)))
-            if is_insert:
-                return self.delta_rules.insert(formula)
-            else:
-                return self.delta_rules.delete(formula)
+            # need to eliminate self-joins here so that we
+            #   call modify_tables_with_rule on all the actual rules.
+            # Yes, the call to eliminate_self_joins is duplicated within
+            #   delta_rules.insert, but it is nice and safe this way.
+            real_rules = DeltaRuleTheory.eliminate_self_joins([formula])
+            changed = False
+            for rule in real_rules:
+                self.modify_tables_with_rule(
+                                    rule, is_insert=is_insert)
+                self.log(formula.head.table, "{}: {}".format(text, str(rule)))
+                if is_insert:
+                    if self.delta_rules.insert(rule):
+                        changed = True
+                else:
+                    if self.delta_rules.delete(rule):
+                        changed = True
+            return changed
 
     def modify_tables_with_rule(self, rule, is_insert):
         """ Add rule (not a DeltaRule) to collection and update
diff --git a/src/policy/tests/test_runtime.py b/src/policy/tests/test_runtime.py
index 7928b4da3..adf334b67 100644
--- a/src/policy/tests/test_runtime.py
+++ b/src/policy/tests/test_runtime.py
@@ -285,6 +285,15 @@ class TestRuntime(unittest.TestCase):
                 'q(2,6) q(2,7)')
         self.check(run, code, "Delete: larger self join")
 
+        # actual bug: insert data first, then
+        #   insert rule with self-join
+        code = ('s(1)'
+                'q(1,1)'
+                'p(x,y) :- q(x,y), not r(x,y)'
+                'r(x,y) :- s(x), s(y)')
+        run = self.prep_runtime(code)
+        self.check(run, 's(1) q(1,1) r(1,1)')
+
     def test_materialized_value_types(self):
         """ Test the different value types """
         # string