Support ref() and DAG dependencies in PropertyGraph - #2258
Conversation
a5f5c57 to
d06d1bd
Compare
| const resolved = this.session.indexedActions.find(rawRef); | ||
| if (resolved.length !== 1) { | ||
| return false; | ||
| } | ||
| const resolvedAction = resolved[0]; | ||
| const target = resolvedAction.getTarget(); | ||
| if (resolvedAction instanceof Declaration) { | ||
| entityOrRel.dataSource = target; | ||
| } else { | ||
| entityOrRel.dataSource = dataform.Target.create({ | ||
| database: target.database && this.session.finalizeDatabase(target.database), | ||
| schema: this.session.finalizeSchema(target.schema), | ||
| name: this.session.finalizeName(target.name) | ||
| }); | ||
| } |
There was a problem hiding this comment.
There was a problem hiding this comment.
resolve() returns SQL string, we need target. I could still unify it introducing shared helper. Is this what you'd expect?
| } | ||
| this.pendingRefs.set(pendingKey, rawRef); | ||
| const depKey = `${rawRef.database || ""}.${rawRef.schema || ""}.${rawRef.name}`; | ||
| if (!this.dependencyKeys.has(depKey)) { |
There was a problem hiding this comment.
I think this logic should respect https://docs.cloud.google.com/dataform/docs/dependencies#set-assertions-as-dependencies
dataform/core/actions/table.ts
Line 370 in b5a1435
There was a problem hiding this comment.
Done, but note graph.yaml has no surface for these flags today (DataSourceRef proto is just {name, schema, database}), so the plumbing is inert.
Would you like keep it that way for now? Revert it? Or add support through proto?
|
|
||
| this.removeNonUniqueActionsFromCompiledGraph(compiledGraph); | ||
|
|
||
| this.finalizePropertyGraphs(); |
There was a problem hiding this comment.
why do we need a separate callback for graph actions here? Regular SQLX actions also have to insert resolved ref in generated SQL body and I'm wondering if it's possible to have it using consistent interfaces inside session
There was a problem hiding this comment.
SQLX ref() runs inside the query closure, which the session doesn't evaluate until every action is indexed - so ref() sees a fully-populated index at call time. YAML has no closure, so we get the equivalent deferral by stashing raw refs in construction and draining them in finalize(). Could contextify graphBody as a Contextable to mirror SQLX, but that changes YAML from data into code.
d06d1bd to
b7b8fda
Compare
Extends PropertyGraph to accept `ref: [dataset, table]` in place of a bare table name for `nodeTables` and `edgeTables`. Refs resolve through the same `Session.resolve` path used by tables and operations so the graph action inherits the standard dependency edges. The resolved dependencies land on the compiled action's `dependencyTargets` which makes the executor run source tables before the graph DDL. Ref resolution respects schema suffixes and database overrides so a graph declared with `ref: [authors]` in a suffixed workspace still targets the suffixed dataset. Lock-in tests cover the resolved-DDL output for named refs, dataset-scoped refs, mixed literal-and-ref entries, and the error surface when a ref points at a nonexistent action.
b7b8fda to
638a793
Compare
2 participants
Extends PropertyGraph to accept
ref: [dataset, table]in place of a bare table name fornodeTablesandedgeTables. Refs resolve through the sameSession.resolvepath used by tables and operations so the graph action inherits the standard dependency edges.The resolved dependencies land on the compiled action's
dependencyTargetswhich makes the executor run source tables before the graph DDL. Ref resolution respects schema suffixes and database overrides so a graph declared withref: [authors]in a suffixed workspace still targets the suffixed dataset.Lock-in tests cover the resolved-DDL output for named refs, dataset-scoped refs, mixed literal-and-ref entries, and the error surface when a ref points at a nonexistent action.