Re: [PATCHES] libpq object hooks (libpq events)
On Mon, May 19, 2008 at 8:22 PM, Tom Lane [EMAIL PROTECTED] wrote: Andrew Chernow [EMAIL PROTECTED] writes: Here is an updated patch for what was called object hooks. This is now called libpq events. If someone has a better name or hates ours, let us know. This is starting to get there, Interesting you haven't commented on two areas, the actual placement of the event callers in the libpq code (i'm assuming these are ok), and the PQcopyResult/PQsetvalue functions. The latter area introduces some new behavior into standard libpq (imo) so it deserves some scrutiny. merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks (libpq events)
On Mon, May 19, 2008 at 10:37 PM, Tom Lane [EMAIL PROTECTED] wrote: Andrew Chernow [EMAIL PROTECTED] writes: 4. add a setter for result instance data - There should also be a PQsetInstanceData(PGconn*, ...) - I see no need for a passThrough setter Check, though I assume we're not expecting PQsetInstanceData to propagate to previously created PGresults? 5. move callback stuff to its own header, maybe pgcallback.h? Should be libpq-something. I was considering libpq-hooks.h or libpq-events.h, but libpq-callback.h would be OK too. I think 'events' best describes what is going on. Your other suggestions are improvements (we maybe tried a little too hard to minimize exports). Losing the global hooks is no problem (it was basically syntax sugar). merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks (libpq events)
On Sat, May 17, 2008 at 8:28 AM, Andrew Chernow [EMAIL PROTECTED] wrote: Here is an updated patch for what was called object hooks. This is now called libpq events. If someone has a better name or hates ours, let us know. Let's decide where to go with this. We have no objections to pushing this back to the next fest (tom's comments on the wiki were noted). If that's the case though, we would like to wrap up a consenus on the general implementation in the meantime. Just give us a heads up and I'll update the wiki. merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] WITH RECURSIVE patch V0.1
On Sun, May 18, 2008 at 5:22 PM, Zoltan Boszormenyi [EMAIL PROTECTED] wrote: Can we get the rows in tree order, please? I.e. something like this: Is ordering by tree order defined in the standard when no explicit order is given? If not, it probably returns them in the order they are pulled up, which might be the fastest way. merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
On Thu, May 15, 2008 at 8:38 PM, Andrew Chernow [EMAIL PROTECTED] wrote: We need to add members to a conn and result, that's pretty much it. To do this, an api user can register callbacks to receive notifications about created/destroyed states of objects. PQhookData is just like PQerrorMessage in that both are public accessor functions to private object data. The difference is that there can be more than one hookData dynamic struct member on a conn/result at a time, unlike errorMessage; thus the need for an additional lookup value when getting hook data (what was hookName). typedef void *(*PGobjectEventProc)(PGobjectEventId evtId, ...); int PQregisterObjectEventProc(PGconn*, PGobjectEventProc); void *PQeventData(PGconn *, PGobjectEventProc); void *PQresultEventData(PGresult *, PGobjectEventProc); This provides what we need...a key to lookup the hook data without using a string. Also, it reduces the number of exports (it's a little easier for us, while not essential, to not have to register each callback individually). Also, this AFAICT does not have any ABI issues (no struct), and adds less exports which is nice. We don't have to 'look up' the data inside the callbacks..it's properly passed through as an argument. While vararg callbacks may be considered unsafe in some scenarios, I think it's a good fit here. The most important part though is that it fits what we think is needed to maintain the data we associate with the libpq with the proper lifetime. I'm not sure that everyone was on the same page in terms of how this works...we may not have explained ourselves properly. In our defense, the interaction between libpq and a wrapping library like libpqtypes is a bit involved and the 'best possible' way to link things up did not necessarily suggest itself the first time out. We would like to wrap this up into some form the community would accept. The event proc style of doing this is better than our initial approach...faster and cleaner. In fact, we are pleased with all the changes that have come about due to community suggestions...there are many positive results. merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
On Fri, May 16, 2008 at 10:59 AM, Tom Lane [EMAIL PROTECTED] wrote: Merlin Moncure [EMAIL PROTECTED] writes: typedef void *(*PGobjectEventProc)(PGobjectEventId evtId, ...); int PQregisterObjectEventProc(PGconn*, PGobjectEventProc); void *PQeventData(PGconn *, PGobjectEventProc); void *PQresultEventData(PGresult *, PGobjectEventProc); This provides what we need...a key to lookup the hook data without using a string. Also, it reduces the number of exports (it's a little easier for us, while not essential, to not have to register each callback individually). Also, this AFAICT does not have any ABI issues (no struct), and adds less exports which is nice. We don't have to 'look up' the data inside the callbacks..it's properly passed through as an argument. While vararg callbacks may be considered unsafe in some scenarios, I think it's a good fit here. I don't think varargs callbacks are a good idea at all. Please adjust this to something that doesn't do that. Also, haven't you forgotten the passthrough void *? We didn't...one of the functions (the init) doesn't need it so we didn't expose it to the fixed arguments...we would just va_arg it off in the other cases. Regardless, your comments below make that moot: If you really want only one callback function, perhaps what would work is typedef void (*PGeventProc) (PGeventId eventId, const void *eventInfo, void *passthrough); int PQregisterEventProc(PGconn *conn, PGeventProc proc, void *passthrough); where eventInfo will point to one of several exported struct types depending on the value of eventId. With this approach, you can add more fields to the end of any one such struct type without creating ABI issues. I have little confidence in being able to make similar changes in a varargs environment. this is fine. Also, even if varargs are safe they'd be notationally unpleasant in the extreme. varargs are just a PITA to work with --- you'd have to do all the decoding in the first-level hook routine, even for items you weren't going to use. With something like the above all you need is a switch() and some pointer casts. Switch, plus struct (basically a union) will do the trick nicely. Can it be a formal union, or is it better as a void*? The main issue was how what we called the 'hook data' was passed back and forth. We'll get a patch up. merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
On Fri, May 16, 2008 at 11:21 AM, Andrew Dunstan [EMAIL PROTECTED] wrote: All of this is getting quite a long way from what was in the commitfest queue. Do we still want to try to get this in this cycle, or should it be marked returned to author for more work? That's your call...we can have a patch up within 24 hours or so. My suggestion would be to let us put up an updated version in the May queue, and if there are any further serious objections we can push it to the next queue giving some more time for things to percolate. merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
On Fri, May 16, 2008 at 2:34 PM, Andrew Chernow [EMAIL PROTECTED] wrote:
Tom Lane wrote:
typedef void (*PGeventProc) (PGeventId eventId, const void *eventInfo,
void *passthrough);
int PQregisterEventProc(PGconn *conn, PGeventProc proc, void
*passthrough);
The above prototypes will work and we will add our 'event instance pointer'
to the event info structures. Should have a patch shortly.
Right. I actually overlooked the 'passthrough' in
PQregisterEventProc. It turns out that we are still not quite on the
same page and this needs to be clarified before we move on. The
passthrough cannot exist...the correct prototypes (reasoning will
follow) are:
typedef void (*PGeventProc) (PGeventId eventId, const void *eventInfo);
int PQregisterEventProc(PGconn *conn, PGeventProc proc);
PQhookData(const PGconn* conn, PGeventProc proc);
...etc.
The purpose of the callbacks is to allow a hooking library to
establish private data that is associated with a PGconn or a PGresult.
Invoking PQregisterEventProc tells libpq 'I am interested in doing
this', for the supplied PGconn, future results created with that
connection, and (if PGconn is passed as null), all future connections.
Once that is established, libpq begins telling the hooking library
when and what needs to be allocated or deleted. This is done for both
connections and results at the appropriate times and the management
always occurs inside a callback function. Thus, while the hooking
library controls what is in the state data, _it does not control when
it is created or destroyed_. Passing a void* into PQregisterEventProc
suggests that is the wrapper's purvue to establish the private
information. It isn't, and it can't be..it's as simple as that.
For example, consider that 'hooked' PGconn then produces a result. If
a passthrough as Tom suggested were passed around, what would you do
to it when the connection was freed (which throws a connection closing
event)? What exactly is passthrough pointing to in a result copy
event? If passed around as a single poitner, they are not
defined.These are pointers to different structures who are created at
particular times that only libpq knows.
All of the callbacks (except for possibly conn reset) are basically
allocation and deletion events. For example, result create is libpq
telling the hooking library: 'a result is coming, please initialize
your result extensions now'. At that point, a very particular
structure is passed back to the hooking library:
{
const PGconn* conn; // the source conn the result
const void *conn_state; // the conn's private state (it might be
interesting, and we don't want to look it up later)
const PGresult* res; // the new result's poiter
void *out_res_state; // we are going to allocate something and set
this in the callback
}
As you can see, there is more going on here than a single passthrough
pointer can handle. We are in fact doing what a passthrough provides
in principle. I can certainly understand why our original 'lookup by
name' concept threw up a smokescreen (it did obfuscate the passing
requirements), but hooking can't be serviced by a single pointer set
at callback registration.
With the above prototypes and carefully designed structures,
everything feels natural. libpq is defining exactly what is supposed
to happen, and the structure parameters define what is coming in and
out. Things are being 'passed through'...but in a more discrete way.
merlin
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
On Fri, May 16, 2008 at 3:49 PM, Merlin Moncure [EMAIL PROTECTED] wrote: On Fri, May 16, 2008 at 2:34 PM, Andrew Chernow [EMAIL PROTECTED] wrote: Tom Lane wrote: typedef void (*PGeventProc) (PGeventId eventId, const void *eventInfo, void *passthrough); int PQregisterEventProc(PGconn *conn, PGeventProc proc, void *passthrough); The above prototypes will work and we will add our 'event instance pointer' to the event info structures. Should have a patch shortly. Right. I actually overlooked the 'passthrough' in PQregisterEventProc. It turns out that we are still not quite on the same page and this needs to be clarified before we move on. The passthrough cannot exist...the correct prototypes (reasoning will follow) are: typedef void (*PGeventProc) (PGeventId eventId, const void *eventInfo); small typo: eventInfo obviously can't be const int PQregisterEventProc(PGconn *conn, PGeventProc proc); PQhookData(const PGconn* conn, PGeventProc proc); PQhookData is the old name...we are going with 'events' nowthe proper names will come with the patch. merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
On Fri, May 16, 2008 at 4:23 PM, Tom Lane [EMAIL PROTECTED] wrote: Merlin Moncure [EMAIL PROTECTED] writes: Right. I actually overlooked the 'passthrough' in PQregisterEventProc. It turns out that we are still not quite on the same page and this needs to be clarified before we move on. The passthrough cannot exist... Yes, it *will* exist. You are assuming that the goals you have for these hooks are the only ones anyone will ever have. There are other possible usage patterns and many of them will need some passthrough state data. I'd venture to say that anytime I've ever used a callback design that did not include a passthrough, I've had reason to curse its designer sooner or later (qsort being a pretty typical example). right. It can exist, just not hold the event data (the extended properties of PGresult/conn). It has a meaning and scope that are not defined for libpq purposes. For the 'result' callbacks (see below), we will just use the passthrough passed in for the connection. libpqtypes will not use this, and we were getting nervous about people understanding what the different pointers were used for (we would call it a user pointer, not a pass through). However, it's clear that we are not on the same page, because what I thought you wanted the PQeventData function to return was the passthrough pointer. Evidently that's not what you want it to do, so you'd better back up a few steps and say what you do want it to do. (I think that a function to get the passthrough value associated with a given hook function might be useful too, but it's clear that what We will do this: void *PQeventData(PGconn*, PGeventProc, void **passthrough); void *PQresultEventData(PGresult*, PGeventProc, void **passthrough); you are after must be something else.) The major challenge is that libpqtypes (and, presumably, other libraries) must essentially store its own data in both conn and result. We allocate our data when these structures are created and free it when they are destroyed. The idea is that libpq fires callbacks at appropriate times when conn/results are constructed/destructed. From these events we set up our private 'event' data and delete it. Each connection and result maintains a 'list' of private event states, one for each registering callback (libpqtypes only requires one). So, registering an event proc is analogous to adding one or fields to a conn or a result with a private meaning. So, libpq is defining 6 events: *) when a connection is created (put into OK status) *) when a connection is reset *) when a connection is destroyed *) when a result is created in non-error state *) when a result is copied (we are adding this to libpq w/PQcopyResult) *) when a result is destroyed In these events we set up or tear down the private state for the libpq objects. We need to sometimes look up the data from outside the library in public (non callback) code. This is what PQeventData, etc. accomplish...provide the private state for the object. The difference is that there can be more than one registered event proc on a conn/result at a time; thus the need for an additional lookup value when getting event data (what was hookName and is now the event proc address). The purpose of the callbacks is to allow a hooking library to establish private data that is associated with a PGconn or a PGresult. So you're imagining that on creation of a PGconn or PGresult, the hook function would be allowed to create some storage and libpq would then be responsible for tracking that storage? Okay, but that's a separate feature from the sort of passthrough I had in mind. Where exactly does the hook hand off the storage pointer to libpq? What are you going to do if the hook fails to create the storage (ie, out of memory during PGresult creation)? Right, this could happen for one of two likely reasons: OOM as you suggest or the callback fails for some reason only known to the hooking library. In either case, the callback would set the return pointer as defined by the structure to null, return FALSE, and libpq would not add the state to the array of 'event state datas' it maintains. Here is the event proc with passthrough added: int PGEventProc(PGEventId event, void *eventInfo, void *passThrough); // FALSE = failure Invoking PQregisterEventProc tells libpq 'I am interested in doing this', for the supplied PGconn, future results created with that connection, and (if PGconn is passed as null), all future connections. I am entirely serious about saying that I will not accept that last bit. To do that we have to buy into having permanent modifiable storage within libpq, protecting it with thread mutexes, etc etc. And I don't like any of the possible usage scenarios for it. I think hooking into a PGconn immediately after its creation is just as useful and a lot easier to manage. Locking is not required so long as you follow certain conventions. Here is our warning
Re: [PATCHES] libpq object hooks
On Wed, May 14, 2008 at 10:44 AM, Tom Lane [EMAIL PROTECTED] wrote: I'm wondering why the hooks need names at all. AFAICS all that libpq needs to know about a hook is a callback function address and a void * passthrough pointer. I'm trying to make this work as you suggest. It's pretty clear that all the callbacks should be modified to send a void* along with the other arguments. This would eliminate the need for hookname there (getting the state inside callback functions). The problem is the functions PQhookData(conn, hookname) and PQresultHookData(result, hookName). We need these to work in functions that are not callbacks. If we eliminate hookname completely, there is no way for libpq to know which private state we are asking for. I'm a little bit stuck here. Is it possible to preserve the hookname outside of the context of the callback functions or is there something else I'm missing? (I'm sorry if I'm being obtuse) Eliminating the need for libpqtypes to need PQhookx functions, while possible, would make libpqtypes a lot more difficult to use and generally more awkward. merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
On Tue, May 13, 2008 at 11:52 PM, Bruce Momjian [EMAIL PROTECTED] wrote: My personal opinion is still that I would like to see a more general usefulness for these functions before adding them to libpq. The complexity of the API just mirrors my gut feeling on this. There has been a lot of demand for the features that libpqtypes provides...a quick search of the archives demonstrates this. Here are a few examples of threads from users asking or even trying to implement some of the things that libpqtypes helps with or indirectly solves...I am speaking to your point of usefulness here. [BUGS] BUG #4053: libpq documentation should express clearly, that integers are passed in network octet order [PATCHES] [PATCH] automatic integer conversion [HACKERS] convert int to bytea [HACKERS] comunication protocol [HACKERS] PQescapeBytea* version for parameters [HACKERS] libpq and Binary Data Formats [GENERAL] binary representation of date and numeric [GENERAL] binding 64-bit integer [HACKERS] Last minute mini-proposal (I know, I know) for PQexecf() [PERFORM] Low throughput of binary inserts from windows to linux [GENERAL] Storing images as BYTEA or large objects merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
On Wed, May 14, 2008 at 10:44 AM, Tom Lane [EMAIL PROTECTED] wrote: No, they could revise their patch to be more stylistically in keeping with libpq. I haven't looked at the current version of the patch yet, but the early versions seemed quite overengineered to me, so your criticism didn't surprise me. I think you'll find the latest version more reasonable. We tried to keep the over-engineering, so to speak, on our side and make the libpq changes surgical. I'm wondering why the hooks need names at all. AFAICS all that libpq needs to know about a hook is a callback function address and a void * passthrough pointer. Here are the proposed API changes [aside: this is one of two breaks from your initial suggestions..the other being PQcopyResult]: + PQcopyResult 142 + PQsetvalue143 + PQresultAlloc 144 + PQaddObjectHooks 145 + PQaddGlobalObjectHooks146 + PQhookData147 + PQresultHookData 148 In question is: + void * + PQhookData(const PGconn *conn, const char *hookName) Basically, libpqtypes has various functions that take a PGconn that need the private data that is stored in libpq with the connection. PQhookData just does simple linear search and returns the data. [thinks] are you suggesting something like + void * + PQhookData(const PGconn *conn, const void *hookHandle) ? I would have to take a quick look at the code with Andrew C (he'll be in in a bit)...but this might be doable. merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
On Wed, May 14, 2008 at 10:52 AM, Bruce Momjian [EMAIL PROTECTED] wrote: Merlin Moncure wrote: Regarding the other comments: *) API structure: Our major objective was to minimize exports to libpq. I think both copyresult and setvalue have some possible sideband usage (footguns or no). Additional functions could be speculated but are not required by libpqtypes. We would have no problem adding a few things to complete the api if necessary. The patch is basically the minimum libpqtypes needs and has to work more or less as written. We tried a few times to suggest implementing the split a different way (basically, more invasion into libpq). We couldn't get any action there. If the patch is rejected on general merits...that signals the death blow for libpqtypes. We have a chicken/egg problem...people can't use it without patching libpq which will really hamper its adoption, which is, uh, needed to justify the patch. For the record, we have had a couple of dozen downloads of the libpqtypes library on pgfoundry since we put it up there last week. Based on how it has simplified and improved our own code vs. libpq, we have absolutely no doubts it is a major improvement over PQexecParams. One idea would be to add the libpq hooks but not document them. This way, we can modify or remove the API as needed in the future. As libpqtypes matures and we are sure what the API should be, we can document it as stable and permanent. The API functions relating to hooks are unlikely to change once settled on...but PQsetvalue/PQcopyResult are a good fit with your idea (they introduce new behaviors that could possibly be used outside of libpqtypes context, and could require changes down the line). merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
On Wed, May 14, 2008 at 10:44 AM, Tom Lane [EMAIL PROTECTED] wrote: I'm wondering why the hooks need names at all. AFAICS all that libpq needs to know about a hook is a callback function address and a void * passthrough pointer. For reference...here is what libpqtypes has to do to bind via the hooking interface: http://libpqtypes.esilo.com/browse_source.html?file=hooks.c Are you proposing something substantially different (not my handle suggestion)? How would it work exactly? merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks
On Wed, May 14, 2008 at 3:47 PM, daveg [EMAIL PROTECTED] wrote: On Wed, May 14, 2008 at 10:52:43AM -0400, Bruce Momjian wrote: One idea would be to add the libpq hooks but not document them. This way, we can modify or remove the API as needed in the future. As libpqtypes matures and we are sure what the API should be, we can document it as stable and permanent. Perhaps it is just me, but undocumented interface are evil. Simply document it with the changable bits labled as such. Well, in defense of Bruce, there is some precedent for that. Anything that queries binary currently falls under that umbrella (mostly undocumented and changeable). For functions exported by libpq though...it's probably better to nail things down as much as possible up front. This is a big advantage of the hooks strategy overall, it allows us to mature the library except for the interaction with libpq (ISTM Bruce was trying to give us a little leeway here as well, and we appreciate that). merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] libpq object hooks
Attached is an updated version of 'libpq object hooks'. The primary
purpose for libpq object hooks is to support our libpqtypes system
(not attached), but could possibly some nice sideband features to
libpq. We are hoping to sneak this into the May commit fest.
regards,
merlin
Index: exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** exports.txt 19 Mar 2008 00:39:33 - 1.19
--- exports.txt 30 Apr 2008 20:05:25 -
***
*** 138,143
--- 138,150
PQsendDescribePortal 136
lo_truncate 137
PQconnectionUsedPassword 138
pg_valid_server_encoding_id 139
PQconnectionNeedsPassword 140
lo_import_with_oid 141
+ PQcopyResult 142
+ PQsetvalue143
+ PQresultAlloc 144
+ PQaddObjectHooks 145
+ PQaddGlobalObjectHooks146
+ PQhookData147
+ PQresultHookData 148
\ No newline at end of file
Index: fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -C6 -r1.357 fe-connect.c
*** fe-connect.c 31 Mar 2008 02:43:14 - 1.357
--- fe-connect.c 30 Apr 2008 20:05:25 -
***
*** 241,253
PQExpBuffer errorMessage);
static char *pwdfMatchesString(char *buf, char *token);
static char *PasswordFromFile(char *hostname, char *port, char *dbname,
char *username);
static void default_threadlock(int acquire);
-
/* global variable because fe-auth.c needs to access it */
pgthreadlock_t pg_g_threadlock = default_threadlock;
/*
* Connecting to a Database
--- 241,252
***
*** 979,990
--- 978,990
* o If your backend wants to use Kerberos authentication then you must
* supply both a host name and a host address, otherwise this function
* may block on gethostname.
*
*
*/
+
PostgresPollingStatusType
PQconnectPoll(PGconn *conn)
{
PGresult *res;
char sebuf[256];
***
*** 998,1009
--- 998,1010
* We really shouldn't have been polled in these two cases, but we
* can handle it.
*/
case CONNECTION_BAD:
return PGRES_POLLING_FAILED;
case CONNECTION_OK:
+ pqInitObjectHooks(conn);
return PGRES_POLLING_OK;
/* These are reading states */
case CONNECTION_AWAITING_RESPONSE:
case CONNECTION_AUTH_OK:
{
***
*** 1816,1827
--- 1817,1829
conn-next_eo = EnvironmentOptions;
return PGRES_POLLING_WRITING;
}
/* Otherwise, we are open for business! */
conn-status = CONNECTION_OK;
+ pqInitObjectHooks(conn);
return PGRES_POLLING_OK;
}
case CONNECTION_SETENV:
/*
***
*** 1848,1859
--- 1850,1862
default:
goto error_return;
}
/* We are open for business! */
conn-status = CONNECTION_OK;
+ pqInitObjectHooks(conn);
return PGRES_POLLING_OK;
default:
printfPQExpBuffer(conn-errorMessage,
libpq_gettext(
invalid connection state %c,
***
*** 1875,1887
* the connection structure must be freed).
*/
conn-status = CONNECTION_BAD;
return PGRES_POLLING_FAILED;
}
-
/*
* makeEmptyPGconn
* - create a PGconn data structure with (as yet) no interesting data
*/
static PGconn *
makeEmptyPGconn(void)
--- 1878,1889
***
*** 1970,1981
--- 1972,2001
* release data that is to be held for the life of the PGconn structure.
* If a value ought to be cleared/freed during PQreset(), do it there not here.
*/
static void
freePGconn(PGconn *conn)
{
+ int i;
+
+ for(i=0; i conn-objHooksCount; i++)
+ {
+ if(conn-objHooks[i].connDestroy)
+ {
+ conn-objHooks[i].connDestroy((const PGconn *)conn);
+ free(conn-objHooks[i].name);
+ }
+ }
+
+ if(conn-objHooks)
+ {
+ free(conn-objHooks);
+ conn-objHooks = NULL;
+ conn-objHooksCount = conn-objHooksSize = 0;
+ }
+
if (conn-pghost)
free(conn-pghost);
if (conn-pghostaddr)
free(conn-pghostaddr);
if (conn-pgport)
free(conn-pgport);
***
*** 2151,2164
PQreset(PGconn *conn)
{
if (conn)
{
closePGconn(conn);
! if (connectDBStart(conn))
! (void) connectDBComplete(conn);
}
}
/*
* PQresetStart:
--- 2171,2189
PQreset(PGconn *conn)
{
if (conn)
{
closePGconn(conn);
! if (connectDBStart(conn) connectDBComplete(conn))
! {
! int i;
! for(i=0; i conn-objHooksCount; i++)
! if(conn-objHooks[i].connReset)
! conn-objHooks[i].connReset((const PGconn *)conn);
! }
}
}
/*
* PQresetStart:
***
*** 2176,2198
return
[PATCHES] PQmakeResult patch
Here is a patch to add a few functions to libpq:
PQmakeResult: creates a new result with attributes but no rows
PQsetValue: set a field inside a result, creating a row if necessary
(but only one at a time)
PQresultAlloc (basically public wrapper to existing internal function).
also, the attribute structure (but not the result) is moved to the
public header.
We thought these functions, particularly PQsetValue, are something
positive that came out of the libpqtypes dicussions.
merlin
Index: interfaces/libpq/exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** interfaces/libpq/exports.txt 19 Mar 2008 00:39:33 - 1.19
--- interfaces/libpq/exports.txt 15 Apr 2008 14:22:00 -
***
*** 138,143
--- 138,146
PQsendDescribePortal 136
lo_truncate 137
PQconnectionUsedPassword 138
pg_valid_server_encoding_id 139
PQconnectionNeedsPassword 140
lo_import_with_oid 141
+ PQmakeResult 142
+ PQsetvalue143
+ PQresultAlloc 144
Index: interfaces/libpq/fe-exec.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.194
diff -C6 -r1.194 fe-exec.c
*** interfaces/libpq/fe-exec.c 1 Jan 2008 19:46:00 - 1.194
--- interfaces/libpq/fe-exec.c 15 Apr 2008 14:22:00 -
***
*** 60,72
int resultFormat);
static void parseInput(PGconn *conn);
static bool PQexecStart(PGconn *conn);
static PGresult *PQexecFinish(PGconn *conn);
static int PQsendDescribe(PGconn *conn, char desc_type,
const char *desc_target);
!
/*
* Space management for PGresult.
*
* Formerly, libpq did a separate malloc() for each field of each tuple
* returned by a query. This was remarkably expensive --- malloc/free
--- 60,73
int resultFormat);
static void parseInput(PGconn *conn);
static bool PQexecStart(PGconn *conn);
static PGresult *PQexecFinish(PGconn *conn);
static int PQsendDescribe(PGconn *conn, char desc_type,
const char *desc_target);
! static int check_field_number(const PGresult *res,
! int field_num);
/*
* Space management for PGresult.
*
* Formerly, libpq did a separate malloc() for each field of each tuple
* returned by a query. This was remarkably expensive --- malloc/free
***
*** 192,203
--- 193,338
result-client_encoding = PG_SQL_ASCII;
}
return result;
}
+ PGresult *
+ PQmakeResult(const PGconn *conn, int numAttributes,
+ PGresAttDesc *attDescs)
+ {
+ int i;
+ PGresult *res;
+
+ if(numAttributes = 0 || !attDescs)
+ return NULL;
+
+ res = PQmakeEmptyPGresult((PGconn *)conn, PGRES_TUPLES_OK);
+ if(!res)
+ return NULL;
+
+ res-attDescs = (PGresAttDesc *)
+ pqResultAlloc(res, numAttributes * sizeof(PGresAttDesc), TRUE);
+
+ if(!res-attDescs)
+ {
+ PQclear(res);
+ return NULL;
+ }
+
+ res-numAttributes = numAttributes;
+ memcpy(res-attDescs, attDescs, numAttributes * sizeof(PGresAttDesc));
+
+ /* resultalloc the attribute names. */
+ res-binary = 1;
+ for(i=0; i numAttributes; i++)
+ {
+ if(attDescs[i].name)
+ res-attDescs[i].name = pqResultStrdup(res, attDescs[i].name);
+ else
+ res-attDescs[i].name = res-null_field;
+
+ if(!res-attDescs[i].name)
+ {
+ PQclear(res);
+ return NULL;
+ }
+
+ /* Although deprecated, because results can have text+binary columns,
+ * its easy enough to deduce so set it for completeness.
+ */
+ if(res-attDescs[i].format == 0)
+ res-binary = 0;
+ }
+
+ return res;
+ }
+
+ int
+ PQsetvalue(PGresult *res, int tup_num, int field_num,
+ char *value, int len)
+ {
+ PGresAttValue *attval;
+
+ if(!check_field_number(res, field_num))
+ return FALSE;
+
+ /* Invalid tup_num, must be = ntups */
+ if(tup_num res-ntups)
+ return FALSE;
+
+ /* need to grow the tuple table */
+ if(res-ntups = res-tupArrSize)
+ {
+ int n = res-tupArrSize ? (res-tupArrSize*3)/2 : 64;
+ PGresAttValue **tups = (PGresAttValue **)
+ (res-tuples ? realloc(res-tuples, n*sizeof(PGresAttValue *)) :
+ malloc(n*sizeof(PGresAttValue *)));
+
+ if(!tups)
+ return FALSE;
+
+ res-tuples = tups;
+ res-tupArrSize = n;
+ }
+
+ /* new to allocate a new tuple */
+ if(tup_num == res-ntups !res-tuples[tup_num])
+ {
+ int i;
+ PGresAttValue *tup = (PGresAttValue *)pqResultAlloc(
+ res, res-numAttributes * sizeof(PGresAttValue), TRUE);
+
+ if(!tup)
+ return FALSE;
+
+ /* initialize each column to NULL */
+ for(i=0; i res-numAttributes; i++)
+ {
+ tup[i].len = NULL_LEN;
+ tup[i].value = res-null_field;
+ }
+
+ res-tuples[tup_num] = tup;
+ res-ntups++;
+ }
+
+ attval = res-tuples[tup_num][field_num];
+
+
[PATCHES] libpq hooks patch v2
Here is an updated version of the 'libpq-side' changes required to
implement the libpqtypes proposal. This fixes the complaints Tom had,
particularly the process lock on the various functions that kept
private state linked up with the PGconn. We did this in a slightly
different way than Tom proposed...by keeping a void* in the
PGobjectHooks struct.
While we think this approach works, it is possibly to complicated for
what it does. This could possibly be said of the hooking approach in
general. We can't think of a good use case for other applications to
use the hooks. The hook system feels awkward on both sides (in libpq,
and libpqtypes). Why keep code abstract when it only has one real
purpose?
Possibly, a better approach than the hooking system is a wholesale
wrapping of libpq, introducing a extended PGconn struct (PQTconn for
example), with the private state stored in there and passed to all the
libpqtypes functions. This would mean a complete new API, something
we wanted to avoid. We see our proposal as a natural extension of
libpq, library code, not application code. This difference of
perception is perhaps what the impasse is.
We think our other proposed strategy is better...namely to load the
libpqtypes functionality with dlopen and keep 'stub' functions inside
libpq which raise a runtime error if the library is not loaded. This
resolves a major raised complaint, library size, and allows the stub
functions to better handle certain things that should be in libpq, for
example error handling.
We understand this raises the bar for libpqtypes in terms of
consensus...since it pushes certain aspects of libpqtypes into libpq,
the whole thing has to be 'signed off' on, and there has naturally
been a certain amount of resistance so far. At this point, function
stubs (PQparamExec and friends) are all we are proposing to add to
libpq. The remainder of the code can exist in core, pgfoundry, etc.
This is much more direct and useful to libpq developers than a
abstract hooking interface.
merlin
Index: exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.19
diff -C6 -r1.19 exports.txt
*** exports.txt 19 Mar 2008 00:39:33 - 1.19
--- exports.txt 14 Apr 2008 14:27:10 -
***
*** 138,143
--- 138,149
PQsendDescribePortal 136
lo_truncate 137
PQconnectionUsedPassword 138
pg_valid_server_encoding_id 139
PQconnectionNeedsPassword 140
lo_import_with_oid 141
+ PQsetObjectHooks 142
+ PQhookData143
+ PQresultHookData 144
+ PQmakeResult 145
+ PQsetvalue146
+ PQresultAlloc 147
\ No newline at end of file
Index: fe-connect.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -C6 -r1.357 fe-connect.c
*** fe-connect.c 31 Mar 2008 02:43:14 - 1.357
--- fe-connect.c 14 Apr 2008 14:27:11 -
***
*** 1970,1981
--- 1970,1984
* release data that is to be held for the life of the PGconn structure.
* If a value ought to be cleared/freed during PQreset(), do it there not here.
*/
static void
freePGconn(PGconn *conn)
{
+ if(conn-objHooks.connDestroy)
+ conn-objHooks.connDestroy((const PGconn *)conn);
+
if (conn-pghost)
free(conn-pghost);
if (conn-pghostaddr)
free(conn-pghostaddr);
if (conn-pgport)
free(conn-pgport);
***
*** 2152,2164
--- 2155,2171
{
if (conn)
{
closePGconn(conn);
if (connectDBStart(conn))
+ {
(void) connectDBComplete(conn);
+ if(conn-objHooks.connReset)
+ conn-objHooks.connReset((const PGconn *)conn);
+ }
}
}
/*
* PQresetStart:
***
*** 2186,2198
* closes the existing connection and makes a new one
*/
PostgresPollingStatusType
PQresetPoll(PGconn *conn)
{
if (conn)
! return PQconnectPoll(conn);
return PGRES_POLLING_FAILED;
}
/*
* PQcancelGet: get a PGcancel structure corresponding to a connection.
--- 2193,2215
* closes the existing connection and makes a new one
*/
PostgresPollingStatusType
PQresetPoll(PGconn *conn)
{
if (conn)
! {
! PostgresPollingStatusType status = PQconnectPoll(conn);
!
! if((status == PGRES_POLLING_OK || status == PGRES_POLLING_FAILED)
! conn-objHooks.connReset)
! {
! conn-objHooks.connReset((const PGconn *)conn);
! }
!
! return status;
! }
return PGRES_POLLING_FAILED;
}
/*
* PQcancelGet: get a PGcancel structure corresponding to a connection.
Index: fe-exec.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.194
diff -C6 -r1.194 fe-exec.c
*** fe-exec.c 1 Jan 2008 19:46:00
Re: [PATCHES] libpq patch for pqtypes hook api and PGresult creation
On Fri, Apr 11, 2008 at 1:47 PM, Andrew Chernow [EMAIL PROTECTED] wrote: Here are the changes to libpq. It adds the ability to register an Object Hook and create a home-grown result. Patch adds 4 functions. We changed the name of PQresultSetFieldValue to PQsetvalue, which better compliments PQgetvalue. If this patch is acceptable, we will move on to making the required changes to pqtypes; some work has already been done. Whoops! One missing thing here...we forgot to make pqResultAlloc pubilc...pqResultAlloc - PQresultAlloc (see discussion in -hackers). Also, we could use pqResultStrdup (or keep it private, in which case we can re-implement in libpqtypes). merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq Win32 Mutex performance patch
On Fri, Apr 11, 2008 at 2:49 PM, Magnus Hagander [EMAIL PROTECTED] wrote: Andrew Chernow wrote: I noticed several months ago, and came across it again today, that libpq's pthread-win32.c implementation is using CreateMutex rather than CRITICAL_SECTION. CreateMutex is like a semaphore in that it is designed to be accessible via name system-wide. Even when you don't give it a name, thus bound to process that created it, it still carries significant overhead compared to using win32 CRITICAL_SECTIONs. The attached patch replaces the win32 mutex calls with critical section calls. The change will not affect the behavior of the windows pthread_xxx functions. First of all, I like this in general :-) But a couple of comments. It changes the behavior when the pointer passed in is invalid from crash to silent working, right? This shouldn't actually matter, since these functions are only ever supposed to run from callers *inside libpq*, so it probalby doesn't matter... I noticed you conjured up a ecpg threading patch sometime around early 2007. You used a mutex there deliberately because that's what libpq did. Maybe that patch should be adjusted? merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq type system 0.9a
On Fri, Apr 4, 2008 at 6:56 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: Merlin Moncure escribió: Yesterday, we notified -hackers of the latest version of the libpq type system. Just to be sure the right people are getting notified, we are posting the latest patch here as well. Would love to get some feedback on this. I had a look at this patch some days ago, and the first question in my mind was: why is it explicitely on libpq? Why not have it as a separate library (say libpqtypes)? That way, applications not using it would not need to link to it. Applications interested in using it would just need to add another -l switch to their link line. I think that is oversimplifying things a little bit. As Andrew stated there are some aspects of the type system that would not so easily abstracted out into a separate library. The type handlers them selves could be moved out...but since they are basically hardcoded in the server (for the built in types), why not do it in the client as well? The libpq type system was deliberately designed so that user types could be 'plugged in' . I think a reasonable objective would be to organize the types a little bit better in both the client and the server so there would be more code reuse. We would support this change, but it would require some changes to the server as well. OTOH, we are proposing to extend the libpq interface. IMO, breaking the libpq interface to separate libraries would only cause confusion. merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq type system 0.9a
On Tue, Mar 25, 2008 at 4:21 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: Merlin Moncure escribió: The latest version of libpq type system is available here: http://www.esilo.com/projects/postgresql/libpq/typesys-0.9a.tar.gz This patch is not in diff -c format ... please provide a diff -c patch, and add the URL to the wiki patch queue: http://wiki.postgresql.org/wiki/CommitFest:March converted to context diff (0.9b): http://www.esilo.com/projects/postgresql/libpq/typesys-0.9b.tar.gz will have wiki set up soon...need to get an account over there. merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] libpq type system 0.9a
Yesterday, we notified -hackers of the latest version of the libpq type system. Just to be sure the right people are getting notified, we are posting the latest patch here as well. Would love to get some feedback on this. The latest version of libpq type system is available here: http://www.esilo.com/projects/postgresql/libpq/typesys-0.9a.tar.gz The following modifications where made: *) documentation fixes *) parameter resets are no longer automatic *) updated to patch vs. REL8_3_STABLE Merlin Moncure Andrew Chernow eSilo -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches
Re: [PATCHES] libpq type system 0.9a
On Wed, Mar 5, 2008 at 5:47 PM, Florian G. Pflug [EMAIL PROTECTED] wrote: Merlin Moncure wrote: Yesterday, we notified -hackers of the latest version of the libpq type system. Just to be sure the right people are getting notified, we are posting the latest patch here as well. Would love to get some feedback on this. Sorry if this has been discussed before, but why is it necessary to specify the type when calling PQgetf on a result? It seems that this formatting string *always* has to match the type list of your select statement, no? yes...it always has to match. the format string requirements could in theory be relaxed (for 'get') but this would break symmetry with 'put' and you would lose a sanity check...getf like scanf writes directly into application memory so the double-specifying (directly in the format string and indirectly in the query) isn't necessarily a bad thing. imagine if your application was 'select * from table' and one of the field types changed...disaster. merlin -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches
[PATCHES] patch to disallow zero length paths in binary (minor bug fix)
Hackers, Following is a patch to force the path type not to accept a path with zero points. This appears to be illegal in the parser, but possible when sending a well formed packed in binary with zero points. The old behavior was this: ERROR: floating-point exception DETAIL: An invalid floating-point operation was signaled. This probably means an out-of-range result or an invalid operation, such as division by zero. now it should properly display the points out of range error merlin eSilo Index: src/backend/utils/adt/geo_ops.c === RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/geo_ops.c,v retrieving revision 1.97 diff -r1.97 geo_ops.c 1459c1459 if (npts 0 || npts = (int32) ((INT_MAX - offsetof(PATH, p[0])) / sizeof(Point))) --- if (npts = 0 || npts = (int32) ((INT_MAX - offsetof(PATH, p[0])) / sizeof(Point))) ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] [PATCH] automatic integer conversion
On Dec 8, 2007 6:50 PM, Tom Lane [EMAIL PROTECTED] wrote: Alvaro Herrera [EMAIL PROTECTED] writes: Please note that I'm not saying that fixing that issue means the patch is acceptable. Personally I'm not sure that this is a worthy goal you are pursuing here. Wouldn't it be a good idea to propose the feature first and write the code later? Indeed. For starters, if we are going to try to provide serious support in libpq for binary-format parameters, it probably ought to cover more than just integers. OTOH, I think we've already seen where that line of thought leads, and it looks pretty ugly too: http://archives.postgresql.org/pgsql-patches/2007-12/msg00014.php Anyway, I'd like to see a design discussion about what any libpq API changes in this area ought to look like, rather than having it be determined by who can submit the quickest-and-dirtiest patch. A major overhaul of this patch is coming...we are addressing some of the issues that were raised. I think that this is an important issue that needs to be solved. Dealing with arrays is a complete mess, and many other things are more difficult than they have to be. User defined types are problem as well, and marshaling everything into text is always a good solution. In the long term, my opinion is that postgresql types have to be converted into a more pluggable interface that is available into both the client and the server. It doesn't really make sense to reimplement the send/receive routines in both the client and the server...and the patch that we proposed (ditto the OP) did not really leave room for this in the future. That said, I strongly feel that libpq should in some fashion do a better job in handling binary data than it currently does. Andrew and I are taking this into consideration and will submit a proposal that we hope that should hopefully deal with things in a more acceptable way. We understand the ramifications of extending the libpq api. merlin ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] PQParam version 0.5
On Dec 5, 2007 2:44 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: Andrew Chernow escribió: Also changed PQputint8's prototype. Previously, it was using a void* as the value argument, due to a lack of a portable 64-bit type in libpq. We found an intersting way around this by using macro and variable argument tricks. I didn't read the patch, but variadic macros are not portable. FWIW uint64 should portable to all platforms that have it (and it should be 32 bits on platforms that don't), but you have to watch for INT64_IS_BUSTED. we don't use variadic macros...just a macro wrapper to a variadic function. merlin ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] enum types and binary queries
On 8/31/07, Tom Lane [EMAIL PROTECTED] wrote:
Andrew Dunstan [EMAIL PROTECTED] writes:
Here's a patch (minus catalog bump) which I think does that.
Looks sane in a very quick once-over, but I didn't test it.
works fine (here was my test). thanks for quick resolution to this
issue. strings returned in binary format is IMO ok.
enum.c:
include libpq-fe.h
#include string.h
#include stdlib.h
int main(int argc, char **argv)
{
PGconn *c = PQconnectdb(user=postgres);
PGresult *r;
r = PQexecParams(c, select 'foo'::foo, 0, NULL, NULL, NULL, NULL, 1);
ExecStatusType t = PQresultStatus(r);
if(t != PGRES_COMMAND_OK t != PGRES_TUPLES_OK)
{
printf(%s, PQresultErrorMessage(r));
exit(1);
}
char* f = PQgetvalue(r,0,0);
int len = 3;
int format = 1;
PQclear(r);
r = PQexecParams(c, select $1::foo, 1, NULL, (const char* const
*)f, len, format, 1);
if(t != PGRES_COMMAND_OK t != PGRES_TUPLES_OK)
{
printf(%s, PQresultErrorMessage(r));
exit(1);
}
PQfinish(c);
}
---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at
http://www.postgresql.org/about/donate
Re: [PATCHES] [HACKERS] enum types and binary queries
On 8/31/07, Merlin Moncure [EMAIL PROTECTED] wrote: On 8/31/07, Tom Lane [EMAIL PROTECTED] wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Here's a patch (minus catalog bump) which I think does that. Looks sane in a very quick once-over, but I didn't test it. works fine (here was my test). thanks for quick resolution to this issue. strings returned in binary format is IMO ok. if(t != PGRES_COMMAND_OK t != PGRES_TUPLES_OK) oops, this line was wrong. the enum is fine though. merlin ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
[PATCHES] PGparam extension version 0.3
Attached is version 0.3 of the proposed PGparam extension to the libpq
API. I think we are wrapping up our changes in the short term and
will begin using our api for our internal projects. There were a lot
of changes and reorganizations, but the big features are that client
side geometry types were introduced and we folded the PGparam struct
into PGconn which simplifies the interface in our opinion. See the
attached changes file for a complete list. Also attached is a small
test which is a good overview of how the proposed API changes work.
The code has been reorganized into a proper patch so that things are
injected into libpq in what we think are the right places along with
an updated makefile and exports.txt.
There are many things we discussed but did not implement because of
time concerns, for example client side support for binary arrays and a
queryf interface which would map input parameters into the various put
functions. These are exciting things but fairly complex features and
may require some reorganization of code on the backend to do properly.
Hopefully this will help developers who would like to use the high
performance binary interface or optimize access to the database from
their particular language. Assuming the code is acceptable to the
community, we will keep the patch up to date through the 8.4 cycle and
write the documentation.
Things are obviously really busy right now with HOT and getting 8.3
locked down...but comments and suggestions are most welcome.
merlin
pg_param.tgz
Description: GNU Zip compressed data
#if defined(_WIN32) || defined(_WIN64)
# define U64FMT %I64u
typedef unsigned __int64 myuint64_t;
#else
# define U64FMT %llu
typedef unsigned long long myuint64_t;
#endif
//#include pg.h
#include /esilo/src/pgsql/src/interfaces/libpq/libpq-fe.h
#include stdlib.h
#include string.h
#include limits.h
#ifndef TRUE
# define TRUE 1
#endif
#ifndef FALSE
# define FALSE 0
#endif
#define countof(array) (sizeof(array)/sizeof(array[0]))
#define TEST_TBLNAME param_test
static const char *create_table =
CREATE TABLE TEST_TBLNAME (
a_char\char\,
a_bool boolean,
a_int2 int2,
a_int4 int4,
a_int8 int8,
a_float4 float4,
a_float8 float8,
a_text text,
a_nulltext text,
a_byteabytea,
a_macaddr macaddr,
a_pointpoint,
a_lseg lseg,
a_box box,
a_circle circle,
a_path path,
a_polygon polygon);
static const char *insert_command =
INSERT INTO TEST_TBLNAME VALUES
($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17);
static int getresfmt(int argc, char **argv);
int main(int argc, char **argv)
{
int i;
char ch = 0;
int b = 0;
short i2 = 0;
int i4 = 0;
myuint64_t i8;
float f4 = 0;
double f8 = 0;
char *text;
int text_isnull;
unsigned char *bytea2;
unsigned char bytea[4] = {1, 10, 220, 255};
PGmacaddr mac;
PGpoint pt;
PGlseg lseg;
PGbox box;
PGcircle circle;
PGpath path;
PGpolygon polygon;
PGconn *conn;
PGresult *res;
ExecStatusType status;
PGpoint points[64];
int resultFormat = getresfmt(argc, argv);
conn = PQconnectdb(hostaddr=127.0.0.1 user=postgres);
if(PQstatus(conn) != CONNECTION_OK)
{
printf(connection failure\n);
return 1;
}
res = PQexec(conn, create_table);
PQclear(res);
/* clear test table */
res = PQexec(conn, DELETE FROM TEST_TBLNAME);
PQclear(res);
i8 = ULLONG_MAX;
PQputchar(conn, UCHAR_MAX);
PQputbool(conn, TRUE);
PQputint2(conn, USHRT_MAX);
PQputint4(conn, UINT_MAX);
PQputint8(conn, i8); /* pqlib has no 64-bit type. */
PQputfloat4(conn, 111.234f);
PQputfloat8(conn, .234567);
PQputtext(conn, This is some text);
PQputnull(conn);
PQput(conn, bytea, (int)sizeof(bytea), InvalidOid, 1);
mac.a = 1;
mac.b = 2;
mac.c = 3;
mac.d = 4;
mac.e = 5;
mac.f = 6;
PQputmacaddr(conn, mac);
pt.x = -11.23;
pt.y = 23.11;
PQputpoint(conn, pt);
lseg.pts[0].x = 6712;
lseg.pts[0].y = 2517.89;
lseg.pts[1].x = 9087.125;
lseg.pts[1].y = 7821.987;
PQputlseg(conn, lseg);
box.high.x = 100;
box.high.y = 10;
box.low.x = 10;
box.low.y = 1;
PQputbox(conn, box);
circle.center.x = 100;
circle.center.y = 200;
circle.radius = 300;
PQputcircle(conn, circle);
path.closed = 0; /* open path */
path.npts = countof(points);
path.pts = points;
for(i=0; i path.npts; i++)
{
path.pts[i].x = i;
path.pts[i].y = i+1;
}
PQputpath(conn, path);
polygon.npts = 16;
polygon.pts = points;
if(!PQputpolygon(conn, polygon))
printf(Polygon: %s\n, PQerrorMessage(conn));
/* Execute query */
PQparamExec(conn, insert_command, 1, res);
/* check result */
status = PQresultStatus(res);
if(status != PGRES_COMMAND_OK status != PGRES_TUPLES_OK)
{
printf(%s\n, res ? PQresultErrorMessage(res) : PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
return 1;
}
printf(\nGetting results in %s
Re: [PATCHES] pgparam extension to the libpq api
On 8/17/07, Merlin Moncure [EMAIL PROTECTED] wrote: Attached are some new functions that extend the libpq api to make calling the parameterized interfaces easier, especially when making binary calls. IMO, this fills one of the two big missing parts of the libpq api when making binary database calls, the other being client side handling of complex structures (arrays, etc). Here is another version of the above with a number of cleanups and fixes. PGparam version 0.2 (from the fixit.txt): While enhancing the API and refactoring to match libpq naming, some bugs were introduced. Below is a list of the fixes. Also, the list includes a few things that cleaned up the code for readability. 1. PGparam had a slab and slabsize member. This is a dead idea and was removed. PQparamClear was referencing the slab member; this has been removed as well. 2. PQparamPutString was accepting a length argument. It specified the strlen of the string to put. Initially, this API only sent params in binary format. Because of this, PQparamPutString was not including the NUL in the value. Since PQparamPutString was changed to send strings in text format, things broke because there was no NUL. The length argument has been removed and PQparamPutString properly handles text format NULL. 3. PQparamPutString has been renamed to PQparamPutText to macth postgres type. 4. PQparamPutFloat has been renamed to PQparamPutFloat4 to match postgres type. 4. PQparamPutDouble has been renamed to PQparamPutFloat8 to match postgres type. 5. PQgetfloat is a macro that did not put () around its floatp arg. was ((void*)flaotp), now its ((void*)(floatp))). 6. PQgetfloat has been renamed to PQgetfloat4 to match postgres type. 7. PQgetdouble has been renamed to PQgetfloat8 to match postgres type. 8. The PARAM_CHKPTR(p) macro was stupidly dereferencing the param ptr ... ONLY IF IT WAS NULL! Yes, lets make sure the ptr is NULL before we access it. Cute :) 9. PQgetchar text format was broken (for use with char type). Apparently, when using text format the char value is converted to A instead if 65. Makes sense, just wasn't initially aware of this. 10. PQparamPutChar, PutInt2, PutInt4, PQgetchar, getint2 and getint4 were all using unsigned types. These have been changed to signed types since postgres server leans more towards signed types. 11. param.c had a long macro named PARAM_ARRAY_ASSIGN. This has been converted to a static function named paramArrayAssign(). 12. Changes some error messages to make them more clear. 13. Comments were added throughout param.c and result.c. libpq-param.tar Description: Unix tar archive ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
[PATCHES] patch to suppress psql timing output in quiet mode
I noticed that when psql accepts input from stdin or -f (but not -c), and timing is set to on in .psqlrc, timing results are printed out to stdout even when -q (quiet) is passed in. This may not be the perfect solution, but it fixes the problem (I'm having problems with bash scripts that are borking on time returned). current behavior: [EMAIL PROTECTED] psql]# echo select 0 | psql -tAq 0 Time: 1.155 ms [EMAIL PROTECTED] psql]# psql -tAqcselect 0 0 merlin Index: common.c === RCS file: /projects/cvsroot/pgsql/src/bin/psql/common.c,v retrieving revision 1.133 diff -c -r1.133 common.c *** common.c8 Feb 2007 11:10:27 - 1.133 --- common.c11 Apr 2007 17:20:21 - *** *** 918,924 PQclear(results); /* Possible microtiming output */ ! if (OK pset.timing) printf(_(Time: %.3f ms\n), elapsed_msec); /* check for events that may occur during query execution */ --- 918,924 PQclear(results); /* Possible microtiming output */ ! if (OK pset.timing !pset.quiet) printf(_(Time: %.3f ms\n), elapsed_msec); /* check for events that may occur during query execution */ ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] \prompt for psql
On 2/9/07, Alvaro Herrera [EMAIL PROTECTED] wrote: Gurjeet Singh wrote: On 2/9/07, Alvaro Herrera [EMAIL PROTECTED] wrote: At least it'd help those poor people trying to do conditional COMMIT or ROLLBACK based on the transaction status. The user doesn't need to check the status of the transaction if he just needs to end it. Just fire the END command and it'll take care of whether to COMMIT or ROLLBACK the transaction: Hmm, right. Maybe I was thinking in savepoints, which Merlin Moncure is so fond of complaining of ;-) I'm still looking for a use for them. When I find one, I'll give you a heads up. (just so you know, with savepoints came exception blocks, one of my all time favorite features). anyways, instead of complaining, consider it more of 'hopeful nudging' :-) merlin ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [pgsql-patches] [PATCHES] pg_standby
On 12/28/06, Simon Riggs [EMAIL PROTECTED] wrote: On Thu, 2006-12-28 at 19:26 +, Simon Riggs wrote: On Thu, 2006-12-14 at 12:04 +, Simon Riggs wrote: pg_standby and test framework, in separate .tar files New version (v2), following further testing. Signal handling not included in this version. Signal handling now added, tested and working correctly in version 3, attached. pg_standby is an example program for a warm standby script as discussed on -hackers: http://archives.postgresql.org/pgsql-hackers/2006-08/msg00407.php Program looks complete and ready for review, to me. I double checked and re-ran all my test and confirmed that pg_standby move (-m) mode is definitely busted in v3 in the sense that a restart of the standby will not resume recovery and requires a pg_resetxlog to become operational -- it needs one more WAL file back than the oldest one available. I am currently working around this by rotating WAL files a couple of versions back in the shell script I am using to receive log files via netcat. move mode is very desirable because it keeps the maintenance down for the standby system. merlin ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [pgsql-patches] [PATCHES] pg_standby
On 1/17/07, Doug Knight [EMAIL PROTECTED] wrote:
I confirm that I am seeing the exact same characteristic. Could you post
your rotating script?
note: this is still a work in progress, the crude but effective sleep
5 is due to be replaced with a lock/fifo and there catch_wal.sh needs
to be rewritten a bit. truncate is a C one-liner I wrote which does a
ftruncate.
*** primary: ***
archive_command = '/home/postgres/send_wal.sh %p %f'
*** send_wal.sh: ***
!/bin/bash
echo archiving: $2 ~/send_wal.log
cat $1 (echo placeholder) (echo $2) | nc $STANDBY 1234 sleep 5
*** secondary: ***
restore_command = 'pg_standby -m -w0 -t/raid/pitr/kill /raid/pitr %f %p'
*** catch_wal.sh ***
!/bin/bash
WALDIR=/raid/pitr
rm -f $WALDIR/*.old
rm -f $WALDIR/*.older
$WALDIR/tmp.older
$WALDIR/tmp.old
while true;
do
tmpfile=`mktemp`
nc -l 1234 $tmpfile || { echo FATAL: nc listen failed; exit 1; }
chown postgres:postgres $tmpfile
file_name=`tail -1 $tmpfile`
./truncate $tmpfile 16777216
rm -f $WALDIR/*.older
for i in `ls $WALDIR/*.old`; do mv $i $WALDIR/`basename $i .old`.older; done
mv $tmpfile $WALDIR/$file_name.old
cp --preserve=ownership $WALDIR/$file_name.old $WALDIR/$file_name
echo LOG: caught file: $file_name
done
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [pgsql-patches] [PATCHES] pg_standby
On 1/17/07, Merlin Moncure [EMAIL PROTECTED] wrote: On 1/17/07, Doug Knight [EMAIL PROTECTED] wrote: I confirm that I am seeing the exact same characteristic. Could you post your rotating script? note: this is still a work in progress, the crude but effective sleep 5 is due to be replaced with a lock/fifo and there catch_wal.sh needs to be rewritten a bit. truncate is a C one-liner I wrote which does a ftruncate. this turned out not to fix the problem...working on it still! merlin ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [pgsql-patches] [PATCHES] pg_standby
On 1/17/07, Simon Riggs [EMAIL PROTECTED] wrote: new v4 Changes - removed -m command, design flaw in original spec, use -l instead - added -k N command to cleanup archive and leave max N files - fflush() points added to allow Windows debug - bug fix: when .history file present - bug fix: command line switch cleanup - readme updated works fantastic. grazi...i guess my rotation would have worked with more files but -k is much cleaner. merlin ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [pgsql-patches] [HACKERS] [PATCHES] Building libpq/psql with Borland BCC5
On 1/10/07, Alvaro Herrera [EMAIL PROTECTED] wrote: What about a Mingw or VC++ psql with a BCC libpq? Is it possible to link something like that? It would be nice to have the libpq at least able to pass the regression tests. you can use microsoft/mingw compiled DLL files but not library files. however, borland provides a command line tool (implib i thnk) to create an import library for it which works ok. (i think you have to pass a switch to fix underscore issue). libpq.lib is not directly usable (coff vs. omf) but digital mars makes a tool which can do this and I have confirmed it works. note: I've used borland compiled libpq (not psql) with borland C++ builder 3 5 with no problems. I had to hack pg_config.h however. merlin ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[PATCHES] docs for advisory locks
ok, here is the promised docs for the advisory locks. some quick notes here: this is my first non trivial patch (albeit only documentation) and i am a complete docbook novice. i also dont have the capability to build docbook at the moment. so consider this a very rough draft. any comments are welcome particularly from someone willing to build the sgml and check for errors...sorry about that but i wanted to get this up as soon as possible. thanks merlin advisory_docs.patch Description: Binary data ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[PATCHES] Fwd: docs for advisory locks
[resending this to the list, was silently dropped this afternoon?] -- Forwarded message -- From: Merlin Moncure [EMAIL PROTECTED] Date: Sep 19, 2006 11:57 PM Subject: docs for advisory locks To: pgsql-patches@postgresql.org ok, here is the promised docs for the advisory locks. some quick notes here: this is my first non trivial patch (albeit only documentation) and i am a complete docbook novice. i also dont have the capability to build docbook at the moment. so consider this a very rough draft. any comments are welcome particularly from someone willing to build the sgml and check for errors...sorry about that but i wanted to get this up as soon as possible. thanks merlin advisory_docs.patch Description: Binary data ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Template0 age is increasing speedily.
On 9/7/06, Nimesh Satam [EMAIL PROTECTED] wrote: We also noticed that the database slow downs heavily at a particular time..Can you suggest any tools which will help in diagnosing the root cause behiond the data load. possible checkpoint? poorly formulated query? it could be any number of things. use standard tools to diagnose the problem, including: unix tools: top, vmstat, etc postgresql query logging, including min_statement_duration explain analyze ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Limit usage of tcop/dest.h
Tom Lane wrote: I thought renaming them was a better idea, actually. Here is a patch for that. I will apply this to HEAD later today. -- I'm getting compiler error (win32) which I think is related to this patch: postmaster.c: In function `SubPostmasterMain': postmaster.c:3189: error: `None' undeclared (first use in this function) postmaster.c:3189: error: (Each undeclared identifier is reported only once postmaster.c:3189: error: for each function it appears in.) make[2]: *** [postmaster.o] Error 1 make[2]: Leaving directory `/postgres/pgsql/src/backend/postmaster' make[1]: *** [postmaster-recursive] Error 2 Merlin ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL
Tom Lane wrote: If the connection gets accepted, I'd expect *something* in the postmaster logs -- can you check? I suspect Merlin's complaint has to do with the fact that the *user* doesn't see any error message. The way you've coded this, setsockopt failure during startup is treated as a communications failure and so there's no attempt to report the problem to the client. Correct. In fact, when I posted I did completely miss the log message due to initdb resetting log_destination. Confirmed: LOG: setsockopt(TCP_KEEPIDLE) not supported in server log. Merlin ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Implement support for TCP_KEEPCNT, TCP_KEEPIDLE, TCP_KEEPINTVL (was Re: [HACKERS] Feature freeze date for 8.1)
Here's a patch that adds four new GUCs: tcp_keepalives (defaults to on, controls SO_KEEPALIVE) tcp_keepalives_idle (controls TCP_KEEPIDLE) tcp_keepalives_interval (controls TCP_KEEPINTVL) tcp_keepalives_count (controls TCP_KEEPCNT) I just tested this on my windows XP machine running rc1. A default configuration reports zeros for the tcp values in a 'show all'. More significantly, if you change a tcp parameter from the default, the server rejects connections without a relevant error message :(. I did some research and the only way to control these parameters is to adjust the system registry plus a reboot. (somebody correct me here). If that is the case IMO it makes the most sense to have the server fail to start if the default parameters are changed. Even better would be a stronger test to make sure o/s supports this feature. Merlin ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [pgsql-hackers-win32] win32 random number generator
Merlin Moncure [EMAIL PROTECTED] writes: Looks like this in lrand48(void): _rand48_seed[1] 1); _rand48_seed[1] 1); ^^ The problem is the shift operator :). Anyways I double checked the results and it works as expected now so here's a patch. I also removed the spurious casts. Merlin fix_random.diff Description: fix_random.diff ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] userlock changes for 8.1/8.2
Alvaro wrote: Please search this message in the archives: right. heh. Well, moving on... tgl wrote: Since subids and offnums are only 16 bits, we could pack all of these cases into 64 bits with a 16-bit type identifier to distinguish the cases. That would mean that LOCKTAG doesn't get any bigger than it is now, and we'd have plenty of room for expansion still with more types. Ok, this makes perfect sense, kind of what I was saying only better. The only thing I can add to it at this point is to reorganize the lock view(s) correspondingly...should be 1 view for each specific lock type plus 1 generic one for all locks. A new datum for the generic lock type (plus some casts) might be worth considering. Is it possible for one backend (with superuser privs) to release a lock held by anotether? Merlin ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
[PATCHES] error in pg_ctl.c
Getting a win32 compiler error based on yesterday's update to pg_ctl.c. It's a 1 liner (1081): if (StartServiceCtrlDispatcher(st)) == 0) to if (StartServiceCtrlDispatcher(st) == 0) or, as I prefer it :) if (0 == StartServiceCtrlDispatcher(st)) Merlin ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] [PERFORM] [pgsql-hackers-win32] scalability issues on win32
Attached patch solves the problem for me.Didn't see anything of the problem you described about hangs in my implementation. Probably because mine exits at the first sign of a linebreak. I confirmed the fix. 8.0.0 now starts up with net start (this is where I first noticed the problem). When I get some time I'll investigate if this is related to the problems I was having with the statistics collector. Merlin ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] Win32 signals sockets
Tom Lane wrote: It's the increase in variance between the Unix and Windows code paths that's really bothering me. We went into this project on the promise that there weren't going to be thousands of lines of #ifdef WIN32 stuff, and I'm not happy in the least with the way postmaster.c looks now, let alone after applying this patch. I've been following this thread for a bit and I have to admit I wouldn't mind seeing the shmmem part of Magnus's patch go in. Windows suffers vs unix generally on process creation times and any improvement here would be welcome. Merlin ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
