bugfix/running-balance-ordering#196
Conversation
fivetran-avinash
left a comment
fivetran-avinash
left a comment
There was a problem hiding this comment.
Good for pre-release, one comment to consider for the future.
| *, | ||
| sum(adjusted_amount) over (partition by account_id, class_id, source_relation | ||
| order by source_relation, transaction_date, account_id, class_id rows unbounded preceding) as running_balance, | ||
| order by source_relation, transaction_date, account_id, class_id, transaction_id, transaction_index rows unbounded preceding) as running_balance, |
There was a problem hiding this comment.
This is not blocking for this pre-release, but I'm wondering if we should optimize these order bys in a future issue. should we be ordering by the same fields account_id, class_id, source_relation if they're already being used in the partition?
There was a problem hiding this comment.
I had the same thought. The source_relation, account, and class order by fields are not necessary given they're included in the partition statement. I'd rather remove these unnecessary fields now and confirm this approach works than consider removing later.
Latest commit remove the unnecessary order by fields and ensures we are only ordering by what we absolutely need to.
fivetran-joemarkiewicz
added
docs:ready
Co-authored-by: fivetran-catfritz <111930712+fivetran-catfritz@users.noreply.github.com>
4 participants
PR Overview
Package version introduced in this PR:
This PR addresses the following Issue/Feature(s):
Summary of changes:
running_balanceandrunning_converted_balancewhen multiple transactions occur on the same date for the same account and class combinationSubmission Checklist
Changelog