Re: libpq object hooks (libpq events)

From: Andrew Chernow <ac(at)esilo(dot)com>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Merlin Moncure <mmoncure(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: libpq object hooks (libpq events)
Date: 2008-09-03 14:09:36
Message-ID: 48BE9AA0.1070606@esilo.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

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/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joshua D. Drake 2008-09-03 14:52:38 Re: [PATCH] Cleanup of GUC units code
Previous Message Hitoshi Harada 2008-09-03 13:33:39 Re: Window functions patch v04 for the September commit fest

Browse pgsql-patches by date

  From Date Subject
Next Message Andrew Chernow 2008-09-03 16:26:43 libpq events patch
Previous Message Brendan Jurd 2008-09-03 07:15:36 pg_typeof() (was: Mysterious Bus Error with get_fn_expr_argtype())