Feature/retained earnings to sub calendar #187

fivetran-catfritz · 2025-11-21T23:22:12Z

PR Overview

Package version introduced in this PR:

v1.3.0

This PR addresses the following Issue/Feature(s):

Summary of changes:

Add field accounting_period_full_name to end models.
When netsuite2__using_to_subsidiary is enabled, netsuite2__balance_sheet applies each transaction’s to_subsidiary fiscal calendar. If to_subsidiary is null, the model falls back to the fiscal calendar of the transaction’s subsidiary_id.

Submission Checklist

Alignment meeting with the reviewer (if needed)
- Timeline and validation requirements discussed
Provide validation details:
- Validation Steps: Check for unintentional effects (e.g., add/run consistency & integrity tests)
- Testing Instructions: Confirm the change addresses the issue(s)
- Focus Areas: Complex logic or queries that need extra attention
Merge any relevant open PRs into this PR

Changelog

Draft changelog for PR
Final changelog for release review

…_with_converted_amounts.to_subsidiary_id, subsidiaries.subsidiary_id)

fivetran-catfritz · 2025-11-25T02:36:56Z

models/netsuite2/netsuite2__balance_sheet.sql

-
-primary_subsidiary_calendar as (
-    select 
-        fiscal_calendar_id, 
-        source_relation 
-    from subsidiaries 
-    where parent_id is null
-),


lines 224 and 329 take over for this.

fivetran-catfritz · 2025-11-25T02:37:52Z

models/netsuite2/netsuite2__balance_sheet.sql

+
+    {% if using_to_subsidiary_and_exchange_rate %}
+    left join subsidiaries as to_subsidiaries
+        on to_subsidiaries.subsidiary_id = coalesce(transactions_with_converted_amounts.to_subsidiary_id, subsidiaries.subsidiary_id)


coalesce because to_subsidiary_id can be null

fivetran-catfritz · 2025-11-25T02:38:04Z

models/netsuite2/netsuite2__balance_sheet.sql


+    {% if using_to_subsidiary_and_exchange_rate %}
+    left join subsidiaries as to_subsidiaries
+        on to_subsidiaries.subsidiary_id = coalesce(transactions_with_converted_amounts.to_subsidiary_id, subsidiaries.subsidiary_id)


coalesce because to_subsidiary_id can be null

fivetran-joemarkiewicz

@fivetran-catfritz thanks for working through this PR! A few comments and questions before approving.

CHANGELOG.md

macros/staging/get_accountingperiodfiscalcalendars_columns.sql

fivetran-joemarkiewicz · 2025-11-25T17:28:56Z

models/netsuite2/netsuite2__balance_sheet.sql

+    left join accounting_periods as reporting_accounting_periods 
        on reporting_accounting_periods.accounting_period_id = transactions_with_converted_amounts.reporting_accounting_period_id
        and reporting_accounting_periods.source_relation = transactions_with_converted_amounts.source_relation
+        and reporting_accounting_periods.fiscal_calendar_id = {{ 'to_subsidiaries' if using_to_subsidiary_and_exchange_rate else 'subsidiaries' }}.fiscal_calendar_id


Out of curiosity, is this join here necessary if not using to_subsidiaries? For example, was this join previously incorrect having the subsdiaries.fiscal_calendar_id component of the join omitted?

Same question for the transaction_accounting_periods join below.

When I take this out I do get fanout. See the row count for dev vs. prod. Previously this was handled by the old line 230 primary_subsidiary_calendar join, but it limited accounting_periods to those only in the primary subsidiary calendar. For our test data, this was not an issue, but we should not limit it to account for more use cases.

fivetran-catfritz · 2025-11-26T00:24:23Z

models/netsuite2/netsuite2__balance_sheet.sql

+    left join accounting_periods as reporting_accounting_periods 
        on reporting_accounting_periods.accounting_period_id = transactions_with_converted_amounts.reporting_accounting_period_id
        and reporting_accounting_periods.source_relation = transactions_with_converted_amounts.source_relation
+        and reporting_accounting_periods.fiscal_calendar_id = {{ 'to_subsidiaries' if using_to_subsidiary_and_exchange_rate else 'subsidiaries' }}.fiscal_calendar_id


When I take this out I do get fanout. See the row count for dev vs. prod. Previously this was handled by the old line 230 primary_subsidiary_calendar join, but it limited accounting_periods to those only in the primary subsidiary calendar. For our test data, this was not an issue, but we should not limit it to account for more use cases.

CHANGELOG.md

macros/staging/get_accountingperiodfiscalcalendars_columns.sql

fivetran-catfritz · 2025-11-26T01:04:31Z

models/netsuite2/netsuite2__balance_sheet.sql

-        or transactions_with_converted_amounts.is_income_statement
+    where (accounts.is_balancesheet 
+        or transactions_with_converted_amounts.is_income_statement)
+        and transactions_with_converted_amounts.reporting_accounting_period_id is not null


Since we no long have an inner join, line 232 accomplishes the same thing.

fivetran-catfritz · 2025-11-26T01:04:54Z

models/netsuite2/netsuite2__balance_sheet.sql

-        or transactions_with_converted_amounts.is_income_statement
+    where (accounts.is_balancesheet
+        or transactions_with_converted_amounts.is_income_statement)
+        and transactions_with_converted_amounts.reporting_accounting_period_id is not null


Same here. Compensates for the lack on inner join.

CHANGELOG.md

Co-authored-by: Joe Markiewicz <[email protected]>

Co-authored-by: Claude README Updater <[email protected]> Co-authored-by: fivetran-catfritz <[email protected]>

fivetran-joemarkiewicz

LGTM

fivetran-catfritz and others added 2 commits November 21, 2025 17:07

feature/retained-earnings-to-sub-calendar

1129129

Roll RE on to_subsidiary fiscal calendar (#183)

7524475

fivetran-catfritz self-assigned this Nov 21, 2025

fivetran-catfritz linked an issue Nov 21, 2025 that may be closed by this pull request

[Bug] Roll RE on to_subsidiary fiscal calendar #182

Open

4 tasks

fivetran-catfritz added 5 commits November 21, 2025 19:18

updates

5d7c10a

fix vars

ec44d16

add coalesce on to_subsidiaries.subsidiary_id = coalesce(transactions…

cd62249

…_with_converted_amounts.to_subsidiary_id, subsidiaries.subsidiary_id)

changelog

96fad8b

simplification

28b05cf

fivetran-catfritz mentioned this pull request Nov 25, 2025

[Bug] Roll RE on to_subsidiary fiscal calendar #182

Open

4 tasks

fivetran-catfritz commented Nov 25, 2025

View reviewed changes

fivetran-joemarkiewicz self-requested a review November 25, 2025 16:30

fivetran-joemarkiewicz requested changes Nov 25, 2025

View reviewed changes

fivetran-catfritz added 2 commits November 25, 2025 19:02

changelog and remove reporting_accounting_period_id nulls

c7c4e49

update tests

9cce631

fivetran-catfritz commented Nov 26, 2025

View reviewed changes

fivetran-joemarkiewicz reviewed Nov 26, 2025

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

fivetran-catfritz and others added 2 commits November 26, 2025 09:46

Update CHANGELOG.md

c798f55

Co-authored-by: Joe Markiewicz <[email protected]>

MagicBot/bump-dbt-to-v3 (#188)

7d52e5c

Co-authored-by: Claude README Updater <[email protected]> Co-authored-by: fivetran-catfritz <[email protected]>

fivetran-catfritz added the docs:ready Triggers the docs generator workflow. label Nov 26, 2025

Generate dbt docs via GitHub Actions

3376743

fivetran-joemarkiewicz approved these changes Nov 26, 2025

View reviewed changes

Feature/retained earnings to sub calendar #187

Are you sure you want to change the base?

Feature/retained earnings to sub calendar #187

Conversation

fivetran-catfritz commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Overview

Submission Checklist

Changelog

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fivetran-catfritz commented Nov 21, 2025 •

edited

Loading