Re: [PATCHES] WIP: rewrite numeric division

2007-07-17 Thread Michael Paesold
Please, let's revisit this, and not postpone it without further 
discussion. I never knew about the correctness issues in div_var(), but 
since I know about it, I feel I am just waiting until that problem will 
hit me or anyone else.
So can you, Tom, please describe in what situations the old code is 
really unsafe?


We usually *round* all values to at maximum 4 decimal places -- are we 
on the save side? Does this only affect high precision division, or any 
divisions?


Best Regards
Michael Paesold

Bruce Momjian wrote:

Because this has not been applied, this has been saved for the 8.4 release:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---

Tom Lane wrote:

I wrote:

I just blew the dust off my old copy of Knuth vol 2, and see that his
algorithm for multi-precision division generates output digits that are
correct to start with (or at least he never needs to revisit a digit
after moving on to the next).  ISTM we should go over to an approach
like that.

The attached proposed patch rewrites div_var() using Knuth's algorithm,
meaning that division should always produce an exact truncated output
when asked to truncate at X number of places.  This passes regression
tests and fixes both of the cases previously exhibited:
http://archives.postgresql.org/pgsql-bugs/2007-06/msg00068.php
http://archives.postgresql.org/pgsql-general/2005-05/msg01109.php

The bad news is that it's significantly slower than the CVS-HEAD code;
it appears that for long inputs, div_var is something like 4X slower
than before, depending on platform.  The numeric_big regression test
takes about twice as long as before on one of my machines, and 50%
longer on another.  This is because the innermost loop now involves
integer division, which it didn't before.  (According to oprofile,
just about all the time goes into the loop that subtracts qhat * divisor
from the working dividend, which is what you'd expect.)

Now it's unlikely that real-world applications are going to be as
dependent on the speed of div_var for long inputs as numeric_big is.
And getting the right answer has to take priority over speed anyway.
Still this is a bit annoying.  Anyone see a way to speed it up, or
have another approach?

regards, tom lane



Content-Description: numeric-div.patch.gz

[ Type application/octet-stream treated as attachment, skipping... ]



---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
  choose an index scan if your joining column's datatypes do not
  match


Re: [PATCHES] WIP: rewrite numeric division

2007-07-17 Thread Tom Lane
Michael Paesold [EMAIL PROTECTED] writes:
 Please, let's revisit this, and not postpone it without further 
 discussion. I never knew about the correctness issues in div_var(), but 
 since I know about it, I feel I am just waiting until that problem will 
 hit me or anyone else.

Yeah.  I was basically waiting to see if anyone could come up with a
faster solution.  Since no one seems to have an idea how to do it
better, I'm inclined to apply the patch for 8.3.

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] Deferred RI trigger for non-key UPDATEs and subxacts

2007-07-17 Thread Tom Lane
Stephan Szabo [EMAIL PROTECTED] writes:
 On Sun, 15 Jul 2007, Tom Lane wrote:
 I don't think this is right.  If the original tuple was inserted by a
 subtransaction of our transaction, it will have been checked at
 subtransaction subcommit, no?

 I don't think the subtransaction subcommit will do the check. Unless I'm
 missing something about the code, a CommitTransaction would but a
 CommitSubTransaction won't, which actually makes sense given that we're
 mapping savepoints on to it, and I don't think we are allowed to check at
 savepoint release time.

OK, that's what I get for opining before checking the code ;-).  It
seems a little weird that a subcommitted subtransaction could still
cause a failure later, but that is how we're doing the triggers.

Given that, the proposed patch seems appropriate.  Will apply.

regards, tom lane

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] WIP: rewrite numeric division

2007-07-17 Thread Gregory Stark
Tom Lane [EMAIL PROTECTED] writes:

 Michael Paesold [EMAIL PROTECTED] writes:
 Please, let's revisit this, and not postpone it without further 
 discussion. I never knew about the correctness issues in div_var(), but 
 since I know about it, I feel I am just waiting until that problem will 
 hit me or anyone else.

 Yeah.  I was basically waiting to see if anyone could come up with a
 faster solution.  Since no one seems to have an idea how to do it
 better, I'm inclined to apply the patch for 8.3.

My only reservation is that I don't have the numeric methods background to
tell if it's really necessary. My fix does resolve the only actual documented
inaccuracy in the existing method.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com


---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] WIP: rewrite numeric division

