From c51b5f3598fb8396f17156eecdc1f8e71c6a88ef Mon Sep 17 00:00:00 2001 From: Yorhel Date: Mon, 3 Feb 2025 16:59:18 +0100 Subject: [PATCH] pg: Better error reporting + basic exec() method --- FU.xs | 5 +++ FU/PG.pm | 6 +++ c/libpq.h | 64 +++++++++++++++++++++++++++++ c/pgconn.c | 109 +++++++++++++++++++++++++++++++++++++------------- t/pgconnect.t | 17 ++++++++ 5 files changed, 173 insertions(+), 28 deletions(-) create mode 100644 c/libpq.h diff --git a/FU.xs b/FU.xs index ee9dd19..674d65d 100644 --- a/FU.xs +++ b/FU.xs @@ -8,6 +8,7 @@ #include "c/common.c" #include "c/jsonfmt.c" #include "c/jsonparse.c" +#include "c/libpq.h" #include "c/pgconn.c" @@ -66,6 +67,10 @@ int server_version(fupg_conn *c) OUTPUT: RETVAL +void exec(fupg_conn *c, SV *sv) + CODE: + ST(0) = fupg_exec(c, SvPVutf8_nolen(sv)); + void DESTROY(fupg_conn *c) CODE: fupg_destroy(c); diff --git a/FU/PG.pm b/FU/PG.pm index 7b8f10b..ab8c90a 100644 --- a/FU/PG.pm +++ b/FU/PG.pm @@ -8,4 +8,10 @@ package FU::PG::conn { sub lib_version { FU::PG::lib_version() } }; +package FU::PG::error { + use overload '""' => sub($e, @) { + $e->{verbose_message} || "$e->{severity}: $e->{message}\n"; + }; +} + 1; diff --git a/c/libpq.h b/c/libpq.h new file mode 100644 index 0000000..8986c79 --- /dev/null +++ b/c/libpq.h @@ -0,0 +1,64 @@ +/* libpq is not being linked directly and there's no build-time dependency on it. + * This means we need to manually copy over some definitions from libpq-fe.h + */ + +typedef struct PGconn PGconn; +typedef struct PGresult PGresult; + +typedef enum { + PGRES_EMPTY_QUERY = 0, PGRES_COMMAND_OK, PGRES_TUPLES_OK, PGRES_COPY_OUT, PGRES_COPY_IN, + PGRES_BAD_RESPONSE, PGRES_NONFATAL_ERROR, PGRES_FATAL_ERROR, PGRES_COPY_BOTH, + PGRES_SINGLE_TUPLE, PGRES_PIPELINE_SYNC, PGRES_PIPELINE_ABORTED, PGRES_TUPLES_CHUNK +} ExecStatusType; +typedef enum { PQERRORS_TERSE, PQERRORS_DEFAULT, PQERRORS_VERBOSE, PQERRORS_SQLSTATE } PGVerbosity; +typedef enum { PQSHOW_CONTEXT_NEVER, PQSHOW_CONTEXT_ERRORS, PQSHOW_CONTEXT_ALWAYS } PGContextVisibility; + +#define PG_DIAG_SEVERITY 'S' +#define PG_DIAG_SEVERITY_NONLOCALIZED 'V' +#define PG_DIAG_SQLSTATE 'C' +#define PG_DIAG_MESSAGE_PRIMARY 'M' +#define PG_DIAG_MESSAGE_DETAIL 'D' +#define PG_DIAG_MESSAGE_HINT 'H' +#define PG_DIAG_STATEMENT_POSITION 'P' +#define PG_DIAG_INTERNAL_POSITION 'p' +#define PG_DIAG_INTERNAL_QUERY 'q' +#define PG_DIAG_CONTEXT 'W' +#define PG_DIAG_SCHEMA_NAME 's' +#define PG_DIAG_TABLE_NAME 't' +#define PG_DIAG_COLUMN_NAME 'c' +#define PG_DIAG_DATATYPE_NAME 'd' +#define PG_DIAG_CONSTRAINT_NAME 'n' +#define PG_DIAG_SOURCE_FILE 'F' +#define PG_DIAG_SOURCE_LINE 'L' +#define PG_DIAG_SOURCE_FUNCTION 'R' + +#define PG_FUNCS \ + X(PQclear, void, PGresult *) \ + X(PQconnectdb, PGconn *, const char *) \ + X(PQcmdTuples, char *, PGresult *) \ + X(PQerrorMessage, char *, const PGconn *) \ + X(PQexec, PGresult *, PGconn *, const char *) \ + X(PQfinish, void, PGconn *) \ + X(PQfreemem, void, void *) \ + X(PQlibVersion, int, void) \ + X(PQresStatus, char *, ExecStatusType) \ + X(PQresultErrorField, char *, const PGresult *, int) \ + X(PQresultErrorMessage, char *, const PGresult *res) \ + X(PQresultStatus, ExecStatusType, const PGresult *) \ + X(PQresultVerboseErrorMessage, char *, const PGresult *, PGVerbosity, PGContextVisibility) \ + X(PQserverVersion, int, const PGconn *) \ + X(PQstatus, int, const PGconn *) + +#define X(n, r, ...) static r (*n)(__VA_ARGS__); +PG_FUNCS +#undef X + +static void fupg_load() { + void *handle = dlopen("libpq.so", RTLD_LAZY); + if (!handle) croak("Unable to load libpq: %s", dlerror()); +#define X(n, ...) if (!(n = dlsym(handle, #n))) croak("Unable to load libpq: %s", dlerror()); +PG_FUNCS +#undef X +} + +#undef PG_FUNCS diff --git a/c/pgconn.c b/c/pgconn.c index 256ff56..e507a54 100644 --- a/c/pgconn.c +++ b/c/pgconn.c @@ -1,37 +1,70 @@ -typedef struct PGconn PGconn; - -#define PG_FUNCS \ - X(PQconnectdb, PGconn *, const char *) \ - X(PQerrorMessage, char *, const PGconn *) \ - X(PQfinish, void, PGconn *) \ - X(PQlibVersion, int, void) \ - X(PQserverVersion, int, const PGconn *) \ - X(PQstatus, int, const PGconn *) - -#define X(n, r, ...) static r (*n)(__VA_ARGS__); -PG_FUNCS -#undef X - -static void fupg_load() { - void *handle = dlopen("libpq.so", RTLD_LAZY); - if (!handle) croak("Unable to load libpq: %s", dlerror()); -#define X(n, ...) if (!(n = dlsym(handle, #n))) croak("Unable to load libpq: %s", dlerror()); -PG_FUNCS -#undef X -} - -#undef PG_FUNCS - - typedef struct { PGconn *conn; } fupg_conn; -fupg_conn *fupg_connect(pTHX_ const char *str) { - SV *sv; + +static SV *fupg_conn_errsv(PGconn *conn, const char *action) { + dTHX; + HV *hv = newHV(); + hv_stores(hv, "action", newSVpv(action, 0)); + hv_stores(hv, "severity", newSVpvs("FATAL")); /* Connection-related errors are always fatal */ + hv_stores(hv, "message", newSVpv(PQerrorMessage(conn), 0)); + return sv_bless(sv_2mortal(newRV_noinc((SV *)hv)), gv_stashpvs("FU::PG::error", GV_ADD)); +} + +static void fupg_conn_croak(fupg_conn *c, const char *action) { + dTHX; + croak_sv(fupg_conn_errsv(c->conn, action)); +} + +/* Takes ownership of the PGresult and croaks. */ +static void fupg_result_croak(PGresult *r, const char *action, const char *query) { + dTHX; + HV *hv = newHV(); + char *s; + hv_stores(hv, "action", newSVpv(action, 0)); + s = PQresultErrorField(r, PG_DIAG_SEVERITY_NONLOCALIZED); + hv_stores(hv, "severity", newSVpv(s ? s : "FATAL", 0)); + if (query) hv_stores(hv, "query", newSVpv(s, 0)); + + /* If the PGresult is not an error, assume it's an unexpected resultStatus */ + s = PQresultErrorField(r, PG_DIAG_MESSAGE_PRIMARY); + hv_stores(hv, "message", s ? newSVpv(s, 0) : newSVpvf("unexpected status code '%s'", PQresStatus(PQresultStatus(r)))); + + /* I like the verbose error messages. Doesn't include anything that's not + * also fetched below, but saves me from having to do the formatting + * manually. */ + if (s) { + s = PQresultVerboseErrorMessage(r, 2 /* PQERRORS_VERBOSE */, 1 /* PQSHOW_CONTEXT_ERRORS */); + if (s) { + hv_stores(hv, "verbose_message", newSVpv(s, 0)); + PQfreemem(s); + } + } + + if ((s = PQresultErrorField(r, PG_DIAG_MESSAGE_DETAIL))) hv_stores(hv, "detail", newSVpv(s, 0)); + if ((s = PQresultErrorField(r, PG_DIAG_MESSAGE_HINT))) hv_stores(hv, "hint", newSVpv(s, 0)); + if ((s = PQresultErrorField(r, PG_DIAG_STATEMENT_POSITION))) hv_stores(hv, "statement_position", newSVpv(s, 0)); + if ((s = PQresultErrorField(r, PG_DIAG_INTERNAL_POSITION))) hv_stores(hv, "internal_position", newSVpv(s, 0)); + if ((s = PQresultErrorField(r, PG_DIAG_INTERNAL_QUERY))) hv_stores(hv, "internal_query", newSVpv(s, 0)); + if ((s = PQresultErrorField(r, PG_DIAG_CONTEXT))) hv_stores(hv, "context", newSVpv(s, 0)); + if ((s = PQresultErrorField(r, PG_DIAG_SCHEMA_NAME))) hv_stores(hv, "schema_name", newSVpv(s, 0)); + if ((s = PQresultErrorField(r, PG_DIAG_TABLE_NAME))) hv_stores(hv, "table_name", newSVpv(s, 0)); + if ((s = PQresultErrorField(r, PG_DIAG_COLUMN_NAME))) hv_stores(hv, "column_name", newSVpv(s, 0)); + if ((s = PQresultErrorField(r, PG_DIAG_DATATYPE_NAME))) hv_stores(hv, "datatype_name", newSVpv(s, 0)); + if ((s = PQresultErrorField(r, PG_DIAG_CONSTRAINT_NAME))) hv_stores(hv, "constraint_name", newSVpv(s, 0)); + if ((s = PQresultErrorField(r, PG_DIAG_SOURCE_FILE))) hv_stores(hv, "source_file", newSVpv(s, 0)); + if ((s = PQresultErrorField(r, PG_DIAG_SOURCE_LINE))) hv_stores(hv, "source_line", newSVpv(s, 0)); + if ((s = PQresultErrorField(r, PG_DIAG_SOURCE_FUNCTION))) hv_stores(hv, "source_function", newSVpv(s, 0)); + + PQclear(r); + croak_sv(sv_bless(sv_2mortal(newRV_noinc((SV *)hv)), gv_stashpvs("FU::PG::error", GV_ADD))); +} + +static fupg_conn *fupg_connect(pTHX_ const char *str) { PGconn *conn = PQconnectdb(str); if (PQstatus(conn) != 0) { - sv = newSVpvf("FU::PG connection error: %s", PQerrorMessage(conn)); + SV *sv = fupg_conn_errsv(conn, "connect"); PQfinish(conn); croak_sv(sv); } @@ -45,3 +78,23 @@ static void fupg_destroy(fupg_conn *c) { PQfinish(c->conn); safefree(c); } + +static SV *fupg_exec(fupg_conn *c, const char *sql) { + PGresult *r = PQexec(c->conn, sql); + if (!r) fupg_conn_croak(c, "exec"); + switch (PQresultStatus(r)) { + case PGRES_EMPTY_QUERY: + case PGRES_COMMAND_OK: + case PGRES_TUPLES_OK: break; + default: fupg_result_croak(r, "exec", sql); + } + SV *ret = &PL_sv_undef; + char *tup = PQcmdTuples(r); + if (tup && *tup) { + ret = sv_2mortal(newSVpv(tup, 0)); + SvIV(ret); + SvIOK_only(ret); + } + PQclear(r); + return ret; +} diff --git a/t/pgconnect.t b/t/pgconnect.t index da10a36..023f0f2 100644 --- a/t/pgconnect.t +++ b/t/pgconnect.t @@ -4,7 +4,15 @@ use Test::More; plan skip_all => $@ if !eval { require FU::PG; } && $@ =~ /Unable to load libpq/; plan skip_all => 'Please set FU_TEST_DB to a PostgreSQL connection string to run these tests' if !$ENV{FU_TEST_DB}; +sub okerr($sev, $act, $msg) { + is ref $@, 'FU::PG::error'; + is $@->{severity}, $sev; + is $@->{action}, $act; + like "$@", $msg; +} + ok !eval { FU::PG->connect("invalid") }; +okerr FATAL => connect => qr/missing "=" after "invalid"/; ok FU::PG::lib_version() > 100000; @@ -14,4 +22,13 @@ is ref $conn, 'FU::PG::conn'; ok $conn->server_version() > 100000; is $conn->lib_version(), FU::PG::lib_version(); +ok !eval { $conn->exec('COPY (SELECT 1) TO STDOUT'); }; +okerr FATAL => exec => qr/unexpected status code/; + +ok !eval { $conn->exec('SELEXT'); }; +okerr ERROR => exec => qr/syntax error/; + +ok !defined $conn->exec(''); +is $conn->exec('SELECT 1'), 1; + done_testing;