Skip to content

Commit cfc0d21

Browse files
committed
fix: remove all use of SUM() in SQL queries
All calculations are now done in the code instead of in the query since the database will introduce floating point errors.
1 parent 95ff85b commit cfc0d21

File tree

6 files changed

+80
-76
lines changed

6 files changed

+80
-76
lines changed

pkg/controllers/account.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,14 @@ func (co Controller) getAccountObject(c *gin.Context, id uuid.UUID) (Account, bo
340340
return Account{}, false
341341
}
342342

343+
account, err = account.WithCalculations(co.DB)
344+
if err != nil {
345+
httperrors.Handler(c, err)
346+
return Account{}, false
347+
}
348+
343349
return Account{
344-
account.WithCalculations(co.DB),
350+
account,
345351
recentEnvelopes,
346352
AccountLinks{
347353
Self: fmt.Sprintf("%s/v1/accounts/%s", c.GetString("baseURL"), account.ID),

pkg/controllers/budget.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,13 @@ func (co Controller) getBudgetResource(c *gin.Context, id uuid.UUID) (models.Bud
657657
return models.Budget{}, false
658658
}
659659

660-
return budget.WithCalculations(co.DB), true
660+
budget, err := budget.WithCalculations(co.DB)
661+
if err != nil {
662+
httperrors.Handler(c, err)
663+
return models.Budget{}, false
664+
}
665+
666+
return budget, true
661667
}
662668

663669
func (co Controller) getBudgetObject(c *gin.Context, id uuid.UUID) (Budget, bool) {

pkg/models/account.go

Lines changed: 49 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,19 @@ type AccountCreate struct {
2929
Hidden bool `json:"hidden" example:"true" default:"false"`
3030
}
3131

32-
func (a Account) WithCalculations(db *gorm.DB) Account {
33-
a.Balance = a.getBalance(db)
34-
a.ReconciledBalance = a.SumReconciledTransactions(db)
32+
func (a Account) WithCalculations(db *gorm.DB) (Account, error) {
33+
balance, _, err := a.GetBalanceMonth(db, types.NewMonth(1, 1))
34+
if err != nil {
35+
return Account{}, err
36+
}
37+
a.Balance = balance
38+
39+
a.ReconciledBalance, err = a.SumReconciled(db)
40+
if err != nil {
41+
return Account{}, err
42+
}
3543

36-
return a
44+
return a, nil
3745
}
3846

3947
// BeforeSave sets OnBudget to false when External is true.
@@ -54,21 +62,33 @@ func (a Account) Transactions(db *gorm.DB) []Transaction {
5462
}
5563

5664
// Transactions returns all transactions for this account.
57-
func (a Account) SumReconciledTransactions(db *gorm.DB) decimal.Decimal {
58-
return a.InitialBalance.Add(TransactionsSum(db,
59-
Transaction{
60-
TransactionCreate: TransactionCreate{
61-
Reconciled: true,
62-
DestinationAccountID: a.ID,
63-
},
64-
},
65-
Transaction{
66-
TransactionCreate: TransactionCreate{
67-
Reconciled: true,
68-
SourceAccountID: a.ID,
69-
},
70-
},
71-
))
65+
func (a Account) SumReconciled(db *gorm.DB) (balance decimal.Decimal, err error) {
66+
var transactions []Transaction
67+
68+
err = db.
69+
Preload("DestinationAccount").
70+
Preload("SourceAccount").
71+
Where(
72+
db.Where(Transaction{TransactionCreate: TransactionCreate{DestinationAccountID: a.ID, Reconciled: true}}).
73+
Or(db.Where(Transaction{TransactionCreate: TransactionCreate{SourceAccountID: a.ID, Reconciled: true}}))).
74+
Find(&transactions).Error
75+
76+
if err != nil {
77+
return decimal.Zero, err
78+
}
79+
80+
balance = a.InitialBalance
81+
82+
// Add incoming transactions, subtract outgoing transactions
83+
for _, t := range transactions {
84+
if t.DestinationAccountID == a.ID {
85+
balance = balance.Add(t.Amount)
86+
} else {
87+
balance = balance.Sub(t.Amount)
88+
}
89+
}
90+
91+
return
7292
}
7393

7494
// GetBalanceMonth calculates the balance and available sums for a specific month.
@@ -78,20 +98,24 @@ func (a Account) SumReconciledTransactions(db *gorm.DB) decimal.Decimal {
7898
func (a Account) GetBalanceMonth(db *gorm.DB, month types.Month) (balance, available decimal.Decimal, err error) {
7999
var transactions []Transaction
80100

81-
err = db.
101+
query := db.
82102
Preload("DestinationAccount").
83103
Preload("SourceAccount").
84-
Where("transactions.date < date(?)", month.AddDate(0, 1)).
85104
Where(
86105
db.Where(Transaction{TransactionCreate: TransactionCreate{DestinationAccountID: a.ID}}).
87-
Or(db.Where(Transaction{TransactionCreate: TransactionCreate{SourceAccountID: a.ID}}))).
88-
Find(&transactions).
89-
Error
106+
Or(db.Where(Transaction{TransactionCreate: TransactionCreate{SourceAccountID: a.ID}})))
107+
108+
// Limit to the month if it is specified
109+
if !month.IsZero() {
110+
query = query.Where("transactions.date < date(?)", month.AddDate(0, 1))
111+
}
112+
113+
err = query.Find(&transactions).Error
90114
if err != nil {
91115
return decimal.Zero, decimal.Zero, err
92116
}
93117

94-
if a.InitialBalanceDate != nil && month.AddDate(0, 1).AfterTime(*a.InitialBalanceDate) {
118+
if month.IsZero() || (a.InitialBalanceDate != nil && month.AddDate(0, 1).AfterTime(*a.InitialBalanceDate)) {
95119
balance = a.InitialBalance
96120
available = a.InitialBalance
97121
}
@@ -116,46 +140,6 @@ func (a Account) GetBalanceMonth(db *gorm.DB, month types.Month) (balance, avail
116140
return
117141
}
118142

119-
// getBalance returns the balance of the account calculated over all transactions.
120-
func (a Account) getBalance(db *gorm.DB) decimal.Decimal {
121-
return a.InitialBalance.Add(TransactionsSum(db,
122-
Transaction{
123-
TransactionCreate: TransactionCreate{
124-
DestinationAccountID: a.ID,
125-
},
126-
},
127-
Transaction{
128-
TransactionCreate: TransactionCreate{
129-
SourceAccountID: a.ID,
130-
},
131-
},
132-
))
133-
}
134-
135-
// TransactionSums returns the sum of all transactions matching two Transaction structs
136-
//
137-
// The incoming Transactions fields is used to add the amount of all matching transactions to the overall sum
138-
// The outgoing Transactions fields is used to subtract the amount of all matching transactions from the overall sum.
139-
func TransactionsSum(db *gorm.DB, incoming, outgoing Transaction) decimal.Decimal {
140-
var outgoingSum, incomingSum decimal.NullDecimal
141-
142-
_ = db.Table("transactions").
143-
Where(&outgoing).
144-
Where("deleted_at is NULL").
145-
Select("SUM(amount)").
146-
Row().
147-
Scan(&outgoingSum)
148-
149-
_ = db.Table("transactions").
150-
Where(&incoming).
151-
Where("deleted_at is NULL").
152-
Select("SUM(amount)").
153-
Row().
154-
Scan(&incomingSum)
155-
156-
return incomingSum.Decimal.Sub(outgoingSum.Decimal)
157-
}
158-
159143
// RecentEnvelopes returns the most common envelopes used in the last 10
160144
// transactions where the account is the destination account.
161145
//

pkg/models/account_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@ func (suite *TestSuiteStandard) TestAccountCalculations() {
108108
suite.Assert().Fail("Resource could not be saved", err)
109109
}
110110

111-
a := account.WithCalculations(suite.db)
111+
a, err := account.WithCalculations(suite.db)
112+
assert.Nil(suite.T(), err)
112113

113114
expected := incomingTransaction.Amount.Sub(outgoingTransaction.Amount).Add(a.InitialBalance).Add(decimal.NewFromFloat(100)) // Add 100 for futureIncomeTransaction
114115
assert.True(suite.T(), a.Balance.Equal(expected), "Balance for account is not correct. Should be: %v but is %v", expected, a.Balance)
@@ -130,7 +131,9 @@ func (suite *TestSuiteStandard) TestAccountCalculations() {
130131
suite.Assert().Fail("Resource could not be deleted", err)
131132
}
132133

133-
a = account.WithCalculations(suite.db)
134+
a, err = account.WithCalculations(suite.db)
135+
assert.Nil(suite.T(), err)
136+
134137
expected = outgoingTransaction.Amount.Neg().Add(a.InitialBalance).Add(decimal.NewFromFloat(100)) // Add 100 for futureIncomeTransaction
135138
assert.True(suite.T(), a.Balance.Equal(expected), "Balance for account is not correct. Should be: %v but is %v", expected, a.Balance)
136139

pkg/models/budget.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ type BudgetMonth struct {
3737
}
3838

3939
// WithCalculations computes all the calculated values.
40-
func (b Budget) WithCalculations(db *gorm.DB) Budget {
40+
func (b Budget) WithCalculations(db *gorm.DB) (Budget, error) {
4141
b.Balance = decimal.Zero
4242

4343
// Get all OnBudget accounts for the budget
@@ -51,10 +51,15 @@ func (b Budget) WithCalculations(db *gorm.DB) Budget {
5151

5252
// Add all their balances to the budget's balance
5353
for _, account := range accounts {
54-
b.Balance = b.Balance.Add(account.WithCalculations(db).Balance)
54+
account, err := account.WithCalculations(db)
55+
if err != nil {
56+
return Budget{}, err
57+
}
58+
59+
b.Balance = b.Balance.Add(account.Balance)
5560
}
5661

57-
return b
62+
return b, nil
5863
}
5964

6065
// Income returns the income for a budget in a given month.
@@ -68,8 +73,7 @@ func (b Budget) Income(db *gorm.DB, month types.Month) (decimal.Decimal, error)
6873
Where("source_account.external = 1").
6974
Where("destination_account.external = 0").
7075
Where("transactions.envelope_id IS NULL").
71-
Where("strftime('%m', transactions.date) = ?", fmt.Sprintf("%02d", time.Time(month).Month())).
72-
Where("strftime('%Y', transactions.date) = ?", fmt.Sprintf("%d", time.Time(month).Year())).
76+
Where("transactions.available_from >= date(?) AND transactions.available_from < date(?)", month, month.AddDate(0, 1)).
7377
Where(&Transaction{
7478
TransactionCreate: TransactionCreate{
7579
BudgetID: b.ID,

pkg/models/budget_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,8 @@ func (suite *TestSuiteStandard) TestBudgetCalculations() {
240240
suite.Assert().Fail("Resource could not be saved", err)
241241
}
242242

243-
budget = budget.WithCalculations(suite.db)
243+
budget, err = budget.WithCalculations(suite.db)
244+
assert.Nil(suite.T(), err)
244245

245246
shouldBalance := decimal.NewFromFloat(7269.38)
246247
assert.True(suite.T(), budget.Balance.Equal(shouldBalance), "Balance for budget is not correct. Should be %s, is %s", shouldBalance, budget.Balance)
@@ -381,7 +382,7 @@ func (suite *TestSuiteStandard) TestMonth() {
381382
budget := suite.createTestBudget(models.BudgetCreate{})
382383
category := suite.createTestCategory(models.CategoryCreate{BudgetID: budget.ID, Name: "Upkeep"})
383384
envelope := suite.createTestEnvelope(models.EnvelopeCreate{CategoryID: category.ID, Name: "Utilities"})
384-
account := suite.createTestAccount(models.AccountCreate{BudgetID: budget.ID, OnBudget: true})
385+
account := suite.createTestAccount(models.AccountCreate{BudgetID: budget.ID, OnBudget: true, Name: "TestMonth"})
385386
externalAccount := suite.createTestAccount(models.AccountCreate{BudgetID: budget.ID, External: true})
386387

387388
allocationJanuary := suite.createTestAllocation(models.AllocationCreate{

0 commit comments

Comments
 (0)