2007-07-17 Thread Tom Lane
Gregory Stark [EMAIL PROTECTED] writes:
 Tom Lane [EMAIL PROTECTED] writes:
 Yeah.  I was basically waiting to see if anyone could come up with a
 faster solution.  Since no one seems to have an idea how to do it
 better, I'm inclined to apply the patch for 8.3.

 My only reservation is that I don't have the numeric methods background to
 tell if it's really necessary. My fix does resolve the only actual documented
 inaccuracy in the existing method.

Well, this doesn't take a lot of numerical methods background: the
fundamental problem is that the existing code generates an *approximate*
answer, whereas people who are doing div and mod on large integers tend
to expect an *exact* answer.  Approximate doesn't cut it --- there will
always be cases where an off-by-one-in-the-last-internal-place error can
carry far enough to the left to be visible to the user.

regards, tom lane

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate


Re: [PATCHES] Deferred RI trigger for non-key UPDATEs and subxacts

2007-07-17 Thread Affan Salman



 OK, that's what I get for opining before checking the code ;-).

Your *cerebral call graph visits* have a knack of being spot on, way
more than often.  :-)


 Will apply.

Thanks, Tom.  We're also back-patching this, right?

--
Affan Salman
EnterpriseDB Corporation  http://www.enterprisedb.com


---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] WIP: rewrite numeric division

2007-07-17 Thread Gregory Stark
Tom Lane [EMAIL PROTECTED] writes:

 Gregory Stark [EMAIL PROTECTED] writes:
 Tom Lane [EMAIL PROTECTED] writes:
 Yeah.  I was basically waiting to see if anyone could come up with a
 faster solution.  Since no one seems to have an idea how to do it
 better, I'm inclined to apply the patch for 8.3.

 My only reservation is that I don't have the numeric methods background to
 tell if it's really necessary. My fix does resolve the only actual documented
 inaccuracy in the existing method.

 Well, this doesn't take a lot of numerical methods background: the
 fundamental problem is that the existing code generates an *approximate*
 answer, whereas people who are doing div and mod on large integers tend
 to expect an *exact* answer.  Approximate doesn't cut it --- there will
 always be cases where an off-by-one-in-the-last-internal-place error can
 carry far enough to the left to be visible to the user.

So does your code behave differently for this or does it round off to the
arbitrary division precision before hitting trunc?

postgres=# select trunc(::numeric / 10::numeric);
trunc 
--
 1000
(1 row)

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com


---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] Deferred RI trigger for non-key UPDATEs and subxacts

2007-07-17 Thread Tom Lane
Affan Salman [EMAIL PROTECTED] writes:
 Thanks, Tom.  We're also back-patching this, right?

Yeah, working on that now.

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] [HACKERS] msvc, build and install with cygwin in the PATH

2007-07-17 Thread Magnus Hagander
I used to have a different patch from Andrew that did part of this, and
more, and conflicted rather badly with it. However, I never got around
to applying that one, and I can't seem to find it anymore.

Andrew -do you recall if you had all this in yours, and is it still
something you want in, or should we just go with this one?

//Magnus

