On Sun, Aug 27, 2017 at 12:12 AM, Tom Lane <tgl@xxxxxxxxxxxxx> wrote: > Michael Paquier <michael.paquier@xxxxxxxxx> writes: >> On Fri, Aug 25, 2017 at 8:10 AM, Tom Lane <tgl@xxxxxxxxxxxxx> wrote: >>> I think the real problem occurs where we realloc the array bigger. > >> Looking at the surroundings, I think that it would be nice to have >> pqAddTuple and PQsetvalue set an error message with this patch. > > Yeah, I was thinking about that myself - the existing design presumes > that the only possible reason for failure of pqAddTuple is OOM, but > it'd be better to distinguish "too many tuples" from true OOM. So > we should also refactor to make pqAddTuple responsible for setting > the error message. Might be best to do the refactoring first. Attached are two patches: 1) 0001 refactors the code around pqAddTuple to be able to handle error messages and assign them in PQsetvalue particularly. 2) 0002 adds sanity checks in pqAddTuple for overflows, maximizing the size of what is allocated to INT_MAX but now more. pqRowProcessor() still has errmsgp, but it is never used on HEAD. At least with this set of patches it comes to be useful. We could rework check_field_number() to use as well an error message string, but I have left that out to keep things simple. Not sure if any complication is worth compared to just copying the error message in case of an unmatching column number. Attached is as well a small program I have used to test PQsetvalue through PQcopyResult to see if results get correctly allocated at each call, looking at the error message stacks on the way. -- Michael
/*
* Script to test PQcopyResult and subsequently PQsetvalue.
* Compile with for example:
* gcc -lpq -g -o pg_copy_res pg_copy_res.c
*/
#include <stdio.h>
#include <stdlib.h>
#include "libpq-fe.h"
#define DEFAULT_PORT "5432"
#define DEFAULT_HOST "/tmp"
#define DEFAULT_DB "postgres"
int
main()
{
char *port = getenv("PGPORT");
char *host = getenv("PGHOST");
char *dbname = getenv("PGDATABASE");
char connstr[512];
PGconn *conn;
PGresult *res, *res_copy;
if (port == NULL)
port = DEFAULT_PORT;
if (host == NULL)
host = DEFAULT_HOST;
if (dbname == NULL)
dbname = DEFAULT_DB;
snprintf(connstr, sizeof(connstr), "port=%s host=%s dbname=%s",
port, host, dbname);
conn = PQconnectdb(connstr);
if (PQstatus(conn) != CONNECTION_OK)
{
fprintf(stderr, "Connection to database failed: %s",
PQerrorMessage(conn));
return 1;
}
res = PQexec(conn, "SELECT 1");
/* Copy the resuld wanted, who care what that is... */
res_copy = PQcopyResult(res, PG_COPYRES_TUPLES | PG_COPYRES_ATTRS);
PQclear(res);
PQclear(res_copy);
PQfinish(conn);
return 0;
}
Attachment:
0001-Refactor-error-message-handling-in-pqAddTuple.patch
Description: Binary data
Attachment:
0002-Improve-overflow-checks-of-pqAddTuple-in-libpq.patch
Description: Binary data
-- Sent via pgsql-general mailing list (pgsql-general@xxxxxxxxxxxxxx) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-general
