Deployment build order should take into account dimension links - #2531
Merged
Conversation
A push that both adds a column to an existing dimension and adds a node linking to that new attribute marks the linking node INVALID with "Dimension attribute '...' not found". Four tests pin the behavior: - reference link: the deploy fails, and the identical retry succeeds - join link: the node is INVALID but the deploy reports success - with query lineage on the dimension: the identical link passes - brand-new dimension in an empty namespace: passes The failure needs the dimension to already be persisted with its old column set. Link validation reads the persisted revision's columns (_prefetch_dimension_link_nodes), and the prefetch runs once per topological level, which is built from query lineage only -- dimension link edges are not ordering edges. With no query dependency between them, the dimension and the linking node share a level, so the link resolves against pre-push columns. Query lineage or an absent dimension both avoid it, which is why the last two cases pass.
The two characterization tests now assert the intended behavior: a push that adds a dimension column and a link onto it deploys the link successfully, for both the reference and the join form. Two cycle cases join them -- a self-join with a role and a mutually linked pair of dimensions -- which link ordering must not turn into a hard failure.
Link validation resolves a dimension attribute against the dimension's persisted columns, and the deploy order came from query lineage alone. A push that added a column to an existing dimension and a link onto that column put both in the same topological level, so the link resolved against the pre-push column set and the linking node landed INVALID; the identical push then succeeded on a retry. Each spec's dimension_links now contribute deploy-ordering edges alongside required_dimensions, kept out of plan.node_graph so they never become parents. Links can be cyclic where query lineage cannot, so an edge that would close a cycle -- a self-join with a role, or two dimensions linking to each other -- is dropped instead, leaving those nodes ordered by lineage as before. Deploy order now differs for specs whose links are not query parents, which reorders the results list of three existing deployment tests.
✅ Deploy Preview for thriving-cassata-78ae72 canceled.
|
shangyian
changed the title
shangyian
marked this pull request as ready for review
September 9, 2026 23:36
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.
Summary
A deployment that both adds a column to an existing dimension and adds a node linking to that new column fails. The root cause is that the deploy order is not taking into account the dimension link dependency, and so the dim is processed before the node-update gets processed.
Problem
A dimension node exists with two columns:
A change adds a third column and, at the same time, a node that links to it:
Deploy ordering is only derived from query lineage, so
us_stateandhard_hatsort into the same level:Both are validated as one batch, and
_prefetch_dimension_link_nodesresolves the dimension's columns from the database, whereus_stateis still the old version without the column.state_abbrexists only in the spec being deployed in that same batch, so the link fails:Solution
This PR adds dimension links to the deploy-ordering graph so that the dimension node lands in an earlier level than anything linking to it.
Each spec's
dimension_linkstargets become ordering edges, added to theordering_graph. They're not added toplan.node_graph, so they influence ordering without being mistaken for parents.Test Plan
make checkpassesmake testshows 100% unit test coverageDeployment Plan
The one exception that we'll fix in a follow-up PR: dimension links can also be cyclic (and query lineage cannot), e.g., join links support self-joins with a role. The case where two dimensions link to each other and where one adds a column the other's link targets would still fail.