Re: [PATCHES] [HACKERS] roundoff problem in time datatype
Patch applied, with documentation updates. --- Tom Lane wrote: Inserting into a time field with limited precision rounds off, which is good except for this case: regression=# select '23:59:59.9'::time(0); time -- 24:00:00 (1 row) This is bad because: regression=# select '24:00:00'::time(0); ERROR: date/time field value out of range: 24:00:00 which means that data originally accepted will fail to dump and reload. I see this behavior in all versions back to 7.3. 7.2 was even more broken: regression=# select '23:59:59.9'::time(0); time -- 00:00:00 (1 row) I think the correct behavior has to be to check for overflow again after rounding off. Alternatively: why are we forbidding the value 24:00:00 anyway? Is there a reason not to allow the hours field to exceed 23? regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/datatype.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/datatype.sgml,v retrieving revision 1.160 diff -c -c -r1.160 datatype.sgml *** doc/src/sgml/datatype.sgml 2 Oct 2005 23:50:06 - 1.160 --- doc/src/sgml/datatype.sgml 14 Oct 2005 03:09:07 - *** *** 1368,1374 entry8 bytes/entry entrytimes of day only/entry entry00:00:00.00/entry ! entry23:59:59.99/entry entry1 microsecond / 14 digits/entry /row row --- 1368,1374 entry8 bytes/entry entrytimes of day only/entry entry00:00:00.00/entry ! entry24:00:00/entry entry1 microsecond / 14 digits/entry /row row *** *** 1376,1382 entry12 bytes/entry entrytimes of day only, with time zone/entry entry00:00:00.00+12/entry ! entry23:59:59.99-12/entry entry1 microsecond / 14 digits/entry /row /tbody --- 1376,1382 entry12 bytes/entry entrytimes of day only, with time zone/entry entry00:00:00.00+12/entry ! entry24:00:00-12/entry entry1 microsecond / 14 digits/entry /row /tbody Index: src/backend/utils/adt/datetime.c === RCS file: /cvsroot/pgsql/src/backend/utils/adt/datetime.c,v retrieving revision 1.158 diff -c -c -r1.158 datetime.c *** src/backend/utils/adt/datetime.c9 Oct 2005 17:21:46 - 1.158 --- src/backend/utils/adt/datetime.c14 Oct 2005 03:09:09 - *** *** 1114,1120 * Check upper limit on hours; other limits checked in * DecodeTime() */ ! if (tm-tm_hour 23) return DTERR_FIELD_OVERFLOW; break; --- 1114,1122 * Check upper limit on hours; other limits checked in * DecodeTime() */ ! /* test for 24:00:00 */ ! if (tm-tm_hour 24 || !(tm-tm_hour == 24 (tm-tm_min 0 || tm-tm_sec 0))) return DTERR_FIELD_OVERFLOW; break; *** *** 2243,2256 else if (mer == PM tm-tm_hour != 12) tm-tm_hour += 12; #ifdef HAVE_INT64_TIMESTAMP ! if (tm-tm_hour 0 || tm-tm_hour 23 || tm-tm_min 0 || ! tm-tm_min 59 || tm-tm_sec 0 || tm-tm_sec 60 || *fsec INT64CONST(0) || *fsec = USECS_PER_SEC) return DTERR_FIELD_OVERFLOW; #else ! if (tm-tm_hour 0 || tm-tm_hour 23 || tm-tm_min 0 || ! tm-tm_min 59 || tm-tm_sec 0 || tm-tm_sec 60 || *fsec 0 || *fsec = 1) return DTERR_FIELD_OVERFLOW; #endif --- 2245,2260 else if (mer == PM tm-tm_hour != 12) tm-tm_hour += 12; + if (tm-tm_hour 0 || tm-tm_min 0 || tm-tm_min 59 || + tm-tm_sec 0 || tm-tm_sec 60 || tm-tm_hour 24 || + /* test for 24:00:00 */ + (tm-tm_hour == 24 (tm-tm_min 0 || tm-tm_sec 0 || #ifdef HAVE_INT64_TIMESTAMP ! *fsec INT64CONST(0))) || *fsec INT64CONST(0) || *fsec =
Re: [PATCHES] [BUGS] BUG #1962: ECPG and VARCHAR
On Thu, Oct 13, 2005 at 09:53:14PM -0400, Bruce Momjian wrote: Good catch! I have backpatched these fixes to the 8.0 and 7.4 branches as you suggested, (identical) patches attached. Thanks Bruce. This was an oversight on my part. I should have committed to 8.0 branch too. I'm sorry about that. On the other hand I agree with Tom. Fortunately it's not a data corruption problem. :-) 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 SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL! ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [BUGS] Bug#333854: pg_group file update problems
Bruce Momjian pgman@candle.pha.pa.us writes: In the patch, notice the old comment that suggests we might need to use CommandCounterIncrement(). ... which you failed to fix in any meaningful way. I'd suggest /* * Advance the commmand counter to ensure we see all results * of current transaction. */ CommandCounterIncrement(); and then change SnapshotSelf to SnapshotNow, since there's no longer any reason for it to be special. Compare to CVS tip which already does it that way. See also the identical code in write_user_file (which perhaps has no bug, but ISTM it should stay identical). regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [BUGS] Bug#333854: pg_group file update problems
Tom Lane wrote:
Bruce Momjian pgman@candle.pha.pa.us writes:
In the patch, notice the old comment that suggests we might need to use
CommandCounterIncrement().
... which you failed to fix in any meaningful way. I'd suggest
/*
* Advance the commmand counter to ensure we see all results
* of current transaction.
*/
CommandCounterIncrement();
and then change SnapshotSelf to SnapshotNow, since there's no longer any
reason for it to be special. Compare to CVS tip which already does it
that way. See also the identical code in write_user_file (which perhaps
has no bug, but ISTM it should stay identical).
OK, updated patch.
--
Bruce Momjian| http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: src/backend/commands/user.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/user.c,v
retrieving revision 1.147
diff -c -c -r1.147 user.c
*** src/backend/commands/user.c 31 Dec 2004 21:59:42 - 1.147
--- src/backend/commands/user.c 14 Oct 2005 17:36:54 -
***
*** 175,184
/*
* Read pg_group and write the file. Note we use SnapshotSelf to
!* ensure we see all effects of current transaction. (Perhaps could
!* do a CommandCounterIncrement beforehand, instead?)
*/
! scan = heap_beginscan(grel, SnapshotSelf, 0, NULL);
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Datum datum,
--- 175,184
/*
* Read pg_group and write the file. Note we use SnapshotSelf to
!* ensure we see all effects of current transaction.
*/
! CommandCounterIncrement(); /* see our current changes */
! scan = heap_beginscan(grel, SnapshotNow, 0, NULL);
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Datum datum,
***
*** 322,331
/*
* Read pg_shadow and write the file. Note we use SnapshotSelf to
!* ensure we see all effects of current transaction. (Perhaps could
!* do a CommandCounterIncrement beforehand, instead?)
*/
! scan = heap_beginscan(urel, SnapshotSelf, 0, NULL);
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Datum datum;
--- 322,331
/*
* Read pg_shadow and write the file. Note we use SnapshotSelf to
!* ensure we see all effects of current transaction.
*/
! CommandCounterIncrement(); /* see our current changes */
! scan = heap_beginscan(urel, SnapshotNow, 0, NULL);
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Datum datum;
***
*** 781,786
--- 781,787
* Set flag to update flat password file at commit.
*/
user_file_update_needed();
+ group_file_update_needed();
}
***
*** 1200,1205
--- 1201,1207
* Set flag to update flat password file at commit.
*/
user_file_update_needed();
+ group_file_update_needed();
}
***
*** 1286,1291
--- 1288,1294
heap_close(rel, NoLock);
user_file_update_needed();
+ group_file_update_needed();
}
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [PATCHES] [BUGS] Bug#333854: pg_group file update problems
Bruce Momjian pgman@candle.pha.pa.us writes: OK, updated patch. I was sort of hoping that you would make the comments agree with the code... regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [BUGS] Bug#333854: pg_group file update problems
Tom Lane wrote:
Bruce Momjian pgman@candle.pha.pa.us writes:
OK, updated patch.
I was sort of hoping that you would make the comments agree with the
code...
Oh, you really read those comments? Fixed and attached.
--
Bruce Momjian| http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: src/backend/commands/user.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/user.c,v
retrieving revision 1.147
diff -c -c -r1.147 user.c
*** src/backend/commands/user.c 31 Dec 2004 21:59:42 - 1.147
--- src/backend/commands/user.c 14 Oct 2005 21:18:42 -
***
*** 174,184
errmsg(could not write to temporary file
\%s\: %m, tempname)));
/*
!* Read pg_group and write the file. Note we use SnapshotSelf to
!* ensure we see all effects of current transaction. (Perhaps could
!* do a CommandCounterIncrement beforehand, instead?)
*/
! scan = heap_beginscan(grel, SnapshotSelf, 0, NULL);
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Datum datum,
--- 174,183
errmsg(could not write to temporary file
\%s\: %m, tempname)));
/*
!* Read pg_group and write the file
*/
! CommandCounterIncrement(); /* see our current changes */
! scan = heap_beginscan(grel, SnapshotNow, 0, NULL);
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Datum datum,
***
*** 321,331
errmsg(could not write to temporary file
\%s\: %m, tempname)));
/*
!* Read pg_shadow and write the file. Note we use SnapshotSelf to
!* ensure we see all effects of current transaction. (Perhaps could
!* do a CommandCounterIncrement beforehand, instead?)
*/
! scan = heap_beginscan(urel, SnapshotSelf, 0, NULL);
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Datum datum;
--- 320,329
errmsg(could not write to temporary file
\%s\: %m, tempname)));
/*
!* Read pg_shadow and write the file
*/
! CommandCounterIncrement(); /* see our current changes */
! scan = heap_beginscan(urel, SnapshotNow, 0, NULL);
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Datum datum;
***
*** 781,786
--- 779,785
* Set flag to update flat password file at commit.
*/
user_file_update_needed();
+ group_file_update_needed();
}
***
*** 1200,1205
--- 1199,1205
* Set flag to update flat password file at commit.
*/
user_file_update_needed();
+ group_file_update_needed();
}
***
*** 1286,1291
--- 1286,1292
heap_close(rel, NoLock);
user_file_update_needed();
+ group_file_update_needed();
}
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [PATCHES] Documentation typos
On Thu, 2005-13-10 at 00:39 -0600, Michael Fuhr wrote: Correct several typos in the documentation. Are there any remaining objections to this patch? Otherwise, I'll apply it within 24 hours. -Neil ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Documentation typos
On Fri, Oct 14, 2005 at 06:18:08PM -0400, Neil Conway wrote: On Thu, 2005-13-10 at 00:39 -0600, Michael Fuhr wrote: Correct several typos in the documentation. Are there any remaining objections to this patch? Otherwise, I'll apply it within 24 hours. I could submit a version with randomi[sz]ed spellings if it would make certain people happier ;-) -- Michael Fuhr ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
