Repository navigation
Remove Staging Introspection Pre-Release - #93
Open
fivetran-catfritz wants to merge 64 commits into
Open
fivetran-catfritz wants to merge 64 commits into
fivetran-catfritz wants to merge 64 commits into
Conversation
Replaces the per-model adapter.get_columns_in_relation() introspection (and the camelCase-guessing add_renamed_columns/coalesce_rename jinja built for the raw-API-casing -> snake_case connector migration) with an exhaustive quickstart-supplied column payload var per table (canonical_name -> current_name in source, renamed or not). New macros/staging/normalize_column_payload.sql and apply_column_payload.sql replace fill_staging_columns + column_list_to_dict + coalesce_rename. get_*_columns() macros now carry alias/datatype overrides directly instead of at coalesce_rename call sites. All 14 stg_salesforce__* models collapsed to a single CTE selecting straight from source, with zero adapter.get_columns_in_relation calls (verified via compiled SQL). integration_tests/dbt_project.yml gains identity-mapped payload vars per table (simulating what the runtime would supply), with one deliberate rename (opportunity.description -> Description__c, see sf_opportunity_data.csv) proving the rename-without-introspection path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two changes based on feedback: 1. Bring back the fields -> final two-CTE shape instead of one collapsed CTE. `fields` (via apply_column_payload, mirroring fivetran_utils.fill_staging_columns' own branching) resolves each column to its one true source reference with zero introspection. `final` goes back to being an explicit, one-line-per-column list -- now via a new cast_and_alias_column macro (coalesce_rename minus the coalesce, since fields already picked the correct spelling) so casting, aliasing, and per-column overrides stay readable and easy to adjust inline, same as before. get_*_columns() macros are back to their original shape (just add_renamed_columns removed) since datatype/alias overrides live at the cast_and_alias_column call sites again, not in the column list. 2. Replaced the 14 per-table payload vars with a single salesforce__column_payload var for the whole package, keyed by schema then table (identifier) -- the shape a real quickstart runtime would actually emit, and one that a future multi-org/union implementation can index by source_relation without a payload format change. normalize_column_payload now drills into payload[schema][table] instead of taking an already-flat dict. Re-verified against the local DuckDB target: all 14 models build, zero adapter.get_columns_in_relation/information_schema in compiled SQL, and the deliberate opportunity.description -> Description__c rename still resolves correctly through fields into final. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
final was still going through a per-column macro + dict lookup (cast_and_alias_column), which is the same shape of indirection as old salesforce's coalesce_rename, just without the coalesce. That's not how other Fivetran packages write their final CTE (see e.g. google_ads' stg_google_ads__campaign_stats: fields resolves/fills columns via a macro, final is hand-written SQL -- bare column, `x as alias`, or `cast(x as type) as alias` only where a real type override is needed). Replaced every cast_and_alias_column(...) call site with the equivalent plain SQL line and dropped cast_and_alias_column.sql and column_list_to_dict.sql (nothing references them anymore). fields is unchanged from the last commit -- apply_column_payload still does the payload-driven resolution there with zero introspection. Re-verified against the local DuckDB target: all 14 models build, zero adapter.get_columns_in_relation/information_schema in compiled SQL, and the opportunity.description -> Description__c rename still resolves (fields: `Description__c as description`; final: `description as opportunity_description`). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The payload's second level was keyed by each customer's actual physical identifier (e.g. sf_account_data), which meant the model still needed the yml-configured identifier var just to know which key to look up. Flipped it: keyed by the package's own standard table name (the literal already passed to source(), e.g. 'account'), with a `__identifier__` entry inside each table's dict giving the actual physical name. normalize_column_payload's table_name param is now that standard name instead of a resolved identifier. Each model builds its FROM relation with api.Relation.create(), using the payload's __identifier__ (falling back to the standard name if absent) instead of relying on source(...).identifier -- verified this resolves correctly via run-operation before rolling it out to all 14 models. This makes the per-table `salesforce__<table>_identifier` vars in src_salesforce.yml redundant for the staging models themselves, though source() there still matters for freshness checks, docs, and catalog generation, which don't go through the payload -- leaving those yml identifier overrides in place rather than removing them. Re-verified against DuckDB: all 14 models build, FROM resolves to the payload's __identifier__ (confirmed against sf_account_data), zero introspection in compiled SQL, and the description -> Description__c rename still resolves end to end. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Opportunity's description -> Description__c was the only rename demo so far. Added a second, more representative example on account: account_number -> AccountNumber (raw camelCase API casing, the actual historical case add_renamed_columns/coalesce_rename existed to paper over) mapped via the account payload, and dropped `website` from the seed entirely (payload has no entry for it) to exercise the null-fill path on a model other than opportunity. Verified via compiled SQL: fields resolves `AccountNumber as account_number` with zero introspection, and final null-fills `cast(null as TEXT) as website` using get_account_columns()'s declared datatype. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both are redundant now: the FROM relation is built from the payload's __identifier__ (see 3e00ed9), so the 14 per-table `identifier: {{ var(...) }}` overrides in src_salesforce.yml no longer do anything for staging. Freshness checks come out in the same pass since they depend on that same identifier mapping being configured in the yml, which it no longer is -- removed the source-level loaded_at_field/warn_after/ error_after config and the 10 per-table `freshness: null` overrides that existed only to opt back out of it. Re-verified against DuckDB: all 14 models still build with the same pre-existing docs warnings (nothing new), since none of them relied on source().identifier for their FROM clause anymore. Added STAGING_V2_POC_NOTES.md summarizing the whole POC: motivation, payload format, the two new macros, the restored fields/final model shape, what got deleted and why, how it was verified, and open questions for anyone building on this. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every model repeated the same three set-lines to resolve source(),
normalize the payload, and rebuild the relation from __identifier__.
Pulled all of it into a new select_payload_fields(table_name,
staging_columns) macro that emits the full `select ... from ...` body
for the fields CTE in one call. get_*_columns() is now called inline in
that macro call too, since nothing else in any model needs the raw
column list anymore -- final is plain SQL and doesn't reference it.
Each model's fields CTE is now just:
with fields as (
{{ salesforce.select_payload_fields('campaign_member', get_campaign_member_columns()) }}
),
Re-verified against DuckDB: all 14 models still build with the same
pre-existing docs warnings (nothing new), zero introspection in compiled
SQL, and the account/opportunity rename and null-fill demos still
resolve identically to before.
Updated STAGING_V2_POC_NOTES.md to describe select_payload_fields and
the simplified model shape.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…erride
Removing the per-table identifier overrides and freshness config two
commits ago broke the only tie dbt itself has to the real physical
table: source('salesforce','account') would resolve to the literal
default 'account', while the model actually queried whatever the
payload's __identifier__ said. Nothing but convention kept those in
sync, and anything that calls source() directly instead of going
through select_payload_fields (docs, dbt source freshness, tests) would
silently point at the wrong table.
Restored src_salesforce.yml exactly as it was on main (14 identifier
overrides + freshness config). select_payload_fields now treats the
payload's __identifier__ as an override on top of that, not a
replacement: falls back to table_source.identifier (the yml/var-
resolved one) when a table is missing from the payload or has no
__identifier__, instead of a bare literal table_name. Verified with the
payload emptied out entirely -- the model still builds and the FROM
clause correctly resolves to the yml-configured identifier
(sf_account_data), not a literal default or a silent wrong-table query.
Rewrote the relevant sections of STAGING_V2_POC_NOTES.md to describe
this as the actual design (source() as the yml tie, __identifier__ as
an override, the safer fallback) rather than the identifier-removal
approach that came before it, and flagged the identifier-lives-in-two-
places tradeoff as an open question.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…vars
The previous commit restored the per-table identifier vars and made the
payload's __identifier__ a runtime override on top of them -- correct,
but left the identifier configured in two independent places that could
silently drift apart, with dbt's own tooling (docs, freshness, lineage)
only ever seeing the yml/var one.
Closed that gap instead of just living with it: added
get_source_identifier(table_name, default=table_name), which reads
salesforce__column_payload's __identifier__ directly (falling back to
the literal standard table name), and pointed every identifier: config
in src_salesforce.yml at it instead of the old
salesforce_<table>_identifier vars (including order's Snowflake
reserved-word branch). Since this runs wherever source('salesforce',
table_name) does, source(...) itself now resolves to the payload's
identifier -- select_payload_fields no longer needs its own
api.Relation.create() override, it can just select from table_source
directly. One source of truth instead of two.
Removed the 14 now-dead salesforce_<table>_identifier vars from
integration_tests/dbt_project.yml (left the _history_identifier ones
alone -- unrelated history models, out of scope here).
Re-verified against DuckDB: all 14 models build with the same
pre-existing warnings, zero introspection in compiled SQL, source()
itself resolves to sf_account_data (not a separately-built relation),
and with the payload emptied out the fallback correctly resolves to the
literal table name (confirmed via a real "table not found" error against
this project's sf_-prefixed seeds, not a crash).
Updated STAGING_V2_POC_NOTES.md throughout to describe this as the
actual design and closed out the "identifier in two places" open
question, replacing it with the parse-time var availability question
this raises instead.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Surfaced on BigQuery, not DuckDB: salesforce__campaign_performance
failed joining stg_salesforce__campaign.campaign_id (string, aliased
from id) against stg_salesforce__opportunity.campaign_id with "No
matching signature for operator = for argument types: STRING, INT64".
Root cause: final only cast a column when the original code had an
explicit datatype= override, on the assumption that matched
stg_google_ads__campaign_stats' style. But fields (apply_column_payload,
like fill_staging_columns before it) never casts a present column, only
a null-filled absent one -- so an uncast column's real type is whatever
the warehouse infers for the raw source column, and BigQuery infers
INT64 for sf_opportunity_data.csv's all-blank campaign_id column with no
explicit schema. The old coalesce_rename macro always cast every column
regardless of override; that was a real safety net this POC had quietly
dropped, not just style DuckDB's more permissive inference didn't expose.
Fixed by making every column in every final CTE an explicit
cast(col as {{ datatype }}) as alias_or_col line -- still hand-written
plain SQL, just with nothing skipped. Regenerated all 14 models
carefully this time: an earlier attempt at this same fix used naive
whole-file string replacement and corrupted stg_salesforce__opportunity
(a bare "type" column match clobbered "dbt.type_timestamp()" elsewhere
in the file) -- reverted that and redid it via anchored whole-block
splicing instead, then verified brace balance and absence of leftover
coalesce_rename calls across all 14 files before testing.
Fixed a config drift bug found in the same BigQuery run: the payload's
top-level schema key was still salesforce_integrations_tests_4 after
salesforce_schema was changed to zz_dbt_catherine for BigQuery testing,
so every identifier lookup missed and fell back to the literal table
name. Updated the payload's key to match.
Re-verified against DuckDB: all 24 models (staging + downstream) build
clean, zero introspection in compiled SQL, and specifically
salesforce__campaign_performance (the model that surfaced the bug)
succeeds.
Rewrote the relevant STAGING_V2_POC_NOTES.md sections: corrected the
model-shape example, added "A real bug this caught" documenting the
casting bug and its BigQuery-vs-DuckDB angle, and confirmed (against a
real dbt-core 1.11.12 install, not inferred) that get_source_identifier
in identifier: config only works on dbt 2.0 -- real dbt-core rejects
custom macros in yml property-file Jinja entirely. Flagged both as open
questions, plus the schema-key drift footgun.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two changes based on feedback: 1. get_*_columns() macros used a mix of literal "boolean" and dbt.type_boolean() across different tables (pre-existing inconsistency from main). Standardized all 14 macros to dbt.type_boolean(). 2. The last commit's "cast every column in final" fix was correct but more than necessary -- the real risk is narrower than "any column, any time": it's specifically columns compared to another table's value (a join key) or combined with one (inside coalesce()), since that's the only place a silently-wrong warehouse-inferred type causes a real failure. Audited every downstream model (models/salesforce/*.sql, models/salesforce/intermediate/*.sql) for join/coalesce references to raw staging columns and narrowed each final CTE to cast only those columns plus the pre-existing datatype= overrides -- everything else went back to a bare reference or `col as alias`. Per table this is 0-7 cast lines out of 6-43 total columns, down from casting everything. Re-verified against DuckDB: all 24 models build clean, zero introspection in compiled SQL, and salesforce__campaign_performance (the model that surfaced the original bug) still succeeds with the narrower cast set. Updated STAGING_V2_POC_NOTES.md: corrected the model-shape example, rewrote "A real bug this caught" with the full per-table audit table and reasoning, and flagged in Open questions that this cast list is a manual, point-in-time audit with nothing enforcing it -- a new downstream join could silently reintroduce the same bug class. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moved the "join/coalesce cast list isn't enforced" note out of Open questions and folded it into "A real bug this caught" as an accepted maintenance expectation instead: keeping the cast list in sync when a new downstream join is added is ordinary package maintenance, the same as the old coalesce_rename pattern already required per-column setup for every new field. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
normalize_column_payload and apply_column_payload were only ever called
from select_payload_fields -- three files to hunt across for one job.
Folded their logic directly into select_payload_fields, and generalized
it to take source_name as its first argument (needed anyway to reuse it
for the salesforce_history source, not just salesforce).
get_source_identifier no longer depends on the now-removed
normalize_column_payload -- inlined the same small drill-down directly,
and changed it to take schema_name as an explicit argument (the caller,
a source yml's identifier: config, already computes it via
var('salesforce_schema', 'salesforce')) instead of hardcoding that var
lookup itself, so it works for salesforce_history's
salesforce_history_schema too.
Updated all 14 model call sites (select_payload_fields('salesforce',
'account', ...)) and all 15 identifier: configs in src_salesforce.yml
(including order's Snowflake branch) to pass these explicitly.
Re-verified against DuckDB: all 24 models build clean with the same
pre-existing warnings, zero introspection in compiled SQL, FROM still
resolves to the payload's identifier (sf_account_data), and the
account_number -> AccountNumber rename still resolves.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The 4 salesforce__*_daily_history models had their own separate
introspection: dbt_utils.star(from=source('salesforce_history', table),
except=[...]) is adapter.get_columns_in_relation under a different
name. Replaced it with a `fields` CTE using select_payload_fields,
reusing each core table's own get_*_columns() macro (via a new
get_history_columns(base_columns, id_alias) macro that adds
_fivetran_start/_fivetran_end and re-aliases id) instead of writing 4
near-duplicate get_*_history_columns() macros.
select_payload_fields and get_source_identifier both gained an explicit
payload_key (defaulting to table_name) to support this: this fixture's
salesforce_history_schema equals salesforce_schema, so the core
`account` table and the history `account` table would otherwise collide
on the same payload[schema]['account'] entry. History models use
payload_key='account_history' etc.; the 14 core calls are unaffected.
Updated src_salesforce_history.yml's 4 identifier: configs to use
get_source_identifier the same way the core source yml does, and added
payload entries for the 4 history tables to integration_tests/
dbt_project.yml.
Auditing each history seed's actual physical columns (not just the
source yml's documented list, which was incomplete) surfaced 4 columns
declared in get_*_columns() that don't exist in this fixture's history
seeds (account: billing_state_code, shipping_country_code,
shipping_state_code; contact: department, mailing_state_code) --
removed from the payload so they null-fill instead of erroring.
Re-verified against DuckDB: all 28 models (14 staging + 10 downstream +
4 history) build clean with --full-refresh, zero
introspection/dbt_utils.star anywhere in compiled SQL.
Updated STAGING_V2_POC_NOTES.md with a new "History models" section and
refreshed the macro list for the select_payload_fields/
get_source_identifier consolidation from the previous commit.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
account_number/AccountNumber and website were both blank in every row of this seed already, so the rename and null-fill mechanisms were only confirmable by reading compiled SQL -- nothing in the actual query output demonstrated them. Populated the seed's first row with a real, easy-to-spot demo: name = 'Acme Test Corp', account_number (renamed from AccountNumber) = 'ACC-100234', plus realistic peer values (industry, type, annual_revenue, description). website (the dropped/missing column) stays null-filled, but now visibly so -- it's the one blank field on an otherwise fully-populated row, instead of blending into a seed where everything is blank. Verified: querying stg_salesforce__account for this row shows account_number = 'ACC-100234' (rename resolved) and website = None (missing column) side by side. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The payload was schema-then-table nested (payload[schema][table]),
justified as future-proofing for multi-org/union. In practice it was a
real, recurring source of failures instead: whenever salesforce_schema
or salesforce_history_schema pointed at a different environment (e.g.
switching integration_tests from DuckDB to BigQuery) without also
updating the payload's top-level schema key to match, every identifier
lookup missed and silently fell back to the literal table name --
exactly the "lots of BQ failures" this session hit.
Salesforce doesn't union across schemas/orgs in staging, so the schema
level was pure risk with no payoff for this package. Flattened
select_payload_fields and get_source_identifier to payload[payload_key]
directly, dropping their schema_name parameters. All 15 identifier:
configs in src_salesforce.yml and 4 in src_salesforce_history.yml
updated to match (no more var('salesforce_schema', ...) argument).
Flattened integration_tests/dbt_project.yml's payload one level to
match -- same 18 table entries, no schema key.
The macro shape itself (resolve source, drill the payload by a single
key, apply_column_payload-style column loop) is still the reusable
part for porting to a package that does need multi-org/union -- that
package should reintroduce a schema or source_relation level deliberately,
not inherit this one by default.
Verified the actual failure mode is gone: compiled with salesforce_schema
overridden to an arbitrary unrelated value and confirmed the FROM clause
still resolves to the payload's __identifier__ (sf_account_data), not a
literal fallback. Full project (28 models) still builds clean against
DuckDB with zero introspection, using the project's own default payload
with no --vars override needed (previously required one matching the
schema in use).
Updated STAGING_V2_POC_NOTES.md throughout: payload format, model shape
example, get_source_identifier signature, the history-model payload_key
rationale (now framed as a whole-package flat namespace collision, not
a same-schema coincidence), and closed out the schema-key-drift open
question as resolved rather than still open.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fully flattening the payload (previous commit) fixed the BigQuery
schema-drift failures but gave up being schema-shaped at all -- a
package that later needs real multi-org/union support would have to
redesign the payload format from scratch rather than extend it.
Restored payload[schema][table] nesting in select_payload_fields and
get_source_identifier, but changed how the schema level is resolved:
both macros check salesforce__column_payload's own length first. If it
has exactly one schema entry, that entry is used directly -- schema_name
isn't even consulted, so a mismatched salesforce_schema/
salesforce_history_schema can't break anything, which is the actual
failure mode this session hit twice. Exact-match lookup against
schema_name only happens once a second schema entry genuinely exists in
the payload, which is exactly when disambiguating matters.
get_source_identifier's schema_name parameter and all 15 (core) + 4
(history) identifier: configs' var('salesforce_schema', ...) /
var('salesforce_history_schema', ...) arguments are back. integration_tests/
dbt_project.yml's payload is re-nested under a zz_dbt_catherine schema key
(same 18 table entries, one level deeper).
Verified all three cases directly: (1) this fixture's single-schema
payload resolves correctly even with salesforce_schema overridden to an
arbitrary unrelated value (shortcut, unaffected by drift); (2) a
synthetic two-schema payload correctly returns the queried schema's own
identifier; (3) querying a schema present in neither entry falls back to
the literal default rather than silently picking either schema's data
(exact match, once real ambiguity exists). Full 28-model project still
builds clean against DuckDB.
Updated STAGING_V2_POC_NOTES.md throughout to describe this as the final
design: payload format, both macros' signatures and docstrings, the
model-shape/identifier examples, the history-model payload_key rationale
(back to a same-schema-collision framing), and reworked the two relevant
Open Questions bullets (schema-key-drift closed out with the actual fix,
multi-org/union noting the framework is ready but untested against a
real union model).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ed columns
This package's own Buildkite build (#273) failed identically on all
three targets (postgres, snowflake, duckdb) at `dbt seed`:
Compilation Error
Could not render {{ salesforce.get_source_identifier(var('salesforce_schema', 'salesforce'), 'account') }}: 'salesforce' is undefined
Real dbt-core's YAML property-file Jinja rendering never exposes custom
macros, only built-in globals (var/env_var/target) -- confirmed this
already, interactively, earlier in this work, but the get_source_identifier
call had been left in identifier: on the assumption of a dbt-2.0-only
target. CI running real dbt-core proved that assumption wrong in
practice, not just in theory.
Fixed by moving the identifier override out of the source yml entirely:
src_salesforce.yml's 14 identifier: configs and src_salesforce_history.yml's
4 are back to exactly what main had -- plain var('salesforce_<table>_identifier',
'<table>') -- with the corresponding vars restored in
integration_tests/dbt_project.yml. select_payload_fields now calls
get_source_identifier itself (a normal macro call from a model, where
custom macros always work) and builds an overriding relation via
api.Relation.create(), falling back to source(...)'s own plain-var
identifier -- not a bare literal -- when the payload has no entry.
get_source_identifier's own implementation didn't need to change, only
who calls it and from where.
Verified against real dbt-core this time, not dbt-oss: `dbt parse`
against a real dbt-core==1.11.12 install succeeds (the exact command
that used to fail).
Found a second, distinct bug building all 28 models against a real
Postgres instance under real dbt-core: stg_salesforce__account/
stg_salesforce__opportunity failed with column "accountnumber"/
"description__c" does not exist -- Postgres folds an unquoted
identifier to lowercase, so the compiled `AccountNumber as
account_number` (bare) didn't match the actual case-preserved column.
select_payload_fields now always quotes a resolved current-name
reference (quote_column with quote=true), reusing the same
per-warehouse quoting logic quote_column already had for exactly this
case (including Snowflake's uppercase-folding special case). Quoting a
plain lowercase/underscore name this way is a no-op everywhere else.
Re-verified end to end: all 28 models build clean against a real
Postgres instance under real dbt-core 1.11.12, and querying the account
demo row directly confirms account_number = 'ACC-100234' (rename,
correctly quoted) and website = NULL (missing column) on a real
warehouse, not just DuckDB.
Rewrote the relevant STAGING_V2_POC_NOTES.md sections to describe the
full arc (three identifier designs tried, why, and what each one broke)
with the actual CI evidence, and closed out both now-resolved Open
Questions bullets.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This package's own Buildkite CI (bigquery v1) failed after the
identifier fix, in the 4 history models this time:
No matching signature for operator <= for argument types: DATETIME, TIMESTAMP
in daily_history's join against spine. The original pre-POC code
explicitly cast _fivetran_start/_fivetran_end to dbt.type_timestamp()
right where they were extracted from dbt_utils.star()'s exclusion.
select_payload_fields never casts a present column by design, so these
two passed straight through as whatever BigQuery actually stored them as
(DATETIME, not TIMESTAMP, in this seed) -- nothing re-cast them before
the comparison that needed them to match.
Missed in the earlier join/coalesce cast audit because that was done
against the 14 core staging models before the history models existed in
their payload-driven form -- same "new join needs its columns added"
maintenance case already flagged as accepted, just for a new model
instead of a new join in an existing one.
Fixed the same way as the earlier campaign_id bug: cast both join
operands explicitly at the comparison
(cast(get_latest_daily_value._fivetran_start as {{ dbt.type_timestamp() }})
<= cast(spine.date_day as {{ dbt.type_timestamp() }})) in all 4 history
models, rather than restructuring their `select *` (portable "select all
except N columns" isn't supported identically across all 6 warehouses
this package targets).
Re-verified against real Postgres (dbt-core 1.11.12) and DuckDB -- no
BigQuery credentials available for interactive testing, so real CI is
the actual confirmation for the BigQuery-specific type this targets.
Also documented in STAGING_V2_POC_NOTES.md that this package's CI runs
every warehouse twice (v1 = real dbt-core, v2 = dbt-oss/Fusion), that
every fix in this POC traced back to a v1-only failure, and that v2
passing is not by itself sufficient (an earlier commit in this PR had
every v2 job failing too, from the schema-key-drift bug).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The move to select_payload_fields (5113e83) was fixing what looked like a real-dbt-core compatibility requirement, but this package only needs to work under dbt-oss/Fusion (v2) going forward -- v1 was never the actual target. The earlier "v2 failures" that caused confusion (build #272) were a separate, unrelated bug (schema-key drift, fixed by the single-schema shortcut), not evidence the yml-based design was broken under v2. src_salesforce.yml's 14 identifier: configs and src_salesforce_history.yml's 4 call salesforce.get_source_identifier(...) directly again. select_payload_fields reads straight from source(source_name, table_name) with no overriding relation. Removed the 18 now-redundant salesforce_<table>[_history]_identifier vars from integration_tests/dbt_project.yml -- the payload's own __identifier__ field is the only source of truth again. Kept the Postgres case-folding quoting fix (quote_column with quote=true), since it's unrelated to identifier placement. Verified against dbt-oss 2.0.5: dbt parse succeeds, all 28 models (history included) build clean against DuckDB with zero errors, and both demos still resolve correctly -- account_number = 'ACC-100234' (rename) / website = NULL (missing column), and opportunity's "Description__c" as description (rename) in compiled SQL. Confirmed real dbt-core 1.11.12 fails as expected and accepted: dbt parse hits Could not render {{ salesforce.get_source_identifier(...) }}: 'salesforce' is undefined, the same failure mode build #273 hit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Restored every boolean column's datatype in the 14 get_*_columns macros to exactly match main's own (admittedly inconsistent) mix of dbt.type_boolean() and plain "boolean" literal -- this POC's only intentional change here is dropping the add_renamed_columns() call per macro, and the boolean datatype swap was unrelated noise bloating the PR diff. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…s to vars.yml
Identifiers: dropped the whole payload-driven identifier concept, not just
the yml-macro part of it. get_source_identifier.sql is deleted,
src_salesforce.yml's 14 identifier: configs and src_salesforce_history.yml's
4 are byte-identical to main again (plain
var('salesforce_<table>_identifier', '<table>')), and the payload's 18
__identifier__ fields are gone -- it only ever carries column rename info
now, never physical table names. Physical table naming already had a
working convention (the per-table _identifier vars); every identifier
design this POC tried was solving a problem that didn't need solving.
select_payload_fields keeps its payload-driven column-presence/rename
logic and reads straight from source(source_name, table_name), no
override.
Vars: per https://docs.getdbt.com/docs/build/project-variables?version=2
(dbt v1.12+/Fusion), moved integration_tests/dbt_project.yml's entire
vars: block into a new integration_tests/vars.yml, parsed before
dbt_project.yml itself. dbt_project.yml no longer has a vars: key of its
own (can't be defined in both files).
Verified against dbt-oss 2.0.5: dbt parse succeeds, all 28 models build
clean against DuckDB, and both demos still resolve correctly --
account_number = 'ACC-100234' (rename) / website = NULL (missing column),
and opportunity's "Description__c" as description (rename) -- with
identifiers now resolving via the restored plain vars in vars.yml rather
than the payload.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Completes the previous commit: that one landed the identifier.yml/macro reverts but missed staging these files. vars.yml: per https://docs.getdbt.com/docs/build/project-variables?version=2 (dbt v1.12+/Fusion), moved integration_tests/dbt_project.yml's entire vars: block (18 per-table identifier vars, salesforce_schema/ salesforce_history_schema, salesforce__column_payload) into a new integration_tests/vars.yml, parsed before dbt_project.yml itself. dbt_project.yml no longer has a vars: key of its own (can't be defined in both files) and the payload's 18 __identifier__ fields are gone from it -- column rename info only now, never physical table names. select_payload_fields.sql: docstring updated to describe the final identifier design (plain main-logic vars, no payload involvement) instead of the now-deleted get_source_identifier. STAGING_V2_POC_NOTES.md: documents both changes and their verification (dbt parse + full 28-model DuckDB build, both demos still resolving correctly). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
integration_tests/vars.yml (added in the previous commit) is a dbt v1.12+/Fusion feature (https://docs.getdbt.com/docs/build/project-variables?version=2). This package's v1 CI track defaults to dbt-core ">=1.3.0,<1.12.0", which excludes 1.12+ entirely, so without this override every v1 warehouse job would silently fail to load any vars from vars.yml. Verified directly against a real dbt-core 1.12.0 install (not dbt-oss): dbt parse and dbt compile both succeed against real Postgres, and the compiled SQL's FROM clause resolves to sf_account_data -- confirming vars.yml's salesforce_account_identifier var was actually loaded, not silently defaulting to the literal table name. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…188)
Per GA-1038188 and the linked Slack thread (Catherine/Fedor Safronov/Joe
Markiewicz): destination/canonical names are unstable as a lookup key --
physical spelling shifts with feature flags, connection settings, and
customer renames -- but the connector's own source field name (shown in
the connection's schema tab, matching the API) never changes.
salesforce__column_payload is renamed to salesforce__source_config, and
its shape changes from schema -> table -> {canonical_name: current_name}
to schema -> SourceTableName -> {name: ..., columns: {SourceFieldName:
current_name}} (Joe's agreed structure). SourceTableName/SourceFieldName
are the Salesforce API's own names (e.g. Account, AccountNumber), not
this package's snake_case ones. The per-table name field is reserved for
a future identifier migration (same Jira issue) and is deliberately not
read yet -- table identifiers stay on the existing separate vars, per
Joe's explicit deferral.
select_payload_fields computes each column's source field name from its
canonical name via name.split('_') | map('capitalize') | join(''), the
same guess the now-deleted add_renamed_columns macro used to make --
grepping every get_*_columns macro on main confirms it never needed an
explicit override for this guess, across every table, in years of
production use. An explicit override (source_name on a column dict) is
still supported for a future column that breaks the pattern, but none of
the 14 existing macros needed one, so none of them changed. Fivetran
metadata columns skip the lookup entirely (synthetic, never renamed,
always present).
History models' explicit payload_key strings updated to match
(account_history -> AccountHistory, etc.) -- needed only because this
test environment deliberately uses the same literal schema value for
salesforce_schema and salesforce_history_schema, so the core and history
variants would otherwise collide in the same schema dict.
Verified against dbt-oss 2.0.5 (dbt parse, full 28-model DuckDB build)
and a real dbt-core==1.12.0 install (dbt compile against real Postgres):
both demos still resolve identically -- "AccountNumber" as account_number
and "Description__c" as description in compiled SQL.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
select_payload_fields.sql and vars.yml's salesforce__source_config comment described how the design evolved (prior rejected approaches, CI build numbers, "now-retired" macro names, "per the agreed design in GA-1038188"). Trimmed to describe only what the code does now -- the evolution is already in STAGING_V2_POC_NOTES.md and GA-1038188 for anyone who wants it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Each column's Salesforce API source_name was being computed at render
time (name.split('_') | map('capitalize') | join('')). Switched to
explicit, hardcoded input instead: every column in every get_*_columns
macro now carries a literal source_name, and every select_payload_fields
call site passes payload_key explicitly (it's now a required param, no
longer defaulted from table_name). The values are unchanged -- that
transform's own track record is why they're trustworthy -- only the
mechanism changed, from inferred to written down. A future column with
an irregular API name is now a one-line addition, not a silent mismatch
against an implicit formula.
Verified against dbt-oss 2.0.5: dbt parse succeeds, all 28 models build
clean against DuckDB, and both demos still resolve identically --
account_number = 'ACC-100234', website = NULL, and opportunity's
"Description__c" as description.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…_*_columns calls select_payload_fields didn't describe what the macro does. Renamed to resolve_source_columns and updated all 18 call sites (14 core staging models, 4 inside the history models' get_history_columns(...) call). Also namespaced every get_*_columns() call site as salesforce.get_*_columns(), for consistency with salesforce.resolve_source_columns(...) and salesforce.get_history_columns(...). Verified against dbt-oss 2.0.5: dbt parse succeeds, all 28 models build clean against DuckDB. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A customer who configures salesforce__source_config for zero or some
tables was getting every column in an unconfigured table null-filled --
the var's absence was indistinguishable from "every column is missing."
That's backwards for anyone not using the exhaustive quickstart payload:
a customer who's customized nothing should get a normal, unrenamed
table, not an empty one.
resolve_source_columns now checks whether this table has an entry in
salesforce__source_config at all (not just whether a given column does).
If it doesn't -- the var is unset entirely, or a customer only
configured other tables -- every column is assumed present under its
own canonical name (the connector's documented default behavior), same
as fivetran_utils.fill_staging_columns' own live-schema check used to
guarantee, just assumed instead of introspected. A customer now only
needs to add an entry for a table they're actually customizing.
Quickstart's exhaustive mode is unaffected -- every table it configures
still works exactly as before.
Verified against dbt-oss 2.0.5: the existing exhaustive config still
builds all 28 models clean with both demos intact; forcing
salesforce__source_config to {} builds every un-customized table
cleanly via the new fallback, and correctly fails loudly (a real DB
error, not silent wrong data) on the one seed with a genuine unconfigured
rename quirk (AccountNumber) -- the expected tradeoff for a customer who
hasn't told us about it. Also confirmed compiling under real
dbt-core==1.12.0.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Per review: trims the long explanation blocks in integration_tests/dbt_project.yml (including a Jira ticket reference and the per-line "see note below" tags), and condenses the resolve_source_columns and get_history_columns headers to a few lines each. Comments only. Verified locally (dbt-core 1.12 + duckdb): full build + test 96/96 with history on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fivetran-data-model-bot
removed
the
docs:ready
Comment on lines
+27
to
+28
| - title: Renames 9 table identifier variables to match the Salesforce API's own object | ||
| names |
Contributor
There was a problem hiding this comment.
I'd maybe call this Under the Hood since it's backwards compatible but that may just be personal preference
| - If your connector left a field in a non-standard spelling and you don't declare | ||
| it, the build now fails with a column-not-found error instead of resolving it | ||
| automatically. | ||
| - See the README for configuration details. |
Contributor
There was a problem hiding this comment.
I'd remove this if we're not gonna have anything in the README about the variable yet. Perhaps we can include an example here or at least link to the integration_tests/dbt_project.yml sample configs?
| it, the build now fails with a column-not-found error instead of resolving it | ||
| automatically. | ||
| - See the README for configuration details. | ||
| - title: Adds naming_standard and an introspective-queries opt-in to salesforce__source_config |
Contributor
There was a problem hiding this comment.
Could you consolidate this into the above entry? It's a bit confusing that it's a new variable but also there is an entry about an addition to it
| details: | ||
| - The old variable names still work as a fallback, so no action is required unless | ||
| you want to adopt the new names. | ||
| date: '2026-10-22' |
Contributor
There was a problem hiding this comment.
should this be updated
Per Jamie's review on the .changes entry: - Fold the naming_standard / introspective-queries entry into the salesforce__source_config entry as details, rather than a separate "addition to" a variable introduced in the same release. - Replace "See the README" (the README doesn't cover the var yet) with a link to the sample config in integration_tests/dbt_project.yml. - Move the identifier var rename to Under the Hood, since it's backwards compatible. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replaces the previous automatic column detection with a new variable for explicit configuration of renamed or unsynced columns, and renames table identifier variables to align with Salesforce API names.
fivetran-catfritz
added
docs:ready
fivetran-data-model-bot
removed
the
docs:ready
Removed unnecessary code block formatting and updated the date line.
fivetran-catfritz
added
docs:ready
fivetran-data-model-bot
removed
the
docs:ready
fivetran-data-model-bot
removed
the
docs:ready
fivetran-jamie
approved these changes
Oct 30, 2026
fivetran-jamie
left a comment
fivetran-jamie
left a comment
Contributor
There was a problem hiding this comment.
Changelog is rendering a bit off but otherwise approved!
Comment on lines
+11
to
+12
| - Set `naming_standard: source` to use Salesforce's original column names by default. | ||
| - Set `fivetran_packages_using_introspective_queries: true` to restore automatic |
Contributor
There was a problem hiding this comment.
The changelog rendered this a bit oddly. Might need to remove the colons or escape them with a backslash
fivetran-data-model-bot
removed
the
docs:ready
fivetran-data-model-bot
removed
the
docs:ready
1 task
1 task
This branch has not been deployed
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.
PR Overview
Package version introduced in this PR:
This PR addresses the following Issue/Feature(s):
Summary of changes:
adapter.get_columns_in_relationintrospection that staging and daily history models ran at everydbt run/dbt compile, replacing it with a singleresolve_source_columnsmacro driven by a newsalesforce__source_configvariable (per the design agreed with the dbt DX team in GA-1038188). A table you haven't configured keeps working as before; add an entry only for a table where a field has been renamed or is genuinely missing.Submission Checklist
Changelog