From 1961bfb918dd8ce14690a02e5a82e5e0f146aa1d Mon Sep 17 00:00:00 2001 From: "AD\\aabdoun" Date: Thu, 29 Oct 2026 14:36:00 +0100 Subject: [PATCH] fix(planning): bound subject sampling in PredicateStatisticsEstimator and demote filter seed when cheaper pattern exists --- .../planning/CardinalityPatternOrdering.java | 18 +- .../memory/PredicateStatisticsEstimator.java | 15 +- .../planning/CardinalityQueryPlannerTest.java | 28 +++ .../PredicateStatisticsEstimatorTest.java | 203 ++++++++++++++++++ 4 files changed, 258 insertions(+), 6 deletions(-) create mode 100644 src/test/java/fr/inria/corese/core/next/storage/impl/memory/PredicateStatisticsEstimatorTest.java diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/planning/CardinalityPatternOrdering.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/planning/CardinalityPatternOrdering.java index 3aa59865c..52e0662a3 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/planning/CardinalityPatternOrdering.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/planning/CardinalityPatternOrdering.java @@ -119,11 +119,27 @@ private static long[] cardinalities(QueryPlan query, Node graph, Exp expression, return counts; } + /** + * Returns true if the seed pattern has the minimum cardinality in the run + * (or is tied for it). When a filter-binding seed has higher cardinality than + * another pattern, the greedy loop will naturally place the cheaper pattern + * first and the filter is still applied as soon as its variable is bound. + */ + private static boolean isCheapestOrTied(int seed, int start, int end, long[] counts) { + long seedCount = counts[seed - start]; + for (int k = start; k < end; k++) { + if (counts[k - start] < seedCount) { + return false; + } + } + return true; + } + private static void orderRun(Exp expression, int start, int end, long[] counts, PlanningContext context) { int first = start; int seed = SelectiveFilterPushdown.seed(expression, start, end); - if (seed >= 0) { + if (seed >= 0 && isCheapestOrTied(seed, start, end, counts)) { moveFirst(expression, counts, start, first, seed); recordBindings(expression.get(first), context.bound()); first++; diff --git a/src/main/java/fr/inria/corese/core/next/storage/impl/memory/PredicateStatisticsEstimator.java b/src/main/java/fr/inria/corese/core/next/storage/impl/memory/PredicateStatisticsEstimator.java index 9c41ee32b..9b4784d8d 100644 --- a/src/main/java/fr/inria/corese/core/next/storage/impl/memory/PredicateStatisticsEstimator.java +++ b/src/main/java/fr/inria/corese/core/next/storage/impl/memory/PredicateStatisticsEstimator.java @@ -21,7 +21,7 @@ private PredicateStatisticsEstimator() { * * @param index POS triple index * @param predicate the predicate IRI - * @param maxSample maximum number of branch entries to sample for distinct subjects + * @param maxSample maximum number of subjects to sample for distinct-subject estimation * @return predicate statistics or null if not found or not in POS order */ static PredicateStatistics estimate(final TripleIndex index, final IRI predicate, final int maxSample) { @@ -66,8 +66,11 @@ void accumulate(final Object entry) { private void accumulateLeaf(final TripleIndex.Leaf leaf) { totalCount += leaf.size(); if (shouldSample()) { - leaf.forEach((subjKey, st) -> subjects.add(subjKey)); - sampled++; + leaf.visitMatching(null, statement -> { + subjects.add(new TermKey(statement.getSubject())); + sampled++; + return !shouldSample(); // true = stop when subject limit reached + }); } } @@ -84,10 +87,12 @@ private boolean shouldSample() { } long estimateDistinctSubjects(final long distinctObjects) { - if (maxSample <= 0 || distinctObjects <= maxSample) { + if (maxSample <= 0 || sampled < maxSample) { + // Processed fewer subjects than the limit: sampled everything exactly. return subjects.size(); } - final double ratio = (double) distinctObjects / Math.max(1, sampled); + // Hit the subject limit — extrapolate from the sample. + final double ratio = (double) totalCount / Math.max(1, sampled); final long estimated = Math.round(subjects.size() * ratio); return Math.min(totalCount, Math.max(subjects.size(), estimated)); } diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/engine/planning/CardinalityQueryPlannerTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/engine/planning/CardinalityQueryPlannerTest.java index 66485e866..09dd389bb 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/engine/planning/CardinalityQueryPlannerTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/engine/planning/CardinalityQueryPlannerTest.java @@ -387,6 +387,34 @@ void zeroCardinalityRemainsCheaperThanFullyBoundProbe() { assertEquals(List.of("urn:bench:empty", "urn:bench:probe"), predicates(query)); } + // ── isCheapestOrTied: selective filter seed is demoted when a cheaper pattern exists ── + + @Test + void selectiveFilterSeedDemotedWhenCheaperPatternExists() { + // ?seed has a FILTER binding hint but count=50, while :cheaper has count=1. + // Before the isCheapestOrTied fix, the seed was always placed first regardless. + // After the fix, :cheaper wins the first slot. + assertPlan("?s :seed ?y . ?s :cheaper ?z . FILTER(?y < 3)", + Map.of("seed", 50L, "cheaper", 1L), + List.of("cheaper", "seed")); + } + + @Test + void selectiveFilterSeedKeptFirstWhenItIsCheapest() { + // Seed has FILTER binding and is also the cheapest — should remain first. + assertPlan("?s :seed ?y . ?s :expensive ?z . FILTER(?y < 3)", + Map.of("seed", 1L, "expensive", 50L), + List.of("seed", "expensive")); + } + + @Test + void selectiveFilterSeedKeptFirstWhenTiedWithOtherPattern() { + // Seed has FILTER binding and is tied — should remain first (tie-keep behaviour). + assertPlan("?s :seed ?y . ?s :tied ?z . FILTER(?y < 3)", + Map.of("seed", 5L, "tied", 5L), + List.of("seed", "tied")); + } + private static void assertPlan(String body, Map counts, List expected) { Map qualified = new java.util.HashMap<>(); counts.forEach((key, value) -> qualified.put("urn:bench:" + key, value)); diff --git a/src/test/java/fr/inria/corese/core/next/storage/impl/memory/PredicateStatisticsEstimatorTest.java b/src/test/java/fr/inria/corese/core/next/storage/impl/memory/PredicateStatisticsEstimatorTest.java new file mode 100644 index 000000000..3f4a39eb4 --- /dev/null +++ b/src/test/java/fr/inria/corese/core/next/storage/impl/memory/PredicateStatisticsEstimatorTest.java @@ -0,0 +1,203 @@ +package fr.inria.corese.core.next.storage.impl.memory; + +import fr.inria.corese.core.next.data.Values; +import fr.inria.corese.core.next.data.api.model.Statement; +import fr.inria.corese.core.next.data.api.term.IRI; +import fr.inria.corese.core.next.data.api.term.Resource; +import fr.inria.corese.core.next.data.api.term.SimpleIRI; +import fr.inria.corese.core.next.data.api.term.SimpleLiteral; +import fr.inria.corese.core.next.storage.api.model.PredicateStatistics; +import fr.inria.corese.core.next.storage.impl.memory.MemoryStorageManager; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Tests for PredicateStatisticsEstimator — bounded subject sampling fix. + * + *

The root cause of the Q15 regression was that accumulateLeaf() iterated ALL + * subjects in a leaf rather than stopping at maxSample subjects. For a predicate + * like rdf:type with many objects but millions of total subjects, this caused + * O(total subjects) work per estimation call.

+ */ +class PredicateStatisticsEstimatorTest { + + private static IRI iri(String local) { + return new SimpleIRI("urn:test:" + local); + } + + private static Statement triple(String s, IRI p, String o) { + return Values.factory().createStatement(iri(s), p, iri(o)); + } + + private static Statement tripleWithLiteral(String s, IRI p, String literal) { + return Values.factory().createStatement(iri(s), p, new SimpleLiteral(literal)); + } + + // ── exact counting when dataset is small ────────────────────────────────── + + @Test + void exactDistinctSubjectsWhenDatasetFitsWithinSampleBudget() { + IRI type = iri("type"); + List stmts = new ArrayList<>(); + for (int i = 0; i < 100; i++) { + stmts.add(triple("s" + i, type, "ClassA")); + } + MemoryStorageManager storage = MemoryStorageManager.builder().build(); + storage.mutations().addAll(stmts); + + PredicateStatistics stats = storage.metadata().getPredicateStatistics(type); + assertNotNull(stats); + assertEquals(100, stats.count()); + assertEquals(100, stats.distinctSubjects()); + assertEquals(1, stats.distinctObjects()); + + storage.close(); + } + + @Test + void exactCountsForMultipleObjectBuckets() { + IRI type = iri("type"); + List stmts = new ArrayList<>(); + // 50 subjects → ClassA, 50 subjects → ClassB + for (int i = 0; i < 50; i++) { + stmts.add(triple("s" + i, type, "ClassA")); + } + for (int i = 50; i < 100; i++) { + stmts.add(triple("s" + i, type, "ClassB")); + } + MemoryStorageManager storage = MemoryStorageManager.builder().build(); + storage.mutations().addAll(stmts); + + PredicateStatistics stats = storage.metadata().getPredicateStatistics(type); + assertNotNull(stats); + assertEquals(100, stats.count()); + assertEquals(100, stats.distinctSubjects()); + assertEquals(2, stats.distinctObjects()); + + storage.close(); + } + + // ── bounded sampling: O(maxSample) work even for large fan-out predicates ─ + + @Test + void samplingBoundedAtMaxSampleSubjectsForHighCardinalityPredicate() { + // Simulate rdf:type pattern: few object classes, millions of subjects. + // Before the fix, all subjects were iterated. After fix: stops at 512. + IRI type = iri("type"); + int totalSubjects = 10_000; + int maxSample = 512; + + List stmts = new ArrayList<>(); + // 10 object classes with 1000 subjects each + for (int i = 0; i < totalSubjects; i++) { + stmts.add(triple("s" + i, type, "Class" + (i % 10))); + } + MemoryStorageManager storage = MemoryStorageManager.builder().build(); + storage.mutations().addAll(stmts); + + PredicateStatistics stats = storage.metadata().getPredicateStatistics(type); + assertNotNull(stats); + assertEquals(totalSubjects, stats.count()); + assertEquals(10, stats.distinctObjects()); + // Distinct subjects is estimated (sampled), must be positive and ≤ totalSubjects + assertTrue(stats.distinctSubjects() > 0, + "distinctSubjects must be positive, got " + stats.distinctSubjects()); + assertTrue(stats.distinctSubjects() <= totalSubjects, + "distinctSubjects must not exceed totalSubjects, got " + stats.distinctSubjects()); + // With uniform distribution, extrapolation should land near totalSubjects + assertTrue(stats.distinctSubjects() >= totalSubjects / 2, + "extrapolated distinctSubjects should be within 2x of actual, got " + stats.distinctSubjects()); + + storage.close(); + } + + @Test + void sampledDistinctSubjectsNeverExceedTotalCount() { + IRI pred = iri("pred"); + List stmts = new ArrayList<>(); + // 2000 subjects → single object (only 1 leaf in POS index) + for (int i = 0; i < 2000; i++) { + stmts.add(triple("s" + i, pred, "obj")); + } + MemoryStorageManager storage = MemoryStorageManager.builder().build(); + storage.mutations().addAll(stmts); + + PredicateStatistics stats = storage.metadata().getPredicateStatistics(pred); + assertNotNull(stats); + assertEquals(2000, stats.count()); + assertEquals(1, stats.distinctObjects()); + assertTrue(stats.distinctSubjects() > 0); + assertTrue(stats.distinctSubjects() <= stats.count(), + "distinctSubjects must not exceed count: " + stats.distinctSubjects() + " > " + stats.count()); + + storage.close(); + } + + // ── zero / missing cases ────────────────────────────────────────────────── + + @Test + void returnsNullForUnknownPredicate() { + MemoryStorageManager storage = MemoryStorageManager.builder().build(); + storage.mutations().addAll(List.of(triple("s0", iri("known"), "obj"))); + + PredicateStatistics stats = storage.metadata().getPredicateStatistics(iri("unknown")); + assertNull(stats); + + storage.close(); + } + + @Test + void returnsZeroStatsForEmptyStorage() { + MemoryStorageManager storage = MemoryStorageManager.builder().build(); + // nothing loaded — no stats for any predicate + assertNull(storage.metadata().getPredicateStatistics(iri("type"))); + storage.close(); + } + + // ── single statement (Statement path vs Leaf path) ──────────────────────── + + @Test + void singleStatementPerObjectIsHandledExactly() { + IRI pred = iri("pred"); + List stmts = new ArrayList<>(); + // 3 subjects, each with a distinct object → hits Statement path (not Leaf) + stmts.add(triple("s0", pred, "obj0")); + stmts.add(triple("s1", pred, "obj1")); + stmts.add(triple("s2", pred, "obj2")); + MemoryStorageManager storage = MemoryStorageManager.builder().build(); + storage.mutations().addAll(stmts); + + PredicateStatistics stats = storage.metadata().getPredicateStatistics(pred); + assertNotNull(stats); + assertEquals(3, stats.count()); + assertEquals(3, stats.distinctSubjects()); + assertEquals(3, stats.distinctObjects()); + + storage.close(); + } + + // ── literal subjects integration ────────────────────────────────────────── + + @Test + void literalObjectsCountedCorrectly() { + IRI name = iri("name"); + List stmts = new ArrayList<>(); + for (int i = 0; i < 50; i++) { + stmts.add(tripleWithLiteral("s" + i, name, "name" + i)); + } + MemoryStorageManager storage = MemoryStorageManager.builder().build(); + storage.mutations().addAll(stmts); + + PredicateStatistics stats = storage.metadata().getPredicateStatistics(name); + assertNotNull(stats); + assertEquals(50, stats.count()); + assertEquals(50, stats.distinctSubjects()); + assertEquals(50, stats.distinctObjects()); + + storage.close(); + } +}