[Feature]Update `_fivetran_deleted` Filtering Strategy

[fivetran-catfritz]

Background

Currently, we filter out _fivetran_deleted rows at the staging layer here. However, this approach prevents deleted rows from flowing downstream, which can cause issues with incremental models recognizing these rows for proper deletion handling.

For example, in cases where a transaction is deleted in the source, the deletion does not propagate to the final models because the _fivetran_deleted rows are removed early in the pipeline.

Proposed Solution

Update the _fivetran_deleted filtering strategy to defer the removal of these rows to downstream transformations. This change would allow incremental models to process deletions correctly while preserving the ability to exclude deleted rows in the final outputs.

To do

• Requires updating all models downstream of staging to ensure _fivetran_deleted rows are handled appropriately.
• Needs validation to ensure no unintended side effects, such as retaining deleted rows in final outputs.

Steps to Implement

1. Remove the _fivetran_deleted filtering logic from staging models.
2. Update downstream models to explicitly filter _fivetran_deleted rows where necessary.
3. Write tests to confirm that deleted rows are processed correctly in incremental and full-refresh scenarios.

Additional Context

This change is proposed as an alternative solution to address incremental data quality issues, particularly for users who cannot schedule full-refreshes.

Open Questions

• What performance implications might this change introduce in larger datasets?

[jmongerlyra]

We experienced this issue of deleted transactions remaining in incremental models. We have scheduled full rebuilds but they only run once a week. I look forward to another solution.

[bryanphuff]

Would like to second this issue. This is preventing our organization from trusting the output of out BALANCE_SHEET object, as it is often times including transactions that have been deleted. In models downstream of the balance sheet, we see failed tests for referential integrity on our transaction ids, as we are now including transactions that do not exist in Netsuite, but are retained in our Balance Sheet model.

A fix here would greatly increase trust in our Netsuite-based models and our data platform at RigUp.

[fivetran-catfritz]

@jmongerlyra @bryanphuff One option we’re considering is carrying the _fivetran_deleted column through to the final models. This would allow you to filter out deleted records in your upstream usage in between full refreshes, while still retaining flexibility on how you want to treat them. Would that be helpful to you?

[fivetran-catfritz]

Another consideration if we took this approach is how to specifically handle the deleted records. I'm thinking we could either:

• Persist all deleted records in the model for consistency or
• Only update rows where _fivetran_deleted changes between full refreshes, which would create fewer records but might be less intuitive to interpret

Welcome thoughts on that as well.

[bryanphuff]

@fivetran-catfritz thank you for the response! I believe the ideal scenario from our perspective would be that the _fivetran_deleted column is carried through to the final model, and, at least in the case of the balance_sheet object, would be used to update any records in the model where the transaction tied to the record has been since been deleted at the source. If the transaction tied to the record ever goes from _fivetran_deleted = false to fivetran_deleted = true, we would want to remove all records from the balance_sheet that are tied to that transaction.

As far as I am aware, once a transaction has been deleted at the source, our teams are no longer interested in the transaction and have likely created a replacement.

[fivetran-catfritz]

@bryanphuff thanks for the extra context. To expand a bit more--one of the limitations with dbt incremental models (merge or delete+insert) is that they only add or update records during an incremental run. They don’t natively support removing rows that no longer exist in the source, so true hard deletes typically aren’t possible without rewriting the entire table, which would erase the performance benefits of incrementals.

I am seeing two paths we can take:

Option 1 (quicker fix): Carry the _fivetran_deleted column through to the final models. This is simple to implement, preserves the benefits of incremental runs, and gives you a clear flag for which rows are deleted. The trade-off is that you would filter on _fivetran_deleted in downstream queries rather than having dbt physically remove the rows.

Option 2 (longer-term): Explore a two-stage approach where the incremental model runs as usual and then a post-hook that goes back and deletes any rows flagged as _fivetran_deleted = true after the model is run. This could bring us closer to true hard deletes, but it would require additional investigation and testing to make sure it works reliably across warehouses since this would be new for us.

