Skip to content

internal/db2/history: rewrite /assets pagination as bounded CTE merge query #210

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hexdigest wants to merge 2 commits into
base: main
Choose a base branch
from
+109 −32
Open

internal/db2/history: rewrite /assets pagination as bounded CTE merge query #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e54e379
Rewrite /assets pagination as bounded CTE merge query
max-chechel-vc Jul 25, 2026
50c864d
Address Copilot review feedback on /assets CTE pagination
hexdigest Jul 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ file. This project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased

### Fixed
- Improved `/assets` endpoint performance by replacing non-sargable pagination over a `FULL OUTER JOIN` with a bounded CTE merge query that restores index-backed filtering and ordering.

## 27.0.0

**This release adds support for Protocol 27**
Expand Down
138 changes: 106 additions & 32 deletions internal/db2/history/asset_stats.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,32 +435,6 @@ func (q *Q) GetAssetStats(ctx context.Context, assetCode, assetIssuer string, pa
//
// (3) is stored in the contract_asset_stats table and is derived by ingesting SAC contract balances.
//
// All 3 tables are joined in the query below to compute the desired list of AssetAndContractStat
// entries.
sql := sq.Select("COALESCE(exp_asset_stats.asset_type, asset_contracts.asset_type) as asset_type, " +
"COALESCE(exp_asset_stats.asset_code, asset_contracts.asset_code) as asset_code, " +
"COALESCE(exp_asset_stats.asset_issuer, asset_contracts.asset_issuer) as asset_issuer, " +
"exp_asset_stats.accounts, " +
"exp_asset_stats.balances, " +
"asset_contracts.contract_id as contract_id, contract_asset_stats.stat as contracts").
From("exp_asset_stats").
JoinClause("FULL OUTER JOIN asset_contracts ON " +
"exp_asset_stats.asset_type = asset_contracts.asset_type AND " +
"exp_asset_stats.asset_code = asset_contracts.asset_code AND " +
"exp_asset_stats.asset_issuer = asset_contracts.asset_issuer").
LeftJoin("contract_asset_stats ON asset_contracts.contract_id = contract_asset_stats.contract_id")
filters := map[string]interface{}{}
if assetCode != "" {
filters["COALESCE(exp_asset_stats.asset_code, asset_contracts.asset_code)"] = assetCode
}
if assetIssuer != "" {
filters["COALESCE(exp_asset_stats.asset_issuer, asset_contracts.asset_issuer)"] = assetIssuer
}

if len(filters) > 0 {
sql = sql.Where(filters)
}

var cursorComparison, orderBy string
switch page.Order {
case "asc":
Expand All @@ -471,21 +445,121 @@ func (q *Q) GetAssetStats(ctx context.Context, assetCode, assetIssuer string, pa
return nil, fmt.Errorf("invalid page order %s", page.Order)
}

cursorCode, cursorIssuer := "", ""
if page.Cursor != "" {
cursorCode, cursorIssuer, err := parseAssetStatsCursor(page.Cursor)
var err error
cursorCode, cursorIssuer, err = parseAssetStatsCursor(page.Cursor)
if err != nil {
return nil, err
}
}

sql = sql.Where("((COALESCE(exp_asset_stats.asset_code, asset_contracts.asset_code), COALESCE(exp_asset_stats.asset_issuer, asset_contracts.asset_issuer)) "+cursorComparison+" (?,?))", cursorCode, cursorIssuer)
expAssetStatsSQL := sq.Select(
"exp_asset_stats.asset_type as asset_type",
"exp_asset_stats.asset_code as asset_code",
"exp_asset_stats.asset_issuer as asset_issuer",
"exp_asset_stats.accounts",
"exp_asset_stats.balances",
"asset_contracts.contract_id as contract_id",
"contract_asset_stats.stat as contracts",
).
From("exp_asset_stats").
LeftJoin("asset_contracts ON " +
"exp_asset_stats.asset_type = asset_contracts.asset_type AND " +
"exp_asset_stats.asset_code = asset_contracts.asset_code AND " +
"exp_asset_stats.asset_issuer = asset_contracts.asset_issuer").
LeftJoin("contract_asset_stats ON asset_contracts.contract_id = contract_asset_stats.contract_id")

contractOnlySQL := sq.Select(
"asset_contracts.asset_type as asset_type",
"asset_contracts.asset_code as asset_code",
"asset_contracts.asset_issuer as asset_issuer",
`'{"authorized":0,"authorized_to_maintain_liabilities":0,"claimable_balances":0,"liquidity_pools":0,"unauthorized":0}'::jsonb as accounts`,
`'{"authorized":"0","authorized_to_maintain_liabilities":"0","claimable_balances":"0","liquidity_pools":"0","unauthorized":"0"}'::jsonb as balances`,
Comment on lines +477 to +478

@tamirms tamirms Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider selecting typed NULLs here instead of hand-written zero JSON:

Suggested change
`'{"authorized":0,"authorized_to_maintain_liabilities":0,"claimable_balances":0,"liquidity_pools":0,"unauthorized":0}'::jsonb as accounts`,
`'{"authorized":"0","authorized_to_maintain_liabilities":"0","claimable_balances":"0","liquidity_pools":"0","unauthorized":"0"}'::jsonb as balances`,
`NULL::jsonb as accounts`,
`NULL::jsonb as balances`,

The old FULL OUTER JOIN produced NULL accounts/balances for contract-only rows, and ExpAssetStatAccounts.Scan / ExpAssetStatBalances.Scan already normalize NULL to exactly these zero values (0 ints, "0" strings). Relying on that gives byte-for-byte parity with the previous behavior and removes two hard-coded copies of the struct field sets, which would silently drift if a field is ever added to ExpAssetStatAccounts/ExpAssetStatBalances.

"asset_contracts.contract_id as contract_id",
"contract_asset_stats.stat as contracts",
).
From("asset_contracts").
LeftJoin("contract_asset_stats ON asset_contracts.contract_id = contract_asset_stats.contract_id").
Where(`NOT EXISTS (
SELECT 1
FROM exp_asset_stats
WHERE exp_asset_stats.asset_type = asset_contracts.asset_type
AND exp_asset_stats.asset_code = asset_contracts.asset_code
AND exp_asset_stats.asset_issuer = asset_contracts.asset_issuer
)`)

if assetCode != "" {
expAssetStatsSQL = expAssetStatsSQL.Where("exp_asset_stats.asset_code = ?", assetCode)
contractOnlySQL = contractOnlySQL.Where("asset_contracts.asset_code = ?", assetCode)
}
if assetIssuer != "" {
expAssetStatsSQL = expAssetStatsSQL.Where("exp_asset_stats.asset_issuer = ?", assetIssuer)
contractOnlySQL = contractOnlySQL.Where("asset_contracts.asset_issuer = ?", assetIssuer)
}
if page.Cursor != "" {
expAssetStatsSQL = expAssetStatsSQL.Where(
"((exp_asset_stats.asset_code, exp_asset_stats.asset_issuer) "+cursorComparison+" (?,?))",
cursorCode,
cursorIssuer,
)
contractOnlySQL = contractOnlySQL.Where(
"((asset_contracts.asset_code, asset_contracts.asset_issuer) "+cursorComparison+" (?,?))",
cursorCode,
cursorIssuer,
)
}

sql = sql.OrderBy("(COALESCE(exp_asset_stats.asset_code, asset_contracts.asset_code), COALESCE(exp_asset_stats.asset_issuer, asset_contracts.asset_issuer)) " + orderBy).Limit(page.Limit)
expAssetStatsSQL = expAssetStatsSQL.
OrderBy("exp_asset_stats.asset_code "+orderBy, "exp_asset_stats.asset_issuer "+orderBy).
Limit(page.Limit)
contractOnlySQL = contractOnlySQL.
OrderBy("asset_contracts.asset_code "+orderBy, "asset_contracts.asset_issuer "+orderBy).
Limit(page.Limit)

expAssetStatsCTE, expAssetStatsArgs, err := expAssetStatsSQL.ToSql()
if err != nil {
return nil, errors.Wrap(err, "could not build exp_asset_stats select query")
}
contractOnlyCTE, contractOnlyArgs, err := contractOnlySQL.ToSql()
if err != nil {
return nil, errors.Wrap(err, "could not build contract-only select query")
}

// fmt.Sprintf is safe here because it only stitches together SQL generated by
// Squirrel plus the fixed order keyword chosen from "asc"/"desc". User input
// remains in the bound args produced by ToSql().
sql := fmt.Sprintf(`
WITH exp_rows AS (
%s
), contract_only_rows AS (
%s
)
SELECT *
FROM (
SELECT * FROM exp_rows
UNION ALL
SELECT * FROM contract_only_rows
) merged
ORDER BY asset_code %s, asset_issuer %s
LIMIT ?`,
expAssetStatsCTE,
contractOnlyCTE,
orderBy,
orderBy,
)

args := append(expAssetStatsArgs, contractOnlyArgs...)
args = append(args, page.Limit)

var results []AssetAndContractStat
if err := q.Select(ctx, &results, sql); err != nil {
sqlString, _, _ := sql.ToSql()
return nil, errors.Wrapf(err, "could not run select query: %s", sqlString)
// Add explicit query type for prometheus metrics, since we use raw sql.
ctx = context.WithValue(ctx, &db.QueryTypeContextKey, db.SelectQueryType)
if err := q.SelectRaw(ctx, &results, sql, args...); err != nil {
return nil, errors.Wrapf(err, "could not run select query: %s", sql)
}
if len(results) == 0 {
return nil, nil
}
Comment on lines +561 to 563

@tamirms tamirms Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block is dead code — results starts nil and sqlx only appends per scanned row, so zero rows already yields nil (clearSliceIfPossible preserves nil-ness too). That's why the existing expected: nil cases in TestGetAssetStatsFiltersAndCursor passed before this change, on the same SelectRaw path.

Suggested change
if len(results) == 0 {
return nil, nil
}


return results, nil
Expand Down