Skip to content

Commit

Permalink
sqldb+invoices: fix ordering bug in FilterInvoices
Browse files Browse the repository at this point in the history
Previously if the `reverse` named arg was unset (value of NULL), then
SQL would order by NULL instead of ID causing undifined ordering of the
returned rows. To fix that we check for NULL and also make sure to set
the `reverse` arg in the code explicitly as it in the generated code it
is an `interface{}` instead of `bool`.
  • Loading branch information
bhandras committed Apr 10, 2024
1 parent a1b6f92 commit fa245f9
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 24 deletions.
19 changes: 11 additions & 8 deletions invoices/sql_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,7 @@ func (i *SQLStore) FetchPendingInvoices(ctx context.Context) (
PendingOnly: true,
NumOffset: int32(offset),
NumLimit: int32(limit),
Reverse: false,
}

rows, err := db.FilterInvoices(ctx, params)
Expand Down Expand Up @@ -683,6 +684,7 @@ func (i *SQLStore) InvoicesSettledSince(ctx context.Context, idx uint64) (
SettleIndexGet: sqldb.SQLInt64(settleIdx + 1),
NumLimit: int32(limit),
NumOffset: int32(offset),
Reverse: false,
}

rows, err := db.FilterInvoices(ctx, params)
Expand Down Expand Up @@ -798,6 +800,7 @@ func (i *SQLStore) InvoicesAddedSince(ctx context.Context, idx uint64) (
AddIndexGet: sqldb.SQLInt64(addIdx + 1),
NumLimit: int32(limit),
NumOffset: int32(offset),
Reverse: false,
}

rows, err := db.FilterInvoices(ctx, params)
Expand Down Expand Up @@ -857,14 +860,6 @@ func (i *SQLStore) QueryInvoices(ctx context.Context,
PendingOnly: q.PendingOnly,
}

if !q.Reversed {
// The invoice with index offset id must not be
// included in the results.
params.AddIndexGet = sqldb.SQLInt64(
q.IndexOffset + uint64(offset) + 1,
)
}

if q.Reversed {
idx := int32(q.IndexOffset)

Expand All @@ -883,6 +878,14 @@ func (i *SQLStore) QueryInvoices(ctx context.Context,
}

params.Reverse = true
} else {
// The invoice with index offset id must not be
// included in the results.
params.AddIndexGet = sqldb.SQLInt64(
q.IndexOffset + uint64(offset) + 1,
)

params.Reverse = false
}

if q.CreationDateStart != 0 {
Expand Down
16 changes: 8 additions & 8 deletions sqldb/sqlc/invoices.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 8 additions & 8 deletions sqldb/sqlc/queries/invoices.sql
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,19 @@ WHERE (
sqlc.narg('created_before') IS NULL
) AND (
CASE
WHEN sqlc.narg('pending_only')=TRUE THEN (state = 0 OR state = 3)
WHEN sqlc.narg('pending_only') = TRUE THEN (state = 0 OR state = 3)
ELSE TRUE
END
)
ORDER BY
CASE
WHEN sqlc.narg('reverse') = FALSE THEN id
ELSE NULL
CASE
WHEN sqlc.narg('reverse') = FALSE OR sqlc.narg('reverse') IS NULL THEN id
ELSE NULL
END ASC,
CASE
WHEN sqlc.narg('reverse') = TRUE THEN id
ELSE NULL
END DESC
CASE
WHEN sqlc.narg('reverse') = TRUE THEN id
ELSE NULL
END DESC
LIMIT @num_limit OFFSET @num_offset;

-- name: UpdateInvoiceState :execresult
Expand Down

0 comments on commit fa245f9

Please sign in to comment.