From fa245f99a0d3ff805daf7494b53a9fb00fc66f44 Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Wed, 3 Apr 2024 16:21:12 +0200 Subject: [PATCH] sqldb+invoices: fix ordering bug in FilterInvoices 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`. --- invoices/sql_store.go | 19 +++++++++++-------- sqldb/sqlc/invoices.sql.go | 16 ++++++++-------- sqldb/sqlc/queries/invoices.sql | 16 ++++++++-------- 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/invoices/sql_store.go b/invoices/sql_store.go index cf829cd43a..bcf3c1f892 100644 --- a/invoices/sql_store.go +++ b/invoices/sql_store.go @@ -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) @@ -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) @@ -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) @@ -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) @@ -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 { diff --git a/sqldb/sqlc/invoices.sql.go b/sqldb/sqlc/invoices.sql.go index 09ca1a1e24..fde02391c6 100644 --- a/sqldb/sqlc/invoices.sql.go +++ b/sqldb/sqlc/invoices.sql.go @@ -82,19 +82,19 @@ WHERE ( $7 IS NULL ) AND ( CASE - WHEN $8=TRUE THEN (state = 0 OR state = 3) + WHEN $8 = TRUE THEN (state = 0 OR state = 3) ELSE TRUE END ) ORDER BY - CASE - WHEN $9 = FALSE THEN id - ELSE NULL +CASE + WHEN $9 = FALSE OR $9 IS NULL THEN id + ELSE NULL END ASC, - CASE - WHEN $9 = TRUE THEN id - ELSE NULL - END DESC +CASE + WHEN $9 = TRUE THEN id + ELSE NULL +END DESC LIMIT $11 OFFSET $10 ` diff --git a/sqldb/sqlc/queries/invoices.sql b/sqldb/sqlc/queries/invoices.sql index c36976a251..07c5ca418b 100644 --- a/sqldb/sqlc/queries/invoices.sql +++ b/sqldb/sqlc/queries/invoices.sql @@ -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