Fix/ma sat v0 inserting non deltas on incremental runs by pkumar-data · Pull Request #486 · ScalefreeCOM/datavault4dbt

pkumar-data · 2026-06-30T14:46:55Z

Description

Title: fix: ma_sat_v0 incremental delta logic - all adapters including SQL Server & Trino

Body:

Summary

Fixes the ma_sat_v0 incremental delta failure affecting all database adapters:

Re-insertion of non-deltas (loading same data multiple times)
Skipping valid deltas (missing changes when values revert within batch)

Root Cause & Solution

Problem: PARTITION BY with |lower caused casing issues; batch reverts weren't detected

Solution:

Removed identifier casing manipulation (|lower)
Added ROW_NUMBER() as rn to track changes within batches
Only compare rn=1 rows against satellite history; keep rn>1 automatically

Adapters Covered

✅ BigQuery, Databricks, Exasol, Fabric, Oracle, Postgres, Redshift, Snowflake, Synapse
✅ SQL Server (newly included)
✅ Trino (newly included)

Status

Branch has been rebased/merged with main to include SQL Server and Trino implementations.
Ready for merge to main.

… fixing identifier casing and adding rn=1 check

…acle

…adapter implementations

Copilot

Pull request overview

This PR updates the ma_sat_v0 incremental delta logic across multiple adapter-specific implementations to prevent re-inserting non-deltas and to correctly retain intra-batch changes (including “revert within batch” scenarios). It does so by removing identifier casing manipulation in window partitions and introducing an rn (row number) to distinguish the first change in a batch from subsequent changes.

Changes:

