Conversation
fivetran-jamie
changed the title
…tsuite into optimize-runtime
fivetran-joemarkiewicz
added
the
docs:ready
fivetran-joemarkiewicz
added
the
pre-release
| partition_by = ({'field': '_fivetran_synced_date', 'data_type': 'date', 'granularity': 'month'} | ||
| if target.type not in ['spark', 'databricks'] else ['_fivetran_synced_date']) if transaction_level | ||
| else ({'field': 'accounting_period_ending', 'data_type': 'date', 'granularity': 'month'} |
There was a problem hiding this comment.
Necessary since _fivetran_synced_date is not present in the aggregated version
| if target.type not in ['spark', 'databricks'] else ['_fivetran_synced_date']) if transaction_level | ||
| else ({'field': 'accounting_period_ending', 'data_type': 'date', 'granularity': 'month'} | ||
| if target.type not in ['spark', 'databricks'] else ['accounting_period_ending']), | ||
| cluster_by = ['transaction_id'] if transaction_level else ['account_id', 'accounting_period_id'], |
There was a problem hiding this comment.
Necessary since transaction_id is not present in the aggregated version
| partition_by = ({'field': '_fivetran_synced_date', 'data_type': 'date', 'granularity': 'month'} | ||
| if target.type not in ['spark', 'databricks'] else ['_fivetran_synced_date']) if transaction_level | ||
| else ({'field': 'accounting_period_ending', 'data_type': 'date', 'granularity': 'month'} | ||
| if target.type not in ['spark', 'databricks'] else ['accounting_period_ending']), |
There was a problem hiding this comment.
Necessary since _fivetran_synced_date is not present in the aggregated version
| if target.type not in ['spark', 'databricks'] else ['_fivetran_synced_date']) if transaction_level | ||
| else ({'field': 'accounting_period_ending', 'data_type': 'date', 'granularity': 'month'} | ||
| if target.type not in ['spark', 'databricks'] else ['accounting_period_ending']), | ||
| cluster_by = ['transaction_id'] if transaction_level else ['account_id', 'accounting_period_id'], |
There was a problem hiding this comment.
Necessary since transaction_id is not present in the aggregated version
fivetran-jamie
changed the title
fivetran-jamie
added
docs:ready
| {% do surrogate_key_fields.append('to_subsidiary_id') if using_to_subsidiary_and_exchange_rate %} | ||
| {% do surrogate_key_fields.append('accounting_book_id') if multibook_accounting_enabled %} | ||
| surrogate_key as ( | ||
| {% set surrogate_key_fields = ['source_relation', 'transaction_line_id', 'transaction_id', 'accounting_period_id', 'account_name', 'account_id'] if transaction_level else ['source_relation', 'accounting_period_id', 'account_name', 'account_id', 'subsidiary_id'] %} |
There was a problem hiding this comment.
had to add subsidiary_id in for the aggregated case
…tsuite into optimize-runtime
|
|
||
| {% if is_incremental() %} | ||
| where _fivetran_synced_date >= {{ netsuite.netsuite_lookback(from_date='max(_fivetran_synced_date)', datepart='day', interval=lookback_window) }} | ||
| {% if transaction_level %} |
There was a problem hiding this comment.
Is this necessary since we already know the incremental config will only be used if the model is configured for transaction_level. Otherwise, no incremental strategy is used.
There was a problem hiding this comment.
Yeah this was necessary for Buildkite. Also if Core users want to override the configs to aggregate + run incrementally this will make things work for them
There was a problem hiding this comment.
It looks like we take a somewhat different approach to aggregating in the income_statement model vs the balance_sheet. In the balance sheet model we first aggregate and then do all dimensional joins. In the income statement we do all dimensional joins and then aggregate.
The approach in the balance sheet looks more performant. Is there a reason there are two different approaches used across the models?
There was a problem hiding this comment.
Nope, adjusting
There was a problem hiding this comment.
Reminder to also update the model descriptions
There was a problem hiding this comment.
Lmk if I should update them in the README as well
There was a problem hiding this comment.
Yes let's update the README to reflect the different states of the model. In the README let's not be highly technical, but explain the two states and how they can be toggled.
fivetran-joemarkiewicz
left a comment
fivetran-joemarkiewicz
left a comment
There was a problem hiding this comment.
A few small requests, but ready to go once updates are applied and docs are regenerated.
There was a problem hiding this comment.
Yes let's update the README to reflect the different states of the model. In the README let's not be highly technical, but explain the two states and how they can be toggled.
| The Cumulative Translation Adjustment total, which in most cases does not have transactions associated with it, is calculated manually. | ||
| columns: | ||
| - name: transaction_id | ||
| description: Netsuite internal transaction ID. |
There was a problem hiding this comment.
Let's add to these descriptions that these fields are only present when the aggregate variables are set to false. Let's apply to all the impacted fields.
|
Also, looks like there are some merge conflicts to address from the patch release before merging. |
Co-authored-by: Joe Markiewicz <74217849+fivetran-joemarkiewicz@users.noreply.github.com>
…tsuite into optimize-runtime
|
@copilot resolve the merge conflicts in this pull request |
3 participants
PR Overview
Package version introduced in this PR:
This PR addresses the following Issue/Feature(s):
Summary of changes:
Submission Checklist
Changelog