From dc49270dcca31b844bfa9454fbd9bfcc2219f9f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20C=C3=A9r=C3=A8s?= Date: Mon, 26 Oct 2026 14:16:16 +0100 Subject: [PATCH 1/4] refactor(engine): purge legacy dead event system and isolate Eval state and visitor dispatching (#643) - Eradicate legacy dead event classes (Event, EventImpl, EventListener, KgramEventDispatcher) - Remove unused event manager references in Memory, Mappings, and Environments - Extract EvaluationState from Eval to isolate execution counters and cooperative cancellation - Extract EvalVisitorDispatcher to isolate ProcessVisitor and ResultListener lifecycle hooks - Encapsulate all Exp fields as private - Add comprehensive unit tests with zero SonarLint alerts and 100% test pass --- .../impl/engine/eval/EnvironmentImpl.java | 6 - .../next/query/impl/engine/eval/Eval.java | 232 +++++------------- .../query/impl/engine/eval/EvalOptional.java | 2 +- .../engine/eval/EvalVisitorDispatcher.java | 125 ++++++++++ .../impl/engine/eval/EvaluationState.java | 120 +++++++++ .../next/query/impl/engine/event/Event.java | 79 ------ .../query/impl/engine/event/EventImpl.java | 135 ---------- .../impl/engine/event/EventListener.java | 20 -- .../engine/event/KgramEventDispatcher.java | 34 --- .../query/impl/engine/event/package-info.java | 2 +- .../next/query/impl/engine/pattern/Exp.java | 36 +-- .../next/query/impl/engine/pattern/Query.java | 4 +- .../query/impl/engine/solution/Mappings.java | 27 -- .../query/impl/engine/solution/Memory.java | 17 -- .../query/impl/engine/spi/Environment.java | 3 - .../storage/StorageBindingEnvironment.java | 6 - .../next/query/impl/engine/eval/EvalTest.java | 24 +- .../eval/EvalVisitorDispatcherTest.java | 130 ++++++++++ .../impl/engine/eval/EvaluationStateTest.java | 83 +++++++ 19 files changed, 551 insertions(+), 534 deletions(-) create mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalVisitorDispatcher.java create mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EvaluationState.java delete mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/engine/event/Event.java delete mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/engine/event/EventImpl.java delete mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/engine/event/EventListener.java delete mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/engine/event/KgramEventDispatcher.java create mode 100644 src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalVisitorDispatcherTest.java create mode 100644 src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvaluationStateTest.java diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EnvironmentImpl.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EnvironmentImpl.java index efffa461c..a9507caf8 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EnvironmentImpl.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EnvironmentImpl.java @@ -11,7 +11,6 @@ import fr.inria.corese.core.next.query.impl.engine.model.Node; import fr.inria.corese.core.next.query.impl.engine.spi.Environment; import fr.inria.corese.core.next.query.impl.engine.event.ProcessVisitor; -import fr.inria.corese.core.next.query.impl.engine.event.KgramEventDispatcher; import fr.inria.corese.core.next.query.impl.engine.path.Path; import fr.inria.corese.core.next.data.api.model.DatatypeValue; @@ -77,11 +76,6 @@ public Query getQuery() { return query; } - @Override - public KgramEventDispatcher getEventManager() { - return null; - } - @Override public Node getGraphNode() { return null; diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/Eval.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/Eval.java index 8d6686457..66302d2b7 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/Eval.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/Eval.java @@ -23,9 +23,6 @@ import fr.inria.corese.core.next.query.api.exception.QueryTypeErrorException; import fr.inria.corese.core.next.data.Values; -import fr.inria.corese.core.next.query.impl.engine.event.Event; -import fr.inria.corese.core.next.query.impl.engine.event.EventImpl; -import fr.inria.corese.core.next.query.impl.engine.event.KgramEventDispatcher; import fr.inria.corese.core.next.query.impl.engine.event.ResultListener; import fr.inria.corese.core.next.query.impl.engine.model.NodeImpl; import fr.inria.corese.core.next.data.api.model.DatatypeValue; @@ -64,8 +61,9 @@ public final class Eval implements ExpType { // Supply incoming mappings independently to both UNION branches. private boolean parameterUnionMappings = true; - KgramEventDispatcher manager; - boolean hasEvent = false; + private final EvaluationState state = new EvaluationState(); + private final EvalVisitorDispatcher visitors = new EvalVisitorDispatcher(); + // Edge and Node producer Producer saveProducer; Provider provider; @@ -83,30 +81,15 @@ public final class Eval implements ExpType { EvalGraph graphEvaluator; EvalJoin join; EvalOptional optional; - int // count number of eval() calls - nbEdge = 0; - int nbCall = 0; - int backjump = -1; - int maxLevel = -1; - int limit = Integer.MAX_VALUE; // subeval = false: eval query/subquery // subeval = true: eval statement (union, optional, minus, graph, join) boolean isSubEval = false; // return only select variables in Mapping boolean optim = true; boolean storeResult = true; - boolean hasFilter = false; - private ResultListener listener; - private ProcessVisitor visitor; // Edge and Node producer private Producer producer; private Stack current; - private boolean hasListener = false; - private int nbResult; - private boolean hasCandidate = false; - private boolean hasStatement = false; - private boolean hasProduce = false; - private volatile boolean stopped = false; private boolean joinMappings = true; /** @@ -189,9 +172,6 @@ public Mappings query(Node graphNode, Query q, Mapping m) { } Mappings queryBasic(Node graphNode, Query q, Mapping m) { - if (hasEvent) { - send(Event.BEGIN, q); - } initMemory(q); share(m); getProducer().start(q); @@ -202,9 +182,6 @@ Mappings queryBasic(Node graphNode, Query q, Mapping m) { Mappings map = eval(graphNode, q, m, null); getVisitor().orderby(map); getVisitor().after(map); - if (hasEvent) { - send(Event.END, q, map); - } return map; } finally { getProducer().finish(q); @@ -298,7 +275,7 @@ void queryWithValues(Node gNode, Query q, Mappings map) { if (!values.isPostpone()) { for (Mapping m : values.getMappings()) { - if (stopped) { + if (state.isStopped()) { return; } if (valuesBinding(values.getNodeList(), m, -1)) { @@ -324,7 +301,7 @@ void evalExp(Node gNode, Exp exp, Mappings map) { * We just counted number of results: nbResult Just build a Mapping */ void countProfile() { - Node n = NodeImpl.forValue(Values.factory().createLiteral(nbResult)); + Node n = NodeImpl.forValue(Values.factory().createLiteral(state.getNbResult())); Mapping m = Mapping.create(getQuery().getSelectFun().getFirst().getNode(), n); getResults().add(m); } @@ -522,9 +499,6 @@ Eval copy(Memory m, Producer p, Evaluator e, boolean extern) { ev.setVisitor(getVisitor()); } ev.startExtFun(); - if (hasEvent) { - ev.setEventManager(manager); - } return ev; } @@ -541,9 +515,6 @@ private Memory copyMemory(Memory memory, Query query, Query sub, Exp exp) { mem.init(sub); } memory.copyInto(sub, mem, exp); - if (hasEvent) { - memory.setEventManager(manager); - } return mem; } @@ -621,9 +592,6 @@ void initMemory(Query q) { getEvaluator().init(memory); // create memory bind stack memory.init(q); - if (hasEvent) { - memory.setEventManager(manager); - } getProducer().init(q); evaluator.start(memory); } @@ -639,17 +607,21 @@ void profile(Query q) { // partial init (for global query and subqueries) public void start(Query q) { - limit = q.getLimitOffset(); + state.setLimit(q.getLimitOffset()); // ASK only needs its first solution after OFFSET. Keep the requested // Query modifiers intact, and evaluate fully when sorting/grouping needs it. if (q.isAsk() && !q.isOrderBy() && !q.hasGroupBy() && !q.isAggregate()) { - limit = (int) Math.min(limit, (long) q.getOffset() + 1); + state.setLimit((int) Math.min(state.getLimit(), (long) q.getOffset() + 1)); } starter(q); } public void setLimit(int n) { - limit = n; + state.setLimit(n); + } + + public int getLimit() { + return state.getLimit(); } // for sub exp @@ -661,9 +633,6 @@ private void starter(Query q) { } else { results = Mappings.create(query, isSubEval); } - if (hasEvent) { - results.setEventManager(manager); - } startExtFun(); // set new results in case of sub query (for aggregates) memory.setEval(this); @@ -671,10 +640,7 @@ private void starter(Query q) { } void startExtFun() { - hasStatement = getVisitor().statement(); - hasProduce = getVisitor().produce(); - hasCandidate = getVisitor().candidate(); - hasFilter = getVisitor().filter(); + visitors.updateVisitorFlags(); } private void complete() { @@ -713,7 +679,7 @@ private int solution(Producer p, Mapping m, int n) { if (status == STOP) { return STOP; } - if (results.size() >= limit) { + if (results.size() >= state.getLimit()) { // backjump to send finish events to listener // and perform 'close instructions' if any return STOP; @@ -721,12 +687,12 @@ private int solution(Producer p, Mapping m, int n) { if (!getVisitor().limit(results)) { return STOP; } - if (backjump != -1) { + if (state.hasBackjump()) { if (!isSubEval && optim) { // use case: select distinct ?x where // backjump where ?x is defined to get a new one - backtrack = backjump; - backjump = -1; + backtrack = state.getBackjump(); + state.clearBackjump(); } } else if (query.isDistinct() && !query.isDistinctRedundantWithGroupBy() && !isSubEval && optim) { int index = memory.getIndex(query.getSelect()); @@ -760,34 +726,26 @@ int eval(Producer p, Node gNode, Stack stack, int n) { @SuppressWarnings("java:S3776") // Core KGRAM evaluation dispatch loop coordinating stack traversal and backtracking int eval(Producer p, Node graphNode, Stack stack, Mappings map, int n) { int backtrack = n - 1; - boolean isEvent = hasEvent; Memory env = getMemory(); - nbCall++; + state.incrementCall(); if (n >= stack.size()) { backtrack = solution(p, null, n); return backtrack; } - if (n > maxLevel) { - maxLevel = n; - } + state.updateMaxLevel(n); Exp exp = stack.get(n); - if (hasListener) { - // rule engine may have a ResultWatcher listener - exp = getListener().listen(exp, n); - } - - if (isEvent) { - send(Event.START, exp, graphNode, stack); + if (visitors.hasListener()) { + exp = visitors.listen(exp, n); } // A false filter was detected at compile time, or this expression was // identified as always failing: no use to evaluate it. if (!exp.isFail()) { - if (hasStatement) { + if (visitors.hasStatement()) { getVisitor().statement(this, getGraphNode(graphNode), exp); } @@ -870,7 +828,7 @@ int eval(Producer p, Node graphNode, Stack stack, Mappings map, int n) { if (getResults().accept(env.getNode(exp.getNode()))) { // backjump here when a mapping will be found with this node // see store() - backjump = n - 1; + state.setBackjump(n - 1); backtrack = eval(p, graphNode, stack, n + 1); } } else { @@ -883,10 +841,6 @@ int eval(Producer p, Node graphNode, Stack stack, Mappings map, int n) { } } - if (isEvent) { - send(Event.FINISH, exp, graphNode, stack); - } - return backtrack; } @@ -895,7 +849,7 @@ private int minus(Producer p, Node graphNode, Exp exp, Mappings data, Stack stac Memory env = getMemory(); Mappings map1 = subEval(p, graphNode, null, exp.first(), exp, data); - if (stopped) { + if (state.isStopped()) { return STOP; } if (map1.isEmpty()) { @@ -914,7 +868,7 @@ private int minus(Producer p, Node graphNode, Exp exp, Mappings data, Stack stac set.start(); for (Mapping map : map1) { - if (stopped) { + if (state.isStopped()) { return STOP; } boolean ok = !set.minusCompatible(map); @@ -941,7 +895,7 @@ private int union(Producer p, Node graphNode, Exp exp, Mappings data, Stack stac // join(A, union(B, C)) ; map = eval(A).distinct(inscopenodes()) Mappings map1 = unionBranch(p, graphNode, exp.first(), exp, data); - if (stopped) { + if (state.isStopped()) { return STOP; } Mappings map2 = unionBranch(p, graphNode, exp.rest(), exp, data); @@ -958,7 +912,7 @@ private int evalUnionMappings( Producer p, Node graphNode, Stack stack, int n, Mappings mappings, int backtrack) { Memory environment = getMemory(); for (Mapping mapping : mappings) { - if (stopped) { + if (state.isStopped()) { return STOP; } if (environment.push(mapping, n, false)) { @@ -1034,7 +988,7 @@ private int bgp(Producer p, Node graphNode, Exp exp, Stack stack, int n) { Mappings map = p.getMappings(graphNode, from, exp, getMemory()); for (Mapping m : map) { - if (stopped) { + if (state.isStopped()) { return STOP; } m.fixQueryNodes(getQuery()); @@ -1065,7 +1019,7 @@ private int service(Producer p, Node graphNode, Exp exp, Mappings data, Stack st Mappings map = getProvider().service(node, exp, selectQueryMappings(data), this); for (Mapping m : map) { - if (stopped) { + if (state.isStopped()) { return STOP; } // push each Mapping in memory and continue @@ -1166,7 +1120,7 @@ private int extBind(Producer p, Node graphNode, Exp exp, Stack stack, int n) { if (map != null) { HashMap tab = toMap(exp.getNodeList()); for (Mapping m : map) { - if (stopped) { + if (state.isStopped()) { return STOP; } if (env.push(tab, m, n)) { @@ -1201,15 +1155,11 @@ private int filter(Producer p, Node graphNode, Exp exp, Stack stack, int n) { if (!exp.isPostpone()) { success = test(graphNode, exp.getFilter(), env, p); - if (hasFilter) { + if (visitors.hasFilter()) { success = getVisitor().filter(this, getGraphNode(graphNode), exp.getFilter().getExp(), success); } } - if (hasEvent) { - send(Event.FILTER, exp, success); - } - if (success) { backtrack = eval(p, graphNode, stack, n + 1); } @@ -1267,8 +1217,8 @@ public Mappings eval(Filter f, Environment env, Producer p, List nodes) { int n = 1; Expr exp = f.getExp(); if (exp.oper() == UNNEST) { - if (hasListener) { - listener.listen(exp); + if (visitors.hasListener()) { + visitors.listen(exp); } if (exp.arity() == 2) { // unnest(exp, 2) @@ -1293,7 +1243,7 @@ private int values(Producer p, Node graphNode, Exp exp, Stack stack, int n) { getVisitor().values(this, getGraphNode(graphNode), exp, exp.getMappings()); for (Mapping map : exp.getMappings()) { - if (stopped) { + if (state.isStopped()) { return STOP; } if (valuesBinding(exp.getNodeList(), map, n)) { @@ -1359,10 +1309,8 @@ void free(List varList, Mapping map) { @SuppressWarnings("java:S3776") // Core KGRAM edge evaluation algorithm managing graph matching and backjump optimization private int edge(Producer p, Node graphNode, Exp exp, Mappings data, Stack stack, int n) { int backtrack = n - 1; - int evENUM = Event.ENUM; boolean isSuccess = false; boolean hasGraphNode = graphNode != null; - boolean isEvent = hasEvent; Edge qEdge = exp.getEdge(); Node graph; Memory env = getMemory(); @@ -1395,7 +1343,7 @@ private int edge(Producer p, Node graphNode, Exp exp, Mappings data, Stack stack } Iterable entities; - if (hasProduce) { + if (visitors.hasProduce()) { // A query visitor may provide candidates before the producer is consulted. entities = produce(graphNode, qEdge); if (entities == null) { @@ -1408,13 +1356,13 @@ private int edge(Producer p, Node graphNode, Exp exp, Mappings data, Stack stack try { for (Edge entity : entities) { - if (stopped) { + if (state.isStopped()) { return STOP; } if (entity != null) { - nbEdge++; - if (hasListener && !listener.listen(exp, qEdge, entity)) { + state.incrementEdge(); + if (visitors.hasListener() && !visitors.listen(exp, qEdge, entity)) { continue; } @@ -1426,7 +1374,7 @@ private int edge(Producer p, Node graphNode, Exp exp, Mappings data, Stack stack } if (bmatch) { - if (hasCandidate) { + if (visitors.hasCandidate()) { DatatypeValue dt = getVisitor().candidate(this, getGraphNode(graphNode), qEdge, entity); if (dt != null) { bmatch = dt.isTrue(); @@ -1436,10 +1384,6 @@ private int edge(Producer p, Node graphNode, Exp exp, Mappings data, Stack stack bmatch &= push(p, qEdge, entity, n); } - if (isEvent) { - send(evENUM, exp, entity, bmatch); - } - if (bmatch) { isSuccess = true; backtrack = eval(p, graphNode, stack, n + 1); @@ -1476,8 +1420,6 @@ private int edge(Producer p, Node graphNode, Exp exp, Mappings data, Stack stack */ private int query(Producer p, Node graphNode, Exp exp, Mappings data, Stack stack, int n) { int backtrack = n - 1; - int evENUM = Event.ENUM; - boolean isEvent = hasEvent; Query subQuery = exp.getQuery(); Memory env = getMemory(); getVisitor().start(subQuery); @@ -1493,15 +1435,11 @@ private int query(Producer p, Node graphNode, Exp exp, Mappings data, Stack stac // enumerate the result of the sub query // bind the select nodes into the stack for (Mapping map : lMap) { - if (stopped) { + if (state.isStopped()) { return STOP; } boolean bmatch = push(subQuery, map, n); - if (isEvent) { - send(evENUM, exp, map, bmatch); - } - if (bmatch) { backtrack = eval(p, graphNode, stack, n + 1); pop(subQuery, map); @@ -1595,7 +1533,7 @@ private int store(Producer p, Mapping m) { store = getListener().process(getMemory()); } if (store) { - nbResult++; + state.incrementResult(); } if (storeResult && store) { Mapping ans = m; @@ -1609,9 +1547,6 @@ private int store(Producer p, Mapping m) { void store(Mapping ans) { if (ans != null && acceptable(ans)) { - if (hasEvent) { - send(Event.RESULT, ans); - } boolean b = true; if (!isSubEval) { b = getVisitor().distinct(this, getQuery(), ans); @@ -1631,7 +1566,7 @@ boolean acceptable(Mapping m) { public int getCount() { - return nbEdge; + return state.getNbEdge(); } private boolean match(Edge qEdge, Edge edge, Node gNode, Node graphNode, Memory memory) { @@ -1650,42 +1585,6 @@ private boolean push(Producer p, Edge qEdge, Edge ent, int n) { } - public void createManager() { - if (manager == null) { - setEventManager(new KgramEventDispatcher()); - if (memory != null) { - memory.setEventManager(manager); - } - if (results != null) { - results.setEventManager(manager); - } - } - } - - public KgramEventDispatcher getEventManager() { - return manager; - } - - public void setEventManager(KgramEventDispatcher man) { - manager = man; - hasEvent = true; - } - - void send(int type, Object obj) { - Event e = EventImpl.create(type, obj); - manager.send(e); - } - - void send(int type, Object obj, Object arg) { - Event e = EventImpl.create(type, obj, arg); - manager.send(e); - } - - void send(int type, Object obj, Object arg, Object arg2) { - Event e = EventImpl.create(type, obj, arg, arg2); - manager.send(e); - } - void set(Stack current) { this.current = current; } @@ -1695,19 +1594,19 @@ public Stack getStack() { } public ProcessVisitor getVisitor() { - return visitor; + return visitors.getVisitor(); } public void setVisitor(ProcessVisitor visitor) { - this.visitor = visitor; + visitors.setVisitor(visitor); } public boolean isStop() { - return stopped; + return state.isStopped(); } public void setStop(boolean stopped) { - this.stopped = stopped; + state.setStopped(stopped); } @@ -1719,7 +1618,7 @@ public void setStop(boolean stopped) { * must only be accessed by the owning execution thread. */ public void finish() { - setStop(true); + state.stop(); join.setStop(); graphEvaluator.setStop(); optional.setStop(true); @@ -1760,9 +1659,6 @@ private int optBind(Producer p, Node gNode, Exp exp, Stack stack, int n) { if (!env.isBound(qNode) && getProducer().isBindable(node)) { // bind qNode with same index as other variable env.push(qNode, node, env.getIndex(exp.get(i).getNode())); - if (hasEvent) { - send(Event.BIND, exp, qNode, node); - } backtrack = eval(p, gNode, stack, n + 1); env.pop(qNode); } else { @@ -1810,9 +1706,6 @@ private int evalBindNodes(Producer p, Node gNode, Exp exp, Stack stack, int n, N Memory env = memory; for (Node node : exp.getNodeList()) { env.push(qNode, node, n); - if (hasEvent) { - send(Event.BIND, exp, qNode, node); - } int backtrack = eval(p, gNode, stack, n + 1); env.pop(qNode); if (backtrack < n) { @@ -1823,20 +1716,14 @@ private int evalBindNodes(Producer p, Node gNode, Exp exp, Stack stack, int n, N } /** - * Draf extension where a Visitor provides Edge iterator + * Draft extension where a Visitor provides Edge iterator */ Iterable produce(Node gNode, Edge edge) { - DatatypeValue res = getVisitor().produce(this, gNode, edge); - if (res == null) { - return null; - } - if (res instanceof Loopable loopable) { - Iterable loop = loopable.getLoop(); - if (loop != null) { - return new IterableEntity(loop); - } - } - return null; + return visitors.produce(this, gNode, edge); + } + + public boolean hasFilter() { + return visitors.hasFilter(); } public boolean isJoinMappings() { @@ -1844,12 +1731,19 @@ public boolean isJoinMappings() { } public ResultListener getListener() { - return listener; + return visitors.getListener(); } public void setListener(ResultListener listener) { - this.listener = listener; - this.hasListener = listener != null; + visitors.setListener(listener); + } + + public EvaluationState getState() { + return state; + } + + public EvalVisitorDispatcher getVisitorDispatcher() { + return visitors; } } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalOptional.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalOptional.java index 184ced3ab..1151cb3ee 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalOptional.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalOptional.java @@ -181,7 +181,7 @@ boolean filter(Environment memory, Producer p, Node gNode, Mapping map, Exp exp) map.setBind(memory.getBind()); map.setEval(engine); boolean b = engine.test(gNode, f.getFilter(), map, p); - if (engine.hasFilter) { + if (engine.hasFilter()) { b = engine.getVisitor().filter(engine, gNode, f.getFilter().getExp(), b); } if (!b) { diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalVisitorDispatcher.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalVisitorDispatcher.java new file mode 100644 index 000000000..1c4bae474 --- /dev/null +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalVisitorDispatcher.java @@ -0,0 +1,125 @@ +package fr.inria.corese.core.next.query.impl.engine.eval; + +import fr.inria.corese.core.next.data.api.model.DatatypeValue; +import fr.inria.corese.core.next.query.impl.engine.event.ProcessVisitor; +import fr.inria.corese.core.next.query.impl.engine.event.ProcessVisitorDefault; +import fr.inria.corese.core.next.query.impl.engine.event.ResultListener; +import fr.inria.corese.core.next.query.impl.engine.model.Edge; +import fr.inria.corese.core.next.query.impl.engine.model.Expr; +import fr.inria.corese.core.next.query.impl.engine.model.Loopable; +import fr.inria.corese.core.next.query.impl.engine.model.Node; +import fr.inria.corese.core.next.query.impl.engine.pattern.Exp; + +import java.util.Objects; + +/** + * Coordinates and dispatches execution lifecycle visitors and result listeners + * for SPARQL evaluation in {@link Eval}. + * + *

Encapsulates the active {@link ProcessVisitor} (for LDScript and federation hooks) + * and the optional {@link ResultListener} (for inference rule execution).

+ */ +public final class EvalVisitorDispatcher { + + private ResultListener listener; + private ProcessVisitor visitor; + + private boolean hasListener = false; + private boolean hasStatement = false; + private boolean hasProduce = false; + private boolean hasCandidate = false; + private boolean hasFilter = false; + + public EvalVisitorDispatcher() { + setVisitor(new ProcessVisitorDefault()); + } + + // --- Visitor --- + + public ProcessVisitor getVisitor() { + return visitor; + } + + public void setVisitor(ProcessVisitor visitor) { + this.visitor = Objects.requireNonNullElseGet(visitor, ProcessVisitorDefault::new); + updateVisitorFlags(); + } + + public void updateVisitorFlags() { + if (visitor != null) { + this.hasStatement = visitor.statement(); + this.hasProduce = visitor.produce(); + this.hasCandidate = visitor.candidate(); + this.hasFilter = visitor.filter(); + } else { + this.hasStatement = false; + this.hasProduce = false; + this.hasCandidate = false; + this.hasFilter = false; + } + } + + public boolean hasStatement() { + return hasStatement; + } + + public boolean hasProduce() { + return hasProduce; + } + + public boolean hasCandidate() { + return hasCandidate; + } + + public boolean hasFilter() { + return hasFilter; + } + + // --- Listener --- + + public ResultListener getListener() { + return listener; + } + + public void setListener(ResultListener listener) { + this.listener = listener; + this.hasListener = (listener != null); + } + + public boolean hasListener() { + return hasListener; + } + + public Exp listen(Exp exp, int n) { + if (hasListener) { + return listener.listen(exp, n); + } + return exp; + } + + public void listen(Expr exp) { + if (hasListener) { + listener.listen(exp); + } + } + + public boolean listen(Exp exp, Edge qEdge, Edge entity) { + if (hasListener) { + return listener.listen(exp, qEdge, entity); + } + return true; + } + + // --- Produce helper --- + + public Iterable produce(Eval eval, Node gNode, Edge edge) { + DatatypeValue res = visitor.produce(eval, gNode, edge); + if (res instanceof Loopable loopable) { + Iterable loop = loopable.getLoop(); + if (loop != null) { + return new IterableEntity(loop); + } + } + return null; + } +} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EvaluationState.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EvaluationState.java new file mode 100644 index 000000000..e2f55a4c6 --- /dev/null +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/EvaluationState.java @@ -0,0 +1,120 @@ +package fr.inria.corese.core.next.query.impl.engine.eval; + +/** + * Encapsulates the execution state, metrics, stack depth, and cooperative cancellation + * flag for SPARQL query evaluation in {@link Eval}. + * + *

Evaluation counters (calls, edges, results, max stack depth) are confined to the + * single evaluation thread. The cancellation flag {@link #isStopped()} is volatile to + * support concurrent interruption by a query timeout watchdog.

+ */ +public final class EvaluationState { + + private int nbEdge = 0; + private int nbCall = 0; + private int nbResult = 0; + private int backjump = -1; + private int maxLevel = -1; + private int limit = Integer.MAX_VALUE; + private volatile boolean stopped = false; + + // --- Counters & Metrics + + public void incrementEdge() { + nbEdge++; + } + + public void incrementCall() { + nbCall++; + } + + public void incrementResult() { + nbResult++; + } + + public int getNbEdge() { + return nbEdge; + } + + public void setNbEdge(int nbEdge) { + this.nbEdge = nbEdge; + } + + public int getNbCall() { + return nbCall; + } + + public void setNbCall(int nbCall) { + this.nbCall = nbCall; + } + + public int getNbResult() { + return nbResult; + } + + public void setNbResult(int nbResult) { + this.nbResult = nbResult; + } + + // --- Backjumping & Depth + + public int getBackjump() { + return backjump; + } + + public void setBackjump(int level) { + this.backjump = level; + } + + public void clearBackjump() { + this.backjump = -1; + } + + public boolean hasBackjump() { + return backjump != -1; + } + + public int getMaxLevel() { + return maxLevel; + } + + public void updateMaxLevel(int level) { + if (level > maxLevel) { + maxLevel = level; + } + } + + // --- Limit + + public int getLimit() { + return limit; + } + + public void setLimit(int limit) { + this.limit = limit; + } + + // --- Cooperative Interruption + + public boolean isStopped() { + return stopped; + } + + public void setStopped(boolean stopped) { + this.stopped = stopped; + } + + public void stop() { + this.stopped = true; + } + + public void reset() { + nbEdge = 0; + nbCall = 0; + nbResult = 0; + backjump = -1; + maxLevel = -1; + limit = Integer.MAX_VALUE; + stopped = false; + } +} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/Event.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/Event.java deleted file mode 100644 index c3767de39..000000000 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/Event.java +++ /dev/null @@ -1,79 +0,0 @@ -package fr.inria.corese.core.next.query.impl.engine.event; - -import fr.inria.corese.core.next.query.impl.engine.pattern.Exp; - -/** - * Event to trace KGRAM execution - * - * @author Olivier Corby, Edelweiss, INRIA 2010 - * - */ -public interface Event { - - int BEGIN = 0; - int START = 1; - int ENUM = 2; - int MATCH = 3; - int FILTER = 4; - int BIND = 5; - int GRAPH = 6; - int PATH = 7; - int PATHSTEP = 8; - - - int AGG = 13; - - int FINISH = 14; - int DISTINCT = 15; - int LIMIT = 16; - - int RESULT = 19; - int END = 20; - - - int ALL = 50; - - - - // events from user to listener - - // next step - int STEP = 101; - - // next edge/node/path - int NEXT = 102; - - // skip trace until next expression in stack - int FORWARD = 103; - - // eval current expression silently - int COMPLETE = 104; - - // eval until current expression succeed - int SUCCESS = 105; - - // resume execution silently - int QUIT = 109; - - - - - // pprint current Mapping - int MAP = 110; - int VERBOSE = 111; - int HELP = 112; - - - - int getSort(); - - Object getObject(); - - Object getArg(int n); - - Exp getExp(); - - boolean isSuccess(); - - -} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/EventImpl.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/EventImpl.java deleted file mode 100644 index 0fc339508..000000000 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/EventImpl.java +++ /dev/null @@ -1,135 +0,0 @@ -package fr.inria.corese.core.next.query.impl.engine.event; - - -import fr.inria.corese.core.next.query.impl.engine.pattern.Exp; -import fr.inria.corese.core.next.query.impl.engine.eval.Stack; - -/** - * Event to trace KGRAM execution - * @author Olivier Corby, Edelweiss, INRIA 2010 - * - */ -public class EventImpl implements Event { - - - int type; - boolean isSuccess = true; - Object object; - Object arg; - Object arg2; - - EventImpl(int n, Object o){ - type = n; - object = o; - } - - EventImpl(int n, Object o, Object o2){ - type = n; - object = o; - arg = o2; - if (o2 instanceof Boolean success){ - isSuccess = success; - } - } - - EventImpl(int n, Object o, Object o2, Object o3){ - type = n; - object = o; - arg = o2; - arg2 = o3; - if (o3 instanceof Boolean success){ - isSuccess = success; - } - } - - public String toString(){ - String str = getTitle(); - if (object != null){ - str += " "; - if (object instanceof Exp exp && exp.isEdge()){ - str += "("+ exp.getEdge().getEdgeIndex() + ") "; - } - str += object; - } - if (arg != null){ - str += "\n"; - str += arg; - } - if (arg2 != null && ! (arg2 instanceof Stack)) str += "\n" + arg2; - - return str; - } - - - String getTitle(){ - return switch (type) { - case BEGIN -> "begin"; - case START -> "start"; - case ENUM -> "enum"; - case FILTER -> "filter"; - case BIND -> "bind"; - case MATCH -> "match"; - case GRAPH -> "graph"; - case PATH -> "path"; - case PATHSTEP, STEP -> "step"; - case FINISH -> "finish"; - case AGG -> "aggregate"; - case DISTINCT -> "distinct"; - case LIMIT -> "limit"; - case RESULT -> "result"; - case END -> "end"; - case COMPLETE -> "complete"; - case FORWARD -> "forward"; - case MAP -> "map"; - case NEXT -> "next"; - case QUIT -> "quit"; - case SUCCESS -> "success"; - case VERBOSE -> "verbose"; - case HELP -> "help"; - default -> "event(" + type + ')'; - }; - } - - - - public static EventImpl create(int type, Object obj){ - return new EventImpl(type, obj); - } - - public static EventImpl create(int type, Object obj, Object arg){ - return new EventImpl(type, obj, arg); - } - - public static EventImpl create(int type, Object obj, Object arg, Object arg2){ - return new EventImpl(type, obj, arg, arg2); - } - - public int getSort(){ - return type; - } - - public Object getObject(){ - return object; - } - - public Object getArg(int n){ - return switch (n) { - case 0 -> object; - case 1 -> arg; - case 2 -> arg2; - default -> null; - }; - } - - public Exp getExp(){ - if (object instanceof Exp expression){ - return expression; - } - return null; - } - - public boolean isSuccess(){ - return isSuccess; - } - -} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/EventListener.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/EventListener.java deleted file mode 100644 index 97bcd1e7d..000000000 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/EventListener.java +++ /dev/null @@ -1,20 +0,0 @@ -package fr.inria.corese.core.next.query.impl.engine.event; - - -/** - * Event Listener to trace KGRAM execution - * @author Olivier Corby, Edelweiss, INRIA 2010 - * - */ -public interface EventListener { - - boolean send(Event e); - - boolean handle(int sort); - - boolean handle(int sort, boolean b); - - void setObject(Object obj); - - -} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/KgramEventDispatcher.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/KgramEventDispatcher.java deleted file mode 100644 index c66473378..000000000 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/KgramEventDispatcher.java +++ /dev/null @@ -1,34 +0,0 @@ -package fr.inria.corese.core.next.query.impl.engine.event; - -import java.util.ArrayList; -import java.util.Iterator; -import java.util.List; - -/** - * Event Manager to trace KGRAM execution - * - * @author Olivier Corby, Edelweiss, INRIA 2010 - * - */ -public class KgramEventDispatcher implements Iterable { - - - List observers = new ArrayList<>(); - - @Override - public Iterator iterator() { - return observers.iterator(); - } - - public boolean send(Event event) { - boolean res = true; - for (EventListener el : observers) { - if (el.handle(event.getSort())) { - boolean b = el.send(event); - res = res && b; - } - } - return res; - } - -} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/package-info.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/package-info.java index e5d3df54b..9124c23ad 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/package-info.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/event/package-info.java @@ -1,4 +1,4 @@ /** - * Engine event dispatching, listeners, and query processing visitors. + * Query processing visitors and evaluation result listeners. */ package fr.inria.corese.core.next.query.impl.engine.event; diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Exp.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Exp.java index 576bff7e1..5385d504b 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Exp.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Exp.java @@ -40,28 +40,28 @@ public class Exp extends PointerObject static final String NEWLINE = "\n"; static final String SP = " "; - Type type; + private Type type; private int bindingIndex = -1; // optional success - boolean status = false; - boolean isFail = false; - boolean isAggregate = false; - boolean isSilent = false; - VExp args; - Edge edge; - Node node; - List lNodes; + private boolean status = false; + private boolean isFail = false; + private boolean isAggregate = false; + private boolean isSilent = false; + private VExp args; + private Edge edge; + private Node node; + private List lNodes; // Expression evaluation contract, independent of parser syntax types. - Filter filter; - List lFilter; + private Filter filter; + private List lFilter; // min(?l, expGroupBy(?x, ?y)) - List expGroupBy; + private List expGroupBy; // for UNION - Stack stack; + private Stack stack; // Optimizer-specific payload, such as candidate values for an equality filter. - Object payload; - Producer producer; - Mappings map; + private Object payload; + private Producer producer; + private Mappings map; private boolean isPostpone = false; private boolean isFunctional = false; private boolean generated = false; @@ -242,6 +242,10 @@ public void add(Exp e) { args.add(e); } + public void clear() { + args.clear(); + } + public void add(Edge e) { args.add(create(Type.EDGE, e)); } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Query.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Query.java index a038bf15f..99a19eeb0 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Query.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Query.java @@ -394,8 +394,8 @@ public Exp getBody() { } public void setBody(Exp exp){ - args.clear(); - args.add(exp); + clear(); + add(exp); } void setGlobalQuery(Query q) { diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java index 14bb642d4..75a69103a 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java @@ -17,9 +17,6 @@ import fr.inria.corese.core.next.query.impl.engine.spi.Environment; import fr.inria.corese.core.next.query.impl.engine.spi.Evaluator; import fr.inria.corese.core.next.query.impl.engine.spi.Producer; -import fr.inria.corese.core.next.query.impl.engine.event.Event; -import fr.inria.corese.core.next.query.impl.engine.event.EventImpl; -import fr.inria.corese.core.next.query.impl.engine.event.KgramEventDispatcher; import fr.inria.corese.core.next.data.api.model.DatatypeValue; import java.util.ArrayList; import java.util.Collections; @@ -53,11 +50,9 @@ public final class Mappings extends PointerObject boolean isDistinct = false; // statisfy having(test) boolean isValid = true; - boolean hasEvent = false; // if true, store all Mapping of the group boolean isListGroup = false; boolean sortWithDesc = true; - KgramEventDispatcher manager; int count = 0; private Query query; private final List list; @@ -120,12 +115,6 @@ public static Mappings create(Query q, boolean subEval) { return lMap; } - - public void setEventManager(KgramEventDispatcher man) { - manager = man; - hasEvent = true; - } - @Override @SuppressWarnings("unchecked") // Every Mapping is an Object; preserve the historical self view. public Iterable getLoop() { @@ -667,9 +656,6 @@ public void aggregate(Query q, Evaluator evaluator, Memory memory, Producer p) { aggregateExpList(q, evaluator, memory, p, q.getOrderBy(), false); if (q.getHaving() != null) { - if (hasEvent) { - manager.send(EventImpl.create(Event.AGG, q.getHaving())); - } aggregateSwitch(q, evaluator, q.getHaving(), memory, p, HAVING); } @@ -685,9 +671,6 @@ void aggregateExpList(Query q, Evaluator evaluator, Memory memory, Producer p, L int n = 0; for (Exp exp : list) { if (exp.isAggregate()) { - if (hasEvent) { - manager.send(EventImpl.create(Event.AGG, exp)); - } // perform group by and then aggregate aggregateSwitch(q, evaluator, exp, memory, p, (isSelect) ? SELECT : n); } @@ -742,9 +725,6 @@ private void aggregateHaving(Evaluator eval, Exp exp, Memory memory, Producer p) if (ev != null) { ev.getVisitor().having(ev, exp.getFilter().getExp(), res); } - if (hasEvent) { - manager.send(EventImpl.create(Event.FILTER, exp, res)); - } setValid(res); } @@ -763,10 +743,6 @@ private void aggregateSelectOrOrder(Evaluator eval, Exp exp, Memory memory, Prod } } - if (hasEvent) { - manager.send(EventImpl.create(Event.FILTER, exp, aggregateValue)); - } - for (Mapping map : this) { if (n == SELECT) { // select (count(?x) as ?c) @@ -792,9 +768,6 @@ Node eval(Filter f, Evaluator eval, Environment env, Producer p) { private void aggregateGroupMembers(Group group, Evaluator eval, Exp exp, Memory mem, Producer p, int n) { int mappingCount = 0; for (Mappings map : group.getValues()) { - if (hasEvent) { - map.setEventManager(manager); - } map.setCount(mappingCount++); mem.setGroup(map); map.aggregate(eval, exp, mem, p, n); diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Memory.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Memory.java index 0a0f9e9d6..4cd4d4fbb 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Memory.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Memory.java @@ -18,8 +18,6 @@ import fr.inria.corese.core.next.query.impl.engine.spi.Evaluator; import fr.inria.corese.core.next.query.impl.engine.spi.Matcher; import fr.inria.corese.core.next.query.impl.engine.spi.Producer; - -import fr.inria.corese.core.next.query.impl.engine.event.KgramEventDispatcher; import fr.inria.corese.core.next.query.impl.engine.path.Path; import fr.inria.corese.core.next.data.api.model.DatatypeValue; @@ -73,8 +71,6 @@ public class Memory extends PointerObject implements Environment { Mapping mapping; // true when processing aggregate at the end boolean isAggregate = false; - KgramEventDispatcher manager; - boolean hasEvent = false; int nbEdge = 0; int nbNode = 0; // service evaluation detail report @@ -91,19 +87,6 @@ public Memory(Matcher m, Evaluator e) { bnode = new HashMap<>(); } - @Override - public KgramEventDispatcher getEventManager() { - if (manager == null) { - kgram.createManager(); - } - return manager; - } - - public void setEventManager(KgramEventDispatcher man) { - manager = man; - hasEvent = true; - } - @Override public Eval getEval() { return kgram; diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/spi/Environment.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/spi/Environment.java index 34731af25..5ff4a01b9 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/spi/Environment.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/spi/Environment.java @@ -11,7 +11,6 @@ import fr.inria.corese.core.next.query.impl.engine.model.Edge; import fr.inria.corese.core.next.query.impl.engine.model.Expr; import fr.inria.corese.core.next.query.impl.engine.model.Node; -import fr.inria.corese.core.next.query.impl.engine.event.KgramEventDispatcher; import fr.inria.corese.core.next.query.impl.engine.path.Path; import fr.inria.corese.core.next.data.api.model.DatatypeValue; @@ -99,8 +98,6 @@ default void setGraphNode(Node n) { int count(); - KgramEventDispatcher getEventManager(); - Object getObject(); void setObject(Object o); diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/storage/StorageBindingEnvironment.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/storage/StorageBindingEnvironment.java index f8b974fe3..e7f255235 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/storage/StorageBindingEnvironment.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/storage/StorageBindingEnvironment.java @@ -2,7 +2,6 @@ import fr.inria.corese.core.next.data.api.model.DatatypeValue; import fr.inria.corese.core.next.query.impl.engine.eval.Eval; -import fr.inria.corese.core.next.query.impl.engine.event.KgramEventDispatcher; import fr.inria.corese.core.next.query.impl.engine.event.ProcessVisitor; import fr.inria.corese.core.next.query.impl.engine.model.BindingContext; import fr.inria.corese.core.next.query.impl.engine.model.Edge; @@ -110,11 +109,6 @@ public int count() { return delegate == null ? 0 : delegate.count(); } - @Override - public KgramEventDispatcher getEventManager() { - return delegate == null ? null : delegate.getEventManager(); - } - @Override public Object getObject() { return delegate == null ? null : delegate.getObject(); diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalTest.java index 7f8a3c1e3..8548cd039 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalTest.java @@ -10,7 +10,6 @@ import fr.inria.corese.core.next.query.impl.engine.spi.Provider; import fr.inria.corese.core.next.query.impl.engine.model.BindingContext; -import fr.inria.corese.core.next.query.impl.engine.event.KgramEventDispatcher; import fr.inria.corese.core.next.query.impl.engine.event.ResultListener; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -218,25 +217,14 @@ void testDefaultVisitor() { } @Nested - @DisplayName("Event Manager Tests") - class EventManagerTests { + @DisplayName("Visitor Dispatcher Tests") + class VisitorDispatcherTests { @Test - @DisplayName("Should set and get event manager") - void testSetAndGetEventManager() { - KgramEventDispatcher mockManager = mock(KgramEventDispatcher.class); - eval.setEventManager(mockManager); - assertEquals(mockManager, eval.getEventManager(), - "Event manager should match"); - } - - @Test - @DisplayName("Should create event manager") - void testCreateEventManager() { - assertDoesNotThrow(() -> eval.createManager(), - "Creating event manager should not throw"); - assertNotNull(eval.getEventManager(), - "Event manager should be created"); + @DisplayName("Should expose visitor dispatcher") + void testVisitorDispatcher() { + assertNotNull(eval.getVisitorDispatcher(), "Visitor dispatcher should be initialized"); + assertNotNull(eval.getVisitor(), "Process visitor should have default instance"); } } diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalVisitorDispatcherTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalVisitorDispatcherTest.java new file mode 100644 index 000000000..ff0a16b82 --- /dev/null +++ b/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalVisitorDispatcherTest.java @@ -0,0 +1,130 @@ +package fr.inria.corese.core.next.query.impl.engine.eval; + +import fr.inria.corese.core.next.data.api.model.DatatypeValue; +import fr.inria.corese.core.next.query.impl.engine.event.ProcessVisitor; +import fr.inria.corese.core.next.query.impl.engine.event.ProcessVisitorDefault; +import fr.inria.corese.core.next.query.impl.engine.event.ResultListener; +import fr.inria.corese.core.next.query.impl.engine.model.Edge; +import fr.inria.corese.core.next.query.impl.engine.model.Expr; +import fr.inria.corese.core.next.query.impl.engine.model.Loopable; +import fr.inria.corese.core.next.query.impl.engine.model.Node; +import fr.inria.corese.core.next.query.impl.engine.pattern.Exp; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +@DisplayName("EvalVisitorDispatcher Tests") +class EvalVisitorDispatcherTest { + + private EvalVisitorDispatcher dispatcher; + + @BeforeEach + void setUp() { + dispatcher = new EvalVisitorDispatcher(); + } + + @Test + @DisplayName("Should initialize with default visitor and false flags") + void testDefaults() { + assertNotNull(dispatcher.getVisitor()); + assertTrue(dispatcher.getVisitor() instanceof ProcessVisitorDefault); + assertFalse(dispatcher.hasStatement()); + assertFalse(dispatcher.hasProduce()); + assertFalse(dispatcher.hasCandidate()); + assertFalse(dispatcher.hasFilter()); + assertFalse(dispatcher.hasListener()); + assertNull(dispatcher.getListener()); + } + + @Test + @DisplayName("Should update flags when custom visitor is configured") + void testCustomVisitorFlags() { + ProcessVisitor mockVisitor = mock(ProcessVisitor.class); + when(mockVisitor.statement()).thenReturn(true); + when(mockVisitor.produce()).thenReturn(true); + when(mockVisitor.candidate()).thenReturn(false); + when(mockVisitor.filter()).thenReturn(true); + + dispatcher.setVisitor(mockVisitor); + + assertSame(mockVisitor, dispatcher.getVisitor()); + assertTrue(dispatcher.hasStatement()); + assertTrue(dispatcher.hasProduce()); + assertFalse(dispatcher.hasCandidate()); + assertTrue(dispatcher.hasFilter()); + } + + @Test + @DisplayName("Should fall back to default visitor when null is passed") + void testNullVisitorFallback() { + dispatcher.setVisitor(null); + assertNotNull(dispatcher.getVisitor()); + assertTrue(dispatcher.getVisitor() instanceof ProcessVisitorDefault); + assertFalse(dispatcher.hasStatement()); + assertFalse(dispatcher.hasProduce()); + assertFalse(dispatcher.hasCandidate()); + assertFalse(dispatcher.hasFilter()); + } + + @Test + @DisplayName("Should dispatch listener calls when listener is present") + void testListenerDispatching() { + ResultListener mockListener = mock(ResultListener.class); + dispatcher.setListener(mockListener); + + assertTrue(dispatcher.hasListener()); + assertSame(mockListener, dispatcher.getListener()); + + Exp exp = mock(Exp.class); + Exp transformedExp = mock(Exp.class); + when(mockListener.listen(exp, 1)).thenReturn(transformedExp); + assertSame(transformedExp, dispatcher.listen(exp, 1)); + + Expr expr = mock(Expr.class); + dispatcher.listen(expr); + verify(mockListener).listen(expr); + + Edge qEdge = mock(Edge.class); + Edge entity = mock(Edge.class); + when(mockListener.listen(exp, qEdge, entity)).thenReturn(false); + assertFalse(dispatcher.listen(exp, qEdge, entity)); + } + + @Test + @DisplayName("Should safely bypass listener calls when listener is null") + void testListenerBypassWhenNull() { + Exp exp = mock(Exp.class); + assertSame(exp, dispatcher.listen(exp, 1)); + + Expr expr = mock(Expr.class); + assertDoesNotThrow(() -> dispatcher.listen(expr)); + + Edge qEdge = mock(Edge.class); + Edge entity = mock(Edge.class); + assertTrue(dispatcher.listen(exp, qEdge, entity)); + } + + @Test + @DisplayName("Should dispatch produce to visitor and wrap loopable results") + void testProduceDispatching() { + ProcessVisitor mockVisitor = mock(ProcessVisitor.class); + Eval mockEval = mock(Eval.class); + Node mockNode = mock(Node.class); + Edge mockEdge = mock(Edge.class); + + DatatypeValue mockDt = mock(DatatypeValue.class, withSettings().extraInterfaces(Loopable.class)); + List elements = Collections.emptyList(); + doReturn(elements).when((Loopable) mockDt).getLoop(); + when(mockVisitor.produce(mockEval, mockNode, mockEdge)).thenReturn(mockDt); + + dispatcher.setVisitor(mockVisitor); + Iterable produced = dispatcher.produce(mockEval, mockNode, mockEdge); + assertNotNull(produced); + } +} diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvaluationStateTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvaluationStateTest.java new file mode 100644 index 000000000..1b2a136c2 --- /dev/null +++ b/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvaluationStateTest.java @@ -0,0 +1,83 @@ +package fr.inria.corese.core.next.query.impl.engine.eval; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class EvaluationStateTest { + + @Test + void initialStateShouldHaveDefaultValues() { + EvaluationState state = new EvaluationState(); + + assertEquals(0, state.getNbEdge()); + assertEquals(0, state.getNbCall()); + assertEquals(0, state.getNbResult()); + assertEquals(-1, state.getBackjump()); + assertFalse(state.hasBackjump()); + assertEquals(-1, state.getMaxLevel()); + assertEquals(Integer.MAX_VALUE, state.getLimit()); + assertFalse(state.isStopped()); + } + + @Test + void countersShouldIncrementCorrectly() { + EvaluationState state = new EvaluationState(); + + state.incrementEdge(); + state.incrementEdge(); + state.incrementCall(); + state.incrementResult(); + + assertEquals(2, state.getNbEdge()); + assertEquals(1, state.getNbCall()); + assertEquals(1, state.getNbResult()); + } + + @Test + void maxLevelShouldTrackHighestDepth() { + EvaluationState state = new EvaluationState(); + + state.updateMaxLevel(2); + assertEquals(2, state.getMaxLevel()); + + state.updateMaxLevel(5); + assertEquals(5, state.getMaxLevel()); + + state.updateMaxLevel(3); + assertEquals(5, state.getMaxLevel()); + } + + @Test + void backjumpLifecycleShouldWork() { + EvaluationState state = new EvaluationState(); + + state.setBackjump(3); + assertTrue(state.hasBackjump()); + assertEquals(3, state.getBackjump()); + + state.clearBackjump(); + assertFalse(state.hasBackjump()); + assertEquals(-1, state.getBackjump()); + } + + @Test + void stopAndResetShouldOperateCorrectly() { + EvaluationState state = new EvaluationState(); + + state.stop(); + assertTrue(state.isStopped()); + + state.setLimit(100); + state.incrementCall(); + state.setBackjump(2); + + state.reset(); + assertFalse(state.isStopped()); + assertEquals(Integer.MAX_VALUE, state.getLimit()); + assertEquals(0, state.getNbCall()); + assertFalse(state.hasBackjump()); + } +} From c645674ba6ff979d3396a7998002acd5158f9a24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20C=C3=A9r=C3=A8s?= Date: Mon, 26 Oct 2026 14:30:43 +0100 Subject: [PATCH 2/4] refactor(engine): reject unsupported SERVICE fallback and purge dead legacy stubs (#643) - Guard federated SERVICE execution against silent local fallback (throw QueryEvaluationException unless SILENT) - Delete dead classes QueryDefinitionChecker and Merge - Purge unused flags (isCheck, isConnect, COUNT_PROFILE, CACHE_MODE) and dead Exp fields (level, num, generated) - Consolidate empty edge arrays in Memory and remove dead Matcher mode - Reduce cognitive complexity of Eval.service by extracting processServiceMappings - Update unit tests with 100% pass --- .../next/query/impl/engine/eval/Eval.java | 85 +++++-------- .../engine/eval/QueryDefinitionChecker.java | 119 ----------------- .../impl/engine/eval/RdfTermMatcher.java | 12 -- .../engine/eval/SparqlKgramEvaluator.java | 25 ---- .../next/query/impl/engine/pattern/Exp.java | 30 ----- .../next/query/impl/engine/pattern/Group.java | 2 +- .../next/query/impl/engine/pattern/Query.java | 26 +--- .../query/impl/engine/solution/Mappings.java | 16 +-- .../query/impl/engine/solution/Memory.java | 7 +- .../query/impl/engine/solution/Merge.java | 120 ------------------ .../next/query/impl/engine/spi/Evaluator.java | 17 ++- .../next/query/impl/engine/spi/Matcher.java | 13 -- .../sparql/bridge/DirectGroupAggregation.java | 2 +- .../next/query/impl/engine/eval/EvalTest.java | 43 +++++++ .../impl/engine/eval/RdfTermMatcherTest.java | 8 -- .../query/impl/engine/pattern/ExpTest.java | 36 ------ .../query/impl/engine/pattern/QueryTest.java | 7 - 17 files changed, 92 insertions(+), 476 deletions(-) delete mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/QueryDefinitionChecker.java delete mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Merge.java diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/Eval.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/Eval.java index 66302d2b7..4bbfafa74 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/Eval.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/Eval.java @@ -20,6 +20,7 @@ import fr.inria.corese.core.next.query.impl.engine.spi.Matcher; import fr.inria.corese.core.next.query.impl.engine.spi.Producer; import fr.inria.corese.core.next.query.impl.engine.spi.Provider; +import fr.inria.corese.core.next.query.api.exception.QueryEvaluationException; import fr.inria.corese.core.next.query.api.exception.QueryTypeErrorException; import fr.inria.corese.core.next.data.Values; @@ -86,7 +87,6 @@ public final class Eval implements ExpType { boolean isSubEval = false; // return only select variables in Mapping boolean optim = true; - boolean storeResult = true; // Edge and Node producer private Producer producer; private Stack current; @@ -223,21 +223,12 @@ Mappings eval(Node gNode, Query q, Mapping m, Mappings map) { // just compile and complete query return getResults(); } - if (q.isCheck()) { - QueryDefinitionChecker check = QueryDefinitionChecker.create(this); - check.check(q); - } if (!q.isFail()) { query(gNode, q, m, map); - - if (q.getQueryProfile() == Query.COUNT_PROFILE) { - countProfile(); - } else { - aggregate(); - // order by - complete(); - } + aggregate(); + // order by + complete(); } getEvaluator().finish(getMemory()); @@ -297,15 +288,6 @@ void evalExp(Node gNode, Exp exp, Mappings map) { eval(getProducer(), gNode, stack, map, 0); } - /** - * We just counted number of results: nbResult Just build a Mapping - */ - void countProfile() { - Node n = NodeImpl.forValue(Values.factory().createLiteral(state.getNbResult())); - Mapping m = Mapping.create(getQuery().getSelectFun().getFirst().getNode(), n); - getResults().add(m); - } - public void setMappings(Mappings lMap) { initialResults = lMap; } @@ -579,7 +561,6 @@ public Mappings getResults() { public void init(Query q) { initMemory(q); start(q); - profile(q); } void initMemory(Query q) { @@ -597,14 +578,6 @@ void initMemory(Query q) { } } - void profile(Query q) { - // select count(*) as ?c where pattern - // do not built Mapping, just count them - if (q.getQueryProfile() == Query.COUNT_PROFILE) { - storeResult = false; - } - } - // partial init (for global query and subqueries) public void start(Query q) { state.setLimit(q.getLimitOffset()); @@ -1005,39 +978,41 @@ private int bgp(Producer p, Node graphNode, Exp exp, Stack stack, int n) { } private int service(Producer p, Node graphNode, Exp exp, Mappings data, Stack stack, int n) { - int backtrack = n - 1; Memory env = getMemory(); Node serv = exp.first().getNode(); - Node node = serv; - - if (serv.isVariable()) { - node = env.getNode(serv); - } + Node node = serv.isVariable() ? env.getNode(serv) : serv; if (getProvider() != null) { - // service delegated to provider Mappings map = getProvider().service(node, exp, selectQueryMappings(data), this); + return processServiceMappings(p, graphNode, stack, n, env, map); + } else if (exp.isSilent()) { + return eval(p, graphNode, stack, n + 1); + } else { + throw new QueryEvaluationException( + "Federated SPARQL SERVICE execution failed: no Provider configured to query endpoint " + node); + } + } - for (Mapping m : map) { - if (state.isStopped()) { - return STOP; - } - // push each Mapping in memory and continue - complete(getQuery(), m, false); + private int processServiceMappings(Producer p, Node graphNode, Stack stack, int n, Memory env, Mappings map) { + int backtrack = n - 1; + if (map == null) { + return backtrack; + } + for (Mapping m : map) { + if (state.isStopped()) { + return STOP; + } + // push each Mapping in memory and continue + complete(getQuery(), m, false); - if (env.push(m, n, false, false)) { - backtrack = eval(p, graphNode, stack, n + 1); - env.pop(m, false); - if (backtrack < n) { - return backtrack; - } + if (env.push(m, n, false, false)) { + backtrack = eval(p, graphNode, stack, n + 1); + env.pop(m, false); + if (backtrack < n) { + return backtrack; } } - } else { - Query q = exp.rest().getQuery(); - return query(p, graphNode, q, data, stack, n); } - return backtrack; } @@ -1535,7 +1510,7 @@ private int store(Producer p, Mapping m) { if (store) { state.incrementResult(); } - if (storeResult && store) { + if (store) { Mapping ans = m; if (m == null) { ans = getMemory().store(getQuery(), p, isSubEval); diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/QueryDefinitionChecker.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/QueryDefinitionChecker.java deleted file mode 100644 index 878e6a4a9..000000000 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/QueryDefinitionChecker.java +++ /dev/null @@ -1,119 +0,0 @@ -package fr.inria.corese.core.next.query.impl.engine.eval; - -import fr.inria.corese.core.next.query.impl.engine.pattern.Exp; -import fr.inria.corese.core.next.query.impl.engine.pattern.Query; -import fr.inria.corese.core.next.query.impl.engine.solution.Mappings; - -import fr.inria.corese.core.next.query.impl.engine.model.Edge; -import fr.inria.corese.core.next.query.impl.engine.model.Node; -import fr.inria.corese.core.next.query.impl.engine.spi.Environment; -import fr.inria.corese.core.next.query.impl.engine.spi.Matcher; -import fr.inria.corese.core.next.query.impl.engine.spi.Producer; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - - -/** - * Query Type QueryDefinitionChecker - *

- * Check occurrence of properties in Producer - * Check if class/property are defined in Producer - * - p rdf:type rdf:Property - * - c rdf:type rdfs:Class - * - * @author Olivier Corby, Edelweiss, INRIA 2011 - */ -final class QueryDefinitionChecker { - - private static final Logger LOGGER = LoggerFactory.getLogger(QueryDefinitionChecker.class); - - private final Eval eval; - private final Producer producer; - private final Matcher matcher; - private Query query; - - QueryDefinitionChecker(Eval e) { - eval = e; - producer = e.getProducer(); - matcher = e.getMatcher(); - } - - static QueryDefinitionChecker create(Eval e) { - return new QueryDefinitionChecker(e); - } - - void check(Query q) { - query = q; - check(null, q.getBody(), eval.getMemory()); - } - - - private void check(Node gNode, Exp exp, Environment env) { - - switch (exp.type()) { - - case EDGE: - edge(gNode, exp, env); - break; - - - case QUERY: - check(gNode, exp.getQuery().getBody(), env); - break; - - default: - for (Exp ee : exp.getExpList()) { - check(gNode, ee, env); - } - } - - - } - - /** - * Check occurrence of edge - * If edge has an associated query, check class/property definition in ontology - */ - private void edge(Node gNode, Exp exp, Environment env) { - Edge edge = exp.getEdge(); - boolean exist = false; - boolean match = false; - boolean define ; - - for (Edge ent : producer.getEdges(gNode, query.getFrom(gNode), edge, env)) { - - if (ent != null) { - exist = true; - if (matcher.match(edge, ent, env)) { - match = true; - break; - } - } - } - - Query q = query.get(edge); - if (q != null) { - Eval ee = Eval.create(producer, eval.getEvaluator(), matcher); - Mappings map = ee.query(q); - define = !map.isEmpty(); - report(edge, exist, match, define); - - } else { - report(edge, exist, match); - } - } - - - private void report(Edge edge, boolean exist, boolean match, boolean define) { - query.addInfo(edge.toString(), - " defined:" + define + " exist: " + exist + " match: " + match); - LOGGER.info("Edge: {}: {} {} {}", edge, exist, match, define); - } - - private void report(Edge edge, boolean exist, boolean match) { - query.addInfo(edge.toString(), " exist: " + exist + " match: " + match); - LOGGER.info("Edge: {}: {} {}", edge, exist, match); - } - - -} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/RdfTermMatcher.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/RdfTermMatcher.java index a32ce5f41..b94bbf5a9 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/RdfTermMatcher.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/RdfTermMatcher.java @@ -19,8 +19,6 @@ */ public final class RdfTermMatcher implements Matcher { - private int mode = Matcher.UNDEF; - @Override public boolean match(Edge query, Edge target, Environment environment) { return match(query.getNode(0), target.getNode(0), environment) @@ -47,16 +45,6 @@ public boolean same(Node queryNode, Node left, Node right, Environment environme return left != null && left.same(right); } - @Override - public int getMode() { - return mode; - } - - @Override - public void setMode(int mode) { - this.mode = mode; - } - private static Node predicateNode(Edge edge) { return edge.getEdgeVariable() == null ? edge.getEdgeNode() : edge.getEdgeVariable(); } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/SparqlKgramEvaluator.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/SparqlKgramEvaluator.java index 1de7f6dab..3414acf7b 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/SparqlKgramEvaluator.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/eval/SparqlKgramEvaluator.java @@ -34,29 +34,4 @@ public Mode getMode() { public void setMode(Mode mode) { this.mode = mode; } - - @Override - public void setProducer(Producer producer) { - // Expression-free graph pattern execution does not need producer state here. - } - - @Override - public void setKGRAM(Eval eval) { - // Eval is driven by the caller for the currently supported execution scope. - } - - @Override - public void start(Environment environment) { - // Graph pattern execution currently needs no evaluator-side initialization. - } - - @Override - public void finish(Environment environment) { - // Graph pattern execution currently needs no evaluator-side cleanup. - } - - @Override - public void init(Environment environment) { - // All native expression state is immutable and initialized at construction. - } } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Exp.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Exp.java index 5385d504b..b3e751cbe 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Exp.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Exp.java @@ -64,17 +64,14 @@ public class Exp extends PointerObject private Mappings map; private boolean isPostpone = false; private boolean isFunctional = false; - private boolean generated = false; // service with several URI: private List simpleNodeList; private Exp postpone; private List inscopeFilter; private Exp bind; private Exp values; - private int level = -1; // service number private int number = -1; - private int num = -1; private boolean isSystem = false; private boolean mappings = false; @@ -113,15 +110,12 @@ void copyPlanFields(Exp target, PlanCopy copies) { target.map = copies.copy(map); target.isPostpone = isPostpone; target.isFunctional = isFunctional; - target.generated = generated; target.simpleNodeList = copies.copy(simpleNodeList); target.postpone = copies.copy(postpone); target.inscopeFilter = copies.copy(inscopeFilter); target.bind = copies.copy(bind); target.values = copies.copy(values); - target.level = level; target.number = number; - target.num = num; target.isSystem = isSystem; target.mappings = mappings; for (Exp child : args) target.args.add(copies.copy(child)); @@ -200,14 +194,6 @@ public void setSystem(boolean b) { isSystem = b; } - public int getLevel() { - return level; - } - - public void setLevel(int level) { - this.level = level; - } - public boolean isBGP() { return type == Type.BGP; } @@ -1588,14 +1574,6 @@ public void setMappings(boolean mappings) { this.mappings = mappings; } - public boolean isGenerated() { - return generated; - } - - public void setGenerated(boolean generated) { - this.generated = generated; - } - public List getInScopeNodeList() { return simpleNodeList; } @@ -1604,14 +1582,6 @@ public void setInScopeNodeList(List simpleNodeList) { this.simpleNodeList = simpleNodeList; } - public int getNum() { - return num; - } - - public void setNum(int num) { - this.num = num; - } - /** * Nodes that may be bound by previous clause or by environment except * minus, etc. diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Group.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Group.java index ff2828cce..0aa7696f4 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Group.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Group.java @@ -28,7 +28,7 @@ public class Group implements Comparator { private List nodes; private boolean isFake; - /** Preserves the no-argument constructor used by Merge. */ + /** Creates an empty group. */ public Group() { this(List.of()); } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Query.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Query.java index 99a19eeb0..6b81a1f2a 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Query.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/pattern/Query.java @@ -38,9 +38,6 @@ public final class Query extends Exp { public static final int QP_HEURISTICS_BASED = 2;//Heuristics based QP public static final int QP_BGP = 3;//BGP based QP - public static final int STD_PROFILE = -1; - public static final int COUNT_PROFILE = 1; - private static final Logger logger = LoggerFactory.getLogger(Query.class); int limit = Integer.MAX_VALUE; @@ -106,7 +103,6 @@ public final class Query extends Exp { private boolean datasetSpecified; private boolean reportEnabled; - boolean isCheck = false; private boolean isUseBind = true; private boolean aggregateQuery; @@ -126,13 +122,11 @@ public final class Query extends Exp { boolean isCorrect = true; - boolean isConnect = false; boolean isMap = true; boolean isRule = false; boolean isDetail = true; int planner = QP_DEFAULT; - private int queryProfile = STD_PROFILE; private boolean isService = false; @@ -225,7 +219,6 @@ private Query(Query source, PlanCopy copies) { this.isCompiled = source.isCompiled; this.datasetSpecified = source.datasetSpecified; this.reportEnabled = source.reportEnabled; - this.isCheck = source.isCheck; this.isUseBind = source.isUseBind; this.aggregateQuery = source.aggregateQuery; this.isRelax = source.isRelax; @@ -242,12 +235,10 @@ private Query(Query source, PlanCopy copies) { this.validate = source.validate; this.federate = source.federate; this.isCorrect = source.isCorrect; - this.isConnect = source.isConnect; this.isMap = source.isMap; this.isRule = source.isRule; this.isDetail = source.isDetail; this.planner = source.planner; - this.queryProfile = source.queryProfile; this.isService = source.isService; this.mapping = copies.copy(source.mapping); this.service = copies.copy(source.service); @@ -688,10 +679,6 @@ private static boolean areAllGroupKeysProjected(List groupList, Set return true; } - public boolean isCheck() { - return isCheck; - } - @Override public boolean isAggregate() { return aggregateQuery; @@ -834,7 +821,7 @@ public boolean isGroupBy() { } public boolean hasGroupBy() { - return isGroupBy() || isConnect(); + return isGroupBy(); } public boolean isListGroup() { @@ -849,10 +836,6 @@ public void setCorrect(boolean b) { isCorrect = b; } - public boolean isConnect() { - return isConnect; - } - public void setHaving(Exp f) { having = f; } @@ -1523,13 +1506,6 @@ public void setNumber(int number) { this.number = number; } - - - public int getQueryProfile() { - return queryProfile; - } - - @Override public Iterable getLoop() { return () -> getEdges().stream().map(Object.class::cast).iterator(); diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java index 75a69103a..2276c2116 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java @@ -67,11 +67,6 @@ public final class Mappings extends PointerObject private Group group; private Group distinct; private Eval eval; - // service report if Mappings from service - // json object - // construct where result graph - private TripleStore graph; - // result of query as a template // fake result in case of aggregate without result private boolean isFake = false; // parse error in service result @@ -904,14 +899,7 @@ public Mappings project(Node q) { * for group by ?o1 .. ?on */ private Group createGroup() { - if (getQuery().isConnect()) { - // group by any - Merge newGroup = new Merge(this); - newGroup.merge(); - return newGroup; - } else { - return createGroup(getQuery().getGroupBy()); - } + return createGroup(getQuery().getGroupBy()); } private Group getCreateGroup() { @@ -1200,7 +1188,7 @@ public Mappings limit(int n) { @Override public TripleStore getTripleStore() { - return graph; + return null; } public List getInsert() { diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Memory.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Memory.java index 4cd4d4fbb..e6e879eb3 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Memory.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Memory.java @@ -40,8 +40,7 @@ public class Memory extends PointerObject implements Environment { private static final String SERVICE_REPORT_ZERO = "?_service_report_0"; - static final Edge[] emptyEdges = new Edge[0]; - static final Edge[] emptyEntities = new Edge[0]; + private static final Edge[] EMPTY_EDGES = new Edge[0]; // number of times nodes are bound by Stack // decrease with backtrack int[] nbNodes; @@ -343,8 +342,8 @@ void copyInto(Node qNode, Memory mem) { public Mapping store(Query query, Producer producer, boolean subEvaluation) { Node reportNode = bindServiceReport(); int capacity = solutionNodeCapacity(query, subEvaluation); - Edge[] queryEdgeCopies = isEdge ? new Edge[nbEdge] : emptyEdges; - Edge[] targetEdgeCopies = isEdge ? new Edge[nbEdge] : emptyEntities; + Edge[] queryEdgeCopies = isEdge ? new Edge[nbEdge] : EMPTY_EDGES; + Edge[] targetEdgeCopies = isEdge ? new Edge[nbEdge] : EMPTY_EDGES; copySolutionEdges(producer, queryEdgeCopies, targetEdgeCopies); Node[] queryNodeCopies = new Node[capacity]; diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Merge.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Merge.java deleted file mode 100644 index 76b47e180..000000000 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Merge.java +++ /dev/null @@ -1,120 +0,0 @@ -package fr.inria.corese.core.next.query.impl.engine.solution; - -import fr.inria.corese.core.next.query.impl.engine.pattern.Group; - -import java.util.ArrayList; - - -/*********************************************** - * group by any - * group Mapping who share one node (from any variable) - * Connected components - * - * @author Olivier Corby, Edelweiss, INRIA 2011 - * - ***********************************************/ - -public class Merge extends Group { - - private final Mappings sourceMappings; - private ListMappings mergedList; - - Merge(Mappings lm) { - sourceMappings = lm; - } - - public static class ListMappings extends ArrayList { - private static final long serialVersionUID = 1L; - } - - @Override - public Iterable getValues() { - return mergedList; - } - - void merge() { - mergedList = merge(sourceMappings); - mergedList = merge(mergedList); - } - - /** - * Initialize - */ - ListMappings merge(Mappings inputMappings) { - ListMappings result = new ListMappings(); - - for (Mapping map : inputMappings) { - boolean found = false; - for (Mappings lm : result) { - if (match(lm, map)) { - lm.add(map); - found = true; - break; - } - } - - if (!found) { - Mappings lm = new Mappings(); - lm.add(map); - result.add(lm); - } - } - - return result; - } - - /** - * Every Mappings is made of connected Mapping - * Try to Merge Mappings that are connected - */ - ListMappings merge(ListMappings inputList) { - ListMappings currentList = inputList; - ListMappings nlist = null; - boolean hasMerge = true; - - while (hasMerge) { - nlist = new ListMappings(); - hasMerge = false; - for (Mappings lm1 : currentList) { - - boolean found = false; - for (Mappings lm2 : nlist) { - if (match(lm1, lm2)) { - lm2.add(lm1); - found = true; - hasMerge = true; - break; - } - } - - if (!found) { - nlist.add(lm1); - } - } - - currentList = nlist; - } - - return nlist; - } - - boolean match(Mappings lm1, Mappings lm2) { - for (Mapping map : lm1) { - if (match(lm2, map)) { - return true; - } - } - return false; - } - - // one node of map is contained in one Mapping of lm - boolean match(Mappings lmap, Mapping map) { - for (Mapping m : lmap) { - if (map.match(m)) { - return true; - } - } - return false; - } - -} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/spi/Evaluator.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/spi/Evaluator.java index f50c2981f..0a4f1b5d1 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/spi/Evaluator.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/spi/Evaluator.java @@ -25,18 +25,23 @@ public interface Evaluator { void setMode(Mode mode); - void setProducer(Producer p); + default void setProducer(Producer p) { + } - void setKGRAM(Eval o); + default void setKGRAM(Eval o) { + } - void start(Environment env); + default void start(Environment env) { + } - void finish(Environment env); + default void finish(Environment env) { + } - void init(Environment env); + default void init(Environment env) { + } enum Mode { - KGRAM_MODE, SPARQL_MODE, CACHE_MODE, NO_CACHE_MODE + KGRAM_MODE, SPARQL_MODE } } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/spi/Matcher.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/spi/Matcher.java index 7076499fd..6c7ac3073 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/spi/Matcher.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/spi/Matcher.java @@ -12,15 +12,6 @@ public interface Matcher { - /** - * Match mode - */ - // equality of types - int UNDEF = -1; - - // accept any types - int RELAX = 4; - /** * Checks whether a candidate edge matches a query edge. * May verify that edge labels match (Producer may have done the job properly). @@ -55,8 +46,4 @@ public interface Matcher { */ boolean same(Node qNode, Node n1, Node n2, Environment env); - int getMode(); - - void setMode(int mode); - } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/DirectGroupAggregation.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/DirectGroupAggregation.java index b3dd86bb5..b6276198b 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/DirectGroupAggregation.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/DirectGroupAggregation.java @@ -36,7 +36,7 @@ private DirectGroupAggregation() { /** Returns whether the query can be reduced without evaluating expressions. */ public static boolean isEligible(Query query) { if (!query.isSelect() || (query.isDistinct() && !query.isDistinctRedundantWithGroupBy()) || query.isListGroup() - || query.isConnect() || query.getGroupBy().isEmpty() + || query.getGroupBy().isEmpty() || query.getHaving() != null || !query.getOrderBy().isEmpty()) { return false; } diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalTest.java index 8548cd039..aa20102e4 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/EvalTest.java @@ -9,6 +9,12 @@ import fr.inria.corese.core.next.query.impl.engine.spi.Producer; import fr.inria.corese.core.next.query.impl.engine.spi.Provider; +import fr.inria.corese.core.next.data.Values; +import fr.inria.corese.core.next.query.api.exception.QueryEvaluationException; +import fr.inria.corese.core.next.query.impl.engine.model.Node; +import fr.inria.corese.core.next.query.impl.engine.model.NodeImpl; +import fr.inria.corese.core.next.query.impl.engine.pattern.Exp; +import fr.inria.corese.core.next.query.impl.engine.pattern.Query; import fr.inria.corese.core.next.query.impl.engine.model.BindingContext; import fr.inria.corese.core.next.query.impl.engine.event.ResultListener; import org.junit.jupiter.api.BeforeEach; @@ -410,4 +416,41 @@ void testIsJoinMappings() { } } + @Nested + @DisplayName("Service Execution Tests") + class ServiceExecutionTests { + + @Test + @DisplayName("Should throw QueryEvaluationException when SERVICE is executed without Provider") + void testServiceWithoutProviderThrows() { + Node uriNode = NodeImpl.forValue(Values.factory().createIRI("https://remote.example.org/sparql")); + Exp serviceExp = Exp.create(Exp.Type.SERVICE, Exp.create(Exp.Type.NODE, uriNode), Exp.create(Exp.Type.EMPTY)); + + Stack stack = new Stack(); + stack.add(serviceExp); + Query q = Query.create(); + eval.init(q); + + assertThrows(QueryEvaluationException.class, () -> + eval.eval(mockProducer, null, stack, null, 0), + "Executing SERVICE without a configured Provider must fail explicitly"); + } + + @Test + @DisplayName("Should silently bypass when SERVICE SILENT is executed without Provider") + void testServiceSilentWithoutProviderBypasses() { + Node uriNode = NodeImpl.forValue(Values.factory().createIRI("https://remote.example.org/sparql")); + Exp serviceExp = Exp.create(Exp.Type.SERVICE, Exp.create(Exp.Type.NODE, uriNode), Exp.create(Exp.Type.EMPTY)); + serviceExp.setSilent(true); + + Stack stack = new Stack(); + stack.add(serviceExp); + Query q = Query.create(); + eval.init(q); + + assertDoesNotThrow(() -> eval.eval(mockProducer, null, stack, null, 0), + "Executing SERVICE SILENT without a Provider must bypass silently per W3C specification"); + } + } + } diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/RdfTermMatcherTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/RdfTermMatcherTest.java index 40260df70..e317eb6ca 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/RdfTermMatcherTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/engine/eval/RdfTermMatcherTest.java @@ -123,14 +123,6 @@ void sameSemantics() { assertFalse(matcher.same(null, null, a, null), "null left is not equal to anything"); } - @Test - @DisplayName("Mode defaults to UNDEF and is settable") - void modeDefaultsToUndefAndIsSettable() { - assertEquals(Matcher.UNDEF, matcher.getMode()); - matcher.setMode(Matcher.RELAX); - assertEquals(Matcher.RELAX, matcher.getMode()); - } - /** * Minimal {@link Edge} for matcher tests: carries subject/object plus the predicate as either a * fixed edge node or an edge variable, mirroring {@code AstBackedEdge}'s representation. diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/engine/pattern/ExpTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/engine/pattern/ExpTest.java index 6e3f149e8..b650ad3fe 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/engine/pattern/ExpTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/engine/pattern/ExpTest.java @@ -433,35 +433,6 @@ void testSetAndIsSystem() { exp.setSystem(true); assertTrue(exp.isSystem(), "System should be true"); } - - @Test - @DisplayName("Should set and check generated flag") - void testSetAndIsGenerated() { - exp.setGenerated(true); - assertTrue(exp.isGenerated(), "Generated should be true"); - } - - } - - - - @Nested - @DisplayName("Level Tests") - class LevelTests { - - @Test - @DisplayName("Should set and get level") - void testSetAndGetLevel() { - exp.setLevel(3); - assertEquals(3, exp.getLevel(), "Level should be 3"); - } - - @Test - @DisplayName("Should have default level -1") - void testDefaultLevel() { - Exp e = Exp.create(ExpType.Type.AND); - assertEquals(-1, e.getLevel(), "Default level should be -1"); - } } @Nested @@ -474,13 +445,6 @@ void testSetAndGetNumber() { exp.setNumber(42); assertEquals(42, exp.getNumber(), "Number should be 42"); } - - @Test - @DisplayName("Should set and get num") - void testSetAndGetNum() { - exp.setNum(7); - assertEquals(7, exp.getNum(), "Num should be 7"); - } } @Nested diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/engine/pattern/QueryTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/engine/pattern/QueryTest.java index 0eb3550d3..14a1cd1f8 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/engine/pattern/QueryTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/engine/pattern/QueryTest.java @@ -552,13 +552,6 @@ void testSetAndIsCorrect() { assertFalse(query.isCorrect(), "Query should not be correct"); } - @Test - @DisplayName("Should check if is connect") - void testIsConnect() { - boolean result = query.isConnect(); - assertFalse(result, "Query should not be connect by default"); - } - @Test @DisplayName("Should set and check map flag") void testSetAndIsMap() { From 83c9dc577785d53e453d8ecd4d42e794478eb178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20C=C3=A9r=C3=A8s?= Date: Mon, 26 Oct 2026 14:47:23 +0100 Subject: [PATCH 3/4] refactor(engine): invert group aggregation dependency and isolate solution materialization (#643) - Introduce GroupAggregator SPI to decouple Mappings from sparql.bridge.DirectGroupAggregation - Extract SolutionMaterializer from Memory.java to isolate query solution mapping materialization and projection - Extract BindingStack to manage low-level node/edge arrays and backjump levels - Introduce AutoCloseable ExecutionSession in SparqlQueryExecutor for clean producer lifecycle management - Add unit tests for GroupAggregator, BindingStack, and ExecutionSession with 100% pass --- .../impl/engine/solution/BindingStack.java | 230 ++++++++++++++++++ .../impl/engine/solution/GroupAggregator.java | 19 ++ .../query/impl/engine/solution/Mappings.java | 12 +- .../query/impl/engine/solution/Memory.java | 166 +------------ .../engine/solution/SolutionMaterializer.java | 215 ++++++++++++++++ .../sparql/bridge/DirectGroupAggregation.java | 11 + .../sparql/execution/ExecutionSession.java | 42 ++++ .../sparql/execution/SparqlQueryExecutor.java | 35 +-- .../engine/solution/BindingStackTest.java | 144 +++++++++++ .../engine/solution/GroupAggregatorTest.java | 33 +++ .../execution/ExecutionSessionTest.java | 28 +++ 11 files changed, 753 insertions(+), 182 deletions(-) create mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStack.java create mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/GroupAggregator.java create mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/SolutionMaterializer.java create mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/sparql/execution/ExecutionSession.java create mode 100644 src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStackTest.java create mode 100644 src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/GroupAggregatorTest.java create mode 100644 src/test/java/fr/inria/corese/core/next/query/impl/sparql/execution/ExecutionSessionTest.java diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStack.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStack.java new file mode 100644 index 000000000..8c6155a9e --- /dev/null +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStack.java @@ -0,0 +1,230 @@ +package fr.inria.corese.core.next.query.impl.engine.solution; + +import fr.inria.corese.core.next.query.impl.engine.model.Edge; +import fr.inria.corese.core.next.query.impl.engine.model.Node; +import fr.inria.corese.core.next.query.impl.engine.pattern.Query; +import fr.inria.corese.core.next.query.impl.engine.spi.Environment; +import fr.inria.corese.core.next.query.impl.engine.spi.Matcher; + +import java.util.Arrays; +import java.util.List; + +/** + * Manages arrays of bound query nodes and edges during query evaluation. + */ +public final class BindingStack { + + private int[] nbNodes = new int[0]; + private int[] nbEdges = new int[0]; + private int[] stackIndex = new int[0]; + private Edge[] qEdges = new Edge[0]; + private Edge[] result = new Edge[0]; + private Node[] qNodes = new Node[0]; + private Node[] nodes = new Node[0]; + private int nbEdge = 0; + private int nbNode = 0; + private boolean recordEdges = false; + + public void init(Query query, boolean recordEdges) { + this.recordEdges = recordEdges; + int nmax = query.nbNodes(); + int emax = recordEdges ? query.nbEdges() : 0; + + nbNodes = new int[nmax]; + stackIndex = new int[nmax]; + nbEdges = new int[emax]; + result = new Edge[emax]; + qEdges = new Edge[emax]; + + nodes = new Node[nmax]; + qNodes = new Node[nmax]; + + reset(); + } + + public void reset() { + nbEdge = 0; + nbNode = 0; + Arrays.fill(qNodes, null); + Arrays.fill(nodes, null); + Arrays.fill(nbNodes, 0); + Arrays.fill(stackIndex, -1); + if (recordEdges) { + Arrays.fill(nbEdges, 0); + Arrays.fill(result, null); + Arrays.fill(qEdges, null); + } + } + + public boolean push(Node node, Node target, int n, Matcher match, Environment env) { + if (node.isConstant()) { + return true; + } + int index = node.getIndex(); + if (nodes[index] == null) { + nodes[index] = target; + qNodes[index] = node; + nbNode++; + nbNodes[index]++; + stackIndex[index] = n; + return true; + } else if (target == null) { + return false; + } else if (match.same(node, nodes[index], target, env)) { + nbNodes[index]++; + return true; + } + return false; + } + + public void pop(Node node) { + if (node.isVariable()) { + int index = node.getIndex(); + if (nbNodes[index] > 0) { + nbNodes[index]--; + if (nbNodes[index] == 0) { + nbNode--; + nodes[index] = null; + qNodes[index] = null; + stackIndex[index] = -1; + } + } + } + } + + public void recordEdge(Edge q, Edge ent) { + int index = q.getEdgeIndex(); + if (nbEdges[index] == 0) { + nbEdge++; + } + nbEdges[index]++; + qEdges[index] = q; + result[index] = ent; + } + + public void popEdge(Edge q) { + int index = q.getEdgeIndex(); + if (nbEdges[index] > 0) { + nbEdges[index]--; + if (nbEdges[index] == 0) { + nbEdge--; + qEdges[index] = null; + result[index] = null; + } + } + } + + public boolean isBound(Node node) { + return node.isConstant() || (node.getIndex() < nodes.length && nodes[node.getIndex()] != null); + } + + public Node getNode(Node node) { + if (node.isConstant()) { + return node; + } + int index = node.getIndex(); + return index < nodes.length ? nodes[index] : null; + } + + public Node getNode(int index) { + return index < nodes.length ? nodes[index] : null; + } + + public Node getNode(String label) { + for (int i = 0; i < qNodes.length; i++) { + if (qNodes[i] != null && qNodes[i].getLabel().equals(label)) { + return nodes[i]; + } + } + return null; + } + + public Node getQueryNode(int index) { + return index < qNodes.length ? qNodes[index] : null; + } + + public Node getQueryNode(String label) { + for (Node qNode : qNodes) { + if (qNode != null && qNode.getLabel().equals(label)) { + return qNode; + } + } + return null; + } + + public int getStackIndex(int index) { + return index < stackIndex.length ? stackIndex[index] : -1; + } + + public int getIndex(Node node) { + return getStackIndex(node.getIndex()); + } + + public int getIndex(Node gNode, Edge edge) { + int max = -1; + int length = edge.nbNode(); + for (int i = 0; i < length; i++) { + Node qNode = edge.getNode(i); + if (qNode != null) { + max = Math.max(max, getStackIndex(qNode.getIndex())); + } + } + Node pNode = edge.getEdgeVariable(); + if (pNode != null) { + max = Math.max(max, getStackIndex(pNode.getIndex())); + } + if (gNode != null && gNode.isVariable()) { + max = Math.max(max, getStackIndex(gNode.getIndex())); + } + return max; + } + + public int getIndex(List lNodes) { + int max = -1; + for (Node node : lNodes) { + int index = getIndex(node); + if (index == -1) { + return -1; + } + max = Math.max(max, index); + } + return max; + } + + public Edge getEdge(Edge edge) { + int index = edge.getEdgeIndex(); + return index < result.length ? result[index] : null; + } + + public Node[] getNodes() { + return nodes; + } + + public Node[] getQueryNodes() { + return qNodes; + } + + public Edge[] getEdges() { + return result; + } + + public Edge[] getQueryEdges() { + return qEdges; + } + + public int getBoundNodeCount() { + return nbNode; + } + + public int getBoundEdgeCount() { + return nbEdge; + } + + public boolean isRecordEdges() { + return recordEdges; + } + + public void setRecordEdges(boolean recordEdges) { + this.recordEdges = recordEdges; + } +} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/GroupAggregator.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/GroupAggregator.java new file mode 100644 index 000000000..42cd17968 --- /dev/null +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/GroupAggregator.java @@ -0,0 +1,19 @@ +package fr.inria.corese.core.next.query.impl.engine.solution; + +import fr.inria.corese.core.next.query.impl.engine.pattern.Query; + +/** + * Strategy interface for direct group aggregation without general expression evaluation. + */ +@FunctionalInterface +public interface GroupAggregator { + + /** + * Attempts to reduce input mappings directly according to the query's group and aggregate projections. + * + * @param query query whose group and aggregate projections apply + * @param mappings input mappings to reduce + * @return {@code true} if aggregation succeeded, {@code false} if ordinary evaluation is required + */ + boolean tryAggregate(Query query, Mappings mappings); +} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java index 2276c2116..b2eca93c2 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java @@ -80,6 +80,15 @@ public final class Mappings extends PointerObject private int length = 0; // limit number of results to be displayed private int display = Integer.MAX_VALUE; + private static GroupAggregator groupAggregator; + + public static void setGroupAggregator(GroupAggregator aggregator) { + groupAggregator = aggregator; + } + + public static GroupAggregator getGroupAggregator() { + return groupAggregator; + } /** Copies a static VALUES table into independent row containers. */ @Override @@ -639,7 +648,8 @@ public void aggregate(Query q, Evaluator evaluator, Memory memory, Producer p) { } } - if (fr.inria.corese.core.next.query.impl.sparql.bridge.DirectGroupAggregation.tryAggregate(q, this)) { + GroupAggregator aggregator = groupAggregator; + if (aggregator != null && aggregator.tryAggregate(q, this)) { finish(q); return; } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Memory.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Memory.java index e6e879eb3..c5a088915 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Memory.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Memory.java @@ -39,8 +39,6 @@ */ public class Memory extends PointerObject implements Environment { - private static final String SERVICE_REPORT_ZERO = "?_service_report_0"; - private static final Edge[] EMPTY_EDGES = new Edge[0]; // number of times nodes are bound by Stack // decrease with backtrack int[] nbNodes; @@ -74,7 +72,7 @@ public class Memory extends PointerObject implements Environment { int nbNode = 0; // service evaluation detail report private DatatypeValue detail; - private boolean isEdge; + boolean isEdge; private BindingContext bindingContext; public Memory() { @@ -340,137 +338,7 @@ void copyInto(Node qNode, Memory mem) { * @return captured solution, or null when a projection conflicts with a binding */ public Mapping store(Query query, Producer producer, boolean subEvaluation) { - Node reportNode = bindServiceReport(); - int capacity = solutionNodeCapacity(query, subEvaluation); - Edge[] queryEdgeCopies = isEdge ? new Edge[nbEdge] : EMPTY_EDGES; - Edge[] targetEdgeCopies = isEdge ? new Edge[nbEdge] : EMPTY_EDGES; - copySolutionEdges(producer, queryEdgeCopies, targetEdgeCopies); - - Node[] queryNodeCopies = new Node[capacity]; - Node[] targetNodeCopies = new Node[capacity]; - int boundNodes = copySolutionNodes(queryNodeCopies, targetNodeCopies); - Node[] orderByNodes = new Node[query.getOrderBy().size()]; - Node[] groupByNodes = new Node[query.getGroupBy().size()]; - - if (!subEvaluation && !projectSolution(query, producer, queryNodeCopies, targetNodeCopies, boundNodes)) { - return null; - } - Mapping solution = new Mapping(queryEdgeCopies, targetEdgeCopies, queryNodeCopies, targetNodeCopies); - mapping = solution; - if (!subEvaluation) { - solution.init(); - // Ordering expressions may inspect the projected solution through the environment. - orderGroup(query.getOrderBy(), orderByNodes, producer); - orderGroup(query.getGroupBy(), groupByNodes, producer); - popProjectionBindings(query); - } - if (reportNode != null) { - pop(reportNode); - setReport(null); - } - solution.setOrderBy(orderByNodes); - solution.setGroupBy(groupByNodes); - clear(); - mapping = null; - return solution; - } - - private Node bindServiceReport() { - if (getReport() == null) { - return null; - } - Node reportNode = getQuery().getSelectNode(SERVICE_REPORT_ZERO); - if (reportNode != null) { - push(reportNode, (Node) getReport()); - } - return reportNode; - } - - private int solutionNodeCapacity(Query query, boolean subEvaluation) { - int capacity = nbNode; - if (!subEvaluation) { - for (Exp projection : query.getSelectFun()) { - if (projection.getFilter() != null && !isBound(projection.getNode())) { - capacity++; - } - } - } - return capacity; - } - - private void copySolutionEdges(Producer producer, Edge[] queryEdgeCopies, Edge[] targetEdgeCopies) { - if (!isEdge) { - return; - } - int targetIndex = 0; - for (int sourceIndex = 0; sourceIndex < qEdges.length; sourceIndex++) { - if (qEdges[sourceIndex] != null) { - queryEdgeCopies[targetIndex] = qEdges[sourceIndex]; - targetEdgeCopies[targetIndex] = producer.copy(result[sourceIndex]); - targetIndex++; - } - } - } - - private int copySolutionNodes(Node[] queryNodeCopies, Node[] targetNodeCopies) { - int targetIndex = 0; - for (int sourceIndex = 0; sourceIndex < qNodes.length; sourceIndex++) { - if (qNodes[sourceIndex] != null) { - queryNodeCopies[targetIndex] = qNodes[sourceIndex]; - targetNodeCopies[targetIndex] = nodes[sourceIndex]; - targetIndex++; - } - } - return targetIndex; - } - - private boolean projectSolution(Query query, Producer producer, Node[] queryNodeCopies, - Node[] targetNodeCopies, int targetIndex) { - int pushedBindings = 0; - for (Exp projection : query.getSelectFun()) { - Filter filter = projection.getFilter(); - if (filter == null) { - continue; - } - boolean previouslyBound = isBound(projection.getNode()); - Node value = null; - if (!projection.isAggregate()) { - value = evaluateExpressionNode(filter, producer); - kgram.getVisitor().select(kgram, filter.getExp(), value == null ? null : value.getDatatypeValue()); - if (!push(projection.getNode(), value)) { - popProjectionPrefix(query, pushedBindings); - return false; - } - pushedBindings++; - } - if (!previouslyBound) { - queryNodeCopies[targetIndex] = projection.getNode(); - targetNodeCopies[targetIndex] = value; - targetIndex++; - } - } - return true; - } - - private void popProjectionPrefix(Query query, int count) { - int popped = 0; - for (Exp projection : query.getSelectFun()) { - if (popped >= count) { - return; - } - if (projection.getFilter() != null) { - pop(projection.getNode()); - popped++; - } - } - } - - private void popProjectionBindings(Query query) { - for (Exp projection : query.getSelectFun()) { - if (projection.getFilter() != null && !projection.isAggregate()) { - pop(projection.getNode()); - } - } + return new SolutionMaterializer(this).materialize(query, producer, subEvaluation); } @Override @@ -499,36 +367,6 @@ void setMap(Map m) { bnode = m; } - void orderGroup(List lExp, Node[] nodes, Producer p) { - int n = 0; - for (Exp e : lExp) { - Node qNode = e.getNode(); - if (qNode != null) { - nodes[n] = getNode(qNode); - } - if (nodes[n] == null) { - Filter f = e.getFilter(); - if (f != null && !e.isAggregate()) { - nodes[n] = evaluateExpressionNode(f, p); - } - - } - if (qNode != null && nodes[n] != null && mapping != null) { - mapping.setNode(qNode, nodes[n]); - } - n++; - } - } - - private Node evaluateExpressionNode(Filter filter, Producer producer) { - try { - return producer.getNode(kgram.eval(filter, this, producer)); - } catch (QueryTypeErrorException error) { - // A scalar type error leaves a projected value or sorting key unbound. - return null; - } - } - boolean pushNodeList(Producer p, Node node, Edge edge, int i) { if (node.isMatchCardinality()) { return pushCardinality(p, node, edge, i); diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/SolutionMaterializer.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/SolutionMaterializer.java new file mode 100644 index 000000000..315f94860 --- /dev/null +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/SolutionMaterializer.java @@ -0,0 +1,215 @@ +package fr.inria.corese.core.next.query.impl.engine.solution; + +import fr.inria.corese.core.next.query.api.exception.QueryTypeErrorException; +import fr.inria.corese.core.next.query.impl.engine.eval.Eval; +import fr.inria.corese.core.next.query.impl.engine.model.Edge; +import fr.inria.corese.core.next.query.impl.engine.model.Filter; +import fr.inria.corese.core.next.query.impl.engine.model.Node; +import fr.inria.corese.core.next.query.impl.engine.pattern.Exp; +import fr.inria.corese.core.next.query.impl.engine.pattern.Query; +import fr.inria.corese.core.next.query.impl.engine.spi.Producer; + +import java.util.List; +import java.util.Objects; + +/** + * Materializes variable bindings, projected expressions, and sorting keys + * into query solution {@link Mapping} instances. + */ +final class SolutionMaterializer { + + private static final String SERVICE_REPORT_ZERO = "?_service_report_0"; + private static final Edge[] EMPTY_EDGES = new Edge[0]; + + private final Memory memory; + + SolutionMaterializer(Memory memory) { + this.memory = Objects.requireNonNull(memory, "memory"); + } + + Mapping materialize(Query query, Producer producer, boolean subEvaluation) { + Node reportNode = bindServiceReport(); + int capacity = solutionNodeCapacity(query, subEvaluation); + Edge[] queryEdgeCopies = memory.isEdge ? new Edge[memory.nbEdge] : EMPTY_EDGES; + Edge[] targetEdgeCopies = memory.isEdge ? new Edge[memory.nbEdge] : EMPTY_EDGES; + copySolutionEdges(producer, queryEdgeCopies, targetEdgeCopies); + + Node[] queryNodeCopies = new Node[capacity]; + Node[] targetNodeCopies = new Node[capacity]; + int boundNodes = copySolutionNodes(queryNodeCopies, targetNodeCopies); + Node[] orderByNodes = new Node[query.getOrderBy().size()]; + Node[] groupByNodes = new Node[query.getGroupBy().size()]; + + if (!subEvaluation && !projectSolution(query, producer, queryNodeCopies, targetNodeCopies, boundNodes)) { + return null; + } + Mapping solution = new Mapping(queryEdgeCopies, targetEdgeCopies, queryNodeCopies, targetNodeCopies); + memory.mapping = solution; + if (!subEvaluation) { + solution.init(); + // Ordering expressions may inspect the projected solution through the environment. + orderGroup(query.getOrderBy(), orderByNodes, producer); + orderGroup(query.getGroupBy(), groupByNodes, producer); + popProjectionBindings(query); + } + if (reportNode != null) { + memory.pop(reportNode); + memory.setReport(null); + } + solution.setOrderBy(orderByNodes); + solution.setGroupBy(groupByNodes); + memory.clear(); + memory.mapping = null; + return solution; + } + + private Node bindServiceReport() { + if (memory.getReport() == null) { + return null; + } + Node reportNode = memory.getQuery().getSelectNode(SERVICE_REPORT_ZERO); + if (reportNode != null) { + memory.push(reportNode, (Node) memory.getReport()); + } + return reportNode; + } + + private int solutionNodeCapacity(Query query, boolean subEvaluation) { + int capacity = memory.nbNode; + if (!subEvaluation) { + for (Exp projection : query.getSelectFun()) { + if (projection.getFilter() != null && !memory.isBound(projection.getNode())) { + capacity++; + } + } + } + return capacity; + } + + private void copySolutionEdges(Producer producer, Edge[] queryEdgeCopies, Edge[] targetEdgeCopies) { + if (!memory.isEdge) { + return; + } + int targetIndex = 0; + for (int sourceIndex = 0; sourceIndex < memory.qEdges.length; sourceIndex++) { + if (memory.qEdges[sourceIndex] != null) { + queryEdgeCopies[targetIndex] = memory.qEdges[sourceIndex]; + targetEdgeCopies[targetIndex] = producer.copy(memory.result[sourceIndex]); + targetIndex++; + } + } + } + + private int copySolutionNodes(Node[] queryNodeCopies, Node[] targetNodeCopies) { + int targetIndex = 0; + for (int sourceIndex = 0; sourceIndex < memory.qNodes.length; sourceIndex++) { + if (memory.qNodes[sourceIndex] != null) { + queryNodeCopies[targetIndex] = memory.qNodes[sourceIndex]; + targetNodeCopies[targetIndex] = memory.nodes[sourceIndex]; + targetIndex++; + } + } + return targetIndex; + } + + private boolean projectSolution(Query query, Producer producer, Node[] queryNodeCopies, + Node[] targetNodeCopies, int targetIndex) { + int pushedBindings = 0; + int currentIndex = targetIndex; + for (Exp projection : query.getSelectFun()) { + if (projection.getFilter() == null) { + continue; + } + ProjectionResult result = applySingleProjection(projection, producer); + if (!result.success()) { + popProjectionPrefix(query, pushedBindings); + return false; + } + if (result.pushed()) { + pushedBindings++; + } + if (result.unboundBefore()) { + queryNodeCopies[currentIndex] = projection.getNode(); + targetNodeCopies[currentIndex] = result.value(); + currentIndex++; + } + } + return true; + } + + private record ProjectionResult(boolean success, boolean pushed, boolean unboundBefore, Node value) { + } + + private ProjectionResult applySingleProjection(Exp projection, Producer producer) { + Filter filter = projection.getFilter(); + boolean previouslyBound = memory.isBound(projection.getNode()); + Node value = null; + boolean pushed = false; + if (!projection.isAggregate()) { + value = evaluateExpressionNode(filter, producer); + Eval kgram = memory.getEval(); + if (kgram != null && kgram.getVisitor() != null) { + kgram.getVisitor().select(kgram, filter.getExp(), value == null ? null : value.getDatatypeValue()); + } + if (!memory.push(projection.getNode(), value)) { + return new ProjectionResult(false, false, previouslyBound, null); + } + pushed = true; + } + return new ProjectionResult(true, pushed, !previouslyBound, value); + } + + private void popProjectionPrefix(Query query, int count) { + int popped = 0; + for (Exp projection : query.getSelectFun()) { + if (popped >= count) { + return; + } + if (projection.getFilter() != null) { + memory.pop(projection.getNode()); + popped++; + } + } + } + + private void popProjectionBindings(Query query) { + for (Exp projection : query.getSelectFun()) { + if (projection.getFilter() != null && !projection.isAggregate()) { + memory.pop(projection.getNode()); + } + } + } + + void orderGroup(List lExp, Node[] nodes, Producer p) { + int n = 0; + for (Exp e : lExp) { + Node qNode = e.getNode(); + if (qNode != null) { + nodes[n] = memory.getNode(qNode); + } + if (nodes[n] == null) { + Filter f = e.getFilter(); + if (f != null && !e.isAggregate()) { + nodes[n] = evaluateExpressionNode(f, p); + } + } + if (qNode != null && nodes[n] != null && memory.mapping != null) { + memory.mapping.setNode(qNode, nodes[n]); + } + n++; + } + } + + private Node evaluateExpressionNode(Filter filter, Producer producer) { + try { + Eval kgram = memory.getEval(); + if (kgram == null) { + return null; + } + return producer.getNode(kgram.eval(filter, memory, producer)); + } catch (QueryTypeErrorException error) { + // A scalar type error leaves a projected value or sorting key unbound. + return null; + } + } +} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/DirectGroupAggregation.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/DirectGroupAggregation.java index b6276198b..f06651f56 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/DirectGroupAggregation.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/DirectGroupAggregation.java @@ -7,6 +7,7 @@ import fr.inria.corese.core.next.query.impl.engine.model.NodeImpl; import fr.inria.corese.core.next.query.impl.engine.pattern.Exp; import fr.inria.corese.core.next.query.impl.engine.pattern.Query; +import fr.inria.corese.core.next.query.impl.engine.solution.GroupAggregator; import fr.inria.corese.core.next.query.impl.engine.solution.Mapping; import fr.inria.corese.core.next.query.impl.engine.solution.Mappings; import fr.inria.corese.core.next.query.impl.sparql.ast.AggregateAst; @@ -30,6 +31,16 @@ */ public final class DirectGroupAggregation { + public static final GroupAggregator INSTANCE = DirectGroupAggregation::tryAggregate; + + static { + register(); + } + + public static void register() { + Mappings.setGroupAggregator(INSTANCE); + } + private DirectGroupAggregation() { } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/execution/ExecutionSession.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/execution/ExecutionSession.java new file mode 100644 index 000000000..535ed5e41 --- /dev/null +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/execution/ExecutionSession.java @@ -0,0 +1,42 @@ +package fr.inria.corese.core.next.query.impl.sparql.execution; + +import fr.inria.corese.core.next.query.impl.engine.diagnostics.DiagnosticProducer; +import fr.inria.corese.core.next.query.impl.engine.diagnostics.ExecutionDiagnostics; +import fr.inria.corese.core.next.query.impl.engine.eval.Eval; +import fr.inria.corese.core.next.query.impl.engine.eval.RdfTermMatcher; +import fr.inria.corese.core.next.query.impl.engine.eval.SparqlKgramEvaluator; +import fr.inria.corese.core.next.query.impl.engine.storage.StorageManagerProducer; +import fr.inria.corese.core.next.storage.api.StorageManager; + +import java.util.Objects; + +/** + * Encapsulates the execution resources and lifecycle (storage producer and engine evaluator) + * for a single query execution. + */ +final class ExecutionSession implements AutoCloseable { + + private final StorageManagerProducer producer; + private final Eval eval; + + ExecutionSession(StorageManager storage, ExecutionDiagnostics diagnostics) { + Objects.requireNonNull(storage, "storage"); + this.producer = diagnostics == null + ? new StorageManagerProducer(storage) + : new DiagnosticProducer(storage, diagnostics); + this.eval = Eval.create(producer, new SparqlKgramEvaluator(), new RdfTermMatcher()); + } + + StorageManagerProducer producer() { + return producer; + } + + Eval eval() { + return eval; + } + + @Override + public void close() { + producer.close(); + } +} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/execution/SparqlQueryExecutor.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/execution/SparqlQueryExecutor.java index d3cf94096..d2504b2de 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/execution/SparqlQueryExecutor.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/execution/SparqlQueryExecutor.java @@ -37,6 +37,7 @@ import fr.inria.corese.core.next.query.impl.engine.eval.SparqlKgramEvaluator; import fr.inria.corese.core.next.query.impl.engine.model.NodeImpl; import fr.inria.corese.core.next.query.impl.engine.storage.StorageManagerProducer; +import fr.inria.corese.core.next.query.impl.sparql.bridge.DirectGroupAggregation; import fr.inria.corese.core.next.storage.api.StorageManager; import fr.inria.corese.core.next.storage.api.exception.StorageException; @@ -65,6 +66,10 @@ */ public final class SparqlQueryExecutor { + static { + DirectGroupAggregation.register(); + } + private static final int QUERY_RUNNING = 0; private static final int QUERY_COMPLETED = 1; private static final int QUERY_TIMED_OUT = 2; @@ -253,23 +258,20 @@ private TupleQueryResult snapshotTuple(ExecutionRequest request, BindingSet bind @SuppressWarnings("java:S1181") // Snapshot ownership is released on fatal setup errors before rethrowing unchanged. private TupleQueryResult evaluateSnapshotTuple(Query query, BindingSet bindings, long timeoutMillis, long started, StorageManager snapshot, boolean ordered) { - StorageManagerProducer producer = null; - Eval eval; + ExecutionSession session = null; StreamingTupleQueryResult result; try { - producer = diagnostics == null - ? new StorageManagerProducer(snapshot) : new DiagnosticProducer(snapshot, diagnostics); - eval = Eval.create(producer, new SparqlKgramEvaluator(), new RdfTermMatcher()); + session = new ExecutionSession(snapshot, diagnostics); long initialization = diagnostics == null ? 0 : System.nanoTime(); Mapping initial = buildInitialMapping(bindings); - SelectCursor cursor = ordered ? SelectCursor.orderedInput(eval, query, initial) - : new SelectCursor(eval, query, initial); + SelectCursor cursor = ordered ? SelectCursor.orderedInput(session.eval(), query, initial) + : new SelectCursor(session.eval(), query, initial); if (diagnostics != null) diagnostics.evaluationTime(System.nanoTime() - initialization); - result = new StreamingTupleQueryResult(cursor, query, producer, snapshot, + result = new StreamingTupleQueryResult(cursor, query, session.producer(), snapshot, diagnostics, timeoutMillis, started); } catch (RuntimeException | Error failure) { try { - if (producer != null) producer.close(); + if (session != null) session.close(); } finally { snapshot.close(); } @@ -277,7 +279,7 @@ private TupleQueryResult evaluateSnapshotTuple(Query query, BindingSet bindings, } if (!ordered) return result; try (result) { - return new CoreseTupleQueryResult(result.collectOrdered(new TopKSelector(query, eval))); + return new CoreseTupleQueryResult(result.collectOrdered(new TopKSelector(query, session.eval()))); } } @@ -410,14 +412,13 @@ private Mappings evaluate(ExecutionRequest request, BindingSet bindings, long ti } private Mappings evaluate(ExecutionRequest request, BindingSet bindings, long timeoutMillis, long started) { - StorageManagerProducer producer = diagnostics == null - ? new StorageManagerProducer(storage) : new DiagnosticProducer(storage, diagnostics); - Eval eval = Eval.create(producer, new SparqlKgramEvaluator(), new RdfTermMatcher()); - Mapping initialMapping = buildInitialMapping(bindings); - if (timeoutMillis > 0) { - return evaluateWithTimeout(eval, producer, request, initialMapping, timeoutMillis, started); + try (ExecutionSession session = new ExecutionSession(storage, diagnostics)) { + Mapping initialMapping = buildInitialMapping(bindings); + if (timeoutMillis > 0) { + return evaluateWithTimeout(session.eval(), session.producer(), request, initialMapping, timeoutMillis, started); + } + return evaluateCore(session.eval(), request, initialMapping); } - return evaluateCore(eval, request, initialMapping); } private Mappings evaluateCore(Eval eval, ExecutionRequest request, Mapping initialMapping) { diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStackTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStackTest.java new file mode 100644 index 000000000..5b6ef9221 --- /dev/null +++ b/src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStackTest.java @@ -0,0 +1,144 @@ +package fr.inria.corese.core.next.query.impl.engine.solution; + +import fr.inria.corese.core.next.query.impl.engine.model.Edge; +import fr.inria.corese.core.next.query.impl.engine.model.Node; +import fr.inria.corese.core.next.query.impl.engine.model.NodeImpl; +import fr.inria.corese.core.next.query.impl.engine.pattern.Query; +import fr.inria.corese.core.next.query.impl.engine.spi.Matcher; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class BindingStackTest { + + private BindingStack stack; + private Matcher matcher; + + @BeforeEach + void setUp() { + stack = new BindingStack(); + matcher = mock(Matcher.class); + } + + private static Node varNode(String name, int index) { + NodeImpl n = NodeImpl.forVariable(name); + n.setIndex(index); + return n; + } + + private static Node litNode(String value) { + return NodeImpl.forLiteral(value, null, null); + } + + @Test + void testNodePushPopAndQuery() { + Query q = mock(Query.class); + when(q.nbNodes()).thenReturn(4); + when(q.nbEdges()).thenReturn(2); + + stack.init(q, false); + assertEquals(0, stack.getBoundNodeCount()); + assertFalse(stack.isRecordEdges()); + + Node qNode0 = varNode("?x", 0); + Node target0 = litNode("foo"); + + assertTrue(stack.push(qNode0, target0, 1, matcher, null)); + assertEquals(1, stack.getBoundNodeCount()); + assertTrue(stack.isBound(qNode0)); + assertSame(target0, stack.getNode(qNode0)); + assertSame(target0, stack.getNode(0)); + assertSame(target0, stack.getNode("?x")); + assertSame(qNode0, stack.getQueryNode(0)); + assertSame(qNode0, stack.getQueryNode("?x")); + assertEquals(1, stack.getIndex(qNode0)); + + // Pushing same binding increases counter + when(matcher.same(any(), any(), any(), any())).thenReturn(true); + assertTrue(stack.push(qNode0, target0, 2, matcher, null)); + assertEquals(1, stack.getBoundNodeCount()); + + stack.pop(qNode0); + assertTrue(stack.isBound(qNode0)); + stack.pop(qNode0); + assertFalse(stack.isBound(qNode0)); + assertNull(stack.getNode(qNode0)); + assertEquals(0, stack.getBoundNodeCount()); + } + + @Test + void testConstantNodeAndConflict() { + Query q = mock(Query.class); + when(q.nbNodes()).thenReturn(3); + stack.init(q, false); + + Node constant = litNode("const"); + assertTrue(stack.push(constant, constant, 0, matcher, null)); + + Node qNode = varNode("?y", 1); + Node val1 = litNode("v1"); + Node val2 = litNode("v2"); + + assertTrue(stack.push(qNode, val1, 0, matcher, null)); + when(matcher.same(eq(qNode), eq(val1), eq(val2), any())).thenReturn(false); + assertFalse(stack.push(qNode, val2, 1, matcher, null)); + } + + @Test + void testEdgeRecordAndPop() { + Query q = mock(Query.class); + when(q.nbNodes()).thenReturn(2); + when(q.nbEdges()).thenReturn(2); + stack.init(q, true); + assertTrue(stack.isRecordEdges()); + + Edge qEdge = mock(Edge.class); + when(qEdge.getEdgeIndex()).thenReturn(0); + Edge ent = mock(Edge.class); + when(ent.getEdgeIndex()).thenReturn(0); + + stack.recordEdge(qEdge, ent); + assertEquals(1, stack.getBoundEdgeCount()); + assertSame(ent, stack.getEdge(qEdge)); + assertNotNull(stack.getEdges()); + assertNotNull(stack.getQueryEdges()); + + stack.popEdge(qEdge); + assertEquals(0, stack.getBoundEdgeCount()); + assertNull(stack.getEdge(qEdge)); + } + + @Test + void testGetIndexAcrossCollections() { + Query q = mock(Query.class); + when(q.nbNodes()).thenReturn(3); + stack.init(q, false); + + Node n0 = varNode("?a", 0); + Node n1 = varNode("?b", 1); + stack.push(n0, litNode("A"), 5, matcher, null); + stack.push(n1, litNode("B"), 10, matcher, null); + + assertEquals(10, stack.getIndex(List.of(n0, n1))); + + Edge edge = mock(Edge.class); + when(edge.nbNode()).thenReturn(2); + when(edge.getNode(0)).thenReturn(n0); + when(edge.getNode(1)).thenReturn(n1); + when(edge.getEdgeVariable()).thenReturn(null); + + assertEquals(10, stack.getIndex(null, edge)); + } +} diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/GroupAggregatorTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/GroupAggregatorTest.java new file mode 100644 index 000000000..52fee9f40 --- /dev/null +++ b/src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/GroupAggregatorTest.java @@ -0,0 +1,33 @@ +package fr.inria.corese.core.next.query.impl.engine.solution; + +import fr.inria.corese.core.next.query.impl.engine.pattern.Query; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class GroupAggregatorTest { + + private GroupAggregator original; + + @BeforeEach + void setUp() { + original = Mappings.getGroupAggregator(); + } + + @AfterEach + void tearDown() { + Mappings.setGroupAggregator(original); + } + + @Test + void canConfigureAndRetrieveCustomGroupAggregator() { + GroupAggregator mockAggregator = (query, mappings) -> true; + Mappings.setGroupAggregator(mockAggregator); + + assertSame(mockAggregator, Mappings.getGroupAggregator()); + assertTrue(Mappings.getGroupAggregator().tryAggregate(new Query(), new Mappings())); + } +} diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/execution/ExecutionSessionTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/execution/ExecutionSessionTest.java new file mode 100644 index 000000000..d51345fde --- /dev/null +++ b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/execution/ExecutionSessionTest.java @@ -0,0 +1,28 @@ +package fr.inria.corese.core.next.query.impl.sparql.execution; + +import fr.inria.corese.core.next.storage.api.StorageManager; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; + +class ExecutionSessionTest { + + @Test + void sessionInitializesProducerAndEvalAndClosesProducer() { + StorageManager storage = mock(StorageManager.class); + ExecutionSession session = new ExecutionSession(storage, null); + + assertNotNull(session.producer()); + assertNotNull(session.eval()); + + assertDoesNotThrow(session::close); + } + + @Test + void rejectsNullStorageManager() { + assertThrows(NullPointerException.class, () -> new ExecutionSession(null, null)); + } +} From de42126095fd849c38b9be661ca8ec3b68929bc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20C=C3=A9r=C3=A8s?= Date: Mon, 26 Oct 2026 14:58:49 +0100 Subject: [PATCH 4/4] refactor(engine): incorporate Copilot review feedback on session safety and volatile aggregator (#643) - Guard ExecutionSession constructor against resource leaks if Eval.create fails - Make Mappings.groupAggregator volatile for thread-safe access - Remove orphan BindingStack to keep codebase strictly pruned of unused classes - Add comprehensive fallback tests in GroupAggregatorTest (null and false returning) --- .../impl/engine/solution/BindingStack.java | 230 ------------------ .../query/impl/engine/solution/Mappings.java | 2 +- .../engine/solution/SolutionMaterializer.java | 2 +- .../sparql/execution/ExecutionSession.java | 10 +- .../engine/solution/BindingStackTest.java | 144 ----------- .../engine/solution/GroupAggregatorTest.java | 43 ++++ 6 files changed, 53 insertions(+), 378 deletions(-) delete mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStack.java delete mode 100644 src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStackTest.java diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStack.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStack.java deleted file mode 100644 index 8c6155a9e..000000000 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStack.java +++ /dev/null @@ -1,230 +0,0 @@ -package fr.inria.corese.core.next.query.impl.engine.solution; - -import fr.inria.corese.core.next.query.impl.engine.model.Edge; -import fr.inria.corese.core.next.query.impl.engine.model.Node; -import fr.inria.corese.core.next.query.impl.engine.pattern.Query; -import fr.inria.corese.core.next.query.impl.engine.spi.Environment; -import fr.inria.corese.core.next.query.impl.engine.spi.Matcher; - -import java.util.Arrays; -import java.util.List; - -/** - * Manages arrays of bound query nodes and edges during query evaluation. - */ -public final class BindingStack { - - private int[] nbNodes = new int[0]; - private int[] nbEdges = new int[0]; - private int[] stackIndex = new int[0]; - private Edge[] qEdges = new Edge[0]; - private Edge[] result = new Edge[0]; - private Node[] qNodes = new Node[0]; - private Node[] nodes = new Node[0]; - private int nbEdge = 0; - private int nbNode = 0; - private boolean recordEdges = false; - - public void init(Query query, boolean recordEdges) { - this.recordEdges = recordEdges; - int nmax = query.nbNodes(); - int emax = recordEdges ? query.nbEdges() : 0; - - nbNodes = new int[nmax]; - stackIndex = new int[nmax]; - nbEdges = new int[emax]; - result = new Edge[emax]; - qEdges = new Edge[emax]; - - nodes = new Node[nmax]; - qNodes = new Node[nmax]; - - reset(); - } - - public void reset() { - nbEdge = 0; - nbNode = 0; - Arrays.fill(qNodes, null); - Arrays.fill(nodes, null); - Arrays.fill(nbNodes, 0); - Arrays.fill(stackIndex, -1); - if (recordEdges) { - Arrays.fill(nbEdges, 0); - Arrays.fill(result, null); - Arrays.fill(qEdges, null); - } - } - - public boolean push(Node node, Node target, int n, Matcher match, Environment env) { - if (node.isConstant()) { - return true; - } - int index = node.getIndex(); - if (nodes[index] == null) { - nodes[index] = target; - qNodes[index] = node; - nbNode++; - nbNodes[index]++; - stackIndex[index] = n; - return true; - } else if (target == null) { - return false; - } else if (match.same(node, nodes[index], target, env)) { - nbNodes[index]++; - return true; - } - return false; - } - - public void pop(Node node) { - if (node.isVariable()) { - int index = node.getIndex(); - if (nbNodes[index] > 0) { - nbNodes[index]--; - if (nbNodes[index] == 0) { - nbNode--; - nodes[index] = null; - qNodes[index] = null; - stackIndex[index] = -1; - } - } - } - } - - public void recordEdge(Edge q, Edge ent) { - int index = q.getEdgeIndex(); - if (nbEdges[index] == 0) { - nbEdge++; - } - nbEdges[index]++; - qEdges[index] = q; - result[index] = ent; - } - - public void popEdge(Edge q) { - int index = q.getEdgeIndex(); - if (nbEdges[index] > 0) { - nbEdges[index]--; - if (nbEdges[index] == 0) { - nbEdge--; - qEdges[index] = null; - result[index] = null; - } - } - } - - public boolean isBound(Node node) { - return node.isConstant() || (node.getIndex() < nodes.length && nodes[node.getIndex()] != null); - } - - public Node getNode(Node node) { - if (node.isConstant()) { - return node; - } - int index = node.getIndex(); - return index < nodes.length ? nodes[index] : null; - } - - public Node getNode(int index) { - return index < nodes.length ? nodes[index] : null; - } - - public Node getNode(String label) { - for (int i = 0; i < qNodes.length; i++) { - if (qNodes[i] != null && qNodes[i].getLabel().equals(label)) { - return nodes[i]; - } - } - return null; - } - - public Node getQueryNode(int index) { - return index < qNodes.length ? qNodes[index] : null; - } - - public Node getQueryNode(String label) { - for (Node qNode : qNodes) { - if (qNode != null && qNode.getLabel().equals(label)) { - return qNode; - } - } - return null; - } - - public int getStackIndex(int index) { - return index < stackIndex.length ? stackIndex[index] : -1; - } - - public int getIndex(Node node) { - return getStackIndex(node.getIndex()); - } - - public int getIndex(Node gNode, Edge edge) { - int max = -1; - int length = edge.nbNode(); - for (int i = 0; i < length; i++) { - Node qNode = edge.getNode(i); - if (qNode != null) { - max = Math.max(max, getStackIndex(qNode.getIndex())); - } - } - Node pNode = edge.getEdgeVariable(); - if (pNode != null) { - max = Math.max(max, getStackIndex(pNode.getIndex())); - } - if (gNode != null && gNode.isVariable()) { - max = Math.max(max, getStackIndex(gNode.getIndex())); - } - return max; - } - - public int getIndex(List lNodes) { - int max = -1; - for (Node node : lNodes) { - int index = getIndex(node); - if (index == -1) { - return -1; - } - max = Math.max(max, index); - } - return max; - } - - public Edge getEdge(Edge edge) { - int index = edge.getEdgeIndex(); - return index < result.length ? result[index] : null; - } - - public Node[] getNodes() { - return nodes; - } - - public Node[] getQueryNodes() { - return qNodes; - } - - public Edge[] getEdges() { - return result; - } - - public Edge[] getQueryEdges() { - return qEdges; - } - - public int getBoundNodeCount() { - return nbNode; - } - - public int getBoundEdgeCount() { - return nbEdge; - } - - public boolean isRecordEdges() { - return recordEdges; - } - - public void setRecordEdges(boolean recordEdges) { - this.recordEdges = recordEdges; - } -} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java index b2eca93c2..218f47815 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/Mappings.java @@ -80,7 +80,7 @@ public final class Mappings extends PointerObject private int length = 0; // limit number of results to be displayed private int display = Integer.MAX_VALUE; - private static GroupAggregator groupAggregator; + private static volatile GroupAggregator groupAggregator; public static void setGroupAggregator(GroupAggregator aggregator) { groupAggregator = aggregator; diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/SolutionMaterializer.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/SolutionMaterializer.java index 315f94860..0b2665371 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/SolutionMaterializer.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/solution/SolutionMaterializer.java @@ -152,7 +152,7 @@ private ProjectionResult applySingleProjection(Exp projection, Producer producer kgram.getVisitor().select(kgram, filter.getExp(), value == null ? null : value.getDatatypeValue()); } if (!memory.push(projection.getNode(), value)) { - return new ProjectionResult(false, false, previouslyBound, null); + return new ProjectionResult(false, false, false, null); } pushed = true; } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/execution/ExecutionSession.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/execution/ExecutionSession.java index 535ed5e41..59c4b0320 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/execution/ExecutionSession.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/execution/ExecutionSession.java @@ -21,10 +21,16 @@ final class ExecutionSession implements AutoCloseable { ExecutionSession(StorageManager storage, ExecutionDiagnostics diagnostics) { Objects.requireNonNull(storage, "storage"); - this.producer = diagnostics == null + StorageManagerProducer p = diagnostics == null ? new StorageManagerProducer(storage) : new DiagnosticProducer(storage, diagnostics); - this.eval = Eval.create(producer, new SparqlKgramEvaluator(), new RdfTermMatcher()); + try { + this.eval = Eval.create(p, new SparqlKgramEvaluator(), new RdfTermMatcher()); + this.producer = p; + } catch (RuntimeException | Error failure) { + p.close(); + throw failure; + } } StorageManagerProducer producer() { diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStackTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStackTest.java deleted file mode 100644 index 5b6ef9221..000000000 --- a/src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/BindingStackTest.java +++ /dev/null @@ -1,144 +0,0 @@ -package fr.inria.corese.core.next.query.impl.engine.solution; - -import fr.inria.corese.core.next.query.impl.engine.model.Edge; -import fr.inria.corese.core.next.query.impl.engine.model.Node; -import fr.inria.corese.core.next.query.impl.engine.model.NodeImpl; -import fr.inria.corese.core.next.query.impl.engine.pattern.Query; -import fr.inria.corese.core.next.query.impl.engine.spi.Matcher; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import java.util.List; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -class BindingStackTest { - - private BindingStack stack; - private Matcher matcher; - - @BeforeEach - void setUp() { - stack = new BindingStack(); - matcher = mock(Matcher.class); - } - - private static Node varNode(String name, int index) { - NodeImpl n = NodeImpl.forVariable(name); - n.setIndex(index); - return n; - } - - private static Node litNode(String value) { - return NodeImpl.forLiteral(value, null, null); - } - - @Test - void testNodePushPopAndQuery() { - Query q = mock(Query.class); - when(q.nbNodes()).thenReturn(4); - when(q.nbEdges()).thenReturn(2); - - stack.init(q, false); - assertEquals(0, stack.getBoundNodeCount()); - assertFalse(stack.isRecordEdges()); - - Node qNode0 = varNode("?x", 0); - Node target0 = litNode("foo"); - - assertTrue(stack.push(qNode0, target0, 1, matcher, null)); - assertEquals(1, stack.getBoundNodeCount()); - assertTrue(stack.isBound(qNode0)); - assertSame(target0, stack.getNode(qNode0)); - assertSame(target0, stack.getNode(0)); - assertSame(target0, stack.getNode("?x")); - assertSame(qNode0, stack.getQueryNode(0)); - assertSame(qNode0, stack.getQueryNode("?x")); - assertEquals(1, stack.getIndex(qNode0)); - - // Pushing same binding increases counter - when(matcher.same(any(), any(), any(), any())).thenReturn(true); - assertTrue(stack.push(qNode0, target0, 2, matcher, null)); - assertEquals(1, stack.getBoundNodeCount()); - - stack.pop(qNode0); - assertTrue(stack.isBound(qNode0)); - stack.pop(qNode0); - assertFalse(stack.isBound(qNode0)); - assertNull(stack.getNode(qNode0)); - assertEquals(0, stack.getBoundNodeCount()); - } - - @Test - void testConstantNodeAndConflict() { - Query q = mock(Query.class); - when(q.nbNodes()).thenReturn(3); - stack.init(q, false); - - Node constant = litNode("const"); - assertTrue(stack.push(constant, constant, 0, matcher, null)); - - Node qNode = varNode("?y", 1); - Node val1 = litNode("v1"); - Node val2 = litNode("v2"); - - assertTrue(stack.push(qNode, val1, 0, matcher, null)); - when(matcher.same(eq(qNode), eq(val1), eq(val2), any())).thenReturn(false); - assertFalse(stack.push(qNode, val2, 1, matcher, null)); - } - - @Test - void testEdgeRecordAndPop() { - Query q = mock(Query.class); - when(q.nbNodes()).thenReturn(2); - when(q.nbEdges()).thenReturn(2); - stack.init(q, true); - assertTrue(stack.isRecordEdges()); - - Edge qEdge = mock(Edge.class); - when(qEdge.getEdgeIndex()).thenReturn(0); - Edge ent = mock(Edge.class); - when(ent.getEdgeIndex()).thenReturn(0); - - stack.recordEdge(qEdge, ent); - assertEquals(1, stack.getBoundEdgeCount()); - assertSame(ent, stack.getEdge(qEdge)); - assertNotNull(stack.getEdges()); - assertNotNull(stack.getQueryEdges()); - - stack.popEdge(qEdge); - assertEquals(0, stack.getBoundEdgeCount()); - assertNull(stack.getEdge(qEdge)); - } - - @Test - void testGetIndexAcrossCollections() { - Query q = mock(Query.class); - when(q.nbNodes()).thenReturn(3); - stack.init(q, false); - - Node n0 = varNode("?a", 0); - Node n1 = varNode("?b", 1); - stack.push(n0, litNode("A"), 5, matcher, null); - stack.push(n1, litNode("B"), 10, matcher, null); - - assertEquals(10, stack.getIndex(List.of(n0, n1))); - - Edge edge = mock(Edge.class); - when(edge.nbNode()).thenReturn(2); - when(edge.getNode(0)).thenReturn(n0); - when(edge.getNode(1)).thenReturn(n1); - when(edge.getEdgeVariable()).thenReturn(null); - - assertEquals(10, stack.getIndex(null, edge)); - } -} diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/GroupAggregatorTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/GroupAggregatorTest.java index 52fee9f40..956aa79f9 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/GroupAggregatorTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/engine/solution/GroupAggregatorTest.java @@ -1,12 +1,19 @@ package fr.inria.corese.core.next.query.impl.engine.solution; import fr.inria.corese.core.next.query.impl.engine.pattern.Query; +import fr.inria.corese.core.next.query.impl.engine.spi.Evaluator; +import fr.inria.corese.core.next.query.impl.engine.spi.Producer; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; class GroupAggregatorTest { @@ -30,4 +37,40 @@ void canConfigureAndRetrieveCustomGroupAggregator() { assertSame(mockAggregator, Mappings.getGroupAggregator()); assertTrue(Mappings.getGroupAggregator().tryAggregate(new Query(), new Mappings())); } + + @Test + void nullAggregatorFallsBackToOrdinaryAggregation() { + Mappings.setGroupAggregator(null); + assertNull(Mappings.getGroupAggregator()); + + Query q = new Query(); + Mappings mappings = new Mappings(); + mappings.add(new Mapping()); + Evaluator eval = mock(Evaluator.class); + Memory mem = mock(Memory.class); + Producer p = mock(Producer.class); + + // Does not throw and finishes cleanly + mappings.aggregate(q, eval, mem, p); + assertFalse(mappings.isFake()); + } + + @Test + void falseReturningAggregatorFallsBackToOrdinaryAggregation() { + AtomicBoolean invoked = new AtomicBoolean(false); + Mappings.setGroupAggregator((query, mappings) -> { + invoked.set(true); + return false; + }); + + Query q = new Query(); + Mappings mappings = new Mappings(); + mappings.add(new Mapping()); + Evaluator eval = mock(Evaluator.class); + Memory mem = mock(Memory.class); + Producer p = mock(Producer.class); + + mappings.aggregate(q, eval, mem, p); + assertTrue(invoked.get()); + } }