fix: handle dbt deferral in get_elementary_relation#1006
Conversation
When --favor-state or --defer is active, Elementary models may not be built in the CI target schema. adapter.get_relation() returns None in this case, causing anomaly tests to fail with 'relation "none" does not exist'. Fall back to constructing a Relation from graph node coordinates when the physical catalog lookup returns None but the node exists in the dbt graph. This allows the generated SQL to reference the correct (deferred) schema. Co-Authored-By: Noy Arie <noyarie1992@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe macro captures the adapter relation lookup into a ChangesRelation Lookup Fallback
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
👋 @NoyaArie |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| {% if identifier_node %} | ||
| {% do return( | ||
| api.Relation.create( | ||
| database=elementary_database, | ||
| schema=elementary_schema, | ||
| identifier=identifier_alias, | ||
| ) | ||
| ) %} | ||
| {% endif %} |
There was a problem hiding this comment.
🔴 Fallback api.Relation.create() breaks truthiness-based existence checks in all callers
When adapter.get_relation() returns None (table doesn't physically exist) but identifier_node is present in the graph, the new fallback constructs a synthetic relation via api.Relation.create(). Since Elementary models are always in the dbt graph once the package is installed, identifier_node is always truthy for them (see macros/utils/graph/get_node.sql). This means get_elementary_relation() now never returns None for any installed Elementary model, even when the underlying table hasn't been materialized yet.
This breaks the contract that numerous callers depend on — they use truthiness of the return value to guard against operating on non-existent tables:
Affected callers that rely on a falsy return
macros/edr/dbt_artifacts/upload_dbt_artifacts.sql:24:{% if elementary.get_elementary_relation(artifacts_model) %}— guard bypassed, attempts upload to non-existent tablemacros/edr/dbt_artifacts/upload_dbt_artifacts.sql:49-52:{% if not artifacts_hash_relation %}— guard bypassed, runs SELECT on non-existent tablemacros/edr/tests/test_execution_sla.sql:196:{% if not run_results_relation %}— error handling bypassedmacros/edr/tests/on_run_end/handle_tests_results.sql:176:{% if not target_relation %} raise_missing...— missing-model detection disabledmacros/edr/tests/on_run_end/handle_tests_results.sql:246: same as above forschema_columns_snapshot- All
upload_dbt_*.sqlfiles use{% if execute and relation %}guards that are now bypassed
For example, on a partial dbt run --select some_model (not including elementary models) or any scenario where the elementary tables don't yet exist, callers will proceed to execute SQL (INSERT/SELECT) against tables that don't exist, causing runtime database errors. The fallback should be gated on actual deferral being active, rather than unconditionally applied whenever the graph node exists.
Prompt for agents
The fallback in get_elementary_relation.sql (lines 33-41) creates a synthetic relation via api.Relation.create() whenever identifier_node exists and adapter.get_relation() returns None. Since identifier_node is always truthy for installed Elementary models (they're always in the dbt graph), this means the function never returns None for any elementary model, even when the physical table doesn't exist.
This breaks all callers that use truthiness checks on the return value to detect non-existent tables (e.g. upload_dbt_artifacts.sql:24, handle_tests_results.sql:176, test_execution_sla.sql:196, and all upload_dbt_*.sql files).
Possible fixes:
1. Gate the fallback on dbt deferral being active. Check if dbt's defer/favor-state flags are set (e.g., via flags or var) before applying the fallback.
2. Only apply the fallback when the node's database/schema differ from the current target, which would indicate deferral is remapping coordinates.
3. If neither is feasible, update all callers to separately verify table existence (e.g., via adapter.get_relation()) rather than relying on this function's return value. But this would be a much larger change.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — fixed in fdcb302. The fallback is now gated on invocation_args_dict.defer or invocation_args_dict.favor_state being truthy, so it only fires when dbt deferral is actually active. Without deferral, get_elementary_relation() returns None as before, preserving the existing contract for all callers.
Only construct a synthetic Relation when dbt deferral is actually active (invocation_args_dict.defer or favor_state). Without this guard, the fallback would fire for any installed Elementary model whose table doesn't exist yet (e.g. partial dbt run), breaking callers that rely on a None return to detect missing tables. Co-Authored-By: Noy Arie <noyarie1992@gmail.com>
NoyaArie
deleted the
devin/1778682933-fix-get-elementary-relation-deferral
branch
2 participants
Summary
When dbt's
--favor-stateor--deferflags are active (common in dbt Cloud CI jobs), Elementary models are deferred to the production schema and not built in the CI target schema. Theget_elementary_relation()macro usesadapter.get_relation()— a physical catalog lookup — to resolve Elementary tables likedata_monitoring_metrics. This returnsNonewhen the table doesn't exist in the CI schema, which gets rendered literally asfrom Nonein the generated SQL, causing all anomaly tests to fail with:This fix adds a fallback gated on deferral being active (
invocation_args_dict.deferorinvocation_args_dict.favor_state): whenadapter.get_relation()returnsNonebut the model node exists in the dbt graph and deferral is active, we construct a Relation object from the graph node coordinates usingapi.Relation.create(). This allows the generated SQL to reference the correct (deferred) schema instead of failing.Without deferral, the function returns
Noneas before, preserving the existing contract for all callers that use truthiness checks to detect missing tables.Affected test types: all anomaly monitors (volume, freshness, all_columns, dimension), plus
test_execution_slaand result-handling macros.Related: Pylon #3812, closed PR #1002.
Review & Testing Checklist for Human
--favor-statewhere Elementary models exist in prod but not in the CI schema. Anomaly tests should now generate SQL referencing the deferred schema instead offrom None.--defer/--favor-state—adapter.get_relation()should still return a real relation as before, and if the table doesn't exist,Noneis returned (fallback is skipped because deferral is not active).dbt run(not including Elementary models) followed by anomaly tests — callers should still getNoneand raise the "missing Elementary models" error as before, since the deferral guard prevents the fallback from firing.Notes
The fallback is gated on
invocation_args_dict.get("defer", false) or invocation_args_dict.get("favor_state", false), which ensures it only activates during dbt commands that explicitly use deferral. This preserves backward compatibility for all existing callers.Link to Devin session: https://app.devin.ai/sessions/b9eb2d85cd8f4489bf67686c7b413af0
Requested by: @NoyaArie
Summary by CodeRabbit