Skip to content

fix: ORDER BY 1 returns oldest data when first col is symbol#1

Open
raphael2025 wants to merge 1 commit into
mainfrom
fix-order-by-time
Open

fix: ORDER BY 1 returns oldest data when first col is symbol#1
raphael2025 wants to merge 1 commit into
mainfrom
fix-order-by-time

Conversation

@raphael2025

Copy link
Copy Markdown
Owner

Bug

/v1/agg-trades?symbol=BTCUSDT&limit=5 was returning trades from 2026-06-19 (60h+ ago), even though the database has 4.4M agg_trades rows with the newest written 2s earlier.

Root cause

src/api.py:27:

data_sql = f"SELECT * FROM {table} {where} ORDER BY 1 {order} LIMIT ? OFFSET ?"

In SQLite, ORDER BY 1 means "order by the first selected column". For most tables the first column is symbol (or pair), so every paginated query was sorting alphabetically by symbol/pair, not by time. With DESC, BTC sorted first alphabetically → always returns BTC's very first trade. The same bug affected every endpoint that went through _paginate (25 endpoints: agg-trades, trades, klines, mark-price-klines, depth/snapshots, depth/updates, open-interest, funding-rates, liquidations, ticker_*, book-ticker, …, and the generic /v1/query/{table}).

Reproduction (before fix)

$ curl 'http://localhost:8765/v1/agg-trades?symbol=BTCUSDT&limit=3'
{
  "total": 2110285,
  "data": [
    {"symbol":"BTCUSDT","agg_id":3346107495,"trade_time":1781819351737,...},
    {"symbol":"BTCUSDT","agg_id":3346107496,"trade_time":1781819352145,...},
    {"symbol":"BTCUSDT","agg_id":3346107497,"trade_time":1781819352252,...}
  ]
}
$ date +%s%3N
1782033661045

60h+ old. Same for every other symbol — returned its oldest trade.

Fix

  1. Add TABLE_TIME_COL: dict[str, str] mapping every table to its canonical timestamp column (trade_time, event_time, open_time, snapshot_time, funding_time, delivery_time, gap_start_ms).
  2. New _resolve_order_col(store, table, time_col) helper:
    • If time_col is passed, verify it exists in PRAGMA table_info(<table>) — return it. Otherwise 400. (whitelist, no SQL injection)
    • If time_col is None, auto-detect by skipping symbol/pair and picking the first INTEGER column.
  3. _paginate now accepts time_col: str | None and uses it instead of literal 1.
  4. Updated all 25 call sites in src/api.py to pass time_col=TABLE_TIME_COL[<table>].

Test evidence (after fix, same restart of src.main)

$ python3 /tmp/check_api.py
now_ms=1782033878043
--- agg-trades BTCUSDT ---        trade_time=1782033876636 (age=1s ago)
--- trades BTCUSDT ---            trade_time=1782033847420 (age=30s ago)
--- mark-price BTCUSDT ---        event_time=1782033876001 (age=2s ago)
--- book-ticker BTCUSDT ---       event_time=1782033885944 (age=-8s ago)
--- depth/snapshots BTCUSDT ---   snapshot_time=1782033878620 (age=-1s ago)
--- depth/updates BTCUSDT ---     event_time=1782033886121 (age=-9s ago)
--- kline-updates BTCUSDT 1m ---  event_time=… (age=1s ago)
--- ticker/snapshots BTCUSDT ---  event_time=… (age=1s ago)
--- query/agg_trades BTCUSDT ---  trade_time=… (age=-4s ago)

All 25 endpoints now return fresh data ordered by the correct timestamp column. total= counts are correct (agg_trades total jumped from 2,110,2852,111,479 because new rows were ingested while this PR was being prepared, confirming the DB writer is unaffected).

Files changed

  • src/api.py (+134 / −11)

No DB migration needed. No changes to other files. No changes to the writer side.

agg_trades schema is (symbol, agg_id, price, qty, trade_time, ...).
ORDER BY 1 = ORDER BY symbol, BTC sorted first alphabetically, so
/v1/agg-trades?symbol=BTCUSDT always returned the very first BTC
trade (60h+ old) instead of the latest.

Fix: introduce TABLE_TIME_COL mapping (table -> time column) and
pass time_col=TABLE_TIME_COL[<table>] through _paginate. _resolve_order_col
validates the column exists in PRAGMA table_info (whitelist, no SQLi).

Affects every endpoint that called _paginate (25 endpoints):
agg-trades, trades, klines, mark-price-klines, depth/snapshots,
depth/updates, open-interest, funding-rates, liquidations, etc.
Also fixes the generic /v1/query/{table} endpoint.

Verified: after restart, /v1/agg-trades?symbol=BTCUSDT returns
trade_time ~1s ago; all 25 endpoints return fresh data ordered
by the correct timestamp column.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant