Skip to content

sql: rewrite scid string literals to use scid() for index support - #9430

Open
vincenzopalazzo wants to merge 3 commits into
ElementsProject:masterfrom
vincenzopalazzo:claude/eloquent-meitner
Open

sql: rewrite scid string literals to use scid() for index support#9430
vincenzopalazzo wants to merge 3 commits into
ElementsProject:masterfrom
vincenzopalazzo:claude/eloquent-meitner

Conversation

@vincenzopalazzo

Copy link
Copy Markdown
Collaborator

Recreated from #9407 (original head fork went private; identical commits, no content changes). Originally opened 2026-08-12.

♻️ Re-creation of #8984, which was auto-closed on 2026-08-11 when my fork was detached from the repository network (GitHub cannot reopen PRs closed by a detachment). Branch content is unchanged (head 84562dfed); the previous review discussion remains on #8984.

Confirmed still relevant on master (2026-08-12): plugins/sql.c has no scid() handling, scid string literals still cannot use indexes.

Summary

  • When querying with 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 present
  • Root cause: json_sql() passes user queries directly to sqlite3_prepare_v2() without translating scid string literals to integers
  • Automatically rewrite scid string literals matching NNNxNNNxNNN format to use the scid() function before query execution, so '735095x480x1' becomes scid('735095x480x1') transparently

Fixes #8941
Changelog-Fixed: sql plugin now automatically translates short_channel_id string literals to integers for efficient index usage.

Test plan

  • CI passes
  • Queries like SELECT * FROM forwards WHERE in_channel='735095x480x1' now use indexes
  • Queries already using scid('...') explicitly are left unchanged
  • Non-scid string literals are unaffected

@vincenzopalazzo vincenzopalazzo added this to the v26.12 milestone Aug 17, 2026
vincenzopalazzo added a commit to vincenzopalazzo/lightning that referenced this pull request Aug 17, 2026
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>
@vincenzopalazzo
vincenzopalazzo force-pushed the claude/eloquent-meitner branch from 6afaed2 to c248e41 Compare August 17, 2026 11:35

@Andezion Andezion left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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??

Comment thread tests/test_plugin.py
'secret': 'BLOB',
'number': 'REAL',
'short_channel_id': 'TEXT'}
'short_channel_id': 'SCID'}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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())?

Comment thread plugins/sql.c
num_rows = 0;
errmsg = NULL;

while ((err = sqlite3_step(dbq->stmt)) == SQLITE_ROW) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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??

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.

SQL plugin: translate short_channel_id strings to integers before querying SQLite

2 participants