Skip to content

Commit

Permalink
Fix claimable_balance_claimants subquery in GetClaimableBalances()
Browse files Browse the repository at this point in the history
  • Loading branch information
tamirms committed Feb 14, 2024
1 parent f232a9b commit f2cd337
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 13 deletions.
32 changes: 21 additions & 11 deletions services/horizon/internal/db2/history/claimable_balances.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {

Check failure on line 61 in services/horizon/internal/db2/history/claimable_balances.go

View workflow job for this annotation

GitHub Actions / golangci

line is 153 characters (lll)
hasPagedLimit := false
if lCursor > 0 && rCursor != "" {
hasPagedLimit = true
Expand All @@ -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)
}
Expand Down Expand Up @@ -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")
}
Expand All @@ -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)

Check failure on line 258 in services/horizon/internal/db2/history/claimable_balances.go

View workflow job for this annotation

GitHub Actions / golangci

line is 145 characters (lll)
if err != nil {
return nil, errors.Wrap(err, "could not apply subquery to page")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit f2cd337

Please sign in to comment.