Wednesday, September 3, 2008

Re: [HACKERS] libpq object hooks (libpq events)

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/


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

No comments: