Skip to content

Support for cockroach db #8662

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Support for cockroach db #8662

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ceb4cca
Some db are stricter with casting
RCasatta Nov 4, 2025
b619ab2
replace _ROWID_ with standard SQL
RCasatta Nov 4, 2025
b90fcd4
support cockroach db 64 bits integer
RCasatta Nov 4, 2025
d584cb9
use OID to determine int column type
RCasatta Nov 7, 2025
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
25 changes: 16 additions & 9 deletions db/db_postgres.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,16 +183,23 @@ static u64 db_postgres_column_u64(struct db_stmt *stmt, int col)
static s64 db_postgres_column_int(struct db_stmt *stmt, int col)
{
PGresult *res = (PGresult*)stmt->inner_stmt;
be32 bin;
size_t expected = sizeof(bin), actual = PQgetlength(res, stmt->row, col);

if (expected != actual)
db_fatal(stmt->db,
"s32 field doesn't match size: expected %zu, actual %zu\n",
expected, actual);
size_t actual = PQgetlength(res, stmt->row, col);

/* CockroachDB returns 8 bytes for INTEGER, PostgreSQL returns 4 */
if (actual == 4) {
be32 bin;
memcpy(&bin, PQgetvalue(res, stmt->row, col), sizeof(bin));
return be32_to_cpu(bin);
} else if (actual == 8) {
be64 bin;
memcpy(&bin, PQgetvalue(res, stmt->row, col), sizeof(bin));
return be64_to_cpu(bin);
}
Copy link
Contributor

@rustyrussell rustyrussell Nov 7, 2025

Choose a reason for hiding this comment

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

That's super-annoying. Postgres was actually checking our types are correct here (sqlite3 is YOLO), so loosening it is not great.

I wonder if it's best to support a cockroachdb:// db URL, which shares everything except has a different column_int function?

Copy link
Contributor Author

@RCasatta RCasatta Nov 7, 2025

Choose a reason for hiding this comment

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

I think there is a better way to do this, without introducing the specific cockroachdb url.
Using OID, see d584cb9

Copy link
Contributor

@rustyrussell rustyrussell Nov 13, 2025

Choose a reason for hiding this comment

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

But I really like that you currently have to set a BIGINT for u64 accesses, and be consistent.

Better is to have a separate db/db_cockroachdb.c and share almost all the functions with the postgres one. Hell, you could even put it in the same file.


memcpy(&bin, PQgetvalue(res, stmt->row, col), sizeof(bin));
return be32_to_cpu(bin);
db_fatal(stmt->db,
"integer field size unexpected: expected 4 or 8, actual %zu\n",
actual);
return 0; /* Never reached, but silences compiler warning */
}

static size_t db_postgres_column_bytes(struct db_stmt *stmt, int col)
Expand Down
8 changes: 4 additions & 4 deletions wallet/db.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,10 @@ static struct migration dbmigrations[] = {
{SQL("UPDATE invoices SET pay_index=id WHERE state=1;"),
NULL}, /* only paid invoice */
/* Create next_pay_index variable (highest pay_index). */
{SQL("INSERT INTO vars(name, val)"
{SQL("INSERT INTO vars(name, val)"
" VALUES('next_pay_index', "
" COALESCE((SELECT MAX(pay_index) FROM invoices WHERE state=1), 0) "
"+ 1"
" CAST(COALESCE((SELECT MAX(pay_index) FROM invoices WHERE state=1), 0) "
"+ 1 AS TEXT)"
" );"),
NULL},
/* Create first_block field; initialize from channel id if any.
Expand Down Expand Up @@ -943,7 +943,7 @@ static struct migration dbmigrations[] = {
" in_channel_scid"
", COALESCE("
" (SELECT channel_htlc_id FROM channel_htlcs WHERE id = forwarded_payments.in_htlc_id),"
" -_ROWID_"
" -row_number() OVER ()"
Comment on lines -939 to +946
Copy link
Contributor

@rustyrussell rustyrussell Nov 7, 2025

Choose a reason for hiding this comment

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

Ah I think you missed the translation layer?

See devtools/sql-rewrite.py...

Copy link
Contributor Author

@RCasatta RCasatta Nov 7, 2025

Choose a reason for hiding this comment

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

I see that sql-rewrite.py rewrite _ROWID_, but I don't get what is exactly the flow to use it, how do I bootstrap a lightning node with cockroach db?

Copy link
Contributor

@rustyrussell rustyrussell Nov 13, 2025

Choose a reason for hiding this comment

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

It gets run over all db source files to map anything in a SQL() macro. These get looked up at runtime.

" )"
", out_channel_scid"
", (SELECT channel_htlc_id FROM channel_htlcs WHERE id = forwarded_payments.out_htlc_id)"
Expand Down
Loading