Removed |lower casing manipulation from PARTITION BY in “latest in sat” lookups.
Added ROW_NUMBER() (rn) into the source dedupe/change-tracking pipeline.
Updated incremental records_to_insert filtering to always keep rn > 1 rows while only comparing the first row per key to existing satellite history.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
macros/tables/bigquery/ma_sat_v0.sql	Removes `
macros/tables/databricks/ma_sat_v0.sql	Removes `
macros/tables/exasol/ma_sat_v0.sql	Removes `
macros/tables/fabric/ma_sat_v0.sql	Adds `rn` and changes incremental insertion predicate logic (Fabric already escapes identifiers).
macros/tables/oracle/ma_sat_v0.sql	Removes `
macros/tables/postgres/ma_sat_v0.sql	Removes `
macros/tables/redshift/ma_sat_v0.sql	Adds `rn` and changes incremental insertion predicate logic.
macros/tables/snowflake/ma_sat_v0.sql	Removes `
macros/tables/synapse/ma_sat_v0.sql	Removes `

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tkiehn

Additionally to the Review from Copilot:

✅ SQL Server (newly included)
✅ Trino (newly included)

Are these adapter already following the new logic? I don't seem them included in this PR.

tkiehn · 2026-07-06T13:50:38Z

Companion regression test: ScalefreeCOM/datavault4dbt-automatic-tests#240

It adds a dedicated multi-batch fixture (ma_sat_v0_05) reproducing the exact scenario this PR fixes — within one incremental load a multi-active group changes and then reverts to the sat's current latest hashdiff (A → B → A across three ascending ldts). Assertions: rsrc='revert' → 2 rows after init, 6 after the delta run (the pre-fix macro drops the reverting group and yields 4); plus a non-delta rsrc='nochange' over-insertion guard (1/1).

Note it exercises the default multi-batch path (source_is_single_batch/disable_hwm left at defaults), which is where the rn logic lives. Two points from reviewing this PR that the test surfaces:

Single-batch + incremental compiles to an invalid reference. records_to_insert hardcodes WHERE deduped_rows.rn > 1 OR ..., but deduped_rows only exists under {% if not source_is_single_batch %}. With source_is_single_batch=true the CTE is absent and source_cte stays source_data, so the incremental run fails (deduped_rows not found). Suggest guarding it, e.g. WHERE {% if not source_is_single_batch %}{{ source_cte }}.rn > 1 OR {% endif %}NOT EXISTS (...). This is already exercised by ma_sat_v0_03 / ma_sat_v0_04 (both source_is_single_batch: true + incremental).
SQL Server and Trino are not actually changed by this PR despite the description marking them ✅ — macros/tables/sqlserver/ma_sat_v0.sql and macros/tables/trino/ma_sat_v0.sql still carry the old WHERE NOT EXISTS (...) with no rn. Either apply the same fix there or adjust the description.

…r/Trino adapters - Wrap deduped_rows.rn > 1 in if not source_is_single_batch across all 9 adapters (bigquery, snowflake, postgres, databricks, exasol, fabric, oracle, redshift, synapse) to prevent invalid CTE reference when source_is_single_batch=true - Add ROW_NUMBER() rn column to SQL Server and Trino lag_source_data, deduped_row_hashdiff, and deduped_rows CTEs to bring them in line with the other adapters - Apply the same guarded WHERE clause to SQL Server and Trino records_to_insert

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

…ental_runs

tkiehn · 2026-07-13T15:28:54Z

https://github.com/ScalefreeCOM/datavault4dbt-ci-cd/actions/runs/29261996817/job/86857442561

…apters Window functions in latest_entries_in_sat and deduped_row_hashdiff were partitioned by parent_hashkey only, causing all MA key rows in the same hashkey group to share a single rn sequence. Rows with rn>1 were always inserted, bypassing the NOT EXISTS delta check. Fixed by adding src_ma_key_list to every PARTITION BY so each MA key gets its own independent row sequence.

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 17 comments.

…in condition deduped_row_hashdiff (and lag_source_data for subquery adapters) were partitioned by MA key but did not project those columns into the CTE output. The join back to source_data on (hashkey, ldts, hashdiff) alone could multiply rows or assign the wrong rn when multiple MA key rows share the same ldts and hashdiff. MA key columns are now selected in deduped_row_hashdiff and included in the join condition across all 11 adapters.

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.

Comments suppressed due to low confidence (1)

macros/tables/synapse/ma_sat_v0.sql:140

The incremental NOT EXISTS check only compares parent_hashkey + hashdiff, but latest_entries_in_sat is now deduped per (parent_hashkey, src_ma_key). If two different multi-active keys share the same hashdiff, this can incorrectly suppress inserts for one key. Include the MA key column(s) in latest_entries_in_sat and add a multikey(src_ma_key, ...) predicate to this NOT EXISTS filter (and ensure the MA key columns are selected in the latest_entries CTE).

            AND {{ datavault4dbt.multikey(ns.hdiff_alias, prefix=['latest_entries_in_sat', source_cte], condition='=') }}
            )
    {%- endif %}

    )

…S check Add MA key column(s) to latest_entries_in_sat SELECT (and latest_entries_in_sat_prep for subquery-based adapters) so the NOT EXISTS deduplication is scoped per (hashkey, MA key) rather than just per hashkey. Add multikey(src_ma_key, ...) predicate to the NOT EXISTS WHERE clause across all 11 adapters, preventing false suppression when two distinct MA keys share the same hashdiff.

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

pkumar-data added 7 commits April 9, 2026 09:32

Make source_is_single_batch required in eff_sat_v0

8ed70b9

Make source_is_single_batch required in eff_sat_v0

1687fd2

fix: ma_sat_v0 inserting non-deltas on subsequent incremental runs by…

9fbf810

… fixing identifier casing and adding rn=1 check

fix: correct column reference for rn in bigquery ma_sat_v0

0f411fb

fix: ma_sat_v0 incremental delta logic across all adapters

66eafdf

fix: ma_sat_v0 incremental delta logic for databricks, fabric, and or…

947e2d7

…acle

Merge main into fix branch to include SQL Server and Trino ma_sat_v0 …

766e92d

…adapter implementations

pkumar-data requested a review from tkiehn June 30, 2026 14:47

tkiehn requested a review from Copilot July 6, 2026 09:14

Copilot started reviewing on behalf of tkiehn July 6, 2026 09:15 View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

tkiehn requested changes Jul 6, 2026

View reviewed changes

pkumar-data requested review from Copilot and tkiehn July 7, 2026 17:20

Copilot started reviewing on behalf of pkumar-data July 7, 2026 17:20 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

tkiehn added the waiting-on-maintainers Waiting for a response from the maintainers. label Jul 13, 2026

Merge branch 'main' into fix/ma_sat_v0_inserting_non_deltas_on_increm…

57b1da4

…ental_runs

pkumar-data requested a review from Copilot July 14, 2026 11:46

Copilot started reviewing on behalf of pkumar-data July 14, 2026 11:46 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

pkumar-data requested a review from Copilot July 14, 2026 12:03

Copilot started reviewing on behalf of pkumar-data July 14, 2026 12:03 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

pkumar-data requested a review from Copilot July 14, 2026 12:25

Copilot started reviewing on behalf of pkumar-data July 14, 2026 12:25 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Uh oh!

Conversation

pkumar-data commented Jun 30, 2026

Description

Summary

Root Cause & Solution

Adapters Covered

Status

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tkiehn left a comment

Choose a reason for hiding this comment

Uh oh!

tkiehn commented Jul 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

tkiehn commented Jul 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants