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/date not recognized #54

Merged
merged 11 commits into from
Jan 15, 2025
34 changes: 24 additions & 10 deletions models/xero__balance_sheet_report.sql
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, I noticed that fields like account_class or source_type have all-caps values... should we do the same with account_name ? (Not really an issue, more of a standardization thing) . I can make an feature flag if it's significant enough cc @fivetran-joemarkiewicz

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line in question is this one.

It is consistent with the account names coming as shown in the seed data, but I believe the question is should everything be all caps?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per conversation with @fivetran-joemarkiewicz, this is the source behavior, so we'll keep as is.

Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,35 @@ with calendar as (

), organization as (

select *
select
*,
cast(extract(year from current_date) as {{ dbt.type_string() }}) as current_year,
cast(extract(year from {{ dbt.dateadd('year', -1, 'current_date') }}) as {{ dbt.type_string() }}) as last_year
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this up here just to make it a little easier to follow.

from {{ var('organization') }}


), year_end as (

-- Calculate the current financial year-end date for each organization:
-- For February, determine last day by subtracting 1 day from March 1, avoiding leap year logic.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant!

-- Compare the year end date to the current date:
-- Use this year's date if it's on or after the current date.
-- Otherwise, use last year's corresponding date.
select
case
when cast(extract(year from current_date) || '-' || financial_year_end_month || '-' || financial_year_end_day as date) >= current_date
then cast(extract(year from current_date) || '-' || financial_year_end_month || '-' || financial_year_end_day as date)
else case when financial_year_end_month = 2 and financial_year_end_day = 29
then cast(extract(year from {{ dbt.dateadd('year', -1, 'current_date') }}) || '-' || financial_year_end_month || '-28' as date) -- Necessary for organizations with a reported fiscal year end of 02-29 as the previous year will not be a leap year and must be the 28th.
else cast(extract(year from {{ dbt.dateadd('year', -1, 'current_date') }}) || '-' || financial_year_end_month || '-' || financial_year_end_day as date)
end
end as current_year_end_date,
source_relation
source_relation,
case when financial_year_end_month = 2 and financial_year_end_day = 29
Copy link
Contributor Author

@fivetran-catfritz fivetran-catfritz Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this condition first appears to prevent the warehouse from trying to execute the other nested conditions that cause the error.

then
case when cast({{ dbt.dateadd('day', -1, "cast(current_year || '-03-01' as date)") }} as date) >= current_date
then cast({{ dbt.dateadd('day', -1, "cast(current_year || '-03-01' as date)") }} as date)
else cast({{ dbt.dateadd('day', -1, "cast(last_year || '-03-01' as date)") }} as date)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case I found it was easiest to go a day back from March to get the last day in Feb.

end
else
case when cast(current_year || '-' || financial_year_end_month || '-' || financial_year_end_day as date) >= current_date
then cast(current_year || '-' || financial_year_end_month || '-' || financial_year_end_day as date)
else cast(last_year || '-' || financial_year_end_month || '-' || financial_year_end_day as date)
end
end as current_year_end_date

from organization

), joined as (
Expand Down