Bruce Momjian wrote:
 Magnus, what is your reaction to this patch?
 
 ---
 
 Hannes Eder wrote:
 Magnus Hagander wrote:
  Hannes Eder wrote:
   Is it worth doing this the Perl-way and using File::Find? If so, I 
 can
   work an a patch for that.
  
   It's certainly cleaner that way, but I don't find it a major issue. 
 But I'd
   rather see that fix than the other one.

 Here we go. See attached patch. Your comments are welcome.

 Hannes.

 
 *** ..\pgsql-cvshead\src\tools\msvc\Install.pm   Mo Mai 14 16:36:10 2007
 --- src\tools\msvc\Install.pmMi Jun  6 20:39:47 2007
 ***
 *** 10,15 
 --- 10,18 
   use Carp;
   use File::Basename;
   use File::Copy;
 + use File::Find;
 + use File::Glob;
 + use File::Spec;
   
   use Exporter;
   our (@ISA,@EXPORT_OK);
 ***
 *** 99,104 
 --- 102,142 
   print \n;
   }
   
 + sub FindFiles
 + {
 + my $spec = shift;
 + my $nonrecursive = shift;
 + my $pat = basename($spec);
 + my $dir = dirname($spec);
 + 
 + if ($dir eq '') { $dir = '.'; }
 + 
 + -d $dir || croak Could not list directory $dir: $!\n;
 + 
 + if ($nonrecursive)
 + {
 + return glob($spec);
 + }
 + 
 + # borrowed from File::DosGlob
 + # escape regex metachars but not glob chars
 + $pat =~ s:([].+^\-\${}[|]):\\$1:g;
 + # and convert DOS-style wildcards to regex
 + $pat =~ s/\*/.*/g;
 + $pat =~ s/\?/.?/g;
 + 
 + $pat = '^' . $pat . '\z';
 + 
 + my @res;
 + find(
 + {
 + wanted = sub { /$pat/s  push (@res, 
 File::Spec-canonpath($File::Find::name)); }
 + },
 + $dir
 + );
 + return @res;
 + }
 + 
   sub CopySetOfFiles
   {
   my $what = shift;
 ***
 *** 106,126 
   my $target = shift;
   my $silent = shift;
   my $norecurse = shift;
 - my $D;
   
 - my $subdirs = $norecurse?'':'/s';
   print Copying $what unless ($silent);
 ! open($D, dir /b $subdirs $spec |) || croak Could not list $spec\n;
 ! while ($D)
   {
 - chomp;
   next if /regress/; # Skip temporary install in regression subdir
 ! my $tgt = $target . basename($_);
   print .;
 ! my $src = $norecurse?(dirname($spec) . '/' . $_):$_;
 ! copy($src, $tgt) || croak Could not copy $src: $!\n;
   }
 ! close($D);
   print \n;
   }
   
 --- 144,161 
   my $target = shift;
   my $silent = shift;
   my $norecurse = shift;
   
   print Copying $what unless ($silent);
 ! 
 ! foreach (FindFiles($spec, $norecurse))
   {
   next if /regress/; # Skip temporary install in regression subdir
 ! my $src = $_;
 ! my $tgt = $target . basename($src);
   print .;
 ! copy($src, $tgt) || croak Could not copy $src to $tgt: $!\n;
   }
 ! 
   print \n;
   }
   
 ***
 *** 371,395 
   {
   my $target = shift;
   my $nlspath = shift;
 - my $D;
   
   print Installing NLS files...;
   EnsureDirectories($target, share/locale);
 ! open($D,dir /b /s nls.mk|) || croak Could not list nls.mk\n;
 ! while ($D)
   {
 - chomp;
   s/nls.mk/po/;
   my $dir = $_;
   next unless ($dir =~ /([^\\]+)\\po$/);
   my $prgm = $1;
   $prgm = 'postgres' if ($prgm eq 'backend');
 - my $E;
 - open($E,dir /b $dir\\*.po|) || croak Could not list contents of 
 $_\n;
   
 ! while ($E)
   {
 - chomp;
   my $lang;
   next unless /^(.*)\.po/;
   $lang = $1;
 --- 406,425 
   {
   my $target = shift;
   my $nlspath = shift;
   
   print Installing NLS files...;
   EnsureDirectories($target, share/locale);
 ! 
 ! foreach (FindFiles(nls.mk))
   {
   s/nls.mk/po/;
   my $dir = $_;
   next unless ($dir =~ /([^\\]+)\\po$/);
   my $prgm = $1;
   $prgm = 'postgres' if ($prgm eq 'backend');
   
 ! foreach (FindFiles($dir\\*.po, 1))
   {
   my $lang;
   next unless /^(.*)\.po/;
   $lang = $1;
 ***
 *** 401,409 
  croak(Could not run msgfmt on $dir\\$_);
   print .;
   }
 - close($E);
   }
 ! close($D);
   print \n;
   }
   
 --- 431,438 
  croak(Could not run msgfmt on $dir\\$_);
   print .;
   }
   }
 ! 
   print \n;
   }
   
 
 

Re: [PATCHES] WIP: rewrite numeric division

2007-07-17 Thread Tom Lane
Gregory Stark [EMAIL PROTECTED] writes:
 Tom Lane [EMAIL PROTECTED] writes:
 Well, this doesn't take a lot of numerical methods background: the
 fundamental problem is that the existing code generates an *approximate*
 answer, whereas people who are doing div and mod on large integers tend
 to expect an *exact* answer.  Approximate doesn't cut it --- there will
 always be cases where an off-by-one-in-the-last-internal-place error can
 carry far enough to the left to be visible to the user.

 So does your code behave differently for this or does it round off to the
 arbitrary division precision before hitting trunc?

 postgres=# select trunc(::numeric / 10::numeric);
 trunc 
 --
  1000
 (1 row)

