Re: [HACKERS] SPI bug.
Neil Conway wrote: We don't currently depend on C99, and not all platforms have a 64-bit datatype. In any case, I'm still unconvinced that using `int' and `long' in backend APIs is a problem. Using long means that you sometimes get a 32-bit value and sometimes a 64-bit value, I think we agree on that. There's no correlation between getting a 64-bit value and the fact that you run on a 64-bit platform since many 64-bit platforms treat a long as 32-bit. I think we agree on that too. If the objective behind using a long is that you get a data-type that followes the CPU register size then that objective is not met. No such data-type exists unless you use C99 intptr_t (an int that can hold a pointer). You could of course explicitly typedef a such in c.h but AFAICS, there is no such definition today. By using a long you will: a) maximize the differences of the SPI interfaces between platforms. b) only enable 64-bit resultset sizes on a limited range of 64-bit platforms. Wouldn't it be better if you: a) Minimized the differences between platforms. b) Made a decision to either use 32- or 64-bit resultset sizes (my preference would be the former) or to conseqently used 32-bit resultset sizes on 32-bit platforms and 64-bit resultset sizes on 64-bit platforms? Regards, Thomas Hallgren ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] SPI bug.
Neil Conway wrote: As I said before, we may or may not want to change the executor itself to use a constant-sized type, but as a matter of interface definition, I think using long makes the most sense. One thing that I forgot. If you indeed will do something like that in the future, the implication is yet another change to the SPI interfaces. Why not decide, once and for all and right now, what the size of this integer should be and then *start* with a change of the interface. The change of the underlying implementation can come later. Now you effectively force a second change that will make things incompatible should you decide to change the executor in the future. Regards, Thomas Hallgren ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] SPI bug.
Neil Conway wrote: My point is that since they are different types, the language itself will need to provide some mechanism for doing this type conversion _anyway_. 'int' and 'long' are used throughout the backend APIs, so I don't see the gain in only converting the SPI functions over to using int32/int64. Mainly because it's easier to do that mapping knowing that the semantics equipped with the involved types never change. There's also a performance issue. I must map a C/C++ long to a 64bit value at all times and thereby get a suboptimal solution. An API should ideally hide the internals of the underlying code so I'm not sure this is a valid reason. Well, the executor allows you to specify a 64-bit count on platforms where long is 64-bit, and a 32-bit count otherwise. Exactly. Why should a user of the SPI API be exposed to or even concerned with this at all? As an application programmer you couldn't care less. You want your app to perform equally well on all platforms without surprises. IMHO, PostgreSQL should make a decision whether the SPI functions support 32-bit or the 64-bit sizes for result sets and the API should reflect that choice. Having the maximum number of rows dependent on platform ports is a bad design. Regards, Thomas Hallgren ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] SPI bug.
Thomas Hallgren [EMAIL PROTECTED] writes: Exactly. Why should a user of the SPI API be exposed to or even concerned with this at all? As an application programmer you couldn't care less. You want your app to perform equally well on all platforms without surprises. IMHO, PostgreSQL should make a decision whether the SPI functions support 32-bit or the 64-bit sizes for result sets and the API should reflect that choice. Having the maximum number of rows dependent on platform ports is a bad design. The fact that 64-bit platforms can tackle bigger problems than 32-bit ones is not a bug to be worked around, and so I don't see any problem with the use of long for tuple counts. Furthermore, we have never promised ABI-level compatibility across versions inside the backend, and we are quite unlikely to make such a promise in the foreseeable future. (Most of the time you are lucky if you get source-level compatibility ;-).) So I can't get excited about avoiding platform dependency in this particular tiny aspect of the API. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] SPI bug.
Tom Lane wrote: Thomas Hallgren [EMAIL PROTECTED] writes: Exactly. Why should a user of the SPI API be exposed to or even concerned with this at all? As an application programmer you couldn't care less. You want your app to perform equally well on all platforms without surprises. IMHO, PostgreSQL should make a decision whether the SPI functions support 32-bit or the 64-bit sizes for result sets and the API should reflect that choice. Having the maximum number of rows dependent on platform ports is a bad design. The fact that 64-bit platforms can tackle bigger problems than 32-bit ones is not a bug to be worked around, and so I don't see any problem with the use of long for tuple counts. I'm not concerned with the use of 32 or 64 bits. I would be equally happy with both. What I am concerned is that the problem that started this SPI bug was caused by the differences in how platforms handle the int and long types. Instead of rectifying this problem once and for all, the type was just changed to a long. Furthermore, we have never promised ABI-level compatibility across versions inside the backend, and we are quite unlikely to make such a promise in the foreseeable future. I know that no promises has been made but PostgreSQL is improved every day and this would be a very easy promise to make. (Most of the time you are lucky if you get source-level compatibility ;-).) So I can't get excited about avoiding platform dependency in this particular tiny aspect of the API. Maybe I've misunderstood the objectives behind the SPI layer altogether but since it's well documented and seems to be the public interface of the backend that extensions are supposed to use, I think it would be an excellent idea to make that interface as stable and platform independent as possible. I can't really see the disadvantages. The use of int, long, and long long is often a source of bugs (as with this one) and many recommend that you avoid them when possible. The definition of int is meant to be a datatype used for storing integers where size of that datatype equals natural size of processor. The long is defined as 'at least as big as int' and the 'long long' is 'bigger than long'. I wonder what that makes 'long long' on a platform where the int is 64 bits. 128 bits? Also, the interpretation of the definition vary between compiler vendors. On Windows Itanium, the int is 32 bit. On Unix it's 64. It's a mess... The 1998 revision of C declares the following types for a good reason: int8_t , int16_t, int32_t int64_t, uint8_t, uint16_t, uint32_t, uint64_t. Why not use them unless you have very specific requirements? And why not *always* use them in a public interface like the SPI? Regards, Thomas Hallgren ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] SPI bug.
Thomas Hallgren wrote: Tom Lane wrote: Furthermore, we have never promised ABI-level compatibility across versions inside the backend, and we are quite unlikely to make such a promise in the foreseeable future. I know that no promises has been made but PostgreSQL is improved every day and this would be a very easy promise to make. Binary compatibility of backend APIs is by no means a very easy promise to make, nor would it be a good idea in any case. Also, the interpretation of the definition vary between compiler vendors. On Windows Itanium, the int is 32 bit. On Unix it's 64. `int' is 32 bit on most modern platforms I can think of. Perhaps you're thinking of `long', which is indeed 64-bit on many 64-bit Unixen but 32-bit on 64-bit Windows (BTW, this likely means that Postgres is completely broken on 64-bit Windows: sizeof(Datum) == sizeof(unsigned long) == 4, sizeof(void *) == 8). The 1998 revision of C declares the following types for a good reason: int8_t , int16_t, int32_t int64_t, uint8_t, uint16_t, uint32_t, uint64_t. We don't currently depend on C99, and not all platforms have a 64-bit datatype. In any case, I'm still unconvinced that using `int' and `long' in backend APIs is a problem. -Neil ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] SPI bug.
Tzahi Fadida wrote: I think the solution can be either changing the FETCH_ALL to INT_MAX or changing the interface parameter count and subsequent usages to long. I think changing SPI_cursor_fetch() and SPI_cursor_move() to take a long for the count parameter is the right fix for HEAD. It would probably not be wise to backport, though, as it is probably not worth breaking ABI compatibility for SPI during a stable release series. -Neil ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] SPI bug.
Neil Conway wrote: I think changing SPI_cursor_fetch() and SPI_cursor_move() to take a long for the count parameter is the right fix for HEAD. Attached is a patch that implements this. A bunch of functions had to be updated: SPI_execute(), SPI_execute_snapshot(), SPI_exec(), SPI_execp(), SPI_execute_plan(), SPI_cursor_fetch(), and SPI_cursor_move(). I also updated PL/Python, which was invoking SPI_execute() with an `int' parameter. PL/Tcl could be updated as well, but it seems the base Tcl package doesn't provide a Tcl_GetLong() function. PL/Perl could also be updated (plperl_spi_exec()), but I don't know XS, so I will leave that to someone else. Barring any objections, I'll apply this to HEAD tomorrow. -Neil Index: doc/src/sgml/spi.sgml === RCS file: /Users/neilc/local/cvs/pgsql/doc/src/sgml/spi.sgml,v retrieving revision 1.40 diff -c -r1.40 spi.sgml *** doc/src/sgml/spi.sgml 29 Mar 2005 02:53:53 - 1.40 --- doc/src/sgml/spi.sgml 1 May 2005 06:29:47 - *** *** 292,298 refsynopsisdiv synopsis ! int SPI_execute(const char * parametercommand/parameter, bool parameterread_only/parameter, int parametercount/parameter) /synopsis /refsynopsisdiv --- 292,298 refsynopsisdiv synopsis ! int SPI_execute(const char * parametercommand/parameter, bool parameterread_only/parameter, long parametercount/parameter) /synopsis /refsynopsisdiv *** *** 423,429 /varlistentry varlistentry ! termliteralint parametercount/parameter/literal/term listitem para maximum number of rows to process or return --- 423,429 /varlistentry varlistentry ! termliterallong parametercount/parameter/literal/term listitem para maximum number of rows to process or return *** *** 598,604 refsynopsisdiv synopsis ! int SPI_exec(const char * parametercommand/parameter, int parametercount/parameter) /synopsis /refsynopsisdiv --- 598,604 refsynopsisdiv synopsis ! int SPI_exec(const char * parametercommand/parameter, long parametercount/parameter) /synopsis /refsynopsisdiv *** *** 627,633 /varlistentry varlistentry ! termliteralint parametercount/parameter/literal/term listitem para maximum number of rows to process or return --- 627,633 /varlistentry varlistentry ! termliterallong parametercount/parameter/literal/term listitem para maximum number of rows to process or return *** *** 963,969 refsynopsisdiv synopsis int SPI_execute_plan(void * parameterplan/parameter, Datum * parametervalues/parameter, const char * parameternulls/parameter, ! bool parameterread_only/parameter, int parametercount/parameter) /synopsis /refsynopsisdiv --- 963,969 refsynopsisdiv synopsis int SPI_execute_plan(void * parameterplan/parameter, Datum * parametervalues/parameter, const char * parameternulls/parameter, ! bool parameterread_only/parameter, long parametercount/parameter) /synopsis /refsynopsisdiv *** *** 1030,1036 /varlistentry varlistentry ! termliteralint parametercount/parameter/literal/term listitem para maximum number of rows to process or return --- 1030,1036 /varlistentry varlistentry ! termliterallong parametercount/parameter/literal/term listitem para maximum number of rows to process or return *** *** 1104,1110 refsynopsisdiv synopsis ! int SPI_execp(void * parameterplan/parameter, Datum * parametervalues/parameter, const char * parameternulls/parameter, int parametercount/parameter) /synopsis /refsynopsisdiv --- 1104,1110 refsynopsisdiv synopsis ! int SPI_execp(void * parameterplan/parameter, Datum * parametervalues/parameter, const char * parameternulls/parameter, long parametercount/parameter) /synopsis /refsynopsisdiv *** *** 1162,1168 /varlistentry varlistentry ! termliteralint parametercount/parameter/literal/term listitem para maximum number of rows to process or return --- 1162,1168 /varlistentry varlistentry ! termliterallong parametercount/parameter/literal/term listitem para maximum number of rows to process or return *** *** 1375,1381 refsynopsisdiv synopsis ! void SPI_cursor_fetch(Portal parameterportal/parameter, bool parameterforward/parameter, int parametercount/parameter) /synopsis /refsynopsisdiv --- 1375,1381 refsynopsisdiv synopsis ! void SPI_cursor_fetch(Portal parameterportal/parameter, bool parameterforward/parameter, long parametercount/parameter) /synopsis /refsynopsisdiv
Re: [HACKERS] SPI bug.
On 2005-05-01, Neil Conway [EMAIL PROTECTED] wrote: Tzahi Fadida wrote: I think the solution can be either changing the FETCH_ALL to INT_MAX or changing the interface parameter count and subsequent usages to long. I think changing SPI_cursor_fetch() and SPI_cursor_move() to take a long for the count parameter is the right fix for HEAD. It would probably not be wise to backport, though, as it is probably not worth breaking ABI compatibility for SPI during a stable release series. While you're at it, how about a way to specify WITH SCROLL for a cursor created in SPI? At the moment, SPI_cursor_open hardwires the scroll option according to the result of ExecSupportsBackwardScan. -- Andrew, Supernews http://www.supernews.com - individual and corporate NNTP services ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] SPI bug.
Neil Conway wrote: Neil Conway wrote: I think changing SPI_cursor_fetch() and SPI_cursor_move() to take a long for the count parameter is the right fix for HEAD. Attached is a patch that implements this. A bunch of functions had to be updated: SPI_execute(), SPI_execute_snapshot(), SPI_exec(), SPI_execp(), SPI_execute_plan(), SPI_cursor_fetch(), and SPI_cursor_move(). I also updated PL/Python, which was invoking SPI_execute() with an `int' parameter. PL/Tcl could be updated as well, but it seems the base Tcl package doesn't provide a Tcl_GetLong() function. PL/Perl could also be updated (plperl_spi_exec()), but I don't know XS, so I will leave that to someone else. Barring any objections, I'll apply this to HEAD tomorrow. Since both int and long are types whos size that vary depending on platform, and since the SPI protocol often interfaces with other languages where the sizes are fixed, wouldn't it be better to use something that is fixed in size here too? I.e. int32 or perhaps int64? Regards, Thomas Hallgren ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] SPI bug.
Thomas Hallgren wrote: Since both int and long are types whos size that vary depending on platform, and since the SPI protocol often interfaces with other languages where the sizes are fixed ISTM there are no languages where the sizes are fixed. In this context, int and long are C and C++ types; types that happen to have the same name but different behavior (e.g. int and long in Java) are not the same type at all. The reason the API types should use long is that the underlying executor APIs (e.g. ExecutorRun()) use long. It might be a good idea to change the executor stuff to use int64s -- then I'd have no issue with making a corresponding change to the SPI APIs. I guess the main objection to doing this is that a 64-bit integral type is not available on all platforms (at least in theory; are there any platforms we care about that don't have one?) -Neil ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] SPI bug.
Neil Conway wrote: Thomas Hallgren wrote: Since both int and long are types whos size that vary depending on platform, and since the SPI protocol often interfaces with other languages where the sizes are fixed ISTM there are no languages where the sizes are fixed. In this context, int and long are C and C++ types; types that happen to have the same name but different behavior (e.g. int and long in Java) are not the same type at all. I fully agree that an int and long in Java is very different from an int or long in C/C++. Hence my proposal :-) What I meant was that SPI will interface with languages where there is no correspondence to a type who's size varies depending on platform and that it therefore would be better to chose a type who's size will not vary. The reason the API types should use long is that the underlying executor APIs (e.g. ExecutorRun()) use long. An API should ideally hide the internals of the underlying code so I'm not sure this is a valid reason. I would instead say that An API should remain consistent over the range of platforms where it is supported. Especially if the intention with this API is to make the life easier for PL/some language authors. It might be a good idea to change the executor stuff to use int64s -- then I'd have no issue with making a corresponding change to the SPI APIs. I guess the main objection to doing this is that a 64-bit integral type is not available on all platforms (at least in theory; are there any platforms we care about that don't have one?) I'm sure there is some obscure platform where this matters. I don't know of one though and in my world there isn't. The Java Native Interface (JNI) uses the jlong type and it's required to be 64 bit. If PostgreSQL could be made to rely the int64, then we could get rid of the integer-datetimes conditional also. That would be nice. For this purpose I wonder if there's a need to use int64's though. An int32 covers extremely huge result-sets. But perhaps I'm not visionary enough. I still remember the days when 640Kb RAM should be enough for all foreseeable future :-) Regards, Thomas Hallgren ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] SPI bug.
Thomas Hallgren wrote: What I meant was that SPI will interface with languages where there is no correspondence to a type who's size varies depending on platform and that it therefore would be better to chose a type who's size will not vary. My point is that since they are different types, the language itself will need to provide some mechanism for doing this type conversion _anyway_. 'int' and 'long' are used throughout the backend APIs, so I don't see the gain in only converting the SPI functions over to using int32/int64. An API should ideally hide the internals of the underlying code so I'm not sure this is a valid reason. Well, the executor allows you to specify a 64-bit count on platforms where long is 64-bit, and a 32-bit count otherwise. ISTM the most straightforward way to expose this to clients is to just make the parameter a long. As I said before, we may or may not want to change the executor itself to use a constant-sized type, but as a matter of interface definition, I think using long makes the most sense. BTW, patch applied to HEAD. -Neil ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[HACKERS] SPI bug.
Hi, While trying to determine if SPI_cursor_move can rewind (and receiving a great help from the guys at the irc), we found out that since the count parameter is int and FETCH_ALL is LONG_MAX then setting the count parameter to FETCH_ALL to rewind will not work on 64bit systems. On my pIII 32 bit system it works since int size=long size. I am using 8.0.2 (i.e. the repositioning bug is already fixed here). I think the solution can be either changing the FETCH_ALL to INT_MAX or changing the interface parameter count and subsequent usages to long. (FETCH_ALL at parsenodes.h) Regards, tzahi. WARNING TO SPAMMERS: see at http://members.lycos.co.uk/my2nis/spamwarning.html ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
