From cd2672b4f0027664db0fc77e543a056eb66ac397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20C=C3=A9r=C3=A8s?= Date: Fri, 18 Sep 2026 15:11:49 +0200 Subject: [PATCH] refactor(next): decouple execution engine from SPARQL AST - Remove AST accessors coreseNextSource() and getFilterExpression() from Filter - Remove TermAst forwarding from Exp - Remove QueryAst field and AST accessors from Query - Remove query.setAST calls from CoreseAstQueryBuilder - Empty EXISTING_ENGINE_AST_DEPENDENCIES in NextModuleBoundaryTest - Update boundary documentation and package-info --- docs/next-engine-ast-boundary.md | 59 +++++-------------- .../next/query/impl/engine/model/Filter.java | 17 +----- .../next/query/impl/engine/package-info.java | 9 ++- .../next/query/impl/engine/pattern/Exp.java | 5 -- .../next/query/impl/engine/pattern/Query.java | 14 ----- .../sparql/bridge/CoreseAstQueryBuilder.java | 4 -- .../impl/sparql/bridge/NextFilterFromAst.java | 2 - .../architecture/NextModuleBoundaryTest.java | 11 ++-- .../query/impl/engine/pattern/QueryTest.java | 22 ------- .../impl/sparql/bridge/AstBackedExprTest.java | 2 +- 10 files changed, 26 insertions(+), 119 deletions(-) diff --git a/docs/next-engine-ast-boundary.md b/docs/next-engine-ast-boundary.md index e4ab1056c..0da162598 100644 --- a/docs/next-engine-ast-boundary.md +++ b/docs/next-engine-ast-boundary.md @@ -1,58 +1,31 @@ # Engine / AST dependency boundary -The target is an execution engine independent of parser and SPARQL AST types. -The current engine still exposes three dependencies. `NextModuleBoundaryTest` -inventories them as exact source-path / referenced-type pairs, rather than -exempting whole files or packages. +The query execution engine is independent of parser and SPARQL AST types. +`NextModuleBoundaryTest` enforces this invariant with zero exceptions across all +engine sources. -## Existing exceptions +## Decoupling history -Paths below are relative to `next/query/impl/engine`. +Previously, three source files in `next/query/impl/engine` retained AST dependencies: -| Source | AST type | Current use and callers | -| --- | --- | --- | -| `model/Filter.java` | `TermAst` | Return types of `getFilterExpression()` and `coreseNextSource()`. `NextFilterFromAst` implements both in the bridge. `Exp` delegates the first; `AstBackedExprTest` checks its result. No production caller of `coreseNextSource()` was found in `next`. | -| `pattern/Exp.java` | `TermAst` | Return type of `getFilterExpression()`, delegating to `Filter`. No production caller of this accessor was found in `next`. | -| `pattern/Query.java` | `QueryAst` | Field `ast` and accessors `getAST()`, `getGlobalAST()`, `setAST()`. `CoreseAstQueryBuilder` sets the source AST for each query form; `QueryTest` tests the accessors. No external production reader of these accessors was found in `next`. | - -This is a source-reference inventory, not a statement that these APIs can be -removed without checking downstream clients. Bridge implementations may still -own AST objects internally after the engine contracts stop exposing their types. +- `model/Filter.java`: returned `TermAst` via `getFilterExpression()` and `coreseNextSource()`. + These AST accessors were removed from the engine interface; the bridge implementation + `NextFilterFromAst` retains AST-level access internally where needed. +- `pattern/Exp.java`: exposed `getFilterExpression()` delegating to `Filter`. Removed as it + had no engine callers. +- `pattern/Query.java`: retained `QueryAst ast` with `getAST()`, `getGlobalAST()`, and `setAST()`. + Removed from `Query`; query builder compilation produces pure engine operator trees. ## Guard behavior +- Zero exceptions: `EXISTING_ENGINE_AST_DEPENDENCIES` is empty. - Parser dependencies have no exceptions. - Both the actual `query.impl.sparql.ast` package and the historical `query.impl.ast` package are checked. - Imports, wildcard imports, static references and fully qualified references - are detected by the existing source-reference matching approach. -- Adding a type to an already listed file is rejected. -- Adding an existing AST type to another file is rejected. -- Removing a dependency requires removing its exception in the same change. - -The source scanner also sees fully qualified names in comments and strings. It -does not perform bytecode or transitive dependency analysis. Its regression test -uses synthetic sources to check new files, new types, wildcard imports and fully -qualified references outside imports. - -## Follow-up: remove the exceptions - -1. Check repository-wide and downstream uses of the source-AST accessors before - removing them; assess compatibility of these implementation classes. -2. Remove the unused `Exp` forwarding accessor and the AST accessors from the - engine `Filter` contract. Keep source-AST access inside the bridge where needed; - adapt `AstBackedExprTest` to test bridge behavior rather than an engine AST API. -3. Remove AST retention from `Query` if no required consumer exists. If diagnostics - need source information, keep it in a bridge-owned structure with an explicit - lifecycle, rather than replacing typed accessors with `Object` or unsafe casts. -4. Remove the corresponding builder writes and accessor-only tests. Preserve - behavioral tests for query forms, expressions and subqueries. -5. Delete each exception as its dependency disappears, then restore the strict - no-AST-dependencies invariant in the engine package documentation. - -Do not introduce new runtime behavior as part of this follow-up. Validate with -the architecture tests, the full core test suite and a comparison against the -existing W3C baseline. + are detected by the source-reference scanner. +- Any new dependency on parser or AST packages within `query/impl/engine` causes + `NextModuleBoundaryTest.queryEngineMustNotAddParserOrAstDependencies` to fail immediately. ## Validation diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/model/Filter.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/model/Filter.java index 16c4a97b0..dc6347d6e 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/model/Filter.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/model/Filter.java @@ -1,27 +1,14 @@ package fr.inria.corese.core.next.query.impl.engine.model; - -import fr.inria.corese.core.next.query.impl.sparql.ast.TermAst; - import java.util.List; -import java.util.Optional; - /** - * Native filter contract backed by the Corese-next SPARQL AST. + * Native filter contract for the Corese-next execution engine. * * @author Olivier Corby, Edelweiss, INRIA 2010 */ public interface Filter { - /** - * When non-empty, the Corese-next AST node this filter was produced from - * (filter expression: {@link TermAst} / constraint subtypes). - */ - default Optional coreseNextSource() { - return Optional.empty(); - } - /** * List of variable names contained in the filter * @@ -33,8 +20,6 @@ default Optional coreseNextSource() { /** Evaluable expression processed by the native KGRAM evaluator. */ Expr getExp(); - TermAst getFilterExpression(); - /** * Does filter contain a bound() function * diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/package-info.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/package-info.java index 190301ed2..c96eb4718 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/package-info.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/package-info.java @@ -7,11 +7,10 @@ * *