No, my proposed patch doesn't change that.  It might be that we should
provide an integer division operator for NUMERIC, so that you can get
at the exact result of trunc(x/y).

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] Async Commit, v21 (now: v22)

2007-07-17 Thread Simon Riggs
On Tue, 2007-07-17 at 01:28 -0400, Bruce Momjian wrote:
 Your patch has been added to the PostgreSQL unapplied patches list at:
 
   http://momjian.postgresql.org/cgi-bin/pgpatches
 
 It will be applied as soon as one of the PostgreSQL committers reviews
 and approves it.
 
 ---
 
 
 Simon Riggs wrote:
  Latest version of synchronous_commit = on | off
  
  Applies cleanly to CVS HEAD. Passes make check with/without
  synchronous_commit on in postgresql.conf, while running
  --enable-cassert.
  
  I expect to review this tomorrow to make sure everything is correctly
  changed before requesting final review/commit.
  
  Docs included at top of main patch.
  
  All comments welcome.
  

Here's the latest version. I've reviewed this to check that this does
what I want it to do, re-written various comments and changed a few
minor points in the code.

I've also added a chunk to transam/README that describes the workings of
the patch from a high level.

Now ready for final review.

-- 
  Simon Riggs
  EnterpriseDB  http://www.enterprisedb.com


sync_commit.v22.tar.bz2
Description: application/bzip-compressed-tar

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


Re: [PATCHES] WIP: rewrite numeric division

2007-07-17 Thread Gregory Stark
Tom Lane [EMAIL PROTECTED] writes:

 No, my proposed patch doesn't change that.  It might be that we should
 provide an integer division operator for NUMERIC, so that you can get
 at the exact result of trunc(x/y).

I was also thinking that if the denominator had only factors of 2 and 5 we
could calculate the precision to be precisely enough to maintain the original
precision. Ie, /1000 should just give you n.nnn not n.nnn and more
importantly it should never round.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com


---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] Async Commit, v21 (now: v22)

2007-07-17 Thread Bruce Momjian

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---


Simon Riggs wrote:
 On Tue, 2007-07-17 at 01:28 -0400, Bruce Momjian wrote:
  Your patch has been added to the PostgreSQL unapplied patches list at:
  
  http://momjian.postgresql.org/cgi-bin/pgpatches
  
  It will be applied as soon as one of the PostgreSQL committers reviews
  and approves it.
  
  ---
  
  
  Simon Riggs wrote:
   Latest version of synchronous_commit = on | off
   
   Applies cleanly to CVS HEAD. Passes make check with/without
   synchronous_commit on in postgresql.conf, while running
   --enable-cassert.
   
   I expect to review this tomorrow to make sure everything is correctly
   changed before requesting final review/commit.
   
   Docs included at top of main patch.
   
   All comments welcome.
   
 
 Here's the latest version. I've reviewed this to check that this does
 what I want it to do, re-written various comments and changed a few
 minor points in the code.
 
 I've also added a chunk to transam/README that describes the workings of
 the patch from a high level.
 
 Now ready for final review.
 
 -- 
   Simon Riggs
   EnterpriseDB  http://www.enterprisedb.com

[ Attachment, skipping... ]

 
 ---(end of broadcast)---
 TIP 3: Have you checked our extensive FAQ?
 
http://www.postgresql.org/docs/faq

-- 
  Bruce Momjian  [EMAIL PROTECTED]  http://momjian.us
  EnterpriseDB   http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] bitmapscan changes patch review

2007-07-17 Thread Bruce Momjian

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---


Alexey Klyukin wrote:
 Hi,
 
 Here is a patch by Heikki Linnakangas with changes for amgetmulti index
 access method to amgetbitmap, which returns all matching tuples at once.
 The patch also introduces support for candidate matches. The original
 post is here:
 http://archives.postgresql.org/pgsql-patches/2007-03/msg00163.php
 
 I've made minor changes to the patch:
 
 - fixed all rejects when applying it to the current CVS head.
 - fixed counting ntids in gistnext if TIDBitmap is not null.
 - added missing expected/bitmapops.out
 
 It passes all regression tests on my system.
 
 Regards,
 -- 
 Alexey Klyukin http://www.commandprompt.com/
 The PostgreSQL Company - Command Prompt, Inc.