While it may not be ideal, would option 1 be usable in the meantime as we work toward option 2?

[bryanphuff]

Hi @fivetran-catfritz yes Option 1 works for now. I have implemented something similar on my end to ensure our models downstream of the balance sheet are not impacted. I am also scheduling a full refresh of the models to ensure we remove deleted transactions on a regular basis.

We look forward to a solution like Option 2, though! Our finance teams have grown accustomed to seeing their changes in Netsuite reflected in the data in a timely manner, so still seeing deleted transactions in our Balance Sheet from time to time is an adjustment we are working through.

[fivetran-joemarkiewicz]

@jmongerlyra @bryanphuff I wanted to chime in on this thread because we are taking a closer look at this issue (along with others) and believe an ideal approach to ensure the integrity of the Balance Sheet along with the other end models would be to no longer materialize as incremental (by default). This will resolve the data drift issue highlighted in this thread; however, it would likely come at the cost of performance.

Before moving forward, I'm looking for feedback to ensure this is the right decision. As highlighted within #127 we found an area of optimization is to remove the cluster_by config in the end models. From our initial testing within PR #189 this looks to offer performance gains! That being said, I'm eager to hear your feedback:

• What are your thoughts if the end models within this package no longer materialized incrementally in order to ensure data accuracy and remove the data drift issue? This likely would result in longer duration job runs (as each run will now be a --full-refresh), but will ensure accurate results.
  • If you are interest, you can install the v1.4.0-a2 pre-release and let me know your thoughts.
• Lastly, I'm also interested in generally understanding how you use the final balance_sheet and income_statement reports. We currently offer these at the transaction_line level. However, I'm wondering if this level of granularity is all that necessary.

[jmongerlyra]

What are your thoughts if the end models within this package no longer materialized incrementally in order to ensure data accuracy and remove the data drift issue? This likely would result in longer duration job runs (as each run will now be a --full-refresh), but will ensure accurate results.
If you are interest, you can install the v1.4.0-a2 pre-release and let me know your thoughts.

Removing incremental materialization would be a step backwards for us. The balance sheet model takes ~1 hour to build with --full-refresh. We have a full refresh scheduled weekly to handle any data drift.

Lastly, I'm also interested in generally understanding how you use the final balance_sheet and income_statement reports. We currently offer these at the transaction_line level. However, I'm wondering if this level of granularity is all that necessary.

The balance_sheet and income_statement models are incredibly helpful since these are the two primary NetSuite reports end users interact with. If they were removed, we would end up rebuilding. We use both models extensively, whereas the transactional model is rarely used.

[fivetran-joemarkiewicz]

@jmongerlyra thanks for your feedback, this is really helpful.

Regarding the balance_sheet and income_statement models. I'm not proposing we remove these models. Instead I'm curious if the level of granularity we currently offer them as is necessary. For example, both the balance_sheet and income_statement models are loosely offered at the transaction_line, transaction, account, period granularity. This results in an end model that is quite large in some cases and is a key reason the incremental logic is currently required to help with runtimes. Our other financial dbt package models of the same type (see QuickBooks, Sage Intacct, and Xero as examples) are provided at the account and period granularity. Effectively presenting the same balance_sheet and income_statement information, but at a higher level of granularity.

Given that, when you use the balance_sheet and income_statement models, do you leverage the transaction_line level granularity? Or do you end up aggregating at the account and period level to replicate the Netsuite reports? If the transaction_line level is necessary for accurate report recreation then we will make sure to retain this structure. However, in an effort to explore improving performance, reducing the total output of this end model could be a possibility if proven to be the right approach.

[jmongerlyra]

@fivetran-joemarkiewicz We leverage drilling down to the transaction line level in both balance_sheet and income_statement models. It does make for large tables, however (for Lyra), the tradeoff is worth it.

[fivetran-joemarkiewicz]

@jmongerlyra thanks for your input, that's helpful to know the transaction line level of granularity is useful.