Move build to getBuildTargets - #259
Merged
Merged
Conversation
Update @malloydata packages to 0.0.429 and port `malloy-cli build` from the deprecated `Model.getBuildPlan` to `Runtime.getBuildTargets`, which answers in tables rather than sources.
A BuildID is one table, but buildFiles plans each file separately, so two files whose sources compile to the same SQL under different name= values never appear in one BuildTargets result. The second file found the first's manifest entry, reported "up to date", and its own table was never built. Track what each BuildID has been named for the whole run.
The claims map guarded only one direction: two names on one BuildID was an error, but two BuildIDs under one name built both and let the second overwrite the first, leaving two manifest entries aimed at a table holding one of the two computations. TableClaims owns both directions and one message. Also: --refresh now reports keys that matched nothing and accepts a renamed source under either name; the conflict message says how to get unstuck; and the source-plus-extension test asserts the merged build line, which it has to, since the manifest looked the same before this PR.
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.
malloy-cli buildmoves toRuntime.getBuildTargets, the first checkbox onmalloydata/malloy#3029. That PR deprecated
Model.getBuildPlanbecause it reportssources while the manifest is keyed by table, and the two are not one to one — a
builder on it has to compute the BuildIDs, find the collisions, and order the walk
itself, and this one got parts of that wrong. Needs @malloydata 0.0.429, bumped here.
Getting tables instead of sources settles half of the identity question; the other half
is that
buildFilesplans one file at a time, so a collision between two files isinvisible to any single
BuildTargets.TableClaimscloses that, in both directions.Impact
both directions are now errors naming every declaration site. Two names for one build
used to let one win silently across files — the loser was reported "up to date". Two
builds under one name still both ran, one overwriting the other, leaving two manifest
entries aimed at a table holding only one of the two computations. Under
strict: truethat substitutes into queries rather than falling back to inline SQL.
sqlValidateTableNameis now computed once and used forboth the
CREATE TABLEand the manifest entry. They could previously diverge forDuckDB's file-path form, which
Manifest.updatewould then reject — reported as abuild failure after the table had already been created.
--refreshreports keys that matched no table, and accepts a renamed source undereither its old or new name. A stale or mistyped key used to be a silent no-op behind a
row of "up to date".
persistence".