Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: Andrew Chernow [EMAIL PROTECTED] writes: What parts of PGconn/PGresult do you need to touch that aren't exposed already? Don't need direct access to PGconn at all. Oh, good, that makes things much easier. Shoot! Feels like you always miss something. The patch uses PGconn's PQExpBuffer to set errors on a conn. Currently, there is no access to this buffer other than PQerrorMessage. Is the below okay? extern PQExpBuffer *PQgetErrorBuffer(PGconn *conn); // OR PQsetErrorMessage(conn, errstr) -- this felt strange to me The expbuffer API is public, so managing the returned PQExpBuffer would not require any additional API calls. While I am on the subject of things I missed, the patch also uses pqSetResultError (mostly during PQgetf). We no longer need direct access to anything inside pg_result. However, we would need 3 new API calls that would dup a result, set field descs and add tuples to a result. Below are prototypes of what I have so far (small footprint for all 3, maybe 100-150 lines). /* numParameters, paramDescs, errFields, curBlock, * curOffset and spaceLeft are not assigned at all, * initialized to zero. errMsg is handled by * PQmakeEmptyPGresult. attDescs and tuples are not * duplicated, only allocated based on 'ntups' and * 'numAttributes'. The idea is to dup the result * but customize attDescs and tuples. */ PGresult *PQresultDup( PGconn *conn, PGresult *source, int ntups, int numAttributes); /* Only for results returned by PQresultDup. This * will set the field descs for 'field_num'. The * PGresAttDesc struct was not used to avoid * exposing it. */ int PQresultSetFieldDesc( PGresult *res, int field_num, const char *name, Oid tableid, int columnid, int format, Oid typid, int typlen, int typmod) /* Only for results returned by PQresultDup. This * will append a new tuple to res. A PGresAttValue * is allocated and put at index 'res-ntups'. This * is similar to pqAddTuple except that the tuples * table has been pre-allocated. In our case, ntups * and numAttributes are known when calling resultDup. */ int PQresultAddTuple( PGresult *res, char *value, int len); The above would allow an external app to dup a result and customize its rows and columns. Our patch uses this for arrays and composites. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow [EMAIL PROTECTED] writes: Shoot! Feels like you always miss something. The patch uses PGconn's PQExpBuffer to set errors on a conn. Currently, there is no access to this buffer other than PQerrorMessage. Is the below okay? Surely you would just provide a function to get pqtypes errors separate from libpq errors? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's PostGIS support! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Gregory Stark wrote: Andrew Chernow [EMAIL PROTECTED] writes: Shoot! Feels like you always miss something. The patch uses PGconn's PQExpBuffer to set errors on a conn. Currently, there is no access to this buffer other than PQerrorMessage. Is the below okay? Surely you would just provide a function to get pqtypes errors separate from libpq errors? That's extremely akward. Consider the below. int getvalues(PGresult *res, int *x, char **t) { return PQgetf(res, get the int and text); } if(getvalues(res, x, t)) printf(%s\n, PQresultErrorMessage(res)); How would the caller of getvalues know whether the error was generated by a libpqtypes API call or by a libpq API call (like PQgetvalue)? PQgetf should behave exactely as PQgetvalue does, in regards to errors. Same holds true for PGconn. Normally, you are using PQputf which sets the error in PQparamErrorMessage. Then there is PQparamCreate(conn) or PQparamExec(conn, param, ...) and friends? PQparamExec calls PQexecParams internally which can return NULL, setting an error message inside PGconn. We felt our light-weight wrappers should be consistent in behavior. If you get a NULL PGresult for a return value, PQerrorMessage should be checked. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow [EMAIL PROTECTED] writes: How would the caller of getvalues know whether the error was generated by a libpqtypes API call or by a libpq API call (like PQgetvalue)? PQgetf should behave exactely as PQgetvalue does, in regards to errors. Hm. Well I was thinking of errors from database operations rather than things like PQgetvalue. I suppose we have to decide whether pqtypes is a wrapper around libpq in which case your functions would have to take the libpq error and copy it into your error state or an additional set of functions which are really part of appendage of libpq -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Thu, Apr 10, 2008 at 10:56 AM, Gregory Stark [EMAIL PROTECTED] wrote: Andrew Chernow [EMAIL PROTECTED] writes: How would the caller of getvalues know whether the error was generated by a libpqtypes API call or by a libpq API call (like PQgetvalue)? PQgetf should behave exactely as PQgetvalue does, in regards to errors. Hm. Well I was thinking of errors from database operations rather than things like PQgetvalue. I suppose we have to decide whether pqtypes is a wrapper around libpq in which case your functions would have to take the libpq error and copy it into your error state or an additional set of functions which are really part of appendage of libpq We see libpq types to be simply extending the api with more functions. We really only introduce new error handling with the PGparam struct that is following the same conventions that exist currently. The separation of libpqtypes out of libpq into a new library is an architectural change for organization purposes. We are not defining a new api or wrapping an existing one for new functionality (which is the same thing imo). With that in mind, libpq's error system is object based, not library based. Thee three objects that set errors are result, conn, (and now), param. In libpq terms there is no global error. (no errno, getlasterror, etc). So, if I understand you correctly, we see libpqtypes is an appendage in your terms...were you thinking along different lines? Consider that we don't reproduce all the things libpq offers, we share the same objects, etc. (In fact, we went though some acrobatics to introduce as little new behavior as possible). merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: /* Only for results returned by PQresultDup. This * will append a new tuple to res. A PGresAttValue * is allocated and put at index 'res-ntups'. This * is similar to pqAddTuple except that the tuples * table has been pre-allocated. In our case, ntups * and numAttributes are known when calling resultDup. */ int PQresultAddTuple( PGresult *res, char *value, int len); PQresultAddTuple is not quite correct. The below is its replacement: int PQresultSetFieldValue( PGresult *res, int tup_num, int field_num, char *value, int len) Recap: PQresultDup, PQresultSetFieldDesc and PQresultSetFieldValue. We feel this approach, which we initially thought wouldn't work, is better than making pg_result public. These functions could be useful outside of pqtypes, providing a way of filtering/modifying a result object ... consider: PGresult *execit(conn, stmt) { res = PQexec(conn, stmt); if(some_app_cond_is_true) { newres = PQresultDup(); // ... customize tups and fields //PQresultSetFieldDesc and PQresultSetFieldValue PQclear(res); res = newres; } return res; } // res could be custom built res = execit(conn, stmt); PQclear(res); This is not an everyday need, but I'm sure it could provide some niche app functionality currently unavailable. Possibly useful to libpq wrapper APIs. Either way, it works great for pqtypes and avoids exposing pg_result. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow [EMAIL PROTECTED] writes: Recap: PQresultDup, PQresultSetFieldDesc and PQresultSetFieldValue. We feel this approach, which we initially thought wouldn't work, is better than making pg_result public. That doesn't seem to me to fit very well with libpq's internals --- in particular the PQresult struct is not designed to support dynamically adding columns, which retail PQresultSetFieldDesc() calls would seem to require, and it's definitely not designed to allow that to happen after you've begun to store data, which the apparent freedom to intermix PQresultSetFieldDesc and PQresultSetFieldValue calls would seem to imply. Instead of PQresultSetFieldDesc, I think it might be better to provide a call that creates an empty (of data) PGresult with a specified tuple descriptor in one go. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow [EMAIL PROTECTED] writes: Gregory Stark wrote: Surely you would just provide a function to get pqtypes errors separate from libpq errors? That's extremely akward. Consider the below. I'm just as suspicious of this as Greg is. In particular, I really disagree with PQgetf storing an error message into a PGresult, because that creates a semantic inconsistency. Up to now, PGresults have come in two flavors, okay (which might hold data) and error (which hold error messages). Now you're proposing to stick an error message into an okay data-holding PGresult. Does that turn it into an error result? Does its PQresultStatus change? Does it maybe forget the data it used to hold? An even more fundamental objection is that surely PQgetf's PGresult argument should be const. int getvalues(PGresult *res, int *x, char **t) { return PQgetf(res, get the int and text); } if(getvalues(res, x, t)) printf(%s\n, PQresultErrorMessage(res)); This example is entirely unconvincing. There is no reason to be checking PQresultErrorMessage at this point --- if you haven't already checked the PGresult to be okay, you should certainly not be trying to extract fields from it. So I don't see that you are saving any steps here. PQgetf should behave exactely as PQgetvalue does, in regards to errors. Uh-huh. You'll notice that PQgetvalue treats the PGresult as const. Same holds true for PGconn. I'm not convinced about that side either. It doesn't fail the basic const-ness test, perhaps, but it still looks mostly like you are trying to avoid the necessity to think hard about how you're going to report errors. You're going to have to confront the issue for operations that only take a PGresult, and once you've got a good solution on that side it might be better to use it for PQputf too. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: Andrew Chernow [EMAIL PROTECTED] writes: Recap: PQresultDup, PQresultSetFieldDesc and PQresultSetFieldValue. We feel this approach, which we initially thought wouldn't work, is better than making pg_result public. That doesn't seem to me to fit very well with libpq's internals --- in particular the PQresult struct is not designed to support dynamically adding columns, which retail PQresultSetFieldDesc() calls would seem to require, and it's definitely not designed to allow that to happen after you've begun to store data, which the apparent freedom to intermix PQresultSetFieldDesc and PQresultSetFieldValue calls would seem to imply. Instead of PQresultSetFieldDesc, I think it might be better to provide a call that creates an empty (of data) PGresult with a specified tuple descriptor in one go. regards, tom lane Are you against exposing PGresAttDesc? PGresult *PQresultDup( PGconn *conn, PGresult *res, int ntups, int numAttributes, PGresAttDesc *attDescs); If you don't want to expose PGresAttDesc, then the only other solution would be to pass parallel arrays of attname[], tableid[], columnid[], etc... Not the most friendly solution, but it would do the trick. Recap: Use new PQresultDup, throw out setfielddesc and keep setfieldvalue the same. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: PQresultErrorMessage at this point --- if you haven't already checked the PGresult to be okay, you should certainly not be trying to extract fields from it. So I don't see that you are saving any steps here. Correct, I agree. Our thinking was flawed. The issue we face is that, unlike getvalue, getf can fail in many ways ... like bad format string or type mismatch. If you would rather us introduce a pqtypes specific error for getf. putf doesn't suffer from this issue because it uses PGparamErrorMessage. Same holds true for PGconn. I'm not convinced about that side either. It doesn't fail the basic const-ness test, perhaps, but it still looks mostly like you are trying to avoid the necessity to think hard about how you're going to report The issue is not a matter of know-how, it is a matter of creating ambiguos situations in regards to where the error is (I'm thinking from a libpq user's perspective). This only applies to PQparamExec and fiends, NOT PQputf. All existing exec/send functions put an error in the conn (if the result is NULL check the conn). paramexec can fail prior to internally calling PQexecParams, in which case it returns NULL because no result has been constructed yet. The question is, where does the error go? res = paramexec(conn, param, ... if(!res) // check pgconn or pgparam? // can conn have an old error (false-pos) Not using the conn's error msg means that one must check the param and conn if the return value is NULL. I think the best behavior here is to check PQerrorMessage when any exec function returns a NULL result, including pqtypes. If not, I think it could get confusing to the API user. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow [EMAIL PROTECTED] writes: PGresult *PQresultDup( PGconn *conn, PGresult *res, int ntups, int numAttributes, PGresAttDesc *attDescs); I don't understand why this is a dup operation. How can you dup if you are specifying a new tuple descriptor? I'd have expected something like PGresult *PQmakeResult(PGconn *conn, int numAttributes, PGresAttDesc *attDescs) producing a zero-row PGRES_TUPLES_OK result that you can then load with data via PQresultSetFieldValue calls. (Even the conn argument is a bit of a wart, but I think we probably need it so we can copy some of its private fields.) Copying an existing PGresult might have some use too, but surely that can't change its tuple descriptor. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: Andrew Chernow [EMAIL PROTECTED] writes: PGresult *PQresultDup( PGconn *conn, PGresult *res, int ntups, int numAttributes, PGresAttDesc *attDescs); I don't understand why this is a dup operation. How can you dup if you are specifying a new tuple descriptor? I'd have expected something like PGresult *PQmakeResult(PGconn *conn, int numAttributes, PGresAttDesc *attDescs) producing a zero-row PGRES_TUPLES_OK result that you can then load with data via PQresultSetFieldValue calls. (Even the conn argument is a bit of a wart, but I think we probably need it so we can copy some of its private fields.) Copying an existing PGresult might have some use too, but surely that can't change its tuple descriptor. regards, tom lane Yeah, dup wasn't the best name. You need more arguments to makeresult though, since you reomved the 'source' result. You need binary, cmdStatus, noticeHooks and client_encoding. PQmakeResult(conn, PQcmdStatus(res), PQbinaryTuples(res), ?client_encoding?, ?noticeHooks?, ntups, /* this interacts with setfieldvalue */ numAttributes, attDescs); For client_encoding and noticeHooks, the conn can be NULL. Previously, we copied this info from the source result when conn was NULL. producing a zero-row PGRES_TUPLES_OK result that you can then load with data via PQresultSetFieldValue calls. I like this idea but you removed the 'ntups' argument. There is no way to adjust ntups after the makeresult call, its a private member and setfieldvalue has no concept of incrementing ntups. Since you are not appending a tuple like pqAddTuple, or inserting one, you can't increment ntups in setfieldvalue. But, you could have a function like PQresultAddEmptyTuple(res) which would have to be called before you can set field values on a tup_num. The empty tuple would grow tuples when needed and increment ntups. The new tuple would be zerod (all NULL values). something like below res = PQmakeResult(...); for(ntups) { PQresultAddEmptyTuple(res); // or PQresultMakeEmptyTuple? for(nfields) PQresultSetFieldValue(res, tup_num, field_num, ); } -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: For client_encoding and noticeHooks, the conn can be NULL. Previously, we copied this info from the source result when conn was NULL. Looks like we don't need client_encoding. My guess is, in our use case noticeHooks can probably be NULL for the created result, when makeresult is not provided a conn. something like below res = PQmakeResult(...); for(ntups) { PQresultAddEmptyTuple(res); // or PQresultMakeEmptyTuple? for(nfields) PQresultSetFieldValue(res, tup_num, field_num, ); } I think a better idea would be to make setfieldvalue more dynamic, removing the need for PQresultAddEmptyTuple. Only allow tup_num to be = res-ntups. This will only allow adding one tuple at a time. The other option would allow arbitray tup_nums to be passed in, like ntups is 1 but the provided tup_num is 67. In this case, we would have to back fill. It seems better to only grow by one tuple at a time. We can get it working either way, we just prefer one tuple at a time allocation. int PQresultSetFieldValue( PGresult *res, int tup_num, int field_num, char *value, int len) { if(!check_field_value(res, field_num)) return FALSE; if(tup_num res-ntups) // error, tup_num must be = res-ntups if(res-ntups = res-tupArrSize) // grow res-tuples if(tup_num == res-ntups !res-tuples[tup_num]) // need to allocate tuples[tup_num] // blah ... blah ... } New PQmakeResult prototype: PQmakeResult( PGconn *conn, const char *cmdStatus, int binaryTuples, int numAttributes, PGresAttDesc *attDescs); -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: res = paramexec(conn, param, ... if(!res) // check pgconn or pgparam? // can conn have an old error (false-pos) We will just always dump the error message into the param. If PQexecParams fails with a NULL result, we will copy PQerrorMessage over to param-errMsg. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Jeff Davis [EMAIL PROTECTED] writes: * Binary formatting The exclusive use of binary formats is worrisome to me. This circumvents one level of indirection that we have (i.e. that everything moves through in/out functions), and will impose a backwards-compatibility requirement on the internal representation of data types, including user-defined data types. As far as I know, we currently have no such requirement, even for built-in types. This is actually incorrect. Binary I/O still goes through a function call, the send/recv functions instead of the in/out functions. In theory these are also supposed to be cross-platform. In practice they are, uhm, less cross-platform. For example they send floats as raw (presumably) IEEE floats. There have also been fewer clients using binary mode, so you're more likely to run into bugs. But the reason fewer clients use binary mode is because they would have to implement all this type-specific knowledge. It doesn't make sense to do it for every driver or application but if you're implementing it once in a library it does start to make more sense. Note however that not every data type will necessarily provide a binary send/recv function. The built-in data types do, but only as a matter of policy. It's not an error to create a type with no binary i/o functions. So I think you have to support using text mode as an option. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Wed, Apr 9, 2008 at 6:13 AM, Gregory Stark [EMAIL PROTECTED] wrote: The exclusive use of binary formats is worrisome to me. This circumvents one level of indirection that we have (i.e. that everything moves through in/out functions), and will impose a backwards-compatibility requirement on the internal representation of data types, including user-defined data types. As far as I know, we currently have no such requirement, even for built-in types. This is actually incorrect. Binary I/O still goes through a function call, the send/recv functions instead of the in/out functions. In theory these are also supposed to be cross-platform. In practice they are, uhm, less cross-platform. For example they send floats as raw (presumably) IEEE floats. There have also been fewer clients using binary mode, so you're more likely to run into bugs. small correction: you _were_ more likely to run into bugs. in the process of developing this patch during the 8.3 cycle, we caught, fixed, and submitted a number of binary format related issues, including some wacky things that are not possible through the text parser (a polygon with zero points for example). we used regression testing heavily in development. But the reason fewer clients use binary mode is because they would have to implement all this type-specific knowledge. It doesn't make sense to do it for every driver or application but if you're implementing it once in a library it does start to make more sense. Note however that not every data type will necessarily provide a binary send/recv function. The built-in data types do, but only as a matter of policy. It's not an error to create a type with no binary i/o functions. So I think you have to support using text mode as an option. You have this option. there is nothing keeping you from using getvalue vs. getf. However, due to libpq limitations, if any datatype must return text the entire result must be text (resultFormat)...this is also interestingly true for functions that return 'void'. So, at present, to use PQgetf, you result set must be binary. In some of the early versions of the patch we introduced parsing code to compute text results in addition to binary results (so for getf('%int), the library does the atoi for you, checking range and such). We ended up dropping this because we were getting nervous about the size of the patch at that point. In any event, I think a better solution would be to change resultformat to mean 'give me binary format if it's available' on a per column basis...libpq already has a method to check field format of the result and getf can just raise a run-time error if you try to pass a native type in when it's not available. This would be a pretty amazing sequence of events...only likely to occur when developing new types for example. Is this (mixed text/binary results) possible with the v3 protocol? merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Merlin Moncure wrote: However, due to libpq limitations, if any datatype must return text the entire result must be text (resultFormat)...this is also interestingly true for functions that return 'void'. So, at present, to use PQgetf, you result set must be binary. I'm surprised you didn't try to address that limitation. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Wed, Apr 9, 2008 at 8:04 AM, Andrew Dunstan [EMAIL PROTECTED] wrote: \ Merlin Moncure wrote: However, due to libpq limitations, if any datatype must return text the entire result must be text (resultFormat)...this is also interestingly true for functions that return 'void'. So, at present, to use PQgetf, you result set must be binary. I'm surprised you didn't try to address that limitation. whoops! we did...thinko on my part. Text results are fully supported save for composites and arrays. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Dunstan wrote: Merlin Moncure wrote: However, due to libpq limitations, if any datatype must return text the entire result must be text (resultFormat)...this is I'm surprised you didn't try to address that limitation. That would change the existing behavior of resultFormat, although not terribly. Currently, the server will spit back an error if you use binary results but some type hasn't implemented a send/recv. Instead of an error, the server could fallback to the type's in/out routines and mark the column as text format. I think the fallback approach is more intelligent behavior but such a change could break libpq clients. They might be blindly ASSuming if the exec worked with resultFormat=1, that everything returned by PQgetvalue will be binary (I'm guilty of this one, prior to libpqtypes). Our patch would work with no changes because it supports text and binary results. So, each type handler already toggles itself based on PQfformat. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: Andrew Dunstan wrote: Merlin Moncure wrote: However, due to libpq limitations, if any datatype must return text the entire result must be text (resultFormat)...this is I'm surprised you didn't try to address that limitation. That would change the existing behavior of resultFormat, although not terribly. Currently, the server will spit back an error if you use binary results but some type hasn't implemented a send/recv. Instead of an error, the server could fallback to the type's in/out routines and mark the column as text format. I think the fallback approach is more intelligent behavior but such a change could break libpq clients. They might be blindly ASSuming if the exec worked with resultFormat=1, that everything returned by PQgetvalue will be binary (I'm guilty of this one, prior to libpqtypes). Our patch would work with no changes because it supports text and binary results. So, each type handler already toggles itself based on PQfformat. That makes it sound more like a protocol limitation, rather than a libpq limitation. Or am I misunderstanding? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Merlin Moncure wrote: On Wed, Apr 9, 2008 at 8:04 AM, Andrew Dunstan [EMAIL PROTECTED] wrote: \ Merlin Moncure wrote: However, due to libpq limitations, if any datatype must return text the entire result must be text (resultFormat)...this is also interestingly true for functions that return 'void'. So, at present, to use PQgetf, you result set must be binary. I'm surprised you didn't try to address that limitation. whoops! we did...thinko on my part. Text results are fully supported save for composites and arrays. merlin Yeah, currently composites and arrays only support binary results in libpqtypes. This forces any array elementType or any member of a composite to have a send/recv routine. Using the fallback to text output approach, this limitation on array elements and composite members would be removed. That makes it sound more like a protocol limitation, rather than a libpq limitation. Or am I misunderstanding? It looks like libpq, message 'T' handling in getRowDescriptions, reads a separate format for each column off the wire. The protocol already has this ability. The backend needs a ?minor? adjustment to make use of the existing capability. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, Apr 08, 2008 at 07:18:31PM -0400, Tom Lane wrote: sure that there's much potential commonality. The thing that's most problematic about ecpg is that it wants to offer client-side equivalents of some server datatype-manipulation functions; and I don't actually see much of any of that in the proposed patch. All that's really here is format conversion stuff, so there's no hope of unifying that code unless this patch adopts ecpg's choices of client-side representation ecpg for instance does not use a struct for time, so there doesn't seem to be an advantance for ecpg in switching. On the other side there's a disadvantage in that you can binary transfer right now given that you're on the same architecture. But if the datatypes differ this would be lost. (which I believe are mostly Informix legacy choices, so I'm not sure we want that). Not really. ecpg's pgtypeslib uses the same datatypes as the backend uses (well, mostly because numeric was changed in the backend after the creation of pgtypeslib). Then there's a compatibility layer that maps Informix functions and datatypes to our functions and datatypes. Michael -- Michael Meskes Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED] Go VfL Borussia! Go SF 49ers! Use Debian GNU/Linux! Use PostgreSQL! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: Yeah, currently composites and arrays only support binary results in libpqtypes. This forces any array elementType or any member of a composite to have a send/recv routine. Using the fallback to text output approach, this limitation on array elements and composite members would be removed. Actually, I am confusing the way the protocol handles arrays and composites (as a single column value, vs. the way libpqtypes handles these (as a separate result object). for instance, the members of a composite inherit the format of the column within the protocol. To allow one member of a composite to be text formatted and another be binary, would require a change to the protocol, an additional format value per member header. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: Well, I can get it working with a very small patch. We actually don't need very much in libpq. Although, making it somehow generic enough to be useful to other extensions is a bit tricky. Please, suggestions would be helpful. Quick question on the hook concept before I try to supply a new patch. From my experience, redhat normally compiles everything into their packages; like apache modules. Why would libpq be any different in regards to libpqtypes? If they don't distribute libpqtypes, how does a libpq user link with libpqtypes? They don't have the library. Where would they get a libpqtypes.so that is compatible with redhat's supplied libpq.so? The core of what I am trying to ask is, there doesn't appear to be an advantage to separating libpqtypes from libpq in terms of space. If redhat follows their normal policy of include all (probably to make their distro as feature rich out-of-the-box as possible), then they would distribute libpqtypes.so which would use the same amount of space as if it were part of libpq. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: Andrew Chernow wrote: Well, I can get it working with a very small patch. We actually don't need very much in libpq. Although, making it somehow generic enough to be useful to other extensions is a bit tricky. Please, suggestions would be helpful. Quick question on the hook concept before I try to supply a new patch. From my experience, redhat normally compiles everything into their packages; like apache modules. Why would libpq be any different in regards to libpqtypes? If they don't distribute libpqtypes, how does a libpq user link with libpqtypes? They don't have the library. Where would they get a libpqtypes.so that is compatible with redhat's supplied libpq.so? The core of what I am trying to ask is, there doesn't appear to be an advantage to separating libpqtypes from libpq in terms of space. If redhat follows their normal policy of include all (probably to make their distro as feature rich out-of-the-box as possible), then they would distribute libpqtypes.so which would use the same amount of space as if it were part of libpq. By the way, I offered up the idea of compiling our patch in or out, ./configure --with-pqtypes. But Tom had said [shrug...] So the packagers will compile it out, and you're still hosed, or at least any users who'd like to use it are. If redhat would compile out our patch, no questions asked, why would they distribute libpqtypes.so. Meaning, separating it out doesn't seem to unhose us :) -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: Andrew Chernow wrote: Well, I can get it working with a very small patch. We actually don't need very much in libpq. Although, making it somehow generic enough to be useful to other extensions is a bit tricky. Please, suggestions would be helpful. Quick question on the hook concept before I try to supply a new patch. From my experience, redhat normally compiles everything into their packages; like apache modules. Why would libpq be any different in regards to libpqtypes? If they don't distribute libpqtypes, how does a libpq user link with libpqtypes? They don't have the library. Where would they get a libpqtypes.so that is compatible with redhat's supplied libpq.so? The core of what I am trying to ask is, there doesn't appear to be an advantage to separating libpqtypes from libpq in terms of space. If redhat follows their normal policy of include all (probably to make their distro as feature rich out-of-the-box as possible), then they would distribute libpqtypes.so which would use the same amount of space as if it were part of libpq. They would get it the same way they would get anything else that uses libpq that isn't packaged with it (e.g. DBD::Pg). They would either get the package that contains it, if it exists, or get the source and build it. The package with the dependent library might belong in the extras collection, while Tom's goal (and it's a good one) is to keep libpq in the core collection. I don't get what you're not seeing about this. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: The core of what I am trying to ask is, there doesn't appear to be an advantage to separating libpqtypes from libpq in terms of space. If redhat follows their normal policy of include all (probably to make their distro as feature rich out-of-the-box as possible), then they would distribute libpqtypes.so which would use the same amount of space as if it were part of libpq. My guess is that if we provide an useful library, Redhat will distribute it some way or another. In the worst case (i.e. Redhat does not distribute it at all), it will still be available on PGDG rpm repositories. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Dunstan wrote: I don't get what you're not seeing about this. All I am trying to say is, redhat's core packages are normally very inclusive. Like apache, which includes many/most modules in the core package. I am still not convinced there is merit to a separate library. But honestly, I'm indifferent at this point. If the community wants it this way, whether or not I agree with the reasons, then consider it done. It would be helpful to get some feedback about what the requirements are for a hooking mechanism for libpqtypes: 1. Do we make the hooking system generic, for other library to use? 2. If #1 is YES, then does this hooking system need to allow registering multiple hooks per app instance. Meaning, should we implement a table of callbacks/hooks? Like a linked list of them. 3. What design is preferred? *) A single msg proc that uses a msg object which is either a big union or something that gets up casted. *) A separate function pointer per hook, like conn_create, conn_destroy *) A combo, where conn hooks are handled by one funcptr and result hooks by another. But each only has one function. Please think carefully about question #1, as this could lead to over-design or guess-design. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow [EMAIL PROTECTED] writes: There is no need to pass hookData to the hook function. libpqtypes already accesses PGconn and PGresult directly so it can just access the hookData member. That's a habit you'd really be best advised to stop, if you're going to be a separate library. Otherwise, any time we make an internal change in the PGconn struct, it'll break your library --- and we have and will feel free to do that without an external soname change. What parts of PGconn/PGresult do you need to touch that aren't exposed already? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: Andrew Dunstan wrote: I don't get what you're not seeing about this. All I am trying to say is, redhat's core packages are normally very inclusive. Like apache, which includes many/most modules in the core package. There are plenty of modules that they don't include, e.g. mod_fastcgi. If you want that you download and build it against your installed apache. Or you get mod_fcgid from extras. Your bolt-on client library would be in exactly the same boat as one of those. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Dunstan wrote: All I am trying to say is, redhat's core packages are normally very inclusive. Like apache, which includes many/most modules in the core package. There are plenty of modules that they don't include, e.g. mod_fastcgi. If you want that you download and build it against your installed apache. Or you get mod_fcgid from extras. Your bolt-on client library would be in exactly the same boat as one of those. I think Andrew Chernow is fundamentally confused about dynamic linking, which apache has to use because it doesn't know what type of file it has to handle, with linking, which is bound to the application code. pgtypes is bound to the application code so it is not like apache --- an application isn't going to be fed arbitrary pgtypes function calls it has to handle. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Bruce Momjian wrote: I think Andrew Chernow is fundamentally confused about dynamic linking, which apache has to use because it doesn't know what type of file it has to handle, with linking, which is bound to the application code. pgtypes is bound to the application code so it is not like apache --- an application isn't going to be fed arbitrary pgtypes function calls it has to handle. This discussion is now completely pointless as we have conceeded to a separate library. The community has spoken. We are trying to move on and open a discussion on the hook design. there are numbered questions: http://archives.postgresql.org/pgsql-hackers/2008-04/msg00565.php -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: Andrew Chernow [EMAIL PROTECTED] writes: There is no need to pass hookData to the hook function. libpqtypes already accesses PGconn and PGresult directly so it can just access the hookData member. That's a habit you'd really be best advised to stop, if you're going to be a separate library. Otherwise, any time we make an internal change in the PGconn struct, it'll break your library --- and we have and will feel free to do that without an external soname change. What parts of PGconn/PGresult do you need to touch that aren't exposed already? regards, tom lane Well, we manually create a result for arrays and composites. We also use pqResultAlloc in several places. I will have to look at the code again but I'm not sure we can be completely abstracted from the result struct. If you are suggesting that libpqtypes should not access internal libpq, than this idea won't work. We can pull all the code out and hook in, as you suggested, but we had no plans of abstracting from internal libpq. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: What parts of PGconn/PGresult do you need to touch that aren't exposed already? Don't need direct access to PGconn at all. result: null_field tupArrSize client_encoding (need a PGconn for this which might be dead) pqSetResultError pqResultAlloc pqResultStrdup Also, we basically need write access to every member inside a result object ... since we create our own for arrays and composites by copying the standard result members over. /* taken from dupresult inside handlers/utils.c */ PGresult *r = (PGresult *)malloc(sizeof(PGresult)); memset(r, 0, sizeof(PGresult)); /* copy some data from source result */ r-binary = args-get.result-binary; r-resultStatus= args-get.result-resultStatus; r-noticeHooks = args-get.result-noticeHooks; r-client_encoding = args-get.result-client_encoding; strcpy(r-cmdStatus, args-get.result-cmdStatus); [snip...] We can read args-get.result properties using PQfuncs with no problem. But we have no way of assign these values to our result 'r'. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Alvaro Herrera [EMAIL PROTECTED] writes: Andrew Chernow wrote: The core of what I am trying to ask is, there doesn't appear to be an advantage to separating libpqtypes from libpq in terms of space. My guess is that if we provide an useful library, Redhat will distribute it some way or another. The key phrase in that being some way or another. Red Hat works with a concept of core vs extras (or another way to look at it being what comes on the CDs vs what you have to download from someplace). I think it's highly likely that libpgtypes would end up in extras. If you do not make it possible to package it that way (ie, separately from libpq), it's more likely that it won't get packaged at all than that it will be put in core. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Wed, Apr 9, 2008 at 11:17 AM, Andrew Chernow [EMAIL PROTECTED] wrote: We can read args-get.result properties using PQfuncs with no problem. But we have no way of assign these values to our result 'r'. By the way, our decision to 'create the result' when exposing arrays and composites saved us from creating lot of interface code to access these structures from user code...the result already gave us what we needed. Just in case anybody missed it, the way arrays of composites would be handled in libpq would be like this: PGarray array; PQgetf(res, 0, %foo[], 0, array); // foo being a composite(a int, b float) PQclear(res); for (i = 0; i PQntuples(array.res); i++) { a int; b float; PQgetf(array.res, i, %int %float, 0, a, 1, b); [...] } PQclear(array.res); In the getf call, the brackets tell libpq that this is an array and a new result is created to present the array...one column for simple array, multiple columns if the array is composite. This can be recursed if the composite is nested. We support all the PGget* functions for the newly created array, providing the OIDs of the internal composite elements for example. This is, IMO, payoff of doing all the work with the type handlers...we don't have to make a special version of getf that operates over arrays for example. The upshot of this is that, however separation occurs, the PGresult is a first class citizen to the types library. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: The key phrase in that being some way or another. Red Hat works with a concept of core vs extras (or another way to look at it being what comes on the CDs vs what you have to download from someplace). I think it's highly likely that libpgtypes would end up in extras. If you do not make it possible to package it that way (ie, separately from libpq), it's more likely that it won't get packaged at all than that it will be put in core. regards, tom lane I'll buy this. Better to be safe then sorry. This patch becomes more flexible and distributable when separated. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow [EMAIL PROTECTED] writes: If you are suggesting that libpqtypes should not access internal libpq, than this idea won't work. We can pull all the code out and hook in, as you suggested, but we had no plans of abstracting from internal libpq. That's exactly what I'm strongly suggesting. If you need to include libpq-int.h at all, then your library will be forever fragile, and could very well end up classified as don't ship this at all, it's too likely to break. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, 2008-04-08 at 21:49 -0400, Merlin Moncure wrote: * an escapeIdent function. not sure what this is... Similar to the quote_ident() function in postgresql: = select quote_ident('foo'); quote_ident - foo (1 row) It could be called something like PQquoteIdent or PQescapeIdent. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: That's exactly what I'm strongly suggesting. If you need to include libpq-int.h at all, then your library will be forever fragile, and could very well end up classified as don't ship this at all, it's too likely to break. regards, tom lane I see your point, and it has a lot of merit. We am completely open to hearing how this can be solved. How do we duplicate a result object and customize many member values after the dup? Do we create a PGresultInfo struct containing all members of a result object that gets passed to PGresuolt *PQresultDup(PGresult *source, PGresultInfo *info);? Maybe it has a flags member that indicates which PQresultInfo members contain values that should override the source result. Any suggestions? This is where we are stumped. Everything else has several solutions. We are not debating this anymore, we are trying to implement it. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow [EMAIL PROTECTED] writes: What parts of PGconn/PGresult do you need to touch that aren't exposed already? Don't need direct access to PGconn at all. Oh, good, that makes things much easier. Also, we basically need write access to every member inside a result object ... since we create our own for arrays and composites by copying the standard result members over. Hmm. I guess it wouldn't be completely out of the question to expose the contents of PGresult as part of libpq's API. We haven't changed it often, and it's hard to imagine a change that wouldn't be associated with a major-version change anyhow. We could do some things to make it a bit more bulletproof too, like change cmdStatus from fixed-size array to pointer to allocated space so that CMDSTATUS_LEN doesn't become part of the API. Alternatively, we could keep it opaque and expose a bunch of manipulator routines, but that might be a lot more cumbersome than it's worth. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: Hmm. I guess it wouldn't be completely out of the question to expose the contents of PGresult as part of libpq's API. We haven't changed it often, and it's hard to imagine a change that wouldn't be associated with a major-version change anyhow. We could do some things to make it a bit more bulletproof too, like change cmdStatus from fixed-size array to pointer to allocated space so that CMDSTATUS_LEN doesn't become part of the API. Alternatively, we could keep it opaque and expose a bunch of manipulator routines, but that might be a lot more cumbersome than it's worth. regards, tom lane How about a proxy header (if such an animal exists). Maybe it is possible to take pg_result, and all structs it references, and put it in result-int.h. libpq-int.h would obviously include this. Also, any external library, like libpqtypes, that need direct access to a result can include result-int.h. This keeps pg_result and referenced structs out of libpq-fe.h. From a libpq user's viewpoint, nothing has changed. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: But I'll agree that cross-version hazards are a much more clear and present danger. We've already broken binary compatibility at least once since the current binary-I/O system was instituted (intervals now have three fields not two) and there are obvious candidates for future breakage, such as text locale/encoding support. But isn't that an argument *for* having support for the binary format in libpq in a form similar to what this patch offers? Then at least you'd be safe as long as your libpq-version is = your server version. Currently, there seems to be no safe way to use the binary format, despite it's benefits for moving around large amounts of data. regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Florian Pflug [EMAIL PROTECTED] writes: But isn't that an argument *for* having support for the binary format in libpq in a form similar to what this patch offers? Then at least you'd be safe as long as your libpq-version is = your server version. Currently, there seems to be no safe way to use the binary format, That's right, there isn't, and it's folly to imagine that anything like this will make it safe. What we really put in the binary I/O support for was for COPY BINARY, with a rather limited use-case of fast dump and reload between similar server versions (you'll notice pg_dump does NOT use it). Yeah, we expose it for clients to use, but they had better understand that they are increasing their risk of cross-version portability problems. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow [EMAIL PROTECTED] writes: Tom Lane wrote: Hmm. I guess it wouldn't be completely out of the question to expose the contents of PGresult as part of libpq's API. How about a proxy header (if such an animal exists). A separate header might be a good idea to discourage unnecessary reliance on the struct, but it doesn't change the basic fact that we'd be adding the struct to our ABI and couldn't change it without a major library version number bump. Perhaps we could do a partial exposure, where the exported struct declaration contains public fields and there are some private ones after that. This would still work for external creation of PGresults, so long as all PGresults are initially manufactured by PQmakeEmptyPGresult. That would give us a little bit of wiggle room ... in particular I'd be very tempted not to expose the fields associated with space allocation (we could export pqResultAlloc instead), nor anything not foreseeably needed by libpgtypes. Maybe it is possible to take pg_result, and all structs it references, and put it in result-int.h. I'd think something like libpq-result.h would be a better choice of name. The other seems likely to collide with who-knows-what from other packages, regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: Perhaps we could do a partial exposure, where the exported struct declaration contains public fields and there are some private ones after that. I have another idea. It would remove a boat load of members that would need to be exposed (may remove them all). Can we make a variant of PQmakeEmptyPGresult? Maybe something like this: PGresult *PQdupPGresult( // maybe not the best name? PGconn *conn, PGresult *source, int numAttributes, int ntups); This starts off by calling PQmakeEmptyPGresult and then copying the below members from the provided 'source' result argument. - ExecStatusType resultStatus; - char cmdStatus[CMDSTATUS_LEN]; - int binary; - PGNoticeHooks noticeHooks; - int client_encoding; It would then preallocate attDescs and tuples which would also set numAttributes, ntups tupArrSize. attdescs and tuples are not duplicated, only allocated based on the 'numAttributes' and 'ntups' arguments. Now libpqtypes only needs direct access to attDescs and tuples, for when it loops array elements or composite fields and copies stuff from the source result. Any interest in this direction? I am still thinking about how to abstract attDesc and tuples, I think it would require more API calls as well. curBlock, curOffset and spaceLeft were never copied (intialized to zero). We don't touch these. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: Tom Lane wrote: Perhaps we could do a partial exposure, where the exported struct declaration contains public fields and there are some private ones after that. I have another idea. It would remove a boat load of members that would need to be exposed (may remove them all). Can we make a variant of PQmakeEmptyPGresult? Maybe something like this: Here is a quick implementation demonstrating the idea. It is very similar to the patches internal dupresult function (handlers/utils.c). /* numParameters, paramDescs, errFields, curBlock, curOffset and spaceLeft * are not assigned at all, initialized to zero. errMsg is handled by * PQmakeEmptyPGresult. */ PGresult *PQdupPGresult( PGconn *conn, PGresult *source, int numAttributes, int ntups) { PGresult *r; if(!source || numAttributes 0 || ntups 0) return NULL; r = PQmakeEmptyPGresult(conn, source-resultStatus); if(!r) return NULL; r-binary = source-binary; strcpy(r-cmdStatus, source-cmdStatus); /* assigned by PQmakeEmptyPGresult when conn is not NULL */ if(!conn) { r-noticeHooks = source-noticeHooks; r-client_encoding = source-client_encoding; } r-attDescs = (PGresAttDesc *) pqResultAlloc(r, numAttributes * sizeof(PGresAttDesc), TRUE); if(!r-attDescs) { PQclear(r); return NULL; } r-numAttributes = numAttributes; r-tuples = (PGresAttValue **) malloc(ntups * sizeof(PGresAttValue *)); if(!r-tuples) { PQclear(r); return NULL; } r-ntups = ntups; r-tupArrSize = ntups; return r; } -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
I know there has been lots of versions and technical feedback related to this proposed feature. However, I have talked to Tom and neither of us see sufficient user request for this capability to add this code into the core server. I recommend you place it on pgfoundry and see if you can get a sufficient user-base there. If you need something exposed by libpq that is not there already, please let us know. One interesting idea would be if ecpg could use this functionality in place of its own type-specific functions --- if that were the case, we could reconsider adding this to core because it would be required by ecpg. Sorry for the bad news. I think we all hoped that enough interest would be generated for this to be accepted. --- Andrew Chernow wrote: Andrew Chernow wrote: Joe Conway wrote: Alvaro Herrera 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. +1 Joe What is gained by having a separate library? Our changes don't bloat the library size so I'm curious what the benefits are to not linking with it? If someone doesn't want to use, they don't have to. Similar to the backend, there is stuff in there I personally don't use (like geo types), but I'm not sure that justifies a link option -lgeotypes. The changes we made are closely tied to libpq's functionality. Adding PQputf to simplify the parameterized API, adding PQgetf to compliment PQgetvalue and added the ability to register user-defined type handlers (used by putf and getf). PQgetf makes extensive use of PGresult's internal API, especially for arrays and composites. Breaking this into a separate library would require an external library to access the private internals of libpq. Personally, I am not really in favor of this idea because it breaks apart code that is very related. Although, it is doable. I poked around to see how this would work. There are some problems. 1. members were added to PGconn so connection-based type handler information can be copied to PGparam and PGresult objects. 2. members were added to PGresult, referenced in #1. To properly getf values, the connection-based type handler information must be available to PGresult. Otherwise, PQgetf would require an additional argument, a PGconn, which may have been closed already. 3. PQfinish calls pqClearTypeHandler to free type info assigned to the PGconn. 4. PQclear also calls pqClearTypeHandlers It would also remove some of the simplicity. Creating a connection would no longer initialized type info, which gets copied to PGparam and PGresult. Type info includes a list of built-in handlers and backend config, like integer_datetimes, server-version, etc... That means an additional function must be called after PQconnectdb. But where would the type info be stored? It wouldn't exist in PGconn anymore? Also, this would require double frees. You have to free the result as well as the type info since they are no longer one object. Same holds true for a pgconn. There is something elegant about not requiring additional API calls to perform a putf or getf. It'll just work if you want to use it. You can use PQgetf on a result returned by PQexec and you can use PQputf, PQparamExec followed by PQgetvalue. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ -- Sent via pgsql-patches mailing list ([EMAIL PROTECTED]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Bruce Momjian wrote: I know there has been lots of versions and technical feedback related to this proposed feature. However, I have talked to Tom and neither of us see sufficient user request for this capability to add this code into the core server. I recommend you place it on pgfoundry and see if you can get a sufficient user-base there. If you need something exposed by libpq that is not there already, please let us know. One interesting idea would be if ecpg could use this functionality in place of its own type-specific functions --- if that were the case, we could reconsider adding this to core because it would be required by ecpg. Sorry for the bad news. I think we all hoped that enough interest would be generated for this to be accepted. I think you should conduct a wider survey before you make that decision. In particular, I'd like to hear from driver writers like Greg Sabino Mullane and Jeff Davis, as well as regular libpq users. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Dunstan wrote: Bruce Momjian wrote: I know there has been lots of versions and technical feedback related to this proposed feature. However, I have talked to Tom and neither of us see sufficient user request for this capability to add this code into the core server. I recommend you place it on pgfoundry and see if you can get a sufficient user-base there. If you need something exposed by libpq that is not there already, please let us know. One interesting idea would be if ecpg could use this functionality in place of its own type-specific functions --- if that were the case, we could reconsider adding this to core because it would be required by ecpg. Sorry for the bad news. I think we all hoped that enough interest would be generated for this to be accepted. I think you should conduct a wider survey before you make that decision. In particular, I'd like to hear from driver writers like Greg Sabino Mullane and Jeff Davis, as well as regular libpq users. Well, they are welcome to chime in but the patch has been out the for a while and they haven't spoken yet. We have to base our decisions on people who do chime in. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Dunstan [EMAIL PROTECTED] writes: I think you should conduct a wider survey before you make that decision. In particular, I'd like to hear from driver writers like Greg Sabino Mullane and Jeff Davis, as well as regular libpq users. Well, the survey's already been taken, pretty much: there's been just about no positive feedback in all the time that this proposal's been discussed. I haven't noticed anyone except Andrew and Merlin saying that they wanted this or would use it. Currently there's about 400K of C source code in libpq. The patch as-is adds more than 100K, and it would surely get larger if we actively pursued this line of development. (For one thing, the density of comments in the submitted patch is well below what I'd find acceptable; by the time it was commented to a level comparable to the existing libpq code, it'd be a lot more than 100K.) That's a pretty big increment for a facility that it appears would be used only by a small minority of users. I think that at minimum we'd have to insist on it being refactored as a separate library, and then the case for it being in core rather than on pgfoundry seems kinda weak. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 I think you should conduct a wider survey before you make that decision. In particular, I'd like to hear from driver writers like Greg Sabino Mullane and Jeff Davis, as well as regular libpq users. I can state that there would be almost zero chance this would ever be used by DBD::Pg, as it would seem to add overhead with no additional functionality over what we already have. Unless I'm misreading what it does and someone can make the case why I should use it. - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200804081349 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAkf7sGkACgkQvJuQZxSWSsi8FgCgkGUGh2ieOAtvNlXX6orjO8oc 0bIAoPF7ojxM1c38kw7+L4Ar7FRZmdrn =U2BM -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, Apr 8, 2008 at 12:59 PM, Bruce Momjian [EMAIL PROTECTED] wrote: Sorry for the bad news. I think we all hoped that enough interest would be generated for this to be accepted. I think that's really unfortunate. Personally, I think that anyone who did any amount of C coding against libpq at all would never have any reason to code in the traditional fashion (PQexec, etc). Anyone who would claim otherwise IMO does not code vs. libpq or does not completely understand what we are trying to do. In particular, I think that the decision to so quickly shut the door on the ability to support arrays and composites in binary on the client side. Contrary to what is stated there have been considerable requests for this on the various lists. I am dismayed that throughout this process there has been no substantive discussion (save for Tom) on what we were trying to do, only to be informed about rejection based on an internal discussion. What issues were raised were opaque and sans reasoning or justification of how that would improve the patch or the functionality (move to separate library for example -- how would this improve things?). Our follow ups were not followed up. We would have been delighted to take suggestions. I attributed the silence to general lack of interest and anticipated this response. However I think that those involved should step back and take a look at what they are walking away from here. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, Apr 8, 2008 at 1:51 PM, Greg Sabino Mullane [EMAIL PROTECTED] wrote: I think you should conduct a wider survey before you make that decision. In particular, I'd like to hear from driver writers like Greg Sabino Mullane and Jeff Davis, as well as regular libpq users. I can state that there would be almost zero chance this would ever be used by DBD::Pg, as it would seem to add overhead with no additional functionality over what we already have. Unless I'm misreading what it does and someone can make the case why I should use it. does DBD::pg move arrays in and out of the server? do you parse them yourself? if you answer yes to either question you should take a second look. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Merlin Moncure escribió: I attributed the silence to general lack of interest and anticipated this response. However I think that those involved should step back and take a look at what they are walking away from here. I suggest you take a survey on a more widely read forum, like pgsql-general or even pgsql-announce. Keep in mind that many of the most active people in pgsql-hackers is not actually writing libpq programs (I know I am not.) -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Greg Sabino Mullane wrote: -BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 I think you should conduct a wider survey before you make that decision. In particular, I'd like to hear from driver writers like Greg Sabino Mullane and Jeff Davis, as well as regular libpq users. I can state that there would be almost zero chance this would ever be used by DBD::Pg, as it would seem to add overhead with no additional functionality over what we already have. Unless I'm misreading what it does and someone can make the case why I should use it. - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200804081349 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAkf7sGkACgkQvJuQZxSWSsi8FgCgkGUGh2ieOAtvNlXX6orjO8oc 0bIAoPF7ojxM1c38kw7+L4Ar7FRZmdrn =U2BM -END PGP SIGNATURE- This idea is for the libpq user, although driver writers could find it handy as well. Really, anyone who uses libpq directly. That's the real audience. I don't know what overhead greg is referring to. How is DBD::pg handling arrays of composites? Are you parsing text output? Wouldn't it be less overhead to avoid text parsing and transmit binary data? no additional functionality over what we already have What about user-defined type registration, sub-classing user or built-in type handlers, handling of all data types in binary. There is a lot of new functionality added by this patch to the existing libpq. I don't think the appropriate audience got a look at this, maybe posting on general or libpq lists. From my perspective as a long time C coder, this made my application code cleaner, easier to maintain and faster in many cases. It removed a lot of code that is now handled by this patch. I am not sure why Tom is worried about source code size, normally the concern is linked size. Code comments were never finished, as the library was changing so much to meet some requests. Instead, we focused on providing API documentation and the overall idea (over 1000 lines). This changed much less than the implementation. I think the real issue is simply the wrong audience. Its the coder in the field making heavy use of libpq that would find this appealing, not really backend hackers. It is disappointing because I was excited to here ideas from others, which never happened. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Merlin Moncure [EMAIL PROTECTED] writes: On Tue, Apr 8, 2008 at 1:51 PM, Greg Sabino Mullane [EMAIL PROTECTED] wrote: I can state that there would be almost zero chance this would ever be used by DBD::Pg, as it would seem to add overhead with no additional functionality over what we already have. Unless I'm misreading what it does and someone can make the case why I should use it. does DBD::pg move arrays in and out of the server? do you parse them yourself? if you answer yes to either question you should take a second look. Better support for arrays and composites is certainly something that people might want, but the problem with this design is that it forces them to buy into a number of other decisions that they don't necessarily want. I could see adding four functions to libpq that create and parse the textual representations of arrays and records. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, 08 Apr 2008 14:34:51 -0400 Andrew Chernow [EMAIL PROTECTED] wrote: I am not sure why Tom is worried about source code size, normally the concern is linked size. Code comments were never finished, as the Every byte added is a byte maintained (or not). Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: Better support for arrays and composites is certainly something that people might want, but the problem with this design is that it forces them to buy into a number of other decisions that they don't necessarily want. I could see adding four functions to libpq that create and parse the textual representations of arrays and records. Well, that was the part that interested me, so let me now speak up in favor of better array/record support in libpq. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, Apr 8, 2008 at 2:49 PM, Andrew Dunstan [EMAIL PROTECTED] wrote: Better support for arrays and composites is certainly something that people might want, but the problem with this design is that it forces them to buy into a number of other decisions that they don't necessarily want. I could see adding four functions to libpq that create and parse the textual representations of arrays and records. Well, that was the part that interested me, so let me now speak up in favor of better array/record support in libpq. by the way, we handle both text and binary array results...and getting things in binary is _much_ faster. not to mention text is destructive. for example, composite types in text do not return the oid of composite member fields. with our patch, since you can 'pop' a result of a returned composite, or array of composite, you have access to all that information in the result api. so I would argue that allowing text only parsing only recovers a portion of the provided functionality. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: Better support for arrays and composites is certainly something that people might want, but the problem with this design is that it forces them to buy into a number of other decisions that they don't necessarily want. regards, tom lane What decisions are we forcing upon the libpq user? Well, most of the functionality is handled by about 3 functions (putf, getf, and paramexec). The difference is, our patch is not limited to only handling text arrays and composites. It can do it all, which we thought would of been a requirement to get approved. There is a performance boost to handling arrays and composites in binary, which we use a lot because there are no stored procedures (note, not trying to take a jab about stored procedures, just giving an example of how we use and abuse arrays and composites). -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Joshua D. Drake [EMAIL PROTECTED] writes: On Tue, 08 Apr 2008 14:34:51 -0400 Andrew Chernow [EMAIL PROTECTED] wrote: I am not sure why Tom is worried about source code size, normally the concern is linked size. Code comments were never finished, as the Every byte added is a byte maintained (or not). Actually I was thinking more about disk footprint. Andrew's comment is correct if you work with statically linked code where the compiler pulls out only the needed .o files from a .a library, but that's pretty out of fashion these days. Most people are dealing with a monolithic libpq.so and might carp a bit if it gets 25% or 50% bigger for stuff that doesn't interest them. Perhaps I'm overly sensitive to this because I'm tuned into Red Hat's constant struggles to fit a Linux distribution onto a reasonable number of CDs ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: Joshua D. Drake [EMAIL PROTECTED] writes: On Tue, 08 Apr 2008 14:34:51 -0400 Andrew Chernow [EMAIL PROTECTED] wrote: I am not sure why Tom is worried about source code size, normally the concern is linked size. Code comments were never finished, as the Every byte added is a byte maintained (or not). Actually I was thinking more about disk footprint. Andrew's comment is correct if you work with statically linked code where the compiler pulls out only the needed .o files from a .a library, but that's pretty out of fashion these days. Most people are dealing with a monolithic libpq.so and might carp a bit if it gets 25% or 50% bigger for stuff that doesn't interest them. Perhaps I'm overly sensitive to this because I'm tuned into Red Hat's constant struggles to fit a Linux distribution onto a reasonable number of CDs ... Also, if we add to libpq we have to document this new functionality. It doesn't make sense to add to the API unless there is a significant number of people who will use it. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow [EMAIL PROTECTED] writes: Tom Lane wrote: Better support for arrays and composites is certainly something that people might want, but the problem with this design is that it forces them to buy into a number of other decisions that they don't necessarily want. What decisions are we forcing upon the libpq user? Well, for starters, using binary format. It is undeniable that that creates more portability risks (cross-architecture and cross-PG-version issues) than text format. Not everyone wants to take those risks for benefits that may not be meaningful for their apps. The other forced decision is the whole PQputf/PQgetf notation, which people may or may not find natural --- it still seems a pretty poor choice to me for this specific problem. PQputf, in the form where you're generating a SQL command string along with some parameters, isn't too unreasonable, but unless you've already bought into binary parameter handling it's not gaining all that much either. And PQgetf is just weird. The format of a PGresult is generally well known by the app; trying to force it into the model of string scanning is a poor fit. For instance there's no mapping at all for what to do with constant parts of the format string. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, Apr 8, 2008 at 3:10 PM, Bruce Momjian [EMAIL PROTECTED] wrote: Actually I was thinking more about disk footprint. Andrew's comment is correct if you work with statically linked code where the compiler pulls out only the needed .o files from a .a library, but that's pretty out of fashion these days. Most people are dealing with a monolithic libpq.so and might carp a bit if it gets 25% or 50% bigger for stuff that doesn't interest them. on my box, the .so went from 118k to 175k. while this is significant in percentage terms, I don't think redhat is going to complain about 57k (correct me if I'm wrong here). Also, if we add to libpq we have to document this new functionality. It doesn't make sense to add to the API unless there is a significant number of people who will use it. We are prepared to write the formal documentation if necessary (and whatever else, code comments and a proper regression test for example). I'll address the 'who will want to use it' in response to Tom's mail. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Merlin Moncure wrote: I attributed the silence to general lack of interest and anticipated this response. However I think that those involved should step back and take a look at what they are walking away from here. Agreed. There are technical issues, but they can be addressed with work. The more fundamental issue is whether we want this functionality in libpq vs pgfoundry, and with a lack of interest, as you stated, having it on pgfoundry seems the only logical direction. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, Apr 8, 2008 at 3:22 PM, Tom Lane [EMAIL PROTECTED] wrote: Andrew Chernow [EMAIL PROTECTED] writes: Tom Lane wrote: Better support for arrays and composites is certainly something that people might want, but the problem with this design is that it forces them to buy into a number of other decisions that they don't necessarily want. What decisions are we forcing upon the libpq user? Well, for starters, using binary format. It is undeniable that that creates more portability risks (cross-architecture and cross-PG-version issues) than text format. Not everyone wants to take those risks for benefits that may not be meaningful for their apps. personally, I think these claims of portability are a bit overblown...they are trivially checked by regressions tests. the version question is more interesting and solutions range from the easy (force version match to access binary types) to the complex. The other forced decision is the whole PQputf/PQgetf notation, which people may or may not find natural --- it still seems a pretty poor choice to me for this specific problem. PQputf, in the form where you're generating a SQL command string along with some parameters, isn't too unreasonable, but unless you've already bought into binary parameter handling it's not gaining all that much either. And PQgetf is just weird. The format of a PGresult is generally well known by the app; trying to force it into the model of string scanning is a poor fit. For instance there's no mapping at all for what to do with constant parts of the format string. We chose to do the PQgetf function that way as a compromise of many different requirements (we went through several alternative implementations before settling on the final one). At minimum, you have to supply at least _some_ type information because getf writes into application memory, and aside from the obvious safety issues, there are user defined types to consider. Since you can register additional type into the handler system at runtime (including 'automagic' discovery of composite types by name), there has to be some way of directing the library to the proper handler. Since everything is driven by typename, it all comes down to how you want to do this. We like the varargs style since it provides some symmetry with putf and feels 'right' in coding. The only other approach to getf that meets all the requirements would be like this: PQgetf(result, tup_num, date, 0, date); // not varargs where the typestring is fixed to one type...you could only pull out one parameter at a time (no variable argument list). We considered this carefully, and we opted for the submitted approach as the best solution. One other benefit to doing things this way is we get to alter the 'format escape character, (%)', to # when lookup by name as opposed to column position is desired. The constant part of the format string in getf are ignored, they have no meaning in that context. Same with putf. Although, there is PQputvf which will take something like this: select %int4 + %int4 and turn it into select $1 + $2. In PQputvf, everything surrounding the type modifier is still ignored. So, the API is symmetrical in terms of type string, except that for query execution there is an extra stage where the query string is mapped. What you find to be weird here I think is rather elegant :-). PQputf/PQgetf notation, which people may or may not find natural --- it still seems a pretty poor choice to me for this specific problem We reference types by there name (can include schema as well). There is no better mnemonic than that ... is there? For example, if I have some funky type 'foo' defined on the server, what is more natural than pulling that type with %foo (which can be optionally schema qualified)? By the way, we ended up with the current implementation based on your concerns with previous attempts (using two letter format codes, or exported static type handlers)...(we really like how things turned out though...using schema qualified type names as the 'typespec' really makes the libpq code look 'pretty'. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, Apr 08, 2008 at 02:34:51PM -0400, Andrew Chernow wrote: This idea is for the libpq user, although driver writers could find it handy as well. Really, anyone who uses libpq directly. That's the real audience. Quite, I'm writing array parsing code right now and this would make my life much easier too. However, my question is: I am not sure why Tom is worried about source code size, normally the concern is linked size. How tight is the link to libpq? Could it exist as a seperate library: libpqbin or something? Still in core, just only used by the people who want it. Have a nice day, -- Martijn van Oosterhout [EMAIL PROTECTED] http://svana.org/kleptog/ Please line up in a tree and maintain the heap invariant while boarding. Thank you for flying nlogn airlines. signature.asc Description: Digital signature
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Martijn van Oosterhout wrote: -- Start of PGP signed section. On Tue, Apr 08, 2008 at 02:34:51PM -0400, Andrew Chernow wrote: This idea is for the libpq user, although driver writers could find it handy as well. Really, anyone who uses libpq directly. That's the real audience. Quite, I'm writing array parsing code right now and this would make my life much easier too. However, my question is: I am not sure why Tom is worried about source code size, normally the concern is linked size. How tight is the link to libpq? Could it exist as a seperate library: libpqbin or something? Still in core, just only used by the people who want it. The idea of pgfoundry was that it would be an independent library and could be used by people who need it. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Martijn van Oosterhout wrote: How tight is the link to libpq? Could it exist as a seperate library: libpqbin or something? Still in core, just only used by the people who want it. I gave this a lot of thought and I do think we could abstract this. The idea is to complie it in or out. Add a --with-typesys to configure, which could enable #ifdef LIBPQ_ENABLE_TYPESYS everywhere. If you don't specify --with-typesys, the API calls would still be there but would return ENOSYS, assign an error string or something. This preserves link capatability. This would trim out the 50k everyone is worried about :) I'm sure there are other ways to acocmplish this, but this one seems easiest and keeps it all centralized. Just like --with-openssl, except that loads libcrypto.so. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: Martijn van Oosterhout wrote: How tight is the link to libpq? Could it exist as a seperate library: libpqbin or something? Still in core, just only used by the people who want it. I gave this a lot of thought and I do think we could abstract this. The idea is to complie it in or out. Add a --with-typesys to configure, which could enable #ifdef LIBPQ_ENABLE_TYPESYS everywhere. If you don't specify --with-typesys, the API calls would still be there but would return ENOSYS, assign an error string or something. This preserves link capatability. This would trim out the 50k everyone is worried about :) I'm sure there are other ways to acocmplish this, but this one seems easiest and keeps it all centralized. Just like --with-openssl, except that loads libcrypto.so. Forgot to say: There is stuff in PGconn, PGresult, PQclear, PQfinish (maybe a couple other places). A separate library would remove the ability to call PQexec followed by PQgetf because the result object is no longer aware of the typesys. You would need the separate library to wrap the result object or something: typesysResult = TypeSysGetResult(PQexec()); Or, you need to wrap the libpq API calls, typesysResult = TypeSysExec();. Both are doable but not nearly as slick as: res = PQexec; PQgetf(res, ..); PQclear(res); -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Merlin Moncure [EMAIL PROTECTED] writes: Actually I was thinking more about disk footprint. Andrew's comment is correct if you work with statically linked code where the compiler pulls out only the needed .o files from a .a library, but that's pretty out of fashion these days. Most people are dealing with a monolithic libpq.so and might carp a bit if it gets 25% or 50% bigger for stuff that doesn't interest them. on my box, the .so went from 118k to 175k. while this is significant in percentage terms, I don't think redhat is going to complain about 57k (correct me if I'm wrong here). Yipes, 48% growth in the binary already? That's much higher than I'd expected from my quick source-code count, even with the scarcity of comments. What happens by the time the feature actually becomes mature? Yes, I could see Red Hat complaining about that. I'd rather have libpq (as it currently stands) included in core RHEL, and libpqtypes as an optional extra, than have a monolithic library get booted out to extras altogether. It could happen. Don't think they don't see us as a secondary objective ... mysql still has a lot more mindshare. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow [EMAIL PROTECTED] writes: I gave this a lot of thought and I do think we could abstract this. The idea is to complie it in or out. [shrug...] So the packagers will compile it out, and you're still hosed, or at least any users who'd like to use it are. Forgot to say: There is stuff in PGconn, PGresult, PQclear, PQfinish (maybe a couple other places). I didn't see anything there that seemed that it *had* to be inside libpq, and certainly not anything that couldn't be handled with a small hook or two. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Bruce Momjian wrote: Martijn van Oosterhout wrote: How tight is the link to libpq? Could it exist as a seperate library: libpqbin or something? Still in core, just only used by the people who want it. The idea of pgfoundry was that it would be an independent library and could be used by people who need it. I don't think phasing it out to pgfoundry is a good idea, because it has some dependency on the OIDs of datatypes. Besides, it is likely that it could be used by ecpg instead of it having its own PGTYPES stuff. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: Andrew Chernow [EMAIL PROTECTED] writes: I gave this a lot of thought and I do think we could abstract this. The idea is to complie it in or out. [shrug...] So the packagers will compile it out, and you're still hosed, or at least any users who'd like to use it are. Forgot to say: There is stuff in PGconn, PGresult, PQclear, PQfinish (maybe a couple other places). I didn't see anything there that seemed that it *had* to be inside libpq, and certainly not anything that couldn't be handled with a small hook or two. regards, tom lane How about using dlopen and dlsym? Sseparate the library as many are suggesting. But leave the functions, the tid-bits in PGconn, PGresult, etc... in there (2 or 3 typedefs would be needed but all data-type typedefs PGtimestamp can be removed). You need something inside the PGconn and PGresult, even if just a function pointer that gets called if not NULL. Yank pqtypes function bodies, and related helper functions, and replace them with something like below: int PQputf(...) { #ifdef HAVE_DLOPEN if(pqtypes-putf) return pqtypes-putf(...); return 1; /* failed, PGparam error = not enabled */ #else return 1; /* failed, PGparam error = cannot load dynamically */ #endif } Then add a PQtypesEnable(bool), which would dlopen(libpqtypes) and dlsym the 10 functions or so we need. Any typedefs you need would be in libpqtypes.h rather than libpq-fe.h. You could disable it as well, which would unload libpqtypes. The default would obviously be disabled. The entire patch would be one small file with a couple 1 line changes to PGconn and PGresult. This would remove all overhead, at least 95% of it. couldn't be handled with a small hook or two. Maybe, have not thought of that. The problem, is that I am trying to make avoid having to keep track of two different APIs. The goal is the libpq user is coding to the libpq API, not some other API like PGTypesExec. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: Forgot to say: There is stuff in PGconn, PGresult, PQclear, PQfinish (maybe a couple other places). Maybe there's a way we can have libpqtypes adding calls into some hypothetical libpq hooks. So libpqtypes registers its hooks in _init() or some such, and it gets picked up automatically by any app that links to it. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Alvaro Herrera wrote: Andrew Chernow wrote: Forgot to say: There is stuff in PGconn, PGresult, PQclear, PQfinish (maybe a couple other places). Maybe there's a way we can have libpqtypes adding calls into some hypothetical libpq hooks. So libpqtypes registers its hooks in _init() or some such, and it gets picked up automatically by any app that links to it. Kinda what my last suggestion was. Some tid-bits need to be reside in libpq, but very little. I was thinking PQtypesEnable(bool) which would dlopen libpqtypes and map all functions needed. This would leave the function bodies of PQputf, PQgetf, PQparamExec, etc... as simple proxy functions to the dynamically loaded functions. This removes any bloat that people don't like right now but still allows one to use libpq as the primary interface, rather than having to fiddle with libpq and some other API. -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: Alvaro Herrera wrote: Andrew Chernow wrote: Forgot to say: There is stuff in PGconn, PGresult, PQclear, PQfinish (maybe a couple other places). Maybe there's a way we can have libpqtypes adding calls into some hypothetical libpq hooks. So libpqtypes registers its hooks in _init() or some such, and it gets picked up automatically by any app that links to it. Kinda what my last suggestion was. Some tid-bits need to be reside in libpq, but very little. I was thinking PQtypesEnable(bool) which would dlopen libpqtypes and map all functions needed. This would leave the function bodies of PQputf, PQgetf, PQparamExec, etc... as simple proxy functions to the dynamically loaded functions. This removes any bloat that people don't like right now but still allows one to use libpq as the primary interface, rather than having to fiddle with libpq and some other API. Please make sure that any scheme you have along these lines will work on Windows DLLs too. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Dunstan wrote: Please make sure that any scheme you have along these lines will work on Windows DLLs too. Ofcourse: LoadLibrary(), GetProcAddress(), __declspec(dllexport). -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, Apr 8, 2008 at 6:09 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: Andrew Chernow wrote: Forgot to say: There is stuff in PGconn, PGresult, PQclear, PQfinish (maybe a couple other places). Maybe there's a way we can have libpqtypes adding calls into some hypothetical libpq hooks. So libpqtypes registers its hooks in _init() or some such, and it gets picked up automatically by any app that links to it. I've been having some thoughts along those lines as well. As currently written there is a already a de facto 'init' as the various static type handlers are assembled into the type handlers for the built in types. About 50% of the patch as written is libpq plumbing which extends the API, defines the structures, and various things like that. IMO, there is very little point to abstracting that out into a separate library. The 50% is the type handlers and various assorted conversion functions. This half will only increase as we introduce new types and other conversions. I think, if there is some reasonable case for separation, it is here (the type handlers)...and I think this might present a reasonable approach for dealing with version compatibility issues. One thought I had was that a 8.4 libpq would be able to load the 8.3 types when dealing with a 8.3 server for example. Maybe this is a non-starter, but it's one case where splitting things out might have some other advantages. If this works the way I'm thinking about it, it would probably better than a compile time flag...I don't think that satisfies anyway since hardly anyone compiles their own libpq any more (presumably everyone would just compile it in, making the check moot). Anyone with an appreciation for irony and a long memory will recall me griping about the odbc driver compiling openssl in, because it required me to package in a bunch of extra dlls on windows :-). In terms of moving the code to pgfoundry, I think this is absolutely the wrong thing to do, except in terms of deciding the patch unsuitable for inclusion into core (or deferring that decision). People that used it would naturally expect changes to happen in concert with the server. Better just to come to a consensus...in, or out :-) merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Alvaro Herrera [EMAIL PROTECTED] writes: Bruce Momjian wrote: The idea of pgfoundry was that it would be an independent library and could be used by people who need it. I don't think phasing it out to pgfoundry is a good idea, because it has some dependency on the OIDs of datatypes. Well, if you'll remind me of the last time we changed the OID of a standard datatype, I'd put some credence in that argument. Besides, it is likely that it could be used by ecpg instead of it having its own PGTYPES stuff. Yeah, Bruce and I were talking about that, but on reflection I'm not sure that there's much potential commonality. The thing that's most problematic about ecpg is that it wants to offer client-side equivalents of some server datatype-manipulation functions; and I don't actually see much of any of that in the proposed patch. All that's really here is format conversion stuff, so there's no hope of unifying that code unless this patch adopts ecpg's choices of client-side representation (which I believe are mostly Informix legacy choices, so I'm not sure we want that). regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow [EMAIL PROTECTED] writes: Kinda what my last suggestion was. Some tid-bits need to be reside in libpq, but very little. I was thinking PQtypesEnable(bool) which would dlopen libpqtypes and map all functions needed. This would leave the function bodies of PQputf, PQgetf, PQparamExec, etc... as simple proxy functions to the dynamically loaded functions. This removes any bloat that people don't like right now but still allows one to use libpq as the primary interface, rather than having to fiddle with libpq and some other API. This is still 100% backwards. My idea of a libpq hook is something that could be used by libpgtypes *and other things*. What you are proposing is something where the entire API of the supposed add-on is hard-wired into libpq. That's just bad design, especially when the adequacy of the proposed API is itself in question. When I say I'd accept some hooks into libpq, I mean some hooks that could be used by either libpgtypes or something that would like to do something roughly similar but with a different API offered to clients. The particular hook that you seem to mostly need is the ability to allocate some private memory associated with a particular PGconn object, and maybe also with individual PGresults, and then to be able to free that at PQclear or PQfinish. Try designing it like that. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: Andrew Chernow [EMAIL PROTECTED] writes: Kinda what my last suggestion was. Some tid-bits need to be reside in libpq, but very little. I was thinking PQtypesEnable(bool) which would dlopen libpqtypes and map all functions needed. This would leave the function bodies of PQputf, PQgetf, PQparamExec, etc... as simple proxy functions to the dynamically loaded functions. This removes any bloat that people don't like right now but still allows one to use libpq as the primary interface, rather than having to fiddle with libpq and some other API. This is still 100% backwards. My idea of a libpq hook is something that could be used by libpgtypes *and other things*. What you are proposing is something where the entire API of the supposed add-on is hard-wired into libpq. That's just bad design, especially when the adequacy of the proposed API is itself in question. When I say I'd accept some hooks into libpq, I mean some hooks that could be used by either libpgtypes or something that would like to do something roughly similar but with a different API offered to clients. The particular hook that you seem to mostly need is the ability to allocate some private memory associated with a particular PGconn object, and maybe also with individual PGresults, and then to be able to free that at PQclear or PQfinish. Try designing it like that. regards, tom lane My idea was not a response to your hook idea. It was different altogether. My idea is trying to create one interface where some parts need to be enabled (nothing wrong with that design, this is a plugin-like model). Your idea creates two interfaces where one of them can hook into the other. These are two different concepts. the question is, are you asking for two different interfaces or are you just looking to minimize overhead. I thought it was the latter which is why I proposed a plugin-like model. It removes bloat as well as maintains a single interface. Nothing wrong with the design unless it doesn't meet your requirements. Andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Tom Lane wrote: This is still 100% backwards. My idea of a libpq hook is something that could be used by libpgtypes *and other things*. What you are proposing is something where the entire API of the supposed add-on is hard-wired into libpq. That's just bad design, especially when the adequacy of the proposed API is itself in question. When I say I'd accept some hooks into libpq, I mean some hooks that could be used by either libpgtypes or something that would like to do something roughly similar but with a different API offered to clients. The particular hook that you seem to mostly need is the ability to allocate some private memory associated with a particular PGconn object, and maybe also with individual PGresults, and then to be able to free that at PQclear or PQfinish. Try designing it like that. Agreed. A minimal change to libpq has a much better chance of being accepted. Let me remind people that the community is not required to accept any patch --- it is accepted based on community feedback. If we accepted everything our API would be a mess and Postgres would be much harder to use. I think a wise thing would be for the patch submitters to give a _basic_ outline of what PQparam is trying to accomplish --- I mean like 10-20 lines with a code snippet, or code snippet with/withouth PQparam. Right now there are too many people trying to guess. Of course, this should have been done at the start. I found this posting from August, 2007 but it isn't short/clear enough: http://archives.postgresql.org/pgsql-hackers/2007-08/msg00630.php I feel this API is foreign enough from what people expect from a typical database interface library that it should be a separate library with its own documentation, whether the library is a separate directory in core or in pgfoundry. FYI, it might be interesting to extend it to cover what ecpg wants --- we have been looking for a way to get the database-dependent parts of ecpg out of the ecpg directory and this might be a solution, _even_ if it makes your library larger. Again, I don't think we have trouble with another library, assuming it does something that many people want to do and it is so tied to the backend that it needs to be in core. (However, Tom's mention that the OIDs don't change weakens the logic that is should be in core.) -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: My idea was not a response to your hook idea. It was different altogether. My idea is trying to create one interface where some parts need to be enabled (nothing wrong with that design, this is a plugin-like model). Your idea creates two interfaces where one of them can hook into the other. These are two different concepts. the question is, are you asking for two different interfaces or are you just looking to minimize overhead. I thought it was the latter which is why I proposed a plugin-like model. It removes bloat as well as maintains a single interface. Nothing wrong with the design unless it doesn't meet your requirements. The idea is that the hooks you need in libpq would be available always, for your interface and for others. This would allow your library to use native libpq on any =8.4 platform. People who want to use your library would have to link in your library and call your additional functions. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: When I say I'd accept some hooks into libpq, I mean some hooks that could be used by either libpgtypes or something that would like to do something roughly similar but with a different API offered to clients. The particular hook that you seem to mostly need is the ability to allocate some private memory associated with a particular PGconn object, and maybe also with individual PGresults, and then to be able to free that at PQclear or PQfinish. Try designing it like that. regards, tom lane Your method would work as well. The only issue is you still have the same issue of binary distributed libpqs. Would redhat distribute a binary linked with libpqtypes? If not, you have the same issue of the end-user having to compile libpq ... passing -lpqtypes to the linker. If redhat did link it, you run into the disk space complaint all over again. My suggestion was trying to work around this by dynamically loading the library, PQtypesEnable(TRUE). In this model, redhat doesn't even have to distribute libpqtypes.so (considering the disk space issue). It could be easily be an additional download. All you need are some proxy functions inside libpq, PQputf calling a dynamically loaded function. This passes the disk space complaints and doesn't require a re-compile if an end-user wants to use it. Andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: Andrew Chernow wrote: When I say I'd accept some hooks into libpq, I mean some hooks that could be used by either libpgtypes or something that would like to do something roughly similar but with a different API offered to clients. The particular hook that you seem to mostly need is the ability to allocate some private memory associated with a particular PGconn object, and maybe also with individual PGresults, and then to be able to free that at PQclear or PQfinish. Try designing it like that. regards, tom lane Your method would work as well. The only issue is you still have the same issue of binary distributed libpqs. Would redhat distribute a binary linked with libpqtypes? If not, you have the same issue of the end-user having to compile libpq ... passing -lpqtypes to the linker. If redhat did link it, you run into the disk space complaint all over again. My suggestion was trying to work around this by dynamically loading the library, PQtypesEnable(TRUE). In this model, redhat doesn't even have to distribute libpqtypes.so (considering the disk space issue). It could be easily be an additional download. All you need are some proxy functions inside libpq, PQputf calling a dynamically loaded function. This passes the disk space complaints and doesn't require a re-compile if an end-user wants to use it. Why would RedHat need to know anything at all about libpqtypes? AIUI Tom is suggesting an API that in libpq that libpqtypes or some other library could use, but not that libpq would be linked against libpqtypes at all. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Andrew Chernow wrote: Your method would work as well. The only issue is you still have the same issue of binary distributed libpqs. Would redhat distribute a binary linked with libpqtypes? If not, you have the same issue of the end-user having to compile libpq ... passing -lpqtypes to the linker. If redhat did link it, you run into the disk space complaint all over again. My suggestion was trying to work around this by dynamically loading the library, PQtypesEnable(TRUE). In this model, redhat doesn't even have to distribute libpqtypes.so (considering the disk space issue). It could be easily be an additional download. All you need are some proxy functions inside libpq, PQputf calling a dynamically loaded function. This passes the disk space complaints and doesn't require a re-compile if an end-user wants to use it. I don't see requiring users to add -lpqtypes to use these functions as a problem. The idea is that the default libpq would have enough hooks that you could use it without modification. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 Andrew states: What about user-defined type registration, sub-classing user or built-in type handlers, handling of all data types in binary. There is a lot of new functionality added by this patch to the existing libpq. All of which may be useful, and may not. Right now DBD::Pg is trying to move further from libpq, rather than binding closer to it, mainly because it's a huge dependency burden and it takes forever to add new features. For example, if this got accepted and put into 8.4, the very earliest DBD::Pg could take advantage of the code would be mid 2009 (assuming an optimistic release schedule). Even then, only once 8.4 was widely in use (2010?) would the average DBD::Pg be compiled against it. And that means lots more forking to handle the old version until then. But don't take my non-use personally: as you say, this may be more helpful towards C programmers. I only weighed in because I was asked to specifically. I don't think the appropriate audience got a look at this, maybe posting on general or libpq lists. From my perspective as a long time C coder, this made my application code cleaner, easier to maintain and faster in many cases. It removed a lot of code that is now handled by this patch. Did you even post on -interfaces? I would think that would be the first place to post, certainly before -patches. Merlin asks: does DBD::pg move arrays in and out of the server? do you parse them yourself? if you answer yes to either question you should take a second look. Yes, we move them into and out of the server, and into and out of Perl arrays in the process. I'd be happy to give the new way a try however: have you any sample code and documentation? Emphasis on the latter. - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200804082025 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iEYEAREDAAYFAkf8DcIACgkQvJuQZxSWSshk/QCfS8Ocsoqo6U/LPNhnz9OH9Bm4 83MAoOvsSWkKuyE+LA1UhLpyX0bICQbZ =2XsW -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Bruce Momjian wrote: I don't see requiring users to add -lpqtypes to use these functions as a problem. The idea is that the default libpq would have enough hooks that you could use it without modification. You are correct, brain fart on my part. Not sure where my mind was at but I scratch those commit about redhat linking in libpqtypes. Sorry about that. I think the hook idea would work. Have to look at composites and arrays, they create their own result objects from scratch. Andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
I think a wise thing would be for the patch submitters to give a _basic_ outline of what PQparam is trying to accomplish --- I mean like 10-20 lines with a code snippet, or code snippet with/withouth PQparam. I found this posting from August, 2007 but it isn't short/clear enough: That is very old. There are tons of examples if you download the patch and look at some of the documentation .txt files. FYI, it might be interesting to extend it to cover what ecpg wants --- we have been looking for a way to get the database-dependent parts of ecpg out of the ecpg directory and this might be a solution, _even_ if it makes your library larger. I am not all to familiar with ecpg. Our idea is nothing more than data type converters, there are no type operators. Our goal is to leverage the binary parameterized API and offer the ability to get C types from an enhanced PQgetvalue, PQgetf. PQgetf understands the external binary format of the backend, so it can convert that to C types. It also understand how to convert text output from the backend. This allows existing applications using text results to drop in PQgetf here and there. You don't have to use PGparam or PQputf at all to use PQgetf. PQputf, allows one to pack parameters into a PQparam object that can be executed at another time. It knows how to take C types and convert them to the backend's external binary format. The patch always sends data in binary format, but can get results in text or binary. Along the way, we devised a way for user-defined types to be used. This introduced PQregisterTypeHandler. This function can allow one to sub-class existing type handlers, like extending %timestamp and making %epoch. It allows registering user-defined types. the type will be looked up in the backend and resolved properly. It also allows one to create aliases .. simple domains. Let's say you have a C type, typedef int user_id;. You could register an alias where user_id=int4. Now you can putf and getf %user_id. this abstracts code from possible integer width changes, you just change your typedef and registration code to user_id=int8. Take a peak at the documentation files in the patch, root of tar *.txt files. they are very verbose and have loads of examples. The regression-test.c file has lots of use cases. latest: http://www.esilo.com/projects/postgresql/libpq/typesys-0.9b.tar.gz To sum it up, the main concepts are PQputf, PQgetf and PQregisterTypeHandler. The other functions maintain a PGparam or exec/send one. -- Andrew Chernow eSilo, LLC 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, 2008-04-08 at 13:08 -0400, Andrew Dunstan wrote: I think you should conduct a wider survey before you make that decision. In particular, I'd like to hear from driver writers like Greg Sabino Mullane and Jeff Davis, as well as regular libpq users. I looked into this today. * Arrays and composites Better ability in libpq to parse and construct arrays and composites would be helpful. I think this is worthwhile to consider, and I would expose the functionality (at least for arrays) in ruby-pg if available. * Binary formatting The exclusive use of binary formats is worrisome to me. This circumvents one level of indirection that we have (i.e. that everything moves through in/out functions), and will impose a backwards-compatibility requirement on the internal representation of data types, including user-defined data types. As far as I know, we currently have no such requirement, even for built-in types. * Type conversion callbacks The type conversion hooks make some sense, but I have reservations about that policy as well. The data types in PostgreSQL will never match perfectly the data types in a host language -- NULL in particular doesn't have a direct counterpart. If there were a perfect mapping of types, it would seem like a much better idea. The problem is that we don't want to have some types that are perfectly mapped (e.g. int, bytea), some that are imperfectly mapped (e.g. dates, numeric), and some types that have no conversion defined and fall back to something else. In this case, the result format is always binary, so we can't even fall back to text. In my experience trying to implicitly map to types doesn't save a lot of time for the developer. You end up spending time referencing the documentation (or some other part of the code) to figure out how the edge cases are being handled rather than just handling them explicitly in the code. For instance, wrapping a value you expect to be a date in Date.parse() is more readable in most cases. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
This patch has an identity crisis. We initially called it PGparam (possibly mispelled several times as PQparam) and then changed it to libpq type system (typesys). Several on patches started to call it libpqtypes, even I did. Any objections to fixing the name to libpqtypes? Any thoughts on the hooking suggested by Tom? It sounds like it should be generic enough so more than just libpqtypes can make use of it. I think something of this nature should have input before I do anything. Possible Hook points: (at least ones needed by libpqtypes) conn_create conn_reset conn_destroy result_create result_destroy I guess libpqtypes would have to maintain a map of conns and results? Right now it can associate type info because we added members to conn and result. When conn_create(conn) is called, libpqtypes would need to map this by pointer address (as it is all it has as an identifier). Still feels like maybe there should be a void* in a conn and result used for per-connection/result based info (libpqtypes or not). -- 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
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, 2008-04-08 at 12:59 -0400, Bruce Momjian wrote: If you need something exposed by libpq that is not there already, please let us know. The things that I found missing in libpq so far are: * The ability to choose some result columns to be binary-formatted and others to be text-formatted. I haven't thought of a reasonable API for this yet, particularly since we already have so many ways of executing a statement. * an escapeIdent function. * PQescapeBytea escapes for both inclusion in a SQL statement and also the escaping for the input function for bytea. This means that, if you are passing a binary value as a text-format parameter, using PQescapeBytea is incorrect, and you need to write your own octal escape routine. This is obviously a minor complaint, and may not even be worth polluting the namespace. I only mention it because it seems like an easy mistake to make. I don't have a proposal ready for any of these yet, but this is what I've found to be awkward or limiting during my work on ruby-pg. If I have a more complete proposal I'll start a new thread. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, Apr 8, 2008 at 8:06 PM, Bruce Momjian [EMAIL PROTECTED] wrote: I think a wise thing would be for the patch submitters to give a _basic_ outline of what PQparam is trying to accomplish --- I mean like 10-20 lines with a code snippet, or code snippet with/withouth PQparam. Right now there are too many people trying to guess. Of course, this should have been done at the start. I found this posting from August, 2007 but it isn't short/clear enough: see [dated jan 8]: http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg102719.html merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, Apr 8, 2008 at 9:28 PM, Jeff Davis [EMAIL PROTECTED] wrote: with proposed changes, (I think) all your suggestions are addressed/moot. see: * The ability to choose some result columns to be binary-formatted and others to be text-formatted. I haven't thought of a reasonable API for this yet, particularly since we already have so many ways of executing a statement. with PQgetf, you are abstracted from resultformat. The library converts your data for you into consistent types (in practice, the result format is always binary)...without the 'negatives' of dealing with binary data. * an escapeIdent function. not sure what this is... * PQescapeBytea escapes for both inclusion in a SQL statement and also the escaping for the input function for bytea. This means that, if you are passing a binary value as a text-format parameter, using PQescapeBytea is incorrect, and you need to write your own octal escape routine. This is obviously a minor complaint, and may not even be worth polluting the namespace. I only mention it because it seems like an easy mistake to make. PQescapeBytea is unnecessary with proposed patch...input parameters are moved to proper format by the library. You simply %bytea the parameter and let the library handle the rest. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, 2008-04-08 at 15:22 -0400, Tom Lane wrote: Well, for starters, using binary format. It is undeniable that that creates more portability risks (cross-architecture and cross-PG-version issues) than text format. Not everyone wants to take those risks for benefits that may not be meaningful for their apps. What are the cross-architecture risks involved? Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Jeff Davis wrote: On Tue, 2008-04-08 at 15:22 -0400, Tom Lane wrote: Well, for starters, using binary format. It is undeniable that that creates more portability risks (cross-architecture and cross-PG-version issues) than text format. Not everyone wants to take those risks for benefits that may not be meaningful for their apps. What are the cross-architecture risks involved? Regards, Jeff Davis What are the cross-architecture risks involved? We didn't run into any issues here. close attention was paid to byte ordering, the rest was handled by libpq's cross-platform handling. cross-PG-version is a different story. the patch already uses server version to toggle (like use int4 for money pre-8.3). If we make this a separate library, it would be easy to plug in a newer or older one. Merlin had some ideas here ... Merlin? Andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
On Tue, Apr 8, 2008 at 9:21 PM, Jeff Davis [EMAIL PROTECTED] wrote: I looked into this today. * Arrays and composites Better ability in libpq to parse and construct arrays and composites would be helpful. I think this is worthwhile to consider, and I would expose the functionality (at least for arrays) in ruby-pg if available. Right..that was the principle objective. In our particular case we move a lot of data in and out via arrays, and in certain case moving data in and out via binary is a huge performance win. * Binary formatting The exclusive use of binary formats is worrisome to me. This circumvents one level of indirection that we have (i.e. that everything moves through in/out functions), and will impose a backwards-compatibility requirement on the internal representation of data types, including user-defined data types. As far as I know, we currently have no such requirement, even for built-in types. This is a valid concern. That said, the in/out functions don't change _that_ much, and even if they did..there are solutions to this problem. Forwards compatibility is the worst case (8.4 libpq connecting to 8.5 server). If this problem was reasonably addressed though, would it alleviate your concerns? * Type conversion callbacks The type conversion hooks make some sense, but I have reservations about that policy as well. The data types in PostgreSQL will never match perfectly the data types in a host language -- NULL in particular doesn't have a direct counterpart. If there were a perfect mapping of types, it would seem like a much better idea. The problem is that we don't want to have some types that are perfectly mapped (e.g. int, bytea), some that are imperfectly mapped (e.g. dates, numeric), and some types that have no conversion defined and fall back to something else. In this case, the result format is always binary, so we can't even fall back to text. We introduced types for which there were no native counterparts...PGtime for example, This choice of which types map and which don't is fairly arbitrary, but not terrible from libpq perspective. We could for example typedef int to PGint, or PGint4 to make things more consistent, I suppose. I would suggest that if you want text from the database, cast it as such in the query and pull it with %text. In my experience trying to implicitly map to types doesn't save a lot of time for the developer. You end up spending time referencing the documentation (or some other part of the code) to figure out how the edge cases are being handled rather than just handling them explicitly in the code. For instance, wrapping a value you expect to be a date in Date.parse() is more readable in most cases. There are many types where this is not the case. Consider the geometric types for example. There is little reason to pull these in text and farily large performance benefits to retrieving in binary. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCHES] libpq type system 0.9a
Jeff Davis [EMAIL PROTECTED] writes: On Tue, 2008-04-08 at 15:22 -0400, Tom Lane wrote: Well, for starters, using binary format. It is undeniable that that creates more portability risks (cross-architecture and cross-PG-version issues) than text format. Not everyone wants to take those risks for benefits that may not be meaningful for their apps. What are the cross-architecture risks involved? The biggie is floating-point format. IEEE standard is not quite universal ... and even for platforms that fully adhere to that standard, it's not entirely clear that we get the endianness issues correct. There used to be platforms where FP and integer endianness were different; is anyone sure that's no longer the case? But I'll agree that cross-version hazards are a much more clear and present danger. We've already broken binary compatibility at least once since the current binary-I/O system was instituted (intervals now have three fields not two) and there are obvious candidates for future breakage, such as text locale/encoding support. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers