Updatable View Matcher - #54
Conversation
xudong963
left a comment
There was a problem hiding this comment.
What consistency semantics do we want during a candidate refresh? After the catalog replaces provider A with B, the matcher continues exposing A while refresh_candidate waits for the lock, looks up B, and analyzes it. Is this intentional RCU/stale-while-refresh behavior, where completion of refresh_candidate is the visibility boundary? Or should new queries stop using A immediately and fall back to the original plan until B is ready?
Relatedly, if schema.table(...).await returns an error, A remains available indefinitely. Is stale-on-error also intentional? If so, could we document and test these semantics explicitly?
When a table is added, this will be applied last; no refresh until the table is "ready". When a table is removed, it should be removed from the view matcher first. So we stop showing it as a candidate before we start to remove the rest of the table. During a change in the table configuration, it gets messy. Any change in the table should, in theory, keep it a valid candidate. As changing the SQL for a materialized view that changes the projection usually results in other issues.
I am honestly not sure what the best behavior here is. Part of me says a failure to refresh the candidate should remove this table and all related tables as valid options. If you have preferred behavior, I would love to hear it. |
- Resolve table references against the session's default catalog/schema so bare, partial, and fully-qualified refs address the same candidate entry. - Look the provider up from the catalog instead of trusting a caller-supplied provider, so entries always reflect what is actually registered. - Serialize mutations (full rescans and single-table refreshes) behind a mutex so a stale analysis cannot overwrite a newer catalog change. - Remove the existing entry when analysis fails, so an analyzer error after a catalog replacement cannot leave the old provider active. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fafe3ad to
a42b383
Compare
Allows the set of view-matching candidates to be updated at runtime, instead of being frozen at
ViewMatcher::try_new_from_statetime.Motivation
ViewMatcherscanned every catalog once at construction and stored the result in a plainHashMapbehind&self. Any materialized view registered, replaced, or dropped afterwards was invisible to query rewriting until the wholeViewMatcher(and therefore theSessionStateholding it) was rebuilt. Long-lived services that register MVs dynamically need the candidate set to track the catalog.What changed
ViewMatcherTableThe map value changes from the tuple
(Arc<dyn TableProvider>, SpjNormalForm)to a named struct:The
SpjNormalFormis now behind anArcso snapshot clones are cheap.Copy-on-write candidate set
OptimizerRule::rewritetakes the read lock only long enough to clone theArc, then does all rewriting against that immutable snapshot — so a concurrent catalog update can never mutate the map out from under an in-flight rewrite, and rewrites never block on updates. Mutations clone the innerHashMap, apply the change, and swap in a newArc. Adds aparking_lotdependency for theRwLock.ViewMatchingRewritercorrespondingly holds the snapshot (Arc<HashMap<..>>) instead of a&ViewMatcherborrow.Three ways to maintain the set
update_candidates(&state)refresh_candidate(&state, table_ref)invalidate_candidate(&state, table_ref)refresh_candidateis the cheap path when a single table changed — it analyzes only that table instead of walking every catalog. It removes the entry when the table was dropped, is not a materialized view, or hasuse_in_query_rewrite = false/ no SPJ normal form.The full-scan and single-table paths share one
build_candidatehelper, so a candidate is computed identically no matter which entry point produced it.Correctness details
Table references are resolved. Both
refresh_candidateandinvalidate_candidateresolve the incoming reference against the session's default catalog/schema before touching the map. The full scan stores fully-qualified keys, so without this a bare"mv"would create a second, duplicate entry rather than updatingdatafusion.public.mv. Bare, partial, and fully-qualified spellings all address the same entry.The provider is read from the catalog, not from the caller.
refresh_candidatelooks theTableProviderup throughcatalog_list()rather than accepting one as an argument, so an entry always reflects what is actually registered — a caller can't inject a provider that was already replaced.Mutations are serialized.
update_lock(an async mutex, held across the.awaiton analysis) ensures a slow full rescan cannot overwrite a newer single-table refresh with stale results. The candidate set converges to the catalog state seen by the last completed call.Failures fail closed. If a refresh errors — catalog lookup failure or analyzer error — the table's candidate is removed and the error returned, rather than leaving a possibly-stale entry live for rewriting. Only the failing table is affected; other candidates are untouched. The table becomes a candidate again on the next successful refresh.
Breaking API changes
ViewMatcher::mv_plans()returnsArc<HashMap<TableReference, ViewMatcherTable>>(an owned snapshot) instead of&HashMap<TableReference, (Arc<dyn TableProvider>, SpjNormalForm)>. A reference can't be handed out from behind the lock.get_potential_mv_candidates_for_table()returnsVec<(TableReference, ViewMatcherTable)>instead of the 3-tupleVec<(TableReference, Arc<dyn TableProvider>, &SpjNormalForm)>.Tests
Six new tests in
tests_incremental_candidatescovering:refresh_candidateignores non-materialized tables and unknown tables — and asserts viaArc::ptr_eqthat a no-op does not swap the snapshot at allrefresh_candidatenormalizes table refs: a bare ref updates the fully-qualified entry rather than creating a duplicate; after deregistration any spelling removes itinvalidate_candidateremoves an entry addressed by a bare ref, is a no-op (no snapshot swap) when absent, and a later refresh brings the candidate backAnalyzerRulethat poisons specific plans)SchemaProviderwhosetable()can be made to error)Full suite: 71 lib tests + 1 integration test pass;
cargo clippy --tests --all-features -- -D warningsandcargo fmt --checkare clean.Rebase note
Rebased onto
branch-54on top of #55 (RewriteReadiness / per-branchCandidateMetadata). Both changes rewrote the candidate-generation loop inf_down; the merged version keeps #55'sNotReadyfiltering and readiness propagation while reading throughViewMatcherTableand the immutable snapshot. Readiness is still read from the live provider on each rewrite, so #55's lazy-index warming behavior is preserved. Every commit in the branch compiles independently.