Repository navigation
getBuildTargets reports a source it cannot compile instead of failing the plan - #3165
Closed
housejester wants to merge 4 commits into
Closed
housejester wants to merge 4 commits into
housejester wants to merge 4 commits into
Conversation
… the plan A persist source whose SQL raises a MalloyCompileError (a given with no default, a virtual source with no map entry), and anything that inlines it, comes back in BuildTargets.unbuildable with the compiler's problem; the rest of the model is planned. Any other error still throws. Signed-off-by: James Estes <james.estes@credibledata.com>
housejester
marked this pull request as ready for review
October 20, 2026 15:50
added 3 commits
October 20, 2026 14:21
…not be persisted The builder and the manifest lookup both compile a persisted source's SQL without resolving givens, so a given with no default has no value there. Both now raise persist-given-no-value, naming the source and the extend-block where: that filters per caller, instead of asking for a value the builder cannot supply and the query already did. Fixes malloydata#3041. Signed-off-by: James Estes <james.estes@credibledata.com>
…names in the present Signed-off-by: James Estes <james.estes@credibledata.com>
… function's invariant Signed-off-by: James Estes <james.estes@credibledata.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
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.
A persisted source that reads a
given:with no default cannot be persisted: its table would have to be one table for every caller (#3041). Today that authoring error surfaces in two bad ways.getBuildTargetsthrows on it and takes the whole model's plan with it, so a builder cannot build any of the model's other tables. And both that throw, and a query over the source once a manifest is in play, carry a message that tells the reader to supply the given, which a builder cannot do and the query already did.With this change:
getBuildTargetsplansdaily_ordersandmy_orders_at_read, and reportsmy_ordersin a newunbuildablelist instead of throwing.runtime-persist-given-no-value: it names the source, says why it cannot be persisted, and points at the shape that can. An extend-blockwhere:is not part of the persisted relation, somy_orders_at_read's table holds every tenant and each query reads its own slice.It also clears the way for the #3029 follow-up of deleting
getBuildPlan. That never compiles SQL, so a builder on it can skip a source likemy_ordersand build the rest; ongetBuildTargetsit cannot.Fixes #3041.
How
persistedSourceSQLinmodel/sql_compiled.tscompiles a persisted source's SQL for both the builder (PersistSource.getSQL) and the manifest lookup (persistedTableFor). Both compile without resolving givens (buildIdOptions), so acompiler-given-no-valueraised there can only mean the source reads a given with no default; it becomesruntime-persist-given-no-value, at the given's location. The live inline of a persisted source still compiles with the query's givens and keeps the ordinary error.getBuildTargetsreports a source whose SQL raises aMalloyCompileErrorinunbuildablewith that problem. Anything else, including a dialect that cannot express the SQL, still fails the call. A source built on an unbuildable one inlines its SQL, so it is reported too.virtualMapentry (runtime-virtual-map-missing).mkBuildTargetstakes the list as an out-array, asresolvePersistWalkdoestagParseLog. The sample builder prints what it skipped and builds the rest.Open questions
#@ persist? Whether a persisted source reads a given with no default is visible at translate time. Proposed: keep it on the build and query paths, as persist: a source depending on a supplied given fails with an error that names neither persistence nor the given #3041 asks. A translator error would fail the whole model's compile because one source can't be persisted, and a builder would lose every other table in the model.persistedSourceSQLcatchescompiler-given-no-valueand raises the persist error. The alternative is a flag on the compile options, so the expression compiler raises it directly. That widensBuildIdOptions, which is deliberately narrow. Proposed: the boundary, since it's one function both sides already share.unbuildableonBuildTargets, or perConnectionBuild? Proposed: onBuildTargets, besidetagParseLog. Both report what's wrong with the model's persist sources, rather than what to build on a connection.Behaviour change: a model that used to throw from
getBuildTargetsnow plans with fewer targets. A builder that relied on the throw has to checkunbuildable;doc/persist/api.mdsays so. A query that used to fail over such a source still fails, with the new message.