fix(persist): a persisted sql_select reads its built table - #3040
Merged
Conversation
Substitution consulted the manifest for a query_source but not for a
sql_select referenced directly, so the source recompiled inline no matter
what had been built, and strict mode never noticed the missing entry.
The lookup is now one function, persistedTableFor(), called from both the
FROM/join site and the %{ } segment site.
Fixes #3016
…under virtualMap The strict-mode error dropped the source name. `sourceID` is deleted rather than inherited when a source is modified, so it either names this source or is absent; `as` and `extends` are the fields that would name the base. A BuildID's SQL was compiled with no options at all, which drops `virtualMap` and `resolvedGivens` along with the manifest. A persisted source over a virtual source could then be neither planned nor queried — both sides threw before they could hash anything. `buildIdOptions()` now says what a BuildID compiles under, and `mkBuildTargets` uses it too, so the key the builder writes stays the key the compiler recomputes.
Compiling a BuildID under resolvedGivens made the two sides disagree. Only the compiler can resolve givens — getBuildTargets takes none and PersistSource.getSQL never resolves them — so a model merely declaring an inline default built one SQL and looked up another, and a strict manifest threw for a table the builder had just described. Both sides now compile givens by not resolving them, which is the behavior #3041 asks to report properly. BuildIdOptions is narrow enough that a manifest cannot be passed, so the builder cannot reintroduce the divergence the projection exists to prevent. Also swept: two comments still crediting query_query.ts with the serve-time recompute, the builder contract and simple_builder still showing getSQL with no options, and the pending-#3016 comment above a test that now passes.
housejester
pushed a commit
to malloydata/publisher
that referenced
this pull request
Sep 2, 2026
Three scenarios asserted contracts that have since moved, each in a change that touched no scenario: a refused `storage=` source is reported in `BuildPlan.refusedSources` rather than carrying a plan handle a manifest can be keyed on (#1045); a part-way failed build now commits a manifest recording what built and why the rest did not, so the tables it wrote stay reachable and reclaim is left for a run that recorded nothing (#1004); and a persisted `sql_select` reads its built table (malloydata/malloy#3040, adopted here in #1063). None was a regression, so none becomes `known-red`. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: James Estes <james.estes@credibledata.com>
housejester
added a commit
to malloydata/publisher
that referenced
this pull request
Sep 2, 2026
…stinguishable from a built one (#1098) * test(hammer): tell a built source from a failed one, and assert the plan's refusals `## Build (orchestrated)` checked only `physicalTableName`, and a failed source is mirrored into `entries` under the name it was headed for, so the step reported a failed build as green. A per-source `(failed)` attribute now asserts against the manifest's `failures`, and an unmarked source is asserted built AND absent from `failures`. Adds `## Build refusals`, asserting `BuildPlan.refusedSources`, so a source that was examined and refused can be told apart from one the plan never contained -- an empty table asserts nothing was refused. Also documents `## Build targets`, which was missing from GRAMMAR.md. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: James Estes <james.estes@credibledata.com> * test(hammer): restate three red scenarios as the contracts that now hold Three scenarios asserted contracts that have since moved, each in a change that touched no scenario: a refused `storage=` source is reported in `BuildPlan.refusedSources` rather than carrying a plan handle a manifest can be keyed on (#1045); a part-way failed build now commits a manifest recording what built and why the rest did not, so the tables it wrote stay reachable and reclaim is left for a run that recorded nothing (#1004); and a persisted `sql_select` reads its built table (malloydata/malloy#3040, adopted here in #1063). None was a regression, so none becomes `known-red`. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: James Estes <james.estes@credibledata.com> * test(hammer): pin the new attribute and section, and record the reclaim decision Review follow-ups. `(failed)` and `## Build refusals` had the same unknown-attribute throws `## Manifest` has but neither of its two spec cases, and `buildRefusals` was missing from the pinned list of kinds that must contribute a check -- so exempting it later would have broken no test. Adds both halves for the attribute, the parse-order pin that keeps `## Build refusals` from being read as `## Build refused`, and the throws for an empty-table-without-source and for `(failed)` on a refused build. The reclaim Note now records the decision rather than leaving the choice open: the guards stay, because the harness being unable to name a path is not evidence production cannot take it. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: James Estes <james.estes@credibledata.com> --------- Signed-off-by: James Estes <james.estes@credibledata.com> Co-authored-by: James Estes <james.estes@credibledata.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Fixes #3016. A persisted
sql_selectwas substituted when%{ }interpolationreached it, but not when it was a query's source or a join — it recompiled
inline whatever had been built, and strict mode never reported the missing
entry.
The manifest lookup is now one function,
persistedTableFor(), called from bothreference sites. Its strict-mode error no longer names a source: by the time it
throws, a reference through an inline
extendhas had its identity cleared, andthe names still on the struct belong to the base.