From: Andrew Chernow <ac@esilo.com>
To: Alvaro Herrera <alvherre@commandprompt.com>
Cc: Merlin Moncure <mmoncure@gmail.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: libpq object hooks (libpq events)
Date: Wed, 03 Sep 2008 10:09:36 -0400
Message-ID: <48BE9AA0.1070606@esilo.com> (raw)
In-Reply-To: <20080903001920.GT12610@alvh.no-ip.org>
References: <19342.1210776271@sss.pgh.pa.us>
<b42b73150805140921v30d24b75m6c230932c61ffdd2@mail.gmail.com>
<482B1480.1070705@esilo.com>
<482B2AC5.7000503@esilo.com>
<482ECF62.7010901@esilo.com>
<12200.1211242959@sss.pgh.pa.us>
<48323593.4060302@esilo.com>
<13642.1211251078@sss.pgh.pa.us>
<48323F94.6010201@esilo.com>
<48325386.2010506@esilo.com>
<20080903001920.GT12610@alvh.no-ip.org>
Alvaro Herrera wrote:
>
> There's one thing that seems a bit baroque, which is the
> PG_COPYRES_USE_ATTRS stuff in PQcopyResult. I think that flag
> introduces different enough behavior that it should be a routine of its
> own, say PQcopyResultAttrs. That way you would leave out the two extra
> params in PQcopyResult.
>
> Oh -- one last thing. I am not really sure about the flags to
> PQcopyResult. Should there really be flags to _remove_ behavior,
> instead of flags that add? i.e. instead of having "0" copy everything,
> and have to pass flags for things not to copy, wouldn't it be cleaner to
> have 0 copy only base stuff, and require flags to copy extra things?
>
> a "name" is attached to every event proc, so that it can be
> reported in error messages
>
Can someone confirm that an event 'name' should be re-introduced, as
suggested by Alvaro?
Can I get a happy or sad face in regards to below?
New options which add instead of remove.
#define PG_COPYRES_ATTRS 0x01
#define PG_COPYRES_TUPLES 0x02 /* Implies PG_COPYRES_ATTRS */
#define PG_COPYRES_EVENTS 0x04
#define PG_COPYRES_NOTICEHOOKS 0x08
// tuples implies attrs, you need the attrs to copy the tuples.
if(options & PG_COPYRES_TUPLES)
options |= PG_COPYRES_ATTRS; // auto set option
In regards to copying the attrs, the PQcopyResult still needs the
ability to copy the source result's attrs. Although, it doesn't need
the ability to provide custom attrs (which I removed). New prototype
for copyresult:
PGresult *
PQcopyResult(const PGresult *src, int options);
I then added a PQsetResultAttrs. copyresultattrs didn't seem like the
correct name because you are no longer copying attrs from a source
result. You are providing the attrs to 'set'.
int
PQsetResultAttrs(PGresult *res, int numAttributes,
PGresAttDesc *attDescs);
If the result provided to setattrs already contains attributes, I have
the function failing (can't overwrite existing attrs). I think this is
good behavior....
When PQcopyResult needs to copy the source result's attrs, it calls
PQsetResultAttrs.
/* Wants attrs */
if((options & PG_COPYRES_ATTRS) &&
!PQsetResultAttrs(dest, src->numAttributes, src->attDescs))
{
PQclear(dest);
return NULL;
}
So, there is some nice code reuse which indicates to me the code is
segmented well (copyres & setattrs).
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-patches@postgresql.org
Cc: ac@esilo.com, alvherre@commandprompt.com, mmoncure@gmail.com, pgsql-hackers@postgresql.org
Subject: Re: libpq object hooks (libpq events)
In-Reply-To: <48BE9AA0.1070606@esilo.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox