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

Reply via email to