215 feat add mandatory satellites parameter to pit#483
Open
jstendel wants to merge 8 commits into
Open
Conversation
Contributor
Author
tkiehn
reviewed
Jun 30, 2026
tkiehn
reviewed
Jun 30, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds optional “satellite coverage” filtering to the PIT macro so PIT rows can be restricted to entities that have matching satellite records at the snapshot time (via per-satellite mandatory flags or a global mandatory_strategy of 'any'/'all'). It also updates adapter-specific PIT implementations and the PIT documentation accordingly.
Changes:
- Extend
pit/adapter PIT macros withmandatory_strategyand supportsat_namesentries as{name, mandatory}dicts (with normalization for backward compatibility). - Apply mandatory satellite match filtering consistently across supported adapters, and drop unknown/ghost records when mandatory filtering is active.
- Document the new
sat_namesdict form andmandatory_strategybehavior in22_pit.md.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| macros/tables/pit.sql | Adds mandatory_strategy, documents it, and normalizes sat_names entries to dicts before dispatch. |
| macros/tables/bigquery/pit.sql | Updates PIT SQL generation to use satellite.name and apply mandatory filtering (default implementation). |
| macros/tables/databricks/pit.sql | Updates PIT SQL generation to use satellite.name and apply mandatory filtering. |
| macros/tables/exasol/pit.sql | Updates PIT SQL generation to use satellite.name and apply mandatory filtering. |
| macros/tables/fabric/pit.sql | Updates PIT SQL generation to use satellite.name and apply mandatory filtering. |
| macros/tables/oracle/pit.sql | Updates PIT SQL generation to use satellite.name and apply mandatory filtering. |
| macros/tables/postgres/pit.sql | Updates PIT SQL generation to use satellite.name and apply mandatory filtering (with WHERE composition). |
| macros/tables/redshift/pit.sql | Updates PIT SQL generation to use satellite.name and apply mandatory filtering via HAVING. |
| macros/tables/snowflake/pit.sql | Updates PIT SQL generation to use satellite.name and apply mandatory filtering (including snapshot optimization path). |
| macros/tables/sqlserver/pit.sql | Updates PIT SQL generation to use satellite.name and apply mandatory filtering. |
| macros/tables/synapse/pit.sql | Updates PIT SQL generation to use satellite.name and apply mandatory filtering. |
| macros/tables/trino/pit.sql | Updates PIT SQL generation to use satellite.name and apply mandatory filtering. |
| docs/01_macro-instructions/22_business-vault/22_pit/22_pit.md | Documents the new sat_names dict entries and the mandatory_strategy parameter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+102
to
+111
| {# Normalize sat_names: plain strings become {'name': ..., 'mandatory': false} for backward compatibility. #} | ||
| {%- set sat_names_normalized = [] -%} | ||
| {%- for sat in sat_names -%} | ||
| {%- if sat is string -%} | ||
| {%- do sat_names_normalized.append({'name': sat, 'mandatory': false}) -%} | ||
| {%- else -%} | ||
| {%- do sat_names_normalized.append({'name': sat['name'], 'mandatory': sat.get('mandatory', false)}) -%} | ||
| {%- endif -%} | ||
| {%- endfor -%} | ||
| {%- set sat_names = sat_names_normalized -%} |
| {%- set refer_to_ghost_records = datavault4dbt.yaml_metadata_parser(name='refer_to_ghost_records', yaml_metadata=yaml_metadata, parameter=refer_to_ghost_records, required=False, documentation=pit_type_description) -%} | ||
| {%- set refer_to_ghost_records = datavault4dbt.yaml_metadata_parser(name='refer_to_ghost_records', yaml_metadata=yaml_metadata, parameter=refer_to_ghost_records, required=False, documentation=refer_to_ghost_records_description) -%} | ||
| {%- set snapshot_optimization = datavault4dbt.yaml_metadata_parser(name='snapshot_optimization', yaml_metadata=yaml_metadata, parameter=snapshot_optimization, required=False, documentation=snapshot_optimization_description) -%} | ||
| {%- set mandatory_strategy = datavault4dbt.yaml_metadata_parser(name='mandatory_strategy', yaml_metadata=yaml_metadata, parameter=mandatory_strategy, required=False, documentation=mandatory_strategy_description) -%} |
| | tracked_entity | string | mandatory | – | Name of the tracked Hub entity. Must be available as a model inside the dbt project. | | ||
| | hashkey | string | mandatory | – | The name of the hashkey column inside the previously referred Hub entity. | | ||
| | sat_names | list of strings| mandatory | – | A list of all the satellites that should be included in this PIT table. Can only be satellites that are attached to the tracked Hub, and should typically include all those satellites. You should always refer here to the version 1 satellites, since those hold the load-end-date. The macro currently supports regular satellites and nh-satellites. | | ||
| | sat_names | list of strings or dicts| mandatory | – | A list of all the satellites that should be included in this PIT table. Can only be satellites that are attached to the tracked Hub, and should typically include all those satellites. You should always refer here to the version 1 satellites, since those hold the load-end-date. The macro currently supports regular satellites and nh-satellites. Each entry can be a plain satellite name, or a dict with `name` and an optional `mandatory` (boolean, default `false`). When `mandatory` is `true`, PIT rows are only included where that satellite found a match at the snapshot time. | |
Collaborator
|
Hi @jstendel, could you please also add a test with defined input and expected output per setting to the testing-repo and mention this PR there? |
tkiehn
added
the
waiting-on-response
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.
Description
Adds optional satellite coverage filtering to the PIT macro, so a PIT can be limited to entities that actually have descriptive data, rather than every hub × snapshot combination:
mandatoryflag:sat_namesnow accepts either a plain satellite name or a{name, mandatory}dict. Whenmandatory: true, a PIT row is only emitted where that satellite has a record at the snapshot timemandatory_strategy: 'any'(row kept if at least one satellite matched, OR) or'all'(row kept only if every satellite matched, AND), applied across all satellites. Mutually exclusive with the per-satellite flags22_pit.mdFixes: 215
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Verified end-to-end on a local Postgres instance and the
215-feat_add_mandatory_satellites_parameter_to_pitbranch inside the datavault4dbt-automatic-tests repo where additional pit models and test for those new models were created.Test Configuration:
215-feat_add_mandatory_satellites_parameter_to_pitChecklist: