fix: close Cypher benchgraph gaps and add benchmark-critical fast paths#1448
Open
bplatz wants to merge 16 commits into
Open
fix: close Cypher benchgraph gaps and add benchmark-critical fast paths#1448bplatz wants to merge 16 commits into
bplatz wants to merge 16 commits into
Conversation
…ite surface) Fixes from the Memgraph benchgraph (Pokec) triage, taking the suite from 24/35 to an expected 33/35 and making the match__pattern_* results correct: - Unary minus folds to a literal at parse time, so SET n.p = -1 and negative inline pattern properties work on both read and write paths. - Untyped single-hop patterns (--> / -[e]->) follow relationships only: rdf:type and data properties are excluded via an edge-set filter, matching the untyped var-length wildcard. - A value-only bound relationship variable (never property-read anywhere in the statement) lowers to the plain base triple + optional annotation probe + coalesce(annotation, MakeRel(s, p, o)): plain-RDF edges now match, reified parallel edges keep one row per annotation, and the probe is skipped when the ledger has no f:reifies* facts. Engine prerequisite: COALESCE gains a binding-path arm so structured values (Rel/List/Map) survive BIND instead of collapsing to null. - Untyped shortestPath / allShortestPaths: ShortestPathPattern.predicate is now Option<Sid>; the wildcard mode reuses the untyped var-length edge-set (SPOT/OPST scans) and resolves per-hop predicates for relationships(p) post-hoc on found paths only. - Bare MATCH (n) whole-graph distinct-subject scans are available behind FLUREE_CYPHER_ALLOW_FULL_SCAN=1 (off by default). - Bare CREATE () / CREATE (n) asserts a hidden db:Node existence marker (filtered from labels()); relationship endpoints need no marker. This also fixes CREATE (n) erroring as an empty transaction. - UNWIND range(1, 100) AS x / inline literal lists before a write clause fold into a synthetic parameter and reuse the $param batch desugar (capped at 10k rows). Also re-wires four fluree-db-api test files that were orphaned from CI (autotests = false with no [[test]] target): it_query_cypher (156 tests), it_policy_cypher, it_csv_import, it_tpch_iceberg_bench — fixing one rotted helper call — and updates the Cypher docs (support matrix, relationship-lowering semantics, configuration). RETURN on a write statement remains deferred: it needs the staging skolem txn_id + solution count exposed through the commit receipt (or a pre-supplied txn_id) before created-entity rows can be reconstructed.
Closes the last two benchgraph gaps (35/35 verbatim expected):
RETURN on a write statement (single_vertex_write / single_edge_write):
- Created-entity Sids are fully deterministic given the skolem key
`fdb-{txn_id}-{solution}-{label}`, so instead of threading a mapping
out of staging, the caller supplies the skolem id up front:
`TxnOpts::skolem_txn_id` (staging uses it in place of the generated
timestamp) travels in the existing TransactionRequest, and the rows
are reconstructed post-commit — statically one row for a pure CREATE,
or one per WHERE solution discovered by one-flake SPOT existence
probes over the contiguous solution indices.
- Named CREATE relationships now use a variable-derived annotation
label (`_:cy_rel_{name}`, reuse rejected) so `RETURN e` can name the
created edge.
- v1 surface (validated pre-submission): bare created-entity variables,
optionally aliased; clear deferred errors for expressions, RETURN
modifiers, MATCH-bound variables, and MERGE. Answered as the read
path's Cypher-JSON tabular envelope on both the HTTP route (which
falls back to the commit receipt when no RETURN is present) and the
new `Fluree::transact_cypher_returning` API.
Write-side WHERE now plans with statistics (create__edge perf):
- `execute_where_streaming` passed `stats = None` to the operator
builder, so a bound-object seek (`?a <id> 4112` → 1 row) tied with a
class scan (`?a rdf:type User` → N rows) at the default selectivity
and lowering order won — an update's anchored MATCH scanned every
label flake and probed per row. It now builds the same cached
novelty-merged stats view the read path uses. A/B on a 10k-user
in-memory ledger: `MATCH (a:User {id:..}), (b:User {id:..}) CREATE
(a)-[:TempEdge]->(b)` went from 82.7s to 25ms (read-path parity;
the indexed benchgraph setup showed the same plan flip at ~360ms).
This applies to every WHERE-driven write surface (Cypher, SPARQL
UPDATE, JSON-LD update), not just Cypher.
MATCH (n:User {id: $id}) RETURN n label-scanned (1032x vs Neo4j at 100k
users, linear in |label|) because PropertyJoinOperator::driver_score
hard-preferred a bound rdf:type object as the leading scan, discarding
the planner's selectivity ordering. Score all fully-bound objects
equally instead: the index tie-break inherits the reordered pattern
order, so a specific bound value ({id: 4112}, ~1 row) drives ahead of
its class pattern, while a class the planner ranks more selective still
wins the tie.
Verified on a bulk-imported pokec ledger: labeled point lookup 4ms ->
0.65ms at 10k users (seek parity with the unlabeled form), SPARQL
equivalent 12ms -> 0.6ms; cost is now constant in label size.
MATCH (n) RETURN count(n), count(n.age) lowers to a DISTINCT-subject subquery + accessor OPTIONAL that no existing fast path matched (the detectors require a single bare triple and exactly one aggregate), so the whole graph was scanned per query — 610ms at 100k nodes vs Neo4j's 22ms count store. New fast_whole_graph_agg recognizes the shape and folds each aggregate exactly: - count(n)/COUNT(*) = N + count(P) - subj(P) (left-join row count; N alone with no accessor) — SPOT lead groups + PSOT directory counts - count(n.P) = the predicate's live row count; count(DISTINCT n) = N; count(DISTINCT n.P) = POST distinct-object lead groups - min/max(n.P) via POST boundary keys, avg/sum(n.P) via the predicate-scoped POST fold Gated on the strict metadata lane (single-ledger, root, no overlay, at HEAD) and declines when the graph carries scan-hidden predicates (f:reifies* / default-graph f:) whose subjects the pipeline would not see. Everything else falls back to the general pipeline. The min/max/avg query previously missed the single-accessor shape because Cypher lowering emitted one accessor OPTIONAL per occurrence; identical accessors now dedup within a clause (cross-WITH re-emits are unaffected). The repeated-optional shape also exposed an indexed-vs- novelty divergence in the general pipeline (repeated identical OPTIONAL skips same-var unification on indexed stores) — no longer reachable from Cypher, tracked separately. pokec (10k): count 27ms -> 0.5ms, min/max/avg 30ms -> 0.55ms; values verified against the general pipeline, including multi-valued property row semantics.
Cypher lowering emitted Ref::Iri / Term::Iri for every label, property, and relationship-type constant. Scans normalize IRIs at open so results were correct, but every Sid-gated plan analysis silently disqualified: the nested-loop join's batched subject/object/exists lanes (is_batched_*_eligible require p.is_sid()) fell back to one scan open per row, and per-row output batches made downstream operators reopen per row too. This was the whole of PERF-4's "flat var-length overhead": the BFS itself was already frontier-bounded (~0.5ms); the per-row tail lanes cost 20ms at 100k nodes and ~1.5s at 1.6M. Encode via encode_iri_strict at the emission sites (labels, inline props, rel types, typed-chain hops, annotation bodies, property accessors), falling back to Ref::Iri for unregistered namespaces — exact parity with the SPARQL lowering. Measured (pokec, real dataset over HTTP at 10k; synthetic 100k in-process): neighbours_2 24.3ms -> 1.5ms / 21ms -> 1.0ms, expansion_2 -> 1.4ms, expansion_3 98ms -> 14ms, expansion_4 346ms -> 124ms (residual is PERF-2 path enumeration), pattern_short 0.94ms -> 0.6ms. Full benchgraph suite 35/35 green. Also adds fluree_db_api::explain::explain_cypher — the Cypher surface previously had no explain entry point, which is how this class of plan-shape defect stayed invisible.
MATCH (n:User) WITH n WHERE n.id = $id RETURN n lowers the accessor to
OPTIONAL { ?n <id> ?v } FILTER(?v = $id). Rows the OPTIONAL leaves
unextended have ?v unbound, the comparison errors, and the filter drops
them — exactly the rows a required triple never produces. The forms are
row-equivalent, but the OPTIONAL wrapper hides the pattern from
equality pushdown and selectivity estimation, so the query ran as a
label scan + post-filter (74x vs Neo4j, linear in |label|). SPARQL
OPTIONAL { … } FILTER(=) hit the same cliff.
New pre-planning rewrite (optional_filter_fold, run before the equijoin
fold): convert a single-triple OPTIONAL to a required triple when a
same-group filter equality-pins a var only that OPTIONAL produces. The
filter is kept — substituting the constant into the triple would switch
value equality to index-term matching, which differs on cross-datatype
numeric equality.
Scope is deliberately narrow (profitability, not soundness): only
equality/IN-with-constants fold — a folded range predicate becomes a
full property scan the planner can misorder as the driving anchor
(measured: benchgraph *_with_filter expansions went from milliseconds
to timeouts when range filters folded) — and queries containing
property-path / shortest-path patterns are skipped so a folded endpoint
accessor cannot become the traversal's driving side.
pokec 10k over HTTP: vertex_on_label_property 15.2ms -> 0.87ms; the
100k in-process WITH form 67ms -> 1.5ms. W3C SPARQL eval suite
unchanged (283/327 with and without the fold); full benchgraph suite
35/35 with no regressions.
MATCH (s:User {id})-->()-->()-->()-->(n:User) RETURN DISTINCT n.id
lowered to four triple joins that materialize one row per WALK
(10^5-10^7 intermediate rows on hub seeds, >120s timeouts at 1.6M
nodes) before the terminal DISTINCT collapses them. With DISTINCT
output the query is semantically per-level reachability.
Runs of >= 2 anonymous untyped outgoing hops now lower to a single
exact-depth wildcard PropertyPathPattern (min=max=k), reusing the
var-length operator's layered BFS with per-(node,depth) dedup — work
bounded by frontier size, not walk count. The interior anonymous nodes
cannot be referenced, so the endpoint set is identical.
Walk multiplicity is observable outside DISTINCT, so fusion is gated
statement-wide: RETURN DISTINCT, no aggregates in the projection or
ORDER BY, and only MATCH / OPTIONAL MATCH / UNWIND / inline-rows
clauses (WITH or CALL could aggregate per-walk rows; the write path
never sets the flag). Non-DISTINCT chains, named intermediates, typed
or non-outgoing hops keep the join lowering — pinned by tests
(count(*) still counts walks, bare RETURN keeps per-walk rows).
Full benchgraph suite 35/35 on a fresh import; expansion results
byte-identical. At small/uniform scale times are tail-dominated and
roughly unchanged — the fix removes the walk-count blowup class
(hub seeds at medium/large).
MATCH (n:User) RETURN n.age, COUNT(*) (and COUNT(DISTINCT n.age)) ran the general label-scan + left-join + hash-group pipeline — 2.2-3.7x vs the vendors' count stores, linear in |class|. Two generalizations of the whole-graph aggregate fold: - Class anchor: `?n rdf:type <C>` joins the DISTINCT-subject subquery as a recognized subject universe. The instance count comes from the per-graph class stats; property-derived folds additionally require the containment proof class_property_flakes(C, P) == predicate_rows(P) — equality means every P-bearing subject is a C, so predicate-scoped folds equal the class-restricted join (both sides current-state-exact at HEAD, class stats per #1266). The scan-hidden-predicate gate applies only to the whole-graph anchor; class anchors read class rows, so annotated graphs still fold. - Histogram kind: GROUP BY the accessor value + one COUNT(*)/COUNT(n) folds to a POST run-length group count (values are physically contiguous per predicate; reuses group_count_v6) plus one null-group row (subjects - subj(P): the left join gives property-less subjects a single unbound-key row). Recognizes the projection-alias Bind the Cypher lowering emits for the group key; capped at 65536 groups. pokec 10k over HTTP: age histogram 6.8ms -> 0.72ms (identical 53 groups), COUNT(DISTINCT n.age) 5.8ms -> 0.61ms. The range-filtered histogram variant stays on the pipeline by design (~6.4ms). Tests pin indexed-vs-pipeline equality incl. multi-valued properties, the null group, and the containment decline (property present on another class).
MATCH (n:User) WHERE n.age >= 18 RETURN n.age, COUNT(*) still ran the general pipeline (6.4ms at 10k, 2.6x): the histogram fold declined on the Filter pattern between the accessor OPTIONAL and the projection Bind. The filter references only the group key, and every row of a group carries the same key value — so it accepts or rejects whole groups. The fold now computes the full histogram (POST group counts + null group) and runs the actual filter expression once per group row via the shared filter_batch evaluator, including on the null group's Unbound key. That is exactly the pipeline's per-row semantics for ANY admitted expression — range, IN, string predicates, even IS NULL (value groups drop, the null group survives) — with no per-shape range machinery. Admission gates: the filter must reference nothing but the accessor var and be synchronously evaluable (no EXISTS / metadata reads); when the group key is the projection alias, the filter is rewritten to it (identity bind). Scalar folds still decline on any filter — their predicate-total reads would be wrong. An all-groups-rejected filter yields an empty batch, not a decline. pokec 10k over HTTP: aggregate_with_filter 6.4ms -> 0.70ms, identical 39 groups; full benchgraph suite 35/35. Tests pin indexed-vs-pipeline equality for the range filter, the all-rejected case, and IS NULL.
Design note for a server-side Bolt (Neo4j wire protocol) adapter: scope (v1 autocommit, versions 4.4/5.x), crate placement and the existing entry points to reuse (query_cypher_with_params, the consensus write path, the cypher-json value mapping and relationship/ path value model), the result-type mapping table, and measured per-request cost breakdowns from the benchgraph effort with expected improvements. Recommends landing a parsed-AST cache first — the current pre-parse param substitution is what defeats text-keyed caching — and defers explicit transactions.
and hidden predicates Investigating a report that whole-graph aggregates fall back to a scan after incremental indexing surfaced three distinct issues; this commit fixes the two query-side ones. 1. COUNT(DISTINCT ?p) returned wrong results (measured 3 vs a true 8) on incrementally-indexed stores: the fold preferred the per-graph property stats, but the incremental index build persists DELTA-ONLY per-graph property stats — base entries are lost and net-zero churn is kept at count 0 (e.g. a store showed only 4 of 8 predicates, one with count 0 despite 10k live values). The fold now always uses its exact PSOT p_const directory walk; the stats shortcut is removed until the indexer-side merge is fixed and regression-tested. 2. The distinct subject/predicate/object folds over `?s ?p ?o` counted scan-hidden facts: the variable-predicate pipeline hides f:reifies* (and default-graph f:) rows, but SPOT/PSOT/OPST directories include them, so one reified edge made COUNT(DISTINCT ?p) answer 9 where the pipeline answers 6 (reifier subjects/objects over-count the other positions the same way). The whole-graph aggregate fold's hidden-predicate gate now guards these folds too — and the gate itself now checks the store's predicate DICTIONARY instead of the per-graph stats, since issue (1) means a stats-driven check can silently pass on exactly the stores that carry hidden facts. Regression test drives the full trigger: full index, then writes introducing a new predicate, net-zero churn, and a reified edge, then an INCREMENTAL index — asserting fold == pipeline for COUNT(DISTINCT ?p), whole-graph count(n), and the class-anchored histogram. Remaining (indexer-side, not addressed here): the incremental build's per-graph property stats merge drops base entries — this also degrades planner selectivity estimates for any predicate missing from the list. Separately, a running server does not swap its cached ledger view to a newly published index until restart/reload, which leaves queries on the novelty-overlay path (observed 0.6ms -> 194ms on the same process while `fluree info` showed index-t == commit-t).
A server that indexes in the background never got its metadata fast paths back after a write: the fold gates checked overlay epoch() == 0, but epoch is a monotonic mutation counter that never resets — after the background indexer published and the cached ledger view swapped snapshots and drained novelty (the event -> reconcile -> apply_index_v2 chain works), the gates kept declining forever. Whole-graph aggregates measured 0.6ms on a fresh server, ~190ms after one write, and stayed there until restart. is_effectively_empty() exists on OverlayProvider for exactly this distinction (drained-after-swap vs never-had-novelty); the gates now use it via a shared overlay_has_novelty() helper. epoch() remains in cache keys, where monotonicity is the point. Also make the scan-hidden-predicate gate row-accurate: the predicate dictionary is global across graphs, so commit-metadata f: predicates (txn-meta graph) are present in every ledger's dictionary — bare membership permanently declined the whole-graph fold on every imported ledger. Each hidden candidate is now confirmed by its row count in the queried graph (count_rows_for_predicate_psot, directory-only), which also keeps the annotation-decline behavior exact. Live verification (pokec 10k, one server process, no restarts): fresh import 0.45ms -> 30 writes 197ms (fold declines correctly while novelty is unmerged) -> background index publishes -> 0.44ms again.
The incremental index build carried base per-graph property stats forward ONLY through the HLL sketch blob (base_root.sketch_ref). Bulk import writes sketch_ref: None (import.rs), so the first incremental over any imported ledger seeded the stats hook from zero and persisted DELTA-ONLY per-graph property stats: base predicates vanished from GraphStatsEntry.properties and net-zero churn was kept at count 0 (observed: 4 of 8 predicates left, completion_percentage count 0 despite 10k live values). The rewritten sketch then made the loss permanent for every subsequent incremental. Class stats were immune because they re-seed from base_root.stats directly (issue #1266); property stats now follow the same pattern. When the sketch blob is absent or unreadable, seed the hook's prior properties from base_root.stats.graphs[].properties (count, datatypes, last_modified_t are exact). HLL registers cannot be reconstructed from the persisted ndv estimates, so the finalized ndv_values/ndv_subjects are clamped to the base root's estimates on every incremental — HLL ndv is monotone (registers only grow), so the base estimate is a valid floor, and applying the clamp unconditionally means ndv also survives register loss in a previously fallback-seeded sketch. Without the floor, count/ndv selectivity estimates collapse after import + write (a bound-value seek estimating count/1 rows re-introduces the label-scan misplans fixed earlier on this branch). Regression tests: an import-based (sketch-less) base + net-zero churn + a new predicate + incremental index must keep sum(property counts) == graph flakes, all counts positive, and base ndv estimates intact (fails pre-fix with delta-only entries); the existing full-rebuild scenario keeps the sketch path covered. Remaining follow-up (not this change): have bulk import produce a sketch_ref so first-incremental ndv merges from real registers.
MATCH (n:User) WITH n WHERE n.id = $id regressed from sub-ms to
12-20ms (BUG-1b) as soon as any write touched the id predicate with a
different subject count than the class (one benchgraph
`CREATE (:UserTemp {id})` sufficed). Reported as a regression from the
novelty-gate change; bisection here shows it pre-existing — the parent
commit reproduces identically. Two compounding causes, both fixed:
1. Join ordering knife-edge (the trigger). The bounds nudge in
where_plan promotes a FILTER-bounded triple to the driving position
only when its estimate is <= the current first triple's — but it
estimated the equality-bounded triple as a FULL property scan
(count), tying the class anchor exactly on clean data and losing to
it the moment novelty made count(id) > count(:User). The plan then
drove the 10k-row class scan with per-subject id probes (batched,
but ~25x slower per subject in overlay-merge mode). An equality
bound pins the object to one value, so the triple now estimates as
a bound-object pattern (count/ndv ~= 1) — the swap holds under any
novelty skew, matching the plan larger datasets already chose.
2. No point seek for integer-equality bounds (the payoff). Once
driving, `?n <id> ?v` + FILTER(?v = k) scanned the predicate's full
POST range with a decoded per-row filter; range narrowing existed
for temporal types only, because cross-type numeric equality (an
xsd:double 4112.0 matches integer 4112) would be missed by a typed
seek. The seek now engages when every persisted leaflet AND every
translated overlay op for the predicate carries the candidate
o_type (directory-only + cached-ops checks) — then no other
representation can exist and the point range is exact, covering
the index and the novelty overlay in one narrowed cursor. Mixed or
unverifiable predicates keep the broad scan + decoded filter.
Verified on the reporter's isolation recipe (pokec 10k over HTTP):
fresh 0.77ms -> +7 UserTemp{id} writes 0.62-0.92ms (was 12-20ms), and
mid-suite match__vertex_on_label_property 0.90ms with
single_vertex_write novelty in play (was 16.9ms); full benchgraph
35/35. Tests pin correctness under predicate novelty: indexed value,
novelty-only value through the narrowed seek, novelty duplicate of an
indexed value, and cross-type decline (double 7.0 still matches
integer equality via the decoded filter).
Driving a star from the most selective bound object (c2d9fae) regressed time-travel over fully-retracted predicates: a predicate whose rows were all retracted has no PSOT/POST partition in the rebuilt index — its historical rows survive only in the subject-keyed SPOT history sidecars — so a direct driver scan of it silently returns nothing. The batched probes replay those sidecars correctly, but only non-driver predicates go through them. Driver selection now takes a replay flag (to_t < index max_t): under replay a bound rdf:type object is hard-preferred again, keeping every other predicate on the replay-correct probe lane. Current-time queries keep the selectivity-ordered bound-object driver (the point-lookup seek win). The underlying hole — a plain single-triple scan of a fully-retracted predicate at a historical t returns 0 rows because the full-rebuild path omits the empty partition instead of emitting a zero-row leaflet with its sidecar — predates this branch and remains open. (cherry picked from commit b27fa26)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Takes the Pokec benchmark to 35/35 supported queries and brings the hot benchmark shapes to seek/count-store parity. Three groups of changes:
Cypher surface gaps
-->/-[e]->) now follow relationships only (rdf:type and data properties excluded), matching the untyped var-length wildcard.shortestPath/allShortestPaths(ShortestPathPattern.predicateis nowOption<Sid>), with per-hop predicates resolved post-hoc forrelationships(p).SET n.p = -1, negative inline properties).CREATE ()/CREATE (n)asserts a hiddendb:Nodeexistence marker;UNWIND range(...)before a write clause reuses the$parambatch desugar.TxnOpts::skolem_txn_id), so rows are reconstructed post-commit — on the HTTP route and the newFluree::transact_cypher_returningAPI.MATCH (n)whole-graph scans available behindFLUREE_CYPHER_ALLOW_FULL_SCAN=1(off by default).Performance
execute_where_streamingpreviously planned with no statistics, so anchored MATCHes label-scanned. Now builds the same cached novelty-merged stats view as the read path (10k-user A/B: 82.7s → 25ms). Applies to every WHERE-driven write surface, not just Cypher.count(n)27ms → 0.5ms), class-anchored aggregates and GROUP-BY histograms, group-key-filtered histograms.Correctness hardening for the fast paths
f:reifies*).Test wiring
Re-wires four
fluree-db-apitest files that were orphaned from CI (autotests = falsewith no[[test]]target):it_query_cypher(156 tests),it_policy_cypher,it_csv_import,it_tpch_iceberg_bench. Also updates the Cypher docs (support matrix, relationship-lowering semantics, configuration) and includes the Bolt protocol adapter design doc.Test plan
cargo nextest run --workspace --all-features(including the newly re-wired test targets)