[HACKERS] Patches applied; initdb time!
I've applied patches to implement an int64-based data/time storage scheme. I've also accumulated some other minor fixes, which result in an initdb being required (sorry!). Note that the *default* timestamp type is now TIMESTAMP WITHOUT TIME ZONE. This is what we discussed previously for the transition to SQL9x compliance. Full cvs log entry is included below. - Thomas Support alternate storage scheme of 64-bit integer for date/time types. Use --enable-integer-datetimes in configuration to use this rather than the original float8 storage. I would recommend the integer-based storage for any platform on which it is available. We perhaps should make this the default for the production release. Change timezone(timestamptz) results to return timestamp rather than a character string. Formerly, we didn't have a way to represent timestamps with an explicit time zone other than freezing the info into a string. Now, we can reasonably omit the explicit time zone from the result and return a timestamp with values appropriate for the specified time zone. Much cleaner, and if you need the time zone in the result you can put it into a character string pretty easily anyway. Allow fractional seconds in date/time types even for dates prior to 1BC. Limit timestamp data types to 6 decimal places of precision. Just right for a micro-second storage of int8 date/time types, and reduces the number of places ad-hoc rounding was occuring for the float8-based types. Use lookup tables for precision/rounding calculations for timestamp and interval types. Formerly used pow() to calculate the desired value but with a more limited range there is no reason to not type in a lookup table. Should be *much* better performance, though formerly there were some optimizations to help minimize the number of times pow() was called. Define a HAVE_INT64_TIMESTAMP variable. Based on the configure option --enable-integer-datetimes and the existing internal INT64_IS_BUSTED. Add explicit date/interval operators and functions for addition and subtraction. Formerly relied on implicit type promotion from date to timestamp with time zone. Change timezone conversion functions for the timetz type from timetz() to timezone(). This is consistant with other time zone coersion functions for other types. Bump the catalog version to 200204201. Fix up regression tests to reflect changes in fractional seconds representation for date/times in BC eras. All regression tests pass on my Linux box. ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] Patches applied; initdb time!
btw, I've updated the regression tests and results for my platform, but other platforms (e.g. Solaris) will need their results files updated... - Thomas ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Patches applied; initdb time!
I'm seeing half a dozen gcc warnings as a result of these patches. Do you want to fix 'em, or shall I? regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/users-lounge/docs/faq.html
Re: [HACKERS] Patches applied; initdb time!
I'm seeing half a dozen gcc warnings as a result of these patches. Do you want to fix 'em, or shall I? Where are they? I haven't noticed anything in the files I have changes; are the warnings elsewhere? - Thomas ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] Patches applied; initdb time!
Thomas Lockhart [EMAIL PROTECTED] writes: I'm seeing half a dozen gcc warnings as a result of these patches. Do you want to fix 'em, or shall I? Where are they? With fairly vanilla configure options, I get make[3]: Entering directory `/home/postgres/pgsql/src/backend/parser' gcc -O1 -Wall -Wmissing-prototypes -Wmissing-declarations -g -I../../../src/include -c -o gram.o gram.c gram.y:6688: warning: `set_name_needs_quotes' defined but not used make[3]: Entering directory `/home/postgres/pgsql/src/backend/commands' gcc -O1 -Wall -Wmissing-prototypes -Wmissing-declarations -g -I../../../src/include -c -o sequence.o sequence.c In file included from sequence.c:25: ../../../src/include/utils/int8.h:33: warning: `INT64CONST' redefined ../../../src/include/utils/pg_crc.h:83: warning: this is the location of the previous definition gcc -O1 -Wall -Wmissing-prototypes -Wmissing-declarations -g -I../../../src/include -c -o variable.o variable.c variable.c: In function `parse_datestyle': variable.c:262: warning: `rstat' might be used uninitialized in this function variable.c:264: warning: `value' might be used uninitialized in this function make[4]: Entering directory `/home/postgres/pgsql/src/backend/utils/adt' gcc -O1 -Wall -Wmissing-prototypes -Wmissing-declarations -g -I../../../../src/include -c -o selfuncs.o selfuncs.c In file included from selfuncs.c:95: ../../../../src/include/utils/int8.h:33: warning: `INT64CONST' redefined ../../../../src/include/utils/pg_crc.h:83: warning: this is the location of the previous definition Seems not good to have INT64CONST separately defined in int8.h and pg_crc.h. Offhand I'd either move it into c.h, or else consider that int8.h is the Right Place for it and make pg_crc.h include int8.h. regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] Patches applied; initdb time!
I'm seeing half a dozen gcc warnings as a result of these patches. Where are they? More specifically, the *only* compiler warning I see (other than the usual yacc/lex symbol warnings) is that a routine in gram.y, set_name_needs_quotes(), is defined but not used. Don't know where that routine came from, and afaik I didn't accidentally remove a reference when trying to merge changes... - Thomas ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] Patches applied; initdb time!
With fairly vanilla configure options, I get... Please be specific on the options and platform. I do *not* see these warnings here with my fairly vanilla configure options ;) Can't fix what I can't see, and we should track down what interactions are happening to get these variables exposed... btw, the INT64CONST must be defined for int8 (which is where I get the definition for the date/time stuff); not sure why it appears in two separate places and not sure why my compiler (gcc-2.96.xxx) does not notice it. - Thomas ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/users-lounge/docs/faq.html
Re: [HACKERS] Patches applied; initdb time!
Thomas Lockhart [EMAIL PROTECTED] writes: btw, I've updated the regression tests and results for my platform, but other platforms (e.g. Solaris) will need their results files updated... I committed a fix for HPUX's horology file, and did some extrapolation to produce a Solaris version; someone please verify that it's OK. regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] Patches applied; initdb time!
Thomas Lockhart [EMAIL PROTECTED] writes: With fairly vanilla configure options, I get... Please be specific on the options and platform. HPUX 10.20, ./configure --with-CXX --with-tcl --enable-cassert regards, tom lane ---(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] Patches applied; initdb time!
Thomas Lockhart wrote: With fairly vanilla configure options, I get... Please be specific on the options and platform. I do *not* see these warnings here with my fairly vanilla configure options ;) Can't fix what I can't see, and we should track down what interactions are happening to get these variables exposed... btw, the INT64CONST must be defined for int8 (which is where I get the definition for the date/time stuff); not sure why it appears in two separate places and not sure why my compiler (gcc-2.96.xxx) does not notice it. I just built from cvs tip using: ./configure --enable-integer-datetimes --enable-locale --enable-debug --enable-cassert --enable-multibyte --enable-syslog --enable-nls --enable-depend and got: gram.y:6688: warning: `set_name_needs_quotes' defined but not used variable.c: In function `parse_datestyle': variable.c:262: warning: `rstat' might be used uninitialized in this function variable.c:264: warning: `value' might be used uninitialized in this function -- and the usual lexer related warnings -- pgc.c: In function `yylex': pgc.c:1249: warning: label `find_rule' defined but not used pgc.l: At top level: pgc.c:3073: warning: `yy_flex_realloc' defined but not used and pl_scan.c: In function `plpgsql_base_yylex': pl_scan.c:1020: warning: label `find_rule' defined but not used scan.l: At top level: pl_scan.c:2321: warning: `yy_flex_realloc' defined but not used but did *not* get the INT64CONST warning that Tom did. I'm using an updated Red Hat 7.2 box. HTH, Joe ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] Patches applied; initdb time!
Thomas Lockhart [EMAIL PROTECTED] writes: More specifically, the *only* compiler warning I see (other than the usual yacc/lex symbol warnings) is that a routine in gram.y, set_name_needs_quotes(), is defined but not used. Don't know where that routine came from, and afaik I didn't accidentally remove a reference when trying to merge changes... Yeah, you did. However the routine could possibly go away now. It was a hack I put in recently to handle cases like regression=# create schema MySchema; CREATE regression=# create schema MyOtherSchema; CREATE regression=# set search_path TO MySchema, MyOtherSchema; ERROR: SET takes only one argument for this parameter Formerly gram.y merged the list items into a single string, and so it needed to double-quote mixed-case names to prevent case folding when the string got re-parsed later. This example worked last week, and probably would work again if the system were applying your new list-argument logic for search_path ... but I'm not sure where to look to learn about that. regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] Patches applied; initdb time!
With fairly vanilla configure options, I get... Please be specific on the options and platform. HPUX 10.20, ./configure --with-CXX --with-tcl --enable-cassert Boy, how plain-vanilla. *My* configure line is all of ./configure --prefix=/home/thomas/local But I do override some parameters in my Makefile.custom: CFLAGS+= -g -O0 -DUSE_ASSERT_CHECKING CFLAGS+= -DCOPY_PARSE_PLAN_TREES Which gives me (except for the plan tree thing) something very similar. I've looked a bit more, and the set_name_needs_quotes() is probably obsoleted by my update, which generalizes parameter handling in SET variables. I'll rip it out unless we get a test case in the regression tests which demonstrates a problem. I'm pretty sure that it may have allowed SET key='par1 w space,par2'; but that would be handled now by SET key='par1 w space',par2; for cases in which key would accept multiple values. We now can allow single parameters with embedded commas *and* whitespace, which would have been impossible before. Not sure why white space is desirable however, so the new behavior seems adequate to me. I'm still not sure why the INT64CONST conflict does not show up as a warning on my machine, but looking at the code I'm not sure why we would ever have had two versions in the first place. Anyone want to take responsibility for consolidating it into The Right Place? If not, I'll go ahead and do it... - Thomas ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Patches applied; initdb time!
Joe Conway [EMAIL PROTECTED] writes: but did *not* get the INT64CONST warning that Tom did. I'm using an updated Red Hat 7.2 box. Probably it depends on compiler version? I'm using gcc 2.95.3. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Patches applied; initdb time!
Thomas Lockhart [EMAIL PROTECTED] writes: I'm still not sure why the INT64CONST conflict does not show up as a warning on my machine, but looking at the code I'm not sure why we would ever have had two versions in the first place. Anyone want to take responsibility for consolidating it into The Right Place? If not, I'll go ahead and do it... I think it was originally needed only for the CRC code, so we put it there to begin with. Clearly should be in a more widely used place now. Do you have any opinion whether c.h or int8.h is the Right Place? I'm still dithering about that. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Patches applied; initdb time!
Tom Lane wrote: Joe Conway [EMAIL PROTECTED] writes: but did *not* get the INT64CONST warning that Tom did. I'm using an updated Red Hat 7.2 box. Probably it depends on compiler version? I'm using gcc 2.95.3. could be: [postgres@jec-linux pgsql]$ gcc -v Reading specs from /usr/lib/gcc-lib/i386-redhat-linux/2.96/specs gcc version 2.96 2731 (Red Hat Linux 7.1 2.96-98) Joe ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/users-lounge/docs/faq.html
Re: [HACKERS] Patches applied; initdb time!
I think it was originally needed only for the CRC code, so we put it there to begin with. Clearly should be in a more widely used place now. Do you have any opinion whether c.h or int8.h is the Right Place? I'm still dithering about that. In looking at the code, istm that the versions should be merged with features from both. The generated constants should be surrounded in parens, but the explicit coersion to (int64) should be omitted at least with the LL version. I've got some other int64 pushups to worry about; let's try fixing those too (though afaict they may need to happen in different places). At the moment, we have INT64_IS_BUSTED as an amalgam of other conditions or undefined variables. I've also got a HAVE_INT64_TIMESTAMP which comes from a configured variable USE_INTEGER_DATETIMES and an undefined INT64_IS_BUSTED. This is now housed in c.h, but istm that we *should* check for conflicting settings in configure itself, and carry forward a consistant set of parameters from there. Anyway, at the moment some of this stuff is in c.h, and that is probably the right place to put the INT64CONST definitions, at least until things sort out differently. btw, I've updated gram.y and variable.c to suppress the reported warnings (which I *still* don't see here; that is very annoying). - Thomas ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] Patches applied; initdb time!
Thomas Lockhart wrote: btw, I've updated gram.y and variable.c to suppress the reported warnings (which I *still* don't see here; that is very annoying). FWIW, I'm still seeing: gram.y:99: warning: `set_name_needs_quotes' declared `static' but never defined Joe ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Patches applied; initdb time!
FWIW, I'm still seeing: gram.y:99: warning: `set_name_needs_quotes' declared `static' but never defined Ack. Sloppy patching. Should be fixed now... - Thomas ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] Patches applied; initdb time!
Thomas Lockhart writes: But I do override some parameters in my Makefile.custom: CFLAGS+= -g -O0 -DUSE_ASSERT_CHECKING If you use -O0 then you miss most of the interesting warnings. -- Peter Eisentraut [EMAIL PROTECTED] ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] Patches applied; initdb time!
But I do override some parameters in my Makefile.custom: CFLAGS+= -g -O0 -DUSE_ASSERT_CHECKING If you use -O0 then you miss most of the interesting warnings. ?? Not in this case. afaik -O0 suppresses most optimizations (and hence does not reorder instructions, which is why I use it for debugging; I know, debuggers nowadays work pretty well even with instruction reordering, but...). Anyway, compiling with -O2 on variable.c still does not show the warnings with my 2.96.x compiler... - Thomas ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] Patches applied; initdb time!
Thomas Lockhart [EMAIL PROTECTED] writes: But I do override some parameters in my Makefile.custom: CFLAGS+= -g -O0 -DUSE_ASSERT_CHECKING If you use -O0 then you miss most of the interesting warnings. ?? Not in this case. afaik -O0 suppresses most optimizations In particular, you don't get unused variable and variable may not have been set before being used warnings at -O0, because the control-flow analysis needed to emit those warnings is not done at -O0. I generally use -O1 for development; it's sometimes a little hairy stepping through the generated code, but usually gcc works well enough at -O1, and I get the important warnings. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Patches applied; initdb time!
Thomas Lockhart wrote: FWIW, I'm still seeing: gram.y:99: warning: `set_name_needs_quotes' declared `static' but never defined Ack. Sloppy patching. Should be fixed now... - Thomas Yup, did the trick. Thanks, Joe ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] Patches applied; initdb time!
... In particular, you don't get unused variable and variable may not have been set before being used warnings at -O0, because the control-flow analysis needed to emit those warnings is not done at -O0. Right. The point is that I don't get those (apparently) with -O2 either, with my particular compiler. Hmm. Actually, I *do* get those if I make sure that some of the other options are set too; my quick test added -O2 but left out some of the -w switches. OK, never mind... - Thomas ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] Patches applied; initdb time!
Thomas Lockhart wrote: But I do override some parameters in my Makefile.custom: CFLAGS+= -g -O0 -DUSE_ASSERT_CHECKING If you use -O0 then you miss most of the interesting warnings. ?? Not in this case. afaik -O0 suppresses most optimizations (and hence does not reorder instructions, which is why I use it for debugging; I know, debuggers nowadays work pretty well even with instruction reordering, but...). Anyway, compiling with -O2 on variable.c still does not show the warnings with my 2.96.x compiler... It's actually the optimiser that allows a large number of the warnings to be uncovered. It generates extra code-path and coverage information, as well as other things, that are needed for the guts of GCC to squawk about a number of odd behaviours. ---(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] Patches applied; initdb time!
Right. The point is that I don't get those (apparently) with -O2 either, with my particular compiler. Hmm. Actually, I *do* get those if I make sure that some of the other options are set too; my quick test added -O2 but left out some of the -w switches. OK, never mind... btw, now that I've started using -O2, my geometry regression test now passes as though it were the standard linux result. It's been a *long* time since that test passed for me, which probably says that it has been quite a while since I didn't force a -O0... - Thomas ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
[HACKERS] Patches applied
I've applied Neale Ferguson's patches for S/390 support, and some fairly extensive patches to repair and improve support for the OVERLAPS operator. I've increased coverage of this in the regression tests, including horology, so those platforms which have variants on these test results will need to be evaluated and those results updated. initdb required. - Thomas
