Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
2 of 4 tasks
priya-cribl opened this issue Oct 22, 2024 · 6 comments · Fixed by #149
Closed
2 of 4 tasks
Assignees
Labels
error:unforced type:bug Something is broken or incorrect

Comments

@priya-cribl
Copy link

Is there an existing issue for this?

  • 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
netsuite2__multibook_accounting_enabled: true # False by default.
netsuite2__using_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.
  • Yes, but I will need assistance.
  • No.
@priya-cribl priya-cribl added the type:bug Something is broken or incorrect label Oct 22, 2024
@fivetran-joemarkiewicz
Copy link
Contributor

@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
Copy link
Author

@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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Author

priya-cribl commented Nov 8, 2024 via email

@fivetran-catfritz
Copy link
Contributor

Hi @priya-cribl This update has been released as v0.16.0. You can install if using the following:

packages:
  - package: fivetran/netsuite
    version: [">=0.16.0", "<0.17.0"]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error:unforced type:bug Something is broken or incorrect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants