sql: rewrite scid string literals to use scid() for index support - #9430
sql: rewrite scid string literals to use scid() for index support#9430vincenzopalazzo wants to merge 3 commits into
Conversation
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Short channel IDs were stored as TEXT strings (e.g., "735095x480x1") in
SQLite, which prevented efficient use of indexes on SCID columns. Change
storage to INTEGER (the u64 encoding), using a custom "SCID" column type
so the result-reading code can detect these columns and format them back
as "NNNxNNNxNNN" strings for backward-compatible JSON output.
Add two new SQL functions:
- scid('NNNxNNNxNNN') -> integer: for efficient WHERE clause filtering
- fmt_scid(integer) -> 'NNNxNNNxNNN': for formatting in SQL expressions
Fixes ElementsProject#8941
When users query with WHERE in_channel='735095x480x1', the string
literal is compared directly against the integer SCID column, forcing
SQLite to perform a full table scan even when an index exists.
Automatically rewrite scid string literals (matching NNNxNNNxNNN format)
to use the scid() function before passing the query to SQLite, so
'735095x480x1' becomes scid('735095x480x1'). This allows SQLite to
use indexes on SCID columns transparently.
Queries already using scid() explicitly are detected and left unchanged.
Changelog-Fixed: sql plugin now automatically translates short_channel_id string literals to integers for efficient index usage.
Fixes: ElementsProject#8941
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
6afaed2 to
c248e41
Compare
Andezion
left a comment
There was a problem hiding this comment.
The character by character copy loop (tal_append_fmt(&result, "%c", *p); p++;) for every non-quote byte is pretty "expensive" - each call goes through tal_append_fmt realloc/vsnprintf machinery for a single byte. Maybe we can find next ' with strchr and append the whole non-quoted span in one tal_append_fmt(&result, "%.*s", ...) call, mirroring what the function already does for the quoted spans??
| 'secret': 'BLOB', | ||
| 'number': 'REAL', | ||
| 'short_channel_id': 'TEXT'} | ||
| 'short_channel_id': 'SCID'} |
There was a problem hiding this comment.
Maybe we should add a test that runs a query like SELECT * FROM forwards WHERE in_channel = '<scid>' and checks it returns the expected row (ideally also EXPLAIN QUERY PLAN showing index usage??), plus a couple of negative/edge cases (malformed literal, literal already wrapped in scid())?
| num_rows = 0; | ||
| errmsg = NULL; | ||
|
|
||
| while ((err = sqlite3_step(dbq->stmt)) == SQLITE_ROW) { |
There was a problem hiding this comment.
As i see here for each row, for each SQLITE_INTEGER column, the code calls sqlite3_column_decltype() and does a streq(). And the decltype of a given column index is constant for the lifetime of the prepared statement so it doesn't depend on the row? Maybe we can compute a bool is_scid[num_cols] once before the row loop and index into it, instead of recomputing per row? What do you think??
Confirmed still relevant on master (2026-08-12):
plugins/sql.chas noscid()handling, scid string literals still cannot use indexes.Summary
WHERE in_channel='735095x480x1', the string literal was compared directly against the integer SCID column, forcing SQLite into a full table scan even with an index presentjson_sql()passes user queries directly tosqlite3_prepare_v2()without translating scid string literals to integersNNNxNNNxNNNformat to use thescid()function before query execution, so'735095x480x1'becomesscid('735095x480x1')transparentlyFixes #8941
Changelog-Fixed: sql plugin now automatically translates short_channel_id string literals to integers for efficient index usage.
Test plan
SELECT * FROM forwards WHERE in_channel='735095x480x1'now use indexesscid('...')explicitly are left unchanged