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

Changes from 1 commit
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
Loading