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.
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
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
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
Thomas Lockhart <thomas@fourpalms.org> 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
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
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
Thomas Lockhart <thomas@fourpalms.org> 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
Thomas Lockhart <thomas@fourpalms.org> 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
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
Thomas Lockhart <thomas@fourpalms.org> 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
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
Joe Conway <mail@joeconway.com> 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
Thomas Lockhart <thomas@fourpalms.org> 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
Tom Lane wrote:
Joe Conway <mail@joeconway.com> 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 20000731 (Red Hat Linux 7.1 2.96-98)
Joe
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
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
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
But I do override some parameters in my Makefile.custom:
CFLAGS+= -g -O0 -DUSE_ASSERT_CHECKINGIf 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
Import Notes
Reference msg id not found: Pine.LNX.4.30.0204211806270.688-100000@peter.localdomain | Resolved by subject fallback
