From f2cd3379b314170b3b6e96cd757078daf20bff90 Mon Sep 17 00:00:00 2001 From: tamirms Date: Wed, 14 Feb 2024 08:28:41 +0000 Subject: [PATCH] Fix claimable_balance_claimants subquery in GetClaimableBalances() --- .../db2/history/claimable_balances.go | 32 ++++++++++++------- .../db2/history/claimable_balances_test.go | 5 +-- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/services/horizon/internal/db2/history/claimable_balances.go b/services/horizon/internal/db2/history/claimable_balances.go index d45780a4c0..c198ee162d 100644 --- a/services/horizon/internal/db2/history/claimable_balances.go +++ b/services/horizon/internal/db2/history/claimable_balances.go @@ -10,6 +10,7 @@ import ( sq "github.com/Masterminds/squirrel" "github.com/guregu/null" + "github.com/stellar/go/services/horizon/internal/db2" "github.com/stellar/go/support/errors" "github.com/stellar/go/xdr" @@ -57,7 +58,7 @@ func (cbq ClaimableBalancesQuery) Cursor() (int64, string, error) { // ApplyCursor applies cursor to the given sql. For performance reason the limit // is not applied here. This allows us to hint the planner later to use the right // indexes. -func applyClaimableBalancesQueriesCursor(sql sq.SelectBuilder, lCursor int64, rCursor string, order string) (sq.SelectBuilder, error) { +func applyClaimableBalancesQueriesCursor(sql sq.SelectBuilder, tableName string, lCursor int64, rCursor string, order string) (sq.SelectBuilder, error) { hasPagedLimit := false if lCursor > 0 && rCursor != "" { hasPagedLimit = true @@ -67,17 +68,26 @@ func applyClaimableBalancesQueriesCursor(sql sq.SelectBuilder, lCursor int64, rC case db2.OrderAscending: if hasPagedLimit { sql = sql. - Where(sq.Expr("(cb.last_modified_ledger, cb.id) > (?, ?)", lCursor, rCursor)) - + Where( + sq.Expr( + fmt.Sprintf("(%s.last_modified_ledger, %s.id) > (?, ?)", tableName, tableName), + lCursor, rCursor, + ), + ) } - sql = sql.OrderBy("cb.last_modified_ledger asc, cb.id asc") + sql = sql.OrderBy(fmt.Sprintf("%s.last_modified_ledger asc, %s.id asc", tableName, tableName)) case db2.OrderDescending: if hasPagedLimit { sql = sql. - Where(sq.Expr("(cb.last_modified_ledger, cb.id) < (?, ?)", lCursor, rCursor)) + Where( + sq.Expr( + fmt.Sprintf("(%s.last_modified_ledger, %s.id) < (?, ?)", tableName, tableName), + lCursor, + rCursor, + ), + ) } - - sql = sql.OrderBy("cb.last_modified_ledger desc, cb.id desc") + sql = sql.OrderBy(fmt.Sprintf("%s.last_modified_ledger desc, %s.id desc", tableName, tableName)) default: return sql, errors.Errorf("invalid order: %s", order) } @@ -216,7 +226,7 @@ func (q *Q) GetClaimableBalances(ctx context.Context, query ClaimableBalancesQue return nil, errors.Wrap(err, "error getting cursor") } - sql, err := applyClaimableBalancesQueriesCursor(selectClaimableBalances, l, r, query.PageQuery.Order) + sql, err := applyClaimableBalancesQueriesCursor(selectClaimableBalances, "cb", l, r, query.PageQuery.Order) if err != nil { return nil, errors.Wrap(err, "could not apply query to page") } @@ -242,10 +252,10 @@ func (q *Q) GetClaimableBalances(ctx context.Context, query ClaimableBalancesQue // does not perform efficiently. Instead, use a subquery (with LIMIT) to retrieve claimable balances based on // the claimant's address. - var selectClaimableBalanceClaimants = sq.Select("id").From("claimable_balance_claimants"). - Where("destination = ?", query.Claimant.Address()).Limit(query.PageQuery.Limit) + var selectClaimableBalanceClaimants = sq.Select("claimable_balance_claimants.id").From("claimable_balance_claimants"). + Where("claimable_balance_claimants.destination = ?", query.Claimant.Address()).Limit(query.PageQuery.Limit) - subSql, err := applyClaimableBalancesQueriesCursor(selectClaimableBalanceClaimants, l, r, query.PageQuery.Order) + subSql, err := applyClaimableBalancesQueriesCursor(selectClaimableBalanceClaimants, "claimable_balance_claimants", l, r, query.PageQuery.Order) if err != nil { return nil, errors.Wrap(err, "could not apply subquery to page") } diff --git a/services/horizon/internal/db2/history/claimable_balances_test.go b/services/horizon/internal/db2/history/claimable_balances_test.go index 769ab3bc13..2e6d621945 100644 --- a/services/horizon/internal/db2/history/claimable_balances_test.go +++ b/services/horizon/internal/db2/history/claimable_balances_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/guregu/null" + "github.com/stellar/go/services/horizon/internal/db2" "github.com/stellar/go/services/horizon/internal/test" "github.com/stellar/go/xdr" @@ -203,7 +204,7 @@ func TestFindClaimableBalancesByDestination(t *testing.T) { tt.Assert.Equal(dest2, cbs[0].Claimants[1].Destination) // this validates the cb query with claimant and cb.id/ledger cursor parameters - query.PageQuery = db2.MustPageQuery(fmt.Sprintf("%v-%s", 150, cbs[0].BalanceID), false, "", 10) + query.PageQuery = db2.MustPageQuery(fmt.Sprintf("%v-%s", 150, cbs[0].BalanceID), false, "asc", 10) query.Claimant = xdr.MustAddressPtr(dest1) cbs, err = q.GetClaimableBalances(tt.Ctx, query) tt.Assert.NoError(err) @@ -212,7 +213,7 @@ func TestFindClaimableBalancesByDestination(t *testing.T) { // this validates the cb query with no claimant parameter, // should still produce working sql, as it triggers different LIMIT position in sql. - query.PageQuery = db2.MustPageQuery("", false, "", 1) + query.PageQuery = db2.MustPageQuery("", false, "desc", 1) query.Claimant = nil cbs, err = q.GetClaimableBalances(tt.Ctx, query) tt.Assert.NoError(err)