[ Attachment, skipping... ]

 
 ---(end of broadcast)---
 TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

-- 
  Bruce Momjian  [EMAIL PROTECTED]  http://momjian.us
  EnterpriseDB   http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] [HACKERS] msvc, build and install with cygwin in the PATH

2007-07-17 Thread Andrew Dunstan


I am fighting some fires in my day job.

My pesonal TODO list for pg up to beta is:

. fix chunking muddle (see recent emails)
. complete CSV logs patch
. harden MSVC builds

I'll get to this when I can.  I can dig up the patch I did if you want 
it again.


cheers

andrew


Magnus Hagander wrote:

I used to have a different patch from Andrew that did part of this, and
more, and conflicted rather badly with it. However, I never got around
to applying that one, and I can't seem to find it anymore.

Andrew -do you recall if you had all this in yours, and is it still
something you want in, or should we just go with this one?

//Magnus

Bruce Momjian wrote:
  

Magnus, what is your reaction to this patch?

---

Hannes Eder wrote:


Magnus Hagander wrote:
 Hannes Eder wrote:
  Is it worth doing this the Perl-way and using File::Find? If so, I 
can

  work an a patch for that.
 
  It's certainly cleaner that way, but I don't find it a major issue. 
But I'd

  rather see that fix than the other one.

Here we go. See attached patch. Your comments are welcome.

Hannes.

  
*** ..\pgsql-cvshead\src\tools\msvc\Install.pm	Mo Mai 14 16:36:10 2007

--- src\tools\msvc\Install.pm   Mi Jun  6 20:39:47 2007
***
*** 10,15 
--- 10,18 
  use Carp;
  use File::Basename;
  use File::Copy;
+ use File::Find;
+ use File::Glob;
+ use File::Spec;
  
  use Exporter;

  our (@ISA,@EXPORT_OK);
***
*** 99,104 
--- 102,142 
  print \n;
  }
  
+ sub FindFiles

+ {
+ my $spec = shift;
+ my $nonrecursive = shift;
+ my $pat = basename($spec);
+ my $dir = dirname($spec);
+ 
+ if ($dir eq '') { $dir = '.'; }
+ 
+ -d $dir || croak Could not list directory $dir: $!\n;
+ 
+ if ($nonrecursive)

+ {
+ return glob($spec);
+ }
+ 
+ # borrowed from File::DosGlob

+ # escape regex metachars but not glob chars
+ $pat =~ s:([].+^\-\${}[|]):\\$1:g;
+ # and convert DOS-style wildcards to regex
+ $pat =~ s/\*/.*/g;
+ $pat =~ s/\?/.?/g;
+ 
+ $pat = '^' . $pat . '\z';
+ 
+ my @res;

+ find(
+ {
+ wanted = sub { /$pat/s  push (@res, 
File::Spec-canonpath($File::Find::name)); }
+ },
+ $dir
+ );
+ return @res;
+ }
+ 
  sub CopySetOfFiles

  {
  my $what = shift;
***
*** 106,126 
  my $target = shift;
  my $silent = shift;
  my $norecurse = shift;
- my $D;
  
- my $subdirs = $norecurse?'':'/s';

  print Copying $what unless ($silent);
! open($D, dir /b $subdirs $spec |) || croak Could not list $spec\n;
! while ($D)
  {
- chomp;
  next if /regress/; # Skip temporary install in regression subdir
! my $tgt = $target . basename($_);
  print .;
! my $src = $norecurse?(dirname($spec) . '/' . $_):$_;
! copy($src, $tgt) || croak Could not copy $src: $!\n;
  }
! close($D);
  print \n;
  }
  
--- 144,161 

  my $target = shift;
  my $silent = shift;
  my $norecurse = shift;
  
  print Copying $what unless ($silent);
! 
! foreach (FindFiles($spec, $norecurse))

  {
  next if /regress/; # Skip temporary install in regression subdir
! my $src = $_;
! my $tgt = $target . basename($src);
  print .;
! copy($src, $tgt) || croak Could not copy $src to $tgt: $!\n;
  }
! 
  print \n;

  }
  
***

