From 043e4aff01aa9a68de781116d51f23ec6ed8fae4 Mon Sep 17 00:00:00 2001
From: Andras Banki-Horvath <bhandras@gmail.com>
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 cf829cd43..bcf3c1f89 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 09ca1a1e2..fde02391c 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 c36976a25..07c5ca418 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