215 feat add mandatory satellites parameter to pit - #483
Conversation
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.
| {# 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) -%} |
|
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
|
Recommended changes are implemented in newest commit and new tests can be found here: https://github.com/ScalefreeCOM/datavault4dbt-automatic-tests/pull/241 Premium Package adjustments can be found here: https://github.com/ScalefreeCOM/datavault4dbt-premium-package/pull/6 |
jstendel
added
waiting-on-maintainers
|
dbt test combined result: ❌ DetailsRESULTS for Synapse: RESULTS for Postgres: RESULTS for BigQuery: RESULTS for Redshift: RESULTS for Snowflake: RESULTS for Exasol: RESULTS for Fabric: RESULTS for Oracle: RESULTS for Databricks: RESULTS for SQL Server: RESULTS for Trino: Link to workflow summary: https://github.com/ScalefreeCOM/datavault4dbt-ci-cd/actions/runs/30883322243 |
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: