Order generators with respect to their dependencies (#2458)

fix #2416

src/ast2ram/seminaive/ClauseTranslator.cpp

@@ -774,9 +774,6 @@ void ClauseTranslator::indexAggregatorBody(const ast::Aggregator& agg) {
 }
 
 void ClauseTranslator::indexAggregators(const ast::Clause& clause) {
-    // Add each aggregator as an internal generator
-    visit(clause, [&](const ast::Aggregator& agg) { indexGenerator(agg); });
-
     // Index aggregator bodies
     visit(clause, [&](const ast::Aggregator& agg) { indexAggregatorBody(agg); });
 
@@ -791,13 +788,6 @@ void ClauseTranslator::indexAggregators(const ast::Clause& clause) {
 }
 
 void ClauseTranslator::indexMultiResultFunctors(const ast::Clause& clause) {
-    // Add each multi-result functor as an internal generator
-    visit(clause, [&](const ast::IntrinsicFunctor& func) {
-        if (ast::analysis::FunctorAnalysis::isMultiResult(func)) {
-            indexGenerator(func);
-        }
-    });
-
     // Add multi-result functor value introductions
     visit(clause, [&](const ast::BinaryConstraint& bc) {
         if (!isEqConstraint(bc.getBaseOperator())) return;
@@ -809,8 +799,94 @@ void ClauseTranslator::indexMultiResultFunctors(const ast::Clause& clause) {
     });
 }
 
+void ClauseTranslator::indexGenerators(const ast::Clause& clause) {
+    // generators must be indexed in topological order according to
+    // dependencies between them.
+    // see issue #2416
+
+    std::map<std::string, const ast::Argument*> varGenerator;
+
+    visit(clause, [&](const ast::BinaryConstraint& bc) {
+        if (!isEqConstraint(bc.getBaseOperator())) return;
+        const auto* lhs = as<ast::Variable>(bc.getLHS());
+        const ast::Argument* rhs = as<ast::IntrinsicFunctor>(bc.getRHS());
+        if (rhs == nullptr) {
+            rhs = as<ast::Aggregator>(bc.getRHS());
+        } else {
+            if (!ast::analysis::FunctorAnalysis::isMultiResult(*as<ast::IntrinsicFunctor>(rhs))) return;
+        }
+        if (lhs == nullptr || rhs == nullptr) return;
+        varGenerator[lhs->getName()] = rhs;
+    });
+
+    // all the generators in the clause
+    std::vector<const ast::Argument*> generators;
+
+    // 'predecessor' mapping from a generator to the generators that must
+    // evaluate before.
+    std::multimap<const ast::Argument*, const ast::Argument*> dependencies;
+
+    // harvest generators and dependencies
+    visit(clause, [&](const ast::Argument& arg) {
+        if (const ast::IntrinsicFunctor* func = as<ast::IntrinsicFunctor>(arg)) {
+            if (ast::analysis::FunctorAnalysis::isMultiResult(*func)) {
+                generators.emplace_back(func);
+                visit(func, [&](const ast::Variable& use) {
+                    if (varGenerator.count(use.getName()) > 0) {
+                        dependencies.emplace(func, varGenerator.at(use.getName()));
+                    }
+                });
+            }
+        } else if (const ast::Aggregator* agg = as<ast::Aggregator>(arg)) {
+            generators.emplace_back(agg);
+            visit(agg, [&](const ast::Variable& use) {
+                if (varGenerator.count(use.getName()) > 0) {
+                    dependencies.emplace(agg, varGenerator.at(use.getName()));
+                }
+            });
+        }
+    });
+
+    // the set of already indexed generators
+    std::set<const ast::Argument*> indexed;
+    // the recursion stack to detect a cycle in the depth-first traversal
+    std::set<const ast::Argument*> recStack;
+
+    // recursive depth-first traversal, perform a post-order indexing of genertors.
+    const std::function<void(const ast::Argument*)> dfs = [&](const ast::Argument* reached) {
+        if (indexed.count(reached)) {
+            return;
+        }
+
+        if (!recStack.emplace(reached).second) {
+            // cycle detected
+            fatal("cyclic dependency");
+        }
+
+        auto range = dependencies.equal_range(reached);
+        for (auto it = range.first; it != range.second; ++it) {
+            if (it->second == reached) {
+                continue;  // ignore self-dependency
+            }
+            dfs(it->second);
+        }
+
+        // index this generator
+        indexGenerator(*reached);
+
+        indexed.insert(reached);
+        recStack.erase(reached);
+    };
+
+    // topological sorting by depth-first search
+    for (const ast::Argument* root : generators) {
+        dfs(root);
+    }
+}
+
 void ClauseTranslator::indexClause(const ast::Clause& clause) {
     indexAtoms(clause);
+    indexGenerators(clause);
     indexAggregators(clause);
     indexMultiResultFunctors(clause);
 }

src/ast2ram/seminaive/ClauseTranslator.h

@@ -87,6 +87,7 @@ class ClauseTranslator : public ast2ram::ClauseTranslator {
 
     /** Indexing */
     void indexClause(const ast::Clause& clause);
+    void indexGenerators(const ast::Clause& clause);
     virtual void indexAtoms(const ast::Clause& clause);
     void indexAggregators(const ast::Clause& clause);
     void indexMultiResultFunctors(const ast::Clause& clause);

tests/semantic/CMakeLists.txt

@@ -272,3 +272,5 @@ positive_output_stdout_test(output_stdout)
 positive_test(iteration_counter)
 positive_test(issue1896)
 positive_test(comp_params)
+positive_test(issue2416)
+positive_test(agg_range)

tests/semantic/agg_range/agg_range.dl

@@ -0,0 +1,44 @@
+.decl a(x: number, y: number, z: number)
+a(1, 2, 0).
+a(3, 4, 1).
+
+
+.decl b(item: number)
+b(1).
+b(2).
+b(3).
+b(4).
+
+.decl c(x: number, y: number)
+.output c()
+c(x, y) :-
+  mr = count : { a(_, y, _) },
+  x = range(0, mr),
+  b(y).
+
+.decl d(x: number, y: number)
+.output d()
+d(x,y) :-
+  y = range(z, z+3),
+  z = v*4,
+  v = x,
+  x = range(0, 4).
+
+.decl e(x: number, y: number)
+.output e()
+e(x,y) :-
+  y = count : { a(_, x, _) },
+  x = range(0, 5).
+
+.decl f(m: number, n: number, x:number)
+.output f()
+f(m,n,x) :-
+  x = range(m,n),
+  m = range(1,3),
+  n = range(4,6).
+
+.decl g(x:number)
+.output g()
+g(x) :-
+  x = count : { a(_, _, x) },
+  x = range(0,3).

tests/semantic/agg_range/c.csv

@@ -0,0 +1,2 @@
+0	2
+0	4

tests/semantic/agg_range/d.csv

@@ -0,0 +1,12 @@
+0	0
+0	1
+0	2
+1	4
+1	5
+1	6
+2	8
+2	9
+2	10
+3	12
+3	13
+3	14

tests/semantic/agg_range/e.csv

@@ -0,0 +1,5 @@
+0	0
+1	0
+2	1
+3	0
+4	1

tests/semantic/agg_range/f.csv

@@ -0,0 +1,12 @@
+1	4	1
+1	4	2
+1	4	3
+1	5	1
+1	5	2
+1	5	3
+1	5	4
+2	4	2
+2	4	3
+2	5	2
+2	5	3
+2	5	4

tests/semantic/agg_range/g.csv

@@ -0,0 +1 @@
+1

tests/semantic/issue2416/c.csv

@@ -0,0 +1 @@
+1	4

tests/semantic/issue2416/issue2416.dl

@@ -0,0 +1,19 @@
+.decl a(x: number, y: number, z: number)
+a(1, 2, 0).
+a(3, 4, 1).
+
+
+.decl b(item: number)
+b(1).
+b(2).
+b(3).
+b(4).
+
+.decl c(x: number, y: number)
+c(x, y) :-
+    //a(_, y, index),   // If the expression is not included, will it result in a segmentation fault
+    1 = count : { a(_, y, index) },
+    index = range(1, 10),
+    x = index,
+    b(y).
+.output c()