priya-cribl commented 1 month ago

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the issue

Noticing that in the netsuite dbt package that the netsuite2__transaction_details.sql is doing an incremental leveraging the transactions table (line 52). However, sometimes a transaction is not updated but the transaction line is (line 42). Since the incremental is placed on the transactions instead of on the transaction_lines it causes the data not to be refreshed.

Relevant error log or model output

No response

Expected behavior

Would expect thenetsuite2__transaction_details.sql incremental logic to catch updates either to transaction or transaction lines, not just hte latter

Possible solution

Shift the incremental logic down and make it an or statement between transation lines last_synced or transaction last_synced

dbt Project configurations

vars: dbt_date:time_zone: "UTC" dbt_constraints_enabled: true

adding vars below to use netsuite package https://hub.getdbt.com/fivetran/netsuite/latest/

netsuite_data_model: netsuite2 netsuite_database: raw netsuite_schema: netsuite netsuite2multibook_accounting_enabled: true # False by default. netsuite2using_to_subsidiary: true # False by default. netsuite2__using_jobs: false netsuite2__using_employees: false balance_sheet_transaction_detail_columns: ['company_name','vendor_name'] income_statement_transaction_detail_columns: ['is_account_intercompany','location_name'] netsuite: lookback_window: 3 # default is 3 transactions_pass_through_columns:

name: "CUSTBODY_CELIGO_SFIO_SF_ID"
name: "CUSTBODY_CRIBL_SUBSCRIPTIONTERM"
name: "CUSTBODY_CRIBL_ENDUSER"
name: "CUSTBODY_CRIBL_RESELLER"
name: "CUSTBODY_CRIBL_DISTRIBUTOR"
name: "TYPEBASEDDOCUMENTNUMBER"
name: "STARTDATE" transaction_lines_pass_through_columns:
name: "QUANTITYBILLED"
name: "RATE"
name: "CUSTCOLCRIBL_ITEMSKU"
name: "BILLINGSCHEDULE"
name: "CUSTCOL_AGENCY_MF_FLIGHT_START_DATE"
name: "CUSTCOL_AGENCY_MF_FLIGHT_END_DATE"
name: "CUSTCOL_AGENCY_MF_LINE_ID"
name: "QUANTITYSHIPRECV"
name: "QUANTITY"

Package versions

packages:

package: calogica/dbt_date version: [">=0.10.1"]

Install dbt_utils separately. dbt_utils removed from dbt_date since 0.7.0. see https://github.com/calogica/dbt-date/issues/52
package: dbt-labs/dbt_utils version: [">=1.1.1"]
package: fivetran/netsuite version: [">=0.14.0", "<0.15.0"]

What database are you using dbt with?

snowflake

How are you running this dbt package?

dbt Cloud™

dbt Version

dbt=1.7.11

Additional Context

No response

Are you willing to open a PR to help address this issue?

[ ] Yes.
[X] Yes, but I will need assistance.
[ ] No.

fivetran-joemarkiewicz commented 4 weeks ago

@priya-cribl thanks for raising this issue and bringing it to our attention. When looking into this further I can see the scenario you're talking about and how our incremental logic could miss this. However, I'm wondering if we could simplify the proposed solution as I worry adding an or statement to the incremental strategy could have unintended consequences.

Currently we have the incremental strategy applied to the transaction table. I wonder if we can swap the incremental strategy to be based on the transaction_line table instead. My only concern here is if there's a risk that we run into a similar problem where a transaction is updated, but the lines are not. Do you see this being a possible issue?

priya-cribl commented 4 weeks ago

@fivetran-joemarkiewicz I tested that hypothesis and I don't have a single transaction that is updated after the transaction line. However, to be on the safe side I would either 1) remove the incremental or 2) do the or incremental

fivetran-joemarkiewicz commented 4 weeks ago

Thanks for confirming you don't see any transactions updated after the transaction lines. I'm still unsure about the "or incremental" and what the unintended consequences that could lead to. I'll investigate this a bit on our end to see how/if this would work.

In the meantime, since you're installing the package in your dbt project you can always turn off the incremental logic. To do so, you can follow the similar steps outlined in our README here, but adjust it to turn off the incremental strategy as opposed to enabling it. It would look something like the following:

models:
  netsuite:
    netsuite2:
      +full_refresh: true

This will ensure that each model in the Netsuite package will run as a full refresh which will ignore any incremental strategy used by default in Netsuite models.

Let me know if the above helps address this in the interim. In the meantime, I'll continue to investigate this and see if something along your suggestion is possible.

fivetran-joemarkiewicz commented 2 weeks ago

@priya-cribl I wanted to share that we were unable to find a scalable and reliable way to modify the incremental logic to check for updates in either the transactions or transaction_lines tables. However, it does look like swapping the incremental logic to work at the line level is feasible and something we can update in a future Netsuite release.

We have a new release planned this sprint; however, I will wait to integrate this change until the subsequent breaking change so we can ideally test with yourself and other customers to ensure quality code before releasing. In the meantime, please feel free to use the above config to turn off the incremental strategy.

Someone from my team will share details once we are committing to this update and we will share a branch which you'll be able to test before we release the update. Thanks again for raising this to our attention and we appreciate your insights!

priya-cribl commented 2 weeks ago

Sounds good and thanks!

On Wed, Nov 6, 2024 at 3:00 PM Joe Markiewicz @.***> wrote:

@priya-cribl https://github.com/priya-cribl I wanted to share that we were unable to find a scalable and reliable way to modify the incremental logic to check for updates in either the transactions or transaction_lines tables. However, it does look like swapping the incremental logic to work at the line level is feasible and something we can update in a future Netsuite release.

We have a new release planned this sprint; however, I will wait to integrate this change until the subsequent breaking change so we can ideally test with yourself and other customers to ensure quality code before releasing. In the meantime, please feel free to use the above config to turn off the incremental strategy.

Someone from my team will share details once we are committing to this update and we will share a branch which you'll be able to test before we release the update. Thanks again for raising this to our attention and we appreciate your insights!

— Reply to this email directly, view it on GitHub https://github.com/fivetran/dbt_netsuite/issues/142#issuecomment-2460964480, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVPEGIXM2FV6F5F2VIUGFN3Z7KNQZAVCNFSM6AAAAABQM2OXGCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRQHE3DINBYGA . You are receiving this because you were mentioned.Message ID: @.***>

fivetran / dbt_netsuite

[Bug] Incremental logic does not address transaction line updates when transaction itself is not updated #142