← Watch feed
sqldb+paymentsdb: improve filterpayments efficiency
Public commit record
73/100 · Adequate
What the developer wrote
Authored by ziggie
sqldb+paymentsdb: improve filterpayments efficiency
In this commit, we fix a gap that was introduced when the payment DB
commits were arranged for the main line. The `migration1` package
already had the updated `FilterPayments` query using `COALESCE` for
the index bounds and a separate `FilterPaymentsDesc` query for
reverse-ordered pagination, but the corresponding changes in
`sqldb/sqlc` and the main `sql_store.go` were accidentally left out.
The old `FilterPayments` used OR-based nullable params for the
`created_at` bounds (e.g., `p.created_at >= $3 OR $3 IS NULL`). On
Postgres, mixing nullable text fallbacks in a `COALESCE` with
timestamp columns causes a type mismatch error. The OR-based approach
also prevents the query planner from using the `created_at` index.
We fix this by providing non-nullable `time.Time` params from the Go
side, defaulting to epoch start and year 9999 when no filter is set.
We also drop the `Reverse` param from `FilterPaymentsParams` and
instead introduce a dedicated `FilterPaymentsDesc` query that orders
by `p.id DESC`. This avoids the conditional `CASE WHEN` ordering
trick, which some planners handle poorly.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit fixes a database query used to list Lightning Network payments. The old query could fail on Postgres with a type-mismatch error and was inefficient because it couldn't use a date index. The fix supplies default date bounds from the Go code, removes a problematic 'reverse' ordering trick, and adds a separate descending-order query. It is a correctness/performance bug fix rather than a security vulnerability.
Recommended action
Treat as a routine bug-fix/correctness patch. Review for normal QA; no emergency security response indicated. If running Postgres-backed LND with payment queries, ensure this fix is included to avoid query failures and poor performance.
Security signals we found
01
Database query correctness fix (Postgres type mismatch)
02
Performance/index-usage improvement for payment filtering
03
No input sanitization, authentication, or authorization changes observed
04
No cryptographic or secret-handling changes observed
Evidence from the diff
The patch updates FilterPayments in LND’s SQL payment store. It replaces nullable OR-based optional filters and a CASE WHEN ordering expression with non-nullable time.Time parameters defaulted to epoch start and year 9999, and introduces a dedicated FilterPaymentsDesc query for reverse pagination. The SQL now uses simple comparisons on created_at and separate ASC/DESC queries, avoiding a Postgres COALESCE type mismatch and enabling better index usage. No privilege escalation, injection, or cryptographic weakness is introduced or fixed.
Changed components
payments/db/sql_store.gosqldb/sqlc/payments.sql.gosqldb/sqlc/querier.gosqldb/sqlc/queries/payments.sqlInspect captured patch +186 / −57
diff --git a/payments/db/sql_store.go b/payments/db/sql_store.go
index 2c97f38..5726504 100644
--- a/payments/db/sql_store.go
+++ b/payments/db/sql_store.go
@@ -47,6 +47,7 @@ type SQLQueries interface {
Payment DB read operations.
*/
FilterPayments(ctx context.Context, query sqlc.FilterPaymentsParams) ([]sqlc.FilterPaymentsRow, error)
+ FilterPaymentsDesc(ctx context.Context, query sqlc.FilterPaymentsDescParams) ([]sqlc.FilterPaymentsDescRow, error)
FetchPayment(ctx context.Context, paymentIdentifier []byte) (sqlc.FetchPaymentRow, error)
FetchPaymentsByIDs(ctx context.Context, paymentIDs []int64) ([]sqlc.FetchPaymentsByIDsRow, error)
@@ -831,12 +832,51 @@ func (s *SQLStore) QueryPayments(ctx context.Context, query Query) (Response,
return nil
}
+ //nolint:ll
+ convertFilterPaymentsDescRows := func(
+ rows []sqlc.FilterPaymentsDescRow) []sqlc.FilterPaymentsRow {
+
+ out := make([]sqlc.FilterPaymentsRow, len(rows))
+ for i, row := range rows {
+ out[i] = sqlc.FilterPaymentsRow{
+ Payment: row.Payment,
+ IntentType: row.IntentType,
+ IntentPayload: row.IntentPayload,
+ }
+ }
+
+ return out
+ }
+
queryFunc := func(ctx context.Context, lastID int64,
limit int32) ([]sqlc.FilterPaymentsRow, error) {
+ // Default date bounds: epoch start and far
+ // future. These are always provided so the SQL
+ // query uses simple comparisons instead of
+ // COALESCE (which causes type mismatch on
+ // Postgres) or OR-based optional filters (which
+ // can prevent index usage).
+ createdAfter := time.Unix(0, 0).UTC()
+ if query.CreationDateStart != 0 {
+ createdAfter = time.Unix(
+ query.CreationDateStart, 0,
+ ).UTC()
+ }
+
+ createdBefore := time.Date(
+ 9999, 12, 31, 23, 59, 59, 0, time.UTC,
+ )
+ if query.CreationDateEnd != 0 {
+ createdBefore = time.Unix(
+ query.CreationDateEnd, 0,
+ ).UTC()
+ }
+
filterParams := sqlc.FilterPaymentsParams{
- NumLimit: limit,
- Reverse: query.Reversed,
+ NumLimit: limit,
+ CreatedAfter: createdAfter,
+ CreatedBefore: createdBefore,
// For now there only BOLT 11 payment intents
// exist.
IntentType: sqldb.SQLInt16(
@@ -854,18 +894,17 @@ func (s *SQLStore) QueryPayments(ctx context.Context, query Query) (Response,
)
}
- // Add potential date filters if specified.
- if query.CreationDateStart != 0 {
- filterParams.CreatedAfter = sqldb.SQLTime(
- time.Unix(query.CreationDateStart, 0).
- UTC(),
- )
- }
- if query.CreationDateEnd != 0 {
- filterParams.CreatedBefore = sqldb.SQLTime(
- time.Unix(query.CreationDateEnd, 0).
- UTC(),
+ if query.Reversed {
+ rows, err := db.FilterPaymentsDesc(
+ ctx, sqlc.FilterPaymentsDescParams(
+ filterParams,
+ ),
)
+ if err != nil {
+ return nil, err
+ }
+
+ return convertFilterPaymentsDescRows(rows), nil
}
return db.FilterPayments(ctx, filterParams)
@@ -1968,7 +2007,12 @@ func (s *SQLStore) DeletePayments(ctx context.Context, failedOnly,
limit int32) ([]sqlc.FilterPaymentsRow, error) {
filterParams := sqlc.FilterPaymentsParams{
- NumLimit: limit,
+ NumLimit: limit,
+ CreatedAfter: time.Unix(0, 0).UTC(),
+ CreatedBefore: time.Date(
+ 9999, 12, 31, 23, 59, 59,
+ 0, time.UTC,
+ ),
IndexOffsetGet: sqldb.SQLInt64(
lastID,
),
diff --git a/sqldb/sqlc/payments.sql.go b/sqldb/sqlc/payments.sql.go
index bf130bd..f5b76b8 100644
--- a/sqldb/sqlc/payments.sql.go
+++ b/sqldb/sqlc/payments.sql.go
@@ -777,35 +777,28 @@ SELECT
i.intent_payload AS "intent_payload"
FROM payments p
LEFT JOIN payment_intents i ON i.payment_id = p.id
-WHERE (
- p.id > $1 OR
- $1 IS NULL
-) AND (
- p.id < $2 OR
- $2 IS NULL
-) AND (
- p.created_at >= $3 OR
- $3 IS NULL
-) AND (
- p.created_at <= $4 OR
- $4 IS NULL
-) AND (
- i.intent_type = $5 OR
- $5 IS NULL OR i.intent_type IS NULL
-)
-ORDER BY
- CASE WHEN $6 = false OR $6 IS NULL THEN p.id END ASC,
- CASE WHEN $6 = true THEN p.id END DESC
-LIMIT $7
+WHERE p.id > COALESCE($1, -1)
+ AND p.id < COALESCE($2, 9223372036854775807)
+ -- NOTE: We use non-nullable time params with Go-side defaults instead of
+ -- COALESCE, because COALESCE with text fallback causes type mismatch on
+ -- Postgres (timestamp vs text), and OR-based optional filters can prevent
+ -- the planner from using the created_at index.
+ AND p.created_at >= $3
+ AND p.created_at <= $4
+ AND (
+ i.intent_type = $5 OR
+ $5 IS NULL OR i.intent_type IS NULL
+ )
+ORDER BY p.id ASC
+LIMIT $6
`
type FilterPaymentsParams struct {
IndexOffsetGet sql.NullInt64
IndexOffsetLet sql.NullInt64
- CreatedAfter sql.NullTime
- CreatedBefore sql.NullTime
+ CreatedAfter time.Time
+ CreatedBefore time.Time
IntentType sql.NullInt16
- Reverse interface{}
NumLimit int32
}
@@ -822,7 +815,6 @@ func (q *Queries) FilterPayments(ctx context.Context, arg FilterPaymentsParams)
arg.CreatedAfter,
arg.CreatedBefore,
arg.IntentType,
- arg.Reverse,
arg.NumLimit,
)
if err != nil {
@@ -854,6 +846,82 @@ func (q *Queries) FilterPayments(ctx context.Context, arg FilterPaymentsParams)
return items, nil
}
+const filterPaymentsDesc = `-- name: FilterPaymentsDesc :many
+SELECT
+ p.id, p.amount_msat, p.created_at, p.payment_identifier, p.fail_reason,
+ i.intent_type AS "intent_type",
+ i.intent_payload AS "intent_payload"
+FROM payments p
+LEFT JOIN payment_intents i ON i.payment_id = p.id
+WHERE p.id > COALESCE($1, -1)
+ AND p.id < COALESCE($2, 9223372036854775807)
+ -- NOTE: We use non-nullable time params with Go-side defaults instead of
+ -- COALESCE, because COALESCE with text fallback causes type mismatch on
+ -- Postgres (timestamp vs text), and OR-based optional filters can prevent
+ -- the planner from using the created_at index.
+ AND p.created_at >= $3
+ AND p.created_at <= $4
+ AND (
+ i.intent_type = $5 OR
+ $5 IS NULL OR i.intent_type IS NULL
+ )
+ORDER BY p.id DESC
+LIMIT $6
+`
+
+type FilterPaymentsDescParams struct {
+ IndexOffsetGet sql.NullInt64
+ IndexOffsetLet sql.NullInt64
+ CreatedAfter time.Time
+ CreatedBefore time.Time
+ IntentType sql.NullInt16
+ NumLimit int32
+}
+
+type FilterPaymentsDescRow struct {
+ Payment Payment
+ IntentType sql.NullInt16
+ IntentPayload []byte
+}
+
+func (q *Queries) FilterPaymentsDesc(ctx context.Context, arg FilterPaymentsDescParams) ([]FilterPaymentsDescRow, error) {
+ rows, err := q.db.QueryContext(ctx, filterPaymentsDesc,
+ arg.IndexOffsetGet,
+ arg.IndexOffsetLet,
+ arg.CreatedAfter,
+ arg.CreatedBefore,
+ arg.IntentType,
+ arg.NumLimit,
+ )
+ if err != nil {
+ return nil, err
+ }
+ defer rows.Close()
+ var items []FilterPaymentsDescRow
+ for rows.Next() {
+ var i FilterPaymentsDescRow
+ if err := rows.Scan(
+ &i.Payment.ID,
+ &i.Payment.AmountMsat,
+ &i.Payment.CreatedAt,
+ &i.Payment.PaymentIdentifier,
+ &i.Payment.FailReason,
+ &i.IntentType,
+ &i.IntentPayload,
+ ); err != nil {
+ return nil, err
+ }
+ items = append(items, i)
+ }
+ if err := rows.Close(); err != nil {
+ return nil, err
+ }
+ if err := rows.Err(); err != nil {
+ return nil, err
+ }
+ return items, nil
+}
+
const insertHtlcAttempt = `-- name: InsertHtlcAttempt :one
INSERT INTO payment_htlc_attempts (
payment_id,
diff --git a/sqldb/sqlc/querier.go b/sqldb/sqlc/querier.go
index c148d66..d2481be 100644
--- a/sqldb/sqlc/querier.go
+++ b/sqldb/sqlc/querier.go
@@ -91,6 +91,7 @@ type Querier interface {
// See FilterInvoicesForward for the expected Go-side defaults.
FilterInvoicesReverse(ctx context.Context, arg FilterInvoicesReverseParams) ([]Invoice, error)
FilterPayments(ctx context.Context, arg FilterPaymentsParams) ([]FilterPaymentsRow, error)
+ FilterPaymentsDesc(ctx context.Context, arg FilterPaymentsDescParams) ([]FilterPaymentsDescRow, error)
GetAMPInvoiceID(ctx context.Context, setID []byte) (int64, error)
GetChannelAndNodesBySCID(ctx context.Context, arg GetChannelAndNodesBySCIDParams) (GetChannelAndNodesBySCIDRow, error)
GetChannelByOutpointWithPolicies(ctx context.Context, arg GetChannelByOutpointWithPoliciesParams) (GetChannelByOutpointWithPoliciesRow, error)
diff --git a/sqldb/sqlc/queries/payments.sql b/sqldb/sqlc/queries/payments.sql
index eabd884..6c0a544 100644
--- a/sqldb/sqlc/queries/payments.sql
+++ b/sqldb/sqlc/queries/payments.sql
@@ -10,25 +10,41 @@ SELECT
i.intent_payload AS "intent_payload"
FROM payments p
LEFT JOIN payment_intents i ON i.payment_id = p.id
-WHERE (
- p.id > sqlc.narg('index_offset_get') OR
- sqlc.narg('index_offset_get') IS NULL
-) AND (
- p.id < sqlc.narg('index_offset_let') OR
- sqlc.narg('index_offset_let') IS NULL
-) AND (
- p.created_at >= sqlc.narg('created_after') OR
- sqlc.narg('created_after') IS NULL
-) AND (
- p.created_at <= sqlc.narg('created_before') OR
- sqlc.narg('created_before') IS NULL
-) AND (
- i.intent_type = sqlc.narg('intent_type') OR
- sqlc.narg('intent_type') IS NULL OR i.intent_type IS NULL
-)
-ORDER BY
- CASE WHEN sqlc.narg('reverse') = false OR sqlc.narg('reverse') IS NULL THEN p.id END ASC,
- CASE WHEN sqlc.narg('reverse') = true THEN p.id END DESC
+WHERE p.id > COALESCE(sqlc.narg('index_offset_get'), -1)
+ AND p.id < COALESCE(sqlc.narg('index_offset_let'), 9223372036854775807)
+ -- NOTE: We use non-nullable time params with Go-side defaults instead of
+ -- COALESCE, because COALESCE with text fallback causes type mismatch on
+ -- Postgres (timestamp vs text), and OR-based optional filters can prevent
+ -- the planner from using the created_at index.
+ AND p.created_at >= @created_after
+ AND p.created_at <= @created_before
+ AND (
+ i.intent_type = sqlc.narg('intent_type') OR
+ sqlc.narg('intent_type') IS NULL OR i.intent_type IS NULL
+ )
+ORDER BY p.id ASC
+LIMIT @num_limit;
+
+-- name: FilterPaymentsDesc :many
+SELECT
+ sqlc.embed(p),
+ i.intent_type AS "intent_type",
+ i.intent_payload AS "intent_payload"
+FROM payments p
+LEFT JOIN payment_intents i ON i.payment_id = p.id
+WHERE p.id > COALESCE(sqlc.narg('index_offset_get'), -1)
+ AND p.id < COALESCE(sqlc.narg('index_offset_let'), 9223372036854775807)
+ -- NOTE: We use non-nullable time params with Go-side defaults instead of
+ -- COALESCE, because COALESCE with text fallback causes type mismatch on
+ -- Postgres (timestamp vs text), and OR-based optional filters can prevent
+ -- the planner from using the created_at index.
+ AND p.created_at >= @created_after
+ AND p.created_at <= @created_before
+ AND (
+ i.intent_type = sqlc.narg('intent_type') OR
+ sqlc.narg('intent_type') IS NULL OR i.intent_type IS NULL
+ )
+ORDER BY p.id DESC
LIMIT @num_limit;
-- name: FetchPayment :one
Risk score
Our methodology →Why this scored 29/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.