*** 371,395 
  {
  my $target = shift;
  my $nlspath = shift;
- my $D;
  
  print Installing NLS files...;

  EnsureDirectories($target, share/locale);
! open($D,dir /b /s nls.mk|) || croak Could not list nls.mk\n;
! while ($D)
  {
- chomp;
  s/nls.mk/po/;
  my $dir = $_;
  next unless ($dir =~ /([^\\]+)\\po$/);
  my $prgm = $1;
  $prgm = 'postgres' if ($prgm eq 'backend');
- my $E;
- open($E,dir /b $dir\\*.po|) || croak Could not list contents of 
$_\n;
  
! while ($E)

  {
- chomp;
  my $lang;
  next unless /^(.*)\.po/;
  $lang = $1;
--- 406,425 
  {
  my $target = shift;
  my $nlspath = shift;
  
  print Installing NLS files...;

  EnsureDirectories($target, share/locale);
! 
! foreach (FindFiles(nls.mk))

  {
  s/nls.mk/po/;
  my $dir = $_;
  next unless ($dir =~ /([^\\]+)\\po$/);
  my $prgm = $1;
  $prgm = 'postgres' if ($prgm eq 'backend');
  
! foreach (FindFiles($dir\\*.po, 1))

  {
  my $lang;
  next unless /^(.*)\.po/;
  $lang = $1;
***
*** 401,409 
 croak(Could not run msgfmt on $dir\\$_);
  print .;
  }
- close($E);
  }
! close($D);
  print \n;
  }
  
--- 431,438 


Re: [PATCHES] [BUGS] BUG #3431: age() gets the days wrong

2007-07-17 Thread Bruce Momjian

I have applied the attached patch that documents the age() behavior,
plus fixes the mismatch sign for seconds by using part of Tom's earlier
patch.  

I agree we want to keep the symmetry we have.  We can call this item
closed.

---

Tom Lane wrote:
 Pelle Johansson [EMAIL PROTECTED] writes:
  If you were to use the result for subtracting from the first value,  
  instead of adding to the second, the conditions are reversed. It's  
  not really as obvious as I first thought whether there's 2 months and  
  29 days or 2 months and 30 days between 2006-11-02 and 2007-02-01...  
 
 Hmm, that's a really good point; perhaps the original author was
 thinking of it in those terms, in which case using the first month of
 the interval is indeed sane.  (Almost: I believe that the loop can
 iterate more than once, and then you need to look to the second month
 etc.  The code's not doing that, so there's still a corner-case bug,
 plus the fsec issue.)
 
 Other than that corner case, it seems the behavior we currently have is
   if x  y, age() produces a positive interval such that
   x - age(x, y) = y
   if x  y, age() produces a negative interval such that
   y + age(x, y) = x
 
 Are we satisfied with just documenting that, or do we want to change it,
 and if so to what?
 
 As the code currently stands, we have the symmetry property
   age(x,y) = - age(y,x)
 for all x,y.  I don't think we can preserve that if we try to simplify
 the relationship to interval addition/subtraction.
 
 Comments?
 
   regards, tom lane
 
 ---(end of broadcast)---
 TIP 7: You can help support the PostgreSQL project by donating at
 
 http://www.postgresql.org/about/donate

-- 
  Bruce Momjian  [EMAIL PROTECTED]  http://momjian.us
  EnterpriseDB   http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/func.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/func.sgml,v
retrieving revision 1.382
diff -c -c -r1.382 func.sgml
*** doc/src/sgml/func.sgml	6 Jun 2007 23:00:35 -	1.382
--- doc/src/sgml/func.sgml	18 Jul 2007 03:09:42 -
***
*** 5895,5900 
--- 5895,5911 
 literalCST7CDT/literal.
/para
  
+   para
+Note that when the functionage/ function operates on multi-month
+intervals, productnamePostgreSQL/ adds days to the earlier date
+until full months can be added.  This yields a different result than
+adding full months first if the interval crosses from one month to the
+next.  For example, literalage('2004-06-01', '2004-04-30')/ yeilds
+literal1 mon 1 day/ using the productnamePostgreSQL/ method,
+while adding the month first would yield literal1 mon 2 days/
+because May has 31 days, while April has only 30.
+   /para
+ 
sect2 id=functions-datetime-extract
 titlefunctionEXTRACT/function, functiondate_part/function/title
  
Index: src/backend/utils/adt/timestamp.c
===
RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.179
diff -c -c -r1.179 timestamp.c
*** src/backend/utils/adt/timestamp.c	6 Jul 2007 04:15:59 -	1.179
--- src/backend/utils/adt/timestamp.c	18 Jul 2007 03:09:44 -
***
*** 3044,3050 
  	if (timestamp2tm(dt1, NULL, tm1, fsec1, NULL, NULL) == 0 
  		timestamp2tm(dt2, NULL, tm2, fsec2, NULL, NULL) == 0)
  	{
! 		fsec = (fsec1 - fsec2);
  		tm-tm_sec = tm1-tm_sec - tm2-tm_sec;
  		tm-tm_min = tm1-tm_min - tm2-tm_min;
  		tm-tm_hour = tm1-tm_hour - tm2-tm_hour;
--- 3044,3051 
  	if (timestamp2tm(dt1, NULL, tm1, fsec1, NULL, NULL) == 0 
  		timestamp2tm(dt2, NULL, tm2, fsec2, NULL, NULL) == 0)
  	{
! 		/* form the symbolic difference */
! 		fsec = fsec1 - fsec2;
  		tm-tm_sec = tm1-tm_sec - tm2-tm_sec;
  		tm-tm_min = tm1-tm_min - tm2-tm_min;
  		tm-tm_hour = tm1-tm_hour - tm2-tm_hour;
***
*** 3064,3069 
--- 3065,3081 
  			tm-tm_year = -tm-tm_year;
  		}
  
+ 		/* propagate any negative fields into the next higher field */
+ 		while (fsec  0)
+ 		{
+ #ifdef HAVE_INT64_TIMESTAMP
+ 			fsec += USECS_PER_SEC;
+ #else
+ 			fsec += 1.0;
+ #endif
+ 			tm-tm_sec--;
+ 		}
+ 
  		while (tm-tm_sec  0)
  		{
  			tm-tm_sec += SECS_PER_MINUTE;
***
*** 3158,3163 
--- 3170,3176 
  	if (timestamp2tm(dt1, tz1, tm1, fsec1, tzn, NULL) == 0 
  		timestamp2tm(dt2, tz2, tm2, fsec2, tzn, NULL) == 0)
  	{
+ 		/* form the symbolic difference */
  		fsec = fsec1 - fsec2;
  		tm-tm_sec = tm1-tm_sec - tm2-tm_sec;
  		tm-tm_min = tm1-tm_min - tm2-tm_min;
***
*** 3178,3183 
--- 3191,3207 
  			tm-tm_year = -tm-tm_year;
  		}
  
+ 		/* propagate any negative fields into 

Re: [PATCHES] execl() sentinel

2007-07-17 Thread Bruce Momjian
Alvaro Herrera wrote:
 Stefan Kaltenbrunner just let me know via Jabber that there's a warning
 in pg_regress.c:
 
 pg_regress.c: In function `spawn_process':
 pg_regress.c:914: warning: missing sentinel in function call
 
 This small patch would seem to fix it, according to 
 http://www.linuxonly.nl/docs/sentinel/

You can apply this, but it sure seems like a compiler/include file bug
to me, even with the 64-bit explaination.

-- 
  Bruce Momjian  [EMAIL PROTECTED]  http://momjian.us
  EnterpriseDB   http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] pg_dump --no-tablespaces patch

2007-07-17 Thread Bruce Momjian

This has been saved for the 8.4 release:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---

Gavin M. Roy wrote:
 This is the patch I proposed on hackers to make pg_dump optionally ignore
 tablespaces.  The patch is against 8.2.4.  If I should be applying it to CVS
 head or what not, please let me know (along with any thoughts, concerns or
 issues).
 
 Gavin

[ Attachment, skipping... ]

 
 ---(end of broadcast)---
 TIP 2: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian  [EMAIL PROTECTED]  http://momjian.us
  EnterpriseDB   http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


Re: [PATCHES] docfix - DELETE doesn't affect auto-analyze

2007-07-17 Thread Bruce Momjian

Patch applied.  Thanks.  Your documentation changes can be viewed in
five minutes using links on the developer's page,
http://www.postgresql.org/developer/testing.


---


ITAGAKI Takahiro wrote:
 I reported an incorrect description for auto-analyze in our documentation.
 http://archives.postgresql.org/pgsql-hackers/2007-06/msg0.php
 Here is a documentation fix for it.
 
 There are the same mistakes in 8.1 and 8.2, not only in HEAD.
 It had been true in contrib/pg_autovacuum at 8.0, but we
 changed the behavior at the integration of autovacuum.
 
 
 Index: doc/src/sgml/maintenance.sgml
 ===
 --- doc/src/sgml/maintenance.sgml (HEAD)
 +++ doc/src/sgml/maintenance.sgml (fixed)
 @@ -533,7 +533,7 @@
  programlisting
  analyze threshold = analyze base threshold + analyze scale factor * number 
 of tuples
  /programlisting
 -is compared to the total number of tuples inserted, updated, or deleted
 +is compared to the total number of tuples inserted or updated
  since the last commandANALYZE/command.
 /para
  
 
 Regards,
 ---
 ITAGAKI Takahiro
 NTT Open Source Software Center
 
 
 ---(end of broadcast)---
 TIP 3: Have you checked our extensive FAQ?
 
http://www.postgresql.org/docs/faq

-- 
  Bruce Momjian  [EMAIL PROTECTED]  http://momjian.us
  EnterpriseDB   http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] execl() sentinel

2007-07-17 Thread Tom Lane
Bruce Momjian [EMAIL PROTECTED] writes:
 Alvaro Herrera wrote:
 pg_regress.c: In function `spawn_process':
 pg_regress.c:914: warning: missing sentinel in function call

 You can apply this, but it sure seems like a compiler/include file bug
 to me, even with the 64-bit explaination.

There are lots of platforms where the include files and the compiler are
not all that compatible, gcc + vendor include files being the prototype
case.  It's too bad that gcc doesn't have a
-Wno-snarkiness-about-system-headers-thank-you switch.

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] docfix - DELETE doesn't affect auto-analyze

2007-07-17 Thread Tom Lane
Bruce Momjian [EMAIL PROTECTED] writes:
 -is compared to the total number of tuples inserted, updated, or deleted
 +is compared to the total number of tuples inserted or updated

As best I can tell, this description is even further away from the
actual CVS HEAD behavior than the previous one.  The code is comparing
against

anltuples = tabentry-n_live_tuples + tabentry-n_dead_tuples -
tabentry-last_anl_tuples;

and deletions surely increase n_dead_tuples.

regards, tom lane

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate


Re: [PATCHES] docfix - DELETE doesn't affect auto-analyze

2007-07-17 Thread ITAGAKI Takahiro

Bruce Momjian [EMAIL PROTECTED] wrote:

 Patch applied.  Thanks.  Your documentation changes can be viewed in
 five minutes using links on the developer's page,
 http://www.postgresql.org/developer/testing.

Thanks. Don't we need to backport it to 8.1 and 8.2?
It was changed at the integration of autovacuum at 8.1.


Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center



---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] docfix - DELETE doesn't affect auto-analyze

2007-07-17 Thread Alvaro Herrera
Tom Lane wrote:
 Bruce Momjian [EMAIL PROTECTED] writes:
  -is compared to the total number of tuples inserted, updated, or 
  deleted
  +is compared to the total number of tuples inserted or updated
 
 As best I can tell, this description is even further away from the
 actual CVS HEAD behavior than the previous one.  The code is comparing
 against
 
 anltuples = tabentry-n_live_tuples + tabentry-n_dead_tuples -
 tabentry-last_anl_tuples;
 
 and deletions surely increase n_dead_tuples.

I think the patch is correct for 8.1 and 8.2 but is wrong for HEAD
(disclaimer: I'm a bit sleepy ATM).

-- 
Alvaro Herrerahttp://www.advogato.org/person/alvherre
Management by consensus: I have decided; you concede.
(Leonard Liu)

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


Re: [PATCHES] docfix - DELETE doesn't affect auto-analyze

2007-07-17 Thread ITAGAKI Takahiro

Tom Lane [EMAIL PROTECTED] wrote:

 Bruce Momjian [EMAIL PROTECTED] writes:
  -is compared to the total number of tuples inserted, updated, or 
  deleted
  +is compared to the total number of tuples inserted or updated
 
 As best I can tell, this description is even further away from the
 actual CVS HEAD behavior than the previous one.  The code is comparing
 against
 
 anltuples = tabentry-n_live_tuples + tabentry-n_dead_tuples -
 tabentry-last_anl_tuples;
 
 and deletions surely increase n_dead_tuples.

Yes, but they also decrease n_live_tuples;
anltuples is not affected by deletions.

if (isCommit)
{
tabstat-t_counts.t_new_live_tuples +=
trans-tuples_inserted - trans-tuples_deleted;
tabstat-t_counts.t_new_dead_tuples += trans-tuples_deleted;
}

Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center



---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org