Architectural Invariants & Development Guardrails

*
    - *
  • AST Boundary: New dependencies on parser or AST classes are forbidden. - * Three existing source/type dependencies in {@code Filter}, {@code Exp}, and - * {@code Query} remain explicitly inventoried by {@code NextModuleBoundaryTest}. - * Removing these exceptions is tracked in {@code docs/next-engine-ast-boundary.md}; - * the target architecture executes compiled logical operators only.
    • + *
  • AST Boundary: Dependencies on parser or AST classes are strictly forbidden. + * The execution engine operates exclusively on compiled logical operators and patterns; + * zero AST dependencies exist or are permitted, as enforced by {@code NextModuleBoundaryTest} + * and documented in {@code docs/next-engine-ast-boundary.md}.
    • *
  • Storage Isolation: Query evaluation and triple access must strictly * transit through the {@link fr.inria.corese.core.next.query.impl.engine.spi.Producer} * and Storage SPI abstractions.
    • 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 2beaf3e58..c57ebc781 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 @@ -14,7 +14,6 @@ import fr.inria.corese.core.next.query.impl.engine.model.PointerType; import fr.inria.corese.core.next.query.impl.engine.model.Regex; import fr.inria.corese.core.next.query.impl.engine.spi.Producer; -import fr.inria.corese.core.next.query.impl.sparql.ast.TermAst; import java.util.ArrayList; import java.util.HashMap; @@ -548,10 +547,6 @@ public void setFilter(Filter f) { } } - public TermAst getFilterExpression() { - return getFilter().getFilterExpression(); - } - public void addFilter(Filter f) { lFilter.add(f); } 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 a822f732a..705f2b5f0 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 @@ -11,7 +11,6 @@ import fr.inria.corese.core.next.query.impl.engine.spi.Producer; import fr.inria.corese.core.next.query.impl.engine.filter.Compile; import fr.inria.corese.core.next.query.impl.engine.eval.Message; -import fr.inria.corese.core.next.query.impl.sparql.ast.QueryAst; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -88,7 +87,6 @@ public final class Query extends Exp { // SPIN graph Query query; Query outerQuery; - QueryAst ast; Object object; // current transformer if any @@ -287,18 +285,6 @@ public void setObject(Object o) { object = o; } - public QueryAst getGlobalAST() { - return getGlobalQuery().getAST(); - } - - public QueryAst getAST() { - return ast; - } - - public void setAST(QueryAst o) { - ast = o; - } - public int getPlanProfile() { return planner; } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/CoreseAstQueryBuilder.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/CoreseAstQueryBuilder.java index e25db4f78..a5d718e2d 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/CoreseAstQueryBuilder.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/CoreseAstQueryBuilder.java @@ -72,7 +72,6 @@ public Query toNextQuery(AskQueryAst askQueryAst) { compiler); SolutionModifierCompiler.applyOrderBy(query, askQueryAst.solutionModifier(), compiler); query.setAsk(true); - query.setAST(askQueryAst); return query; } @@ -104,7 +103,6 @@ public Query toNextQuery(SelectQueryAst selectQueryAst) { if (query.getHaving() != null && !query.hasGroupBy()) { query.setAggregate(true); } - query.setAST(selectQueryAst); return query; } @@ -131,7 +129,6 @@ public Query toNextQuery(DescribeQueryAst describeQueryAst) { compiler); SolutionModifierCompiler.applyOrderBy(query, describeQueryAst.solutionModifier(), compiler); DescribeQueryCompiler.compile(query, describeQueryAst, compiler); - query.setAST(describeQueryAst); return query; } @@ -157,7 +154,6 @@ public Query toNextQuery(ConstructQueryAst constructQueryAst) { compiler); SolutionModifierCompiler.applyOrderBy(query, constructQueryAst.solutionModifier(), compiler); ConstructQueryCompiler.compile(query, constructQueryAst, compiler); - query.setAST(constructQueryAst); return query; } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/NextFilterFromAst.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/NextFilterFromAst.java index 59bc84115..e04c3da3d 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/NextFilterFromAst.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/NextFilterFromAst.java @@ -53,7 +53,6 @@ public Expr getExp() { return owner; } - @Override public TermAst getFilterExpression() { return owner.sourceAst().orElseThrow(); } @@ -83,7 +82,6 @@ public boolean isRecExist() { return owner.isRecExist(); } - @Override public Optional coreseNextSource() { return owner.sourceAst(); } diff --git a/src/test/java/fr/inria/corese/core/next/architecture/NextModuleBoundaryTest.java b/src/test/java/fr/inria/corese/core/next/architecture/NextModuleBoundaryTest.java index 1df6f0ec0..afd2b0211 100644 --- a/src/test/java/fr/inria/corese/core/next/architecture/NextModuleBoundaryTest.java +++ b/src/test/java/fr/inria/corese/core/next/architecture/NextModuleBoundaryTest.java @@ -27,12 +27,9 @@ class NextModuleBoundaryTest { "\\bfr\\.inria\\.corese\\.core(?:\\.[A-Za-z_$][A-Za-z0-9_$]*)+"); private static final Pattern STATIC_IMPORT_PREFIX = Pattern.compile("^static\\s+"); + // Engine must not depend on parser or AST types: zero exceptions. // Exact source/type pairs only: see docs/next-engine-ast-boundary.md. - // Equality below also requires removing exceptions when their dependencies disappear. - private static final Set EXISTING_ENGINE_AST_DEPENDENCIES = Set.of( - "model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.ast.TermAst", - "pattern/Exp.java -> fr.inria.corese.core.next.query.impl.sparql.ast.TermAst", - "pattern/Query.java -> fr.inria.corese.core.next.query.impl.sparql.ast.QueryAst"); + private static final Set EXISTING_ENGINE_AST_DEPENDENCIES = Set.of(); @Test void sharedCodeMustNotDependOnDomainModules() throws IOException { @@ -105,7 +102,7 @@ void queryEngineMustNotAddParserOrAstDependencies() throws IOException { } @Test - void engineBoundaryFindsSyntaxReferencesBeyondExistingExceptions(@TempDir Path sources) + void engineBoundaryDetectsForbiddenSyntaxReferences(@TempDir Path sources) throws IOException { Path filter = sources.resolve("model/Filter.java"); Files.createDirectories(filter.getParent()); @@ -121,8 +118,8 @@ class Filter { Files.writeString(sources.resolve("NewOperator.java"), "import fr.inria.corese.core.next.query.impl.sparql.ast.TermAst;"); Set unexpected = engineSyntaxDependencies(sources); - unexpected.removeAll(EXISTING_ENGINE_AST_DEPENDENCIES); assertEquals(Set.of( + "model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.ast.TermAst", "model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.ast.QueryAst", "model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.parser.SparqlParser", "model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.ast.VarAst", 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 943604921..80f53229e 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 @@ -4,8 +4,6 @@ import fr.inria.corese.core.next.query.impl.engine.model.ExpType; 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.sparql.ast.QueryAst; -import fr.inria.corese.core.next.query.impl.sparql.parser.SparqlParser; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; @@ -649,26 +647,6 @@ void testIsTransformationTemplate() { } } - @Nested - @DisplayName("AST Tests") - class ASTTests { - - @Test - @DisplayName("Should set and get AST") - void testSetAndGetAST() { - QueryAst ast = new SparqlParser().parse("SELECT * WHERE { ?s ?p ?o }"); - query.setAST(ast); - assertSame(ast, query.getAST(), "AST should match"); - } - - @Test - @DisplayName("Should get global AST") - void testGetGlobalAST() { - assertDoesNotThrow(() -> query.getGlobalAST(), - "Getting global AST should not throw"); - } - } - @Nested @DisplayName("Global Query Tests") class GlobalQueryTests { diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/bridge/AstBackedExprTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/bridge/AstBackedExprTest.java index af39c82a3..87ef677ca 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/bridge/AstBackedExprTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/bridge/AstBackedExprTest.java @@ -66,7 +66,7 @@ void boundExpressionAndFilterView() { assertNotNull(boundExpr.getFilter()); assertTrue(boundExpr.getFilter().isBound()); assertEquals(List.of("y"), boundExpr.getFilter().getVariables()); - assertEquals(boundAst, boundExpr.getFilter().getFilterExpression()); + assertEquals(boundAst, ((NextFilterFromAst) boundExpr.getFilter()).getFilterExpression()); } @Test