Re: [HACKERS] 9.2rc1 build requirements

2012-09-01 Thread Robert Haas
On Thu, Aug 30, 2012 at 8:31 PM, Jeff Janes jeff.ja...@gmail.com wrote:
 patch to do that attached.

Committed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] too much pgbench init output

2012-09-01 Thread Robert Haas
On Sat, Sep 1, 2012 at 12:00 AM, Peter Eisentraut pete...@gmx.net wrote:
 When initializing a large database, pgbench writes tons of %d tuples
 done lines.  I propose to change this to a sort of progress counter
 that stays on the same line, as in the attached patch.

I'm not sure I like this - what if the output is being saved off to a file?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Re: [COMMITTERS] pgsql: Cross-link to doc build requirements from install requirements.

2012-09-01 Thread Stefan Kaltenbrunner
On 09/01/2012 12:28 PM, Robert Haas wrote:
 Cross-link to doc build requirements from install requirements.
 
 Jeff Janes
 
 Branch
 --
 master
 
 Details
 ---
 http://git.postgresql.org/pg/commitdiff/e8d6c98c2f082bead1202b23e9d70e0fbde49129
 
 Modified Files
 --
 doc/src/sgml/installation.sgml |8 
 1 files changed, 8 insertions(+), 0 deletions(-)
 

this seems to have broken the docs build:


http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=guaibasaurusdt=2012-09-01%2012%3A17%3A01



Stefan


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Yet another failure mode in pg_upgrade

2012-09-01 Thread Bruce Momjian
On Mon, Aug 13, 2012 at 12:46:43PM +0200, Magnus Hagander wrote:
 On Mon, Aug 13, 2012 at 4:34 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  I've been experimenting with moving the Unix socket directory to
  /var/run/postgresql for the Fedora distribution (don't ask :-().
  It's mostly working, but I found out yet another way that pg_upgrade
  can crash and burn: it doesn't consider the possibility that the
  old or new postmaster is compiled with a different default
  unix_socket_directory than what is compiled into the libpq it's using
  or that pg_dump is using.
 
  This is another hazard that we could forget about if we had some way for
  pg_upgrade to run standalone backends instead of starting a postmaster.
 
 Yeah, that would be nice.
 
 
  But in the meantime, I suggest it'd be a good idea for pg_upgrade to
  explicitly set unix_socket_directory (or unix_socket_directories in
  HEAD) when starting the postmasters, and also explicitly set PGHOST
  to ensure that the client-side code plays along.
 
 That sounds like a good idea for other reasons as well - manual
 connections attempting to get in during an upgrade will just fail with
 a no connection error, which makes sense...
 
 So, +1.

OK, I looked this over, and I have a patch, attached.

Because we are already playing with socket directories, this patch creates
the socket files in the current directory for upgrades and non-live
checks, but not live checks.  This eliminates the someone accidentally
connects problem, at least on Unix, plus we are using port 50432
already.  This also turns off TCP connections for unix domain socket
systems.

For live check operation, you are checking a running server, so
assuming the socket is in the current directory is not going to work.
What the code does is to read the 5th line from the running server's
postmaster.pid file, which has the socket directory in PG = 9.1.  For
pre-9.1, pg_upgrade uses the compiled-in defaults for socket directory.
If the defaults are different between the two servers, the new binaries,
e.g. pg_dump, will not work.  The fix is for the user to set pg_upgrade
-O to match the old socket directory, and set PGHOST before running
pg_upgrade.  I could not find a good way to generate a proper error
message because we are blind to the socket directory in pre-9.1. 
Frankly, this is a problem if the old pre-9.1 server is running in a
user-configured socket directory too, so a documentation addition seems
right here.

So, in summary, this patch moves the socket directory to the current
directory all but live check operation, and handles different socket
directories for old cluster = 9.1.  I have added a documentation
mention of how to make this work for for pre-9.1 old servers.

Thus completes another surgery on a moving train that is pg_upgrade
development.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c
new file mode 100644
index 94bce50..8b35d8f
*** a/contrib/pg_upgrade/option.c
--- b/contrib/pg_upgrade/option.c
*** adjust_data_dir(ClusterInfo *cluster)
*** 376,378 
--- 376,431 
  
  	check_ok();
  }
+ 
+ 
+ #if HAVE_UNIX_SOCKETS
+ /*
+  * set_sockdir_and_pghost
+  *
+  * Set the socket directory to either the configured location (live check)
+  * or the current directory.
+  */
+ void
+ set_sockdir_and_pghost(bool live_check)
+ {
+ 	if (!live_check)
+ 	{
+ 		/* Use the current directory for the socket */
+ 		if (!getcwd(os_info.sockdir, MAXPGPATH))
+ 			pg_log(PG_FATAL, cannot find current directory\n);
+ 	}
+ 	else
+ 	{
+ 		/*
+ 		 *	If we are doing a live check, we will use the old cluster's Unix
+ 		 *	domain socket directory so we can connect to the live server.
+ 		 */
+ 
+ 		/* sockdir added to the 5th line of postmaster.pid in PG 9.1 */
+ 		if (GET_MAJOR_VERSION(old_cluster.major_version) = 901)
+ 		{
+ 			char		filename[MAXPGPATH];
+ 			FILE		*fp;
+ 			int			i;
+ 
+ 			snprintf(filename, sizeof(filename), %s/postmaster.pid, old_cluster.pgdata);
+ 			if ((fp = fopen(filename, r)) == NULL)
+ pg_log(PG_FATAL, Could not get socket directory of the old server\n);
+ 
+ 			for (i = 0; i  5; i++)
+ if (fgets(os_info.sockdir, sizeof(os_info.sockdir), fp) == NULL)
+ 	pg_log(PG_FATAL, Could not get socket directory of the old server\n);
+ 			fclose(fp);
+ 		}
+ 		else
+ 			return;		/* Can't get live sockdir, so use the default. */
+ 
+ 		/* Remove trailing newline */
+ 		if (strchr(os_info.sockdir, '\n') != NULL)
+ 			*strchr(os_info.sockdir, '\n') = '\0';
+ 	}
+ 
+ 	/* For client communication */
+ 	pg_putenv(PGHOST, os_info.sockdir);
+ }
+ #endif
diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c
new file mode 100644
index c47c8bb..8cad5c3
*** a/contrib/pg_upgrade/pg_upgrade.c
--- b/contrib/pg_upgrade/pg_upgrade.c
*** 

Re: [HACKERS] Yet another failure mode in pg_upgrade

2012-09-01 Thread Bruce Momjian
On Sat, Sep  1, 2012 at 11:45:58AM -0400, Bruce Momjian wrote:
 So, in summary, this patch moves the socket directory to the current
 directory all but live check operation, and handles different socket
 directories for old cluster = 9.1.  I have added a documentation
 mention of how to make this work for for pre-9.1 old servers.
 
 Thus completes another surgery on a moving train that is pg_upgrade
 development.

Oh, one more thing.  We have talked about creating some special pipe for
pg_upgrade to communicate the a backend directly, but live check mode
hightlights that we will _still_ need traditional connection abilities
even if we add the pipe ability.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Yet another failure mode in pg_upgrade

2012-09-01 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 On Sat, Sep  1, 2012 at 11:45:58AM -0400, Bruce Momjian wrote:
 So, in summary, this patch moves the socket directory to the current
 directory all but live check operation, and handles different socket
 directories for old cluster = 9.1.  I have added a documentation
 mention of how to make this work for for pre-9.1 old servers.
 
 Thus completes another surgery on a moving train that is pg_upgrade
 development.

 Oh, one more thing.  We have talked about creating some special pipe for
 pg_upgrade to communicate the a backend directly, but live check mode
 hightlights that we will _still_ need traditional connection abilities
 even if we add the pipe ability.

So?  By definition, the live check mode is not guaranteed to produce
correct answers, since other connections could be changing the
database's contents.  The problem we are interested in solving here is
preventing other connections from occurring when we're doing the upgrade
for real.  All this stuff with moving sockets around is nothing but
security by obscurity; it cannot positively guarantee that there's
nobody else connecting to the database while pg_upgrade runs.  (Most
notably, on Windows there's no guarantee at all.)

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Yet another failure mode in pg_upgrade

2012-09-01 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 + /*
 +  *  Report the Unix domain socket directory location to the 
 postmaster.
 +  */

Report seems entirely the wrong verb there.

 + #define LISTEN_STR   -c listen_addresses=''
 + 
 + /* Have a sockdir to use? */
 + if (strlen(os_info.sockdir) != 0)
 + snprintf(socket_string, sizeof(socket_string) - 
 strlen(LISTEN_STR),
 +  -c %s='%s',
 + (GET_MAJOR_VERSION(cluster-major_version)  903) ?
 + unix_socket_directory : 
 unix_socket_directories,
 + os_info.sockdir);
 + 
 + /* prevent TCP/IP connections */
 + strcat(socket_string, LISTEN_STR);

IMO this would be simpler and more readable if you got rid of the LISTEN_STR
#define and just included -c listen_addresses='' in the snprintf format
string.  The comment for the whole thing should be something like
If we have a socket directory to use, command the postmaster to use it,
and disable TCP/IP connections altogether.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Yet another failure mode in pg_upgrade

2012-09-01 Thread Bruce Momjian
On Sat, Sep  1, 2012 at 02:23:22PM -0400, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  +   /*
  +*  Report the Unix domain socket directory location to the 
  postmaster.
  +*/
 
 Report seems entirely the wrong verb there.
 
  + #define LISTEN_STR -c listen_addresses=''
  + 
  +   /* Have a sockdir to use? */
  +   if (strlen(os_info.sockdir) != 0)
  +   snprintf(socket_string, sizeof(socket_string) - 
  strlen(LISTEN_STR),
  +-c %s='%s',
  +   (GET_MAJOR_VERSION(cluster-major_version)  903) ?
  +   unix_socket_directory : 
  unix_socket_directories,
  +   os_info.sockdir);
  +   
  +   /* prevent TCP/IP connections */
  +   strcat(socket_string, LISTEN_STR);
 
 IMO this would be simpler and more readable if you got rid of the LISTEN_STR
 #define and just included -c listen_addresses='' in the snprintf format
 string.  The comment for the whole thing should be something like
 If we have a socket directory to use, command the postmaster to use it,
 and disable TCP/IP connections altogether.

Well, you only want the unix_socket* if sockdir is defined, but you want
LISTEN_STR unconditionally, even if there is no sockdir.  Not sure how
that could cleanly be in a single snprintf.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Yet another failure mode in pg_upgrade

2012-09-01 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 Well, you only want the unix_socket* if sockdir is defined, but you want
 LISTEN_STR unconditionally, even if there is no sockdir.

Really?  What will happen when the installation's default is to not
listen on any Unix socket?  (unix_socket_directories = '' in
postgresql.conf.)

I'm inclined to think that the no sockdir case is broken and you
should get rid of it.  If you're starting a postmaster, you can and
should tell it a sockdir, period.  If you're running a live check this
code is all moot anyway.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Yet another failure mode in pg_upgrade

2012-09-01 Thread Bruce Momjian
On Sat, Sep  1, 2012 at 02:18:59PM -0400, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  On Sat, Sep  1, 2012 at 11:45:58AM -0400, Bruce Momjian wrote:
  So, in summary, this patch moves the socket directory to the current
  directory all but live check operation, and handles different socket
  directories for old cluster = 9.1.  I have added a documentation
  mention of how to make this work for for pre-9.1 old servers.
  
  Thus completes another surgery on a moving train that is pg_upgrade
  development.
 
  Oh, one more thing.  We have talked about creating some special pipe for
  pg_upgrade to communicate the a backend directly, but live check mode
  hightlights that we will _still_ need traditional connection abilities
  even if we add the pipe ability.
 
 So?  By definition, the live check mode is not guaranteed to produce
 correct answers, since other connections could be changing the
 database's contents.  The problem we are interested in solving here is

True.

 preventing other connections from occurring when we're doing the upgrade
 for real.  All this stuff with moving sockets around is nothing but
 security by obscurity; it cannot positively guarantee that there's
 nobody else connecting to the database while pg_upgrade runs.  (Most
 notably, on Windows there's no guarantee at all.)

My point is that we are still going to need traditional connections for
live checks.  If we could find a solution for Windows, the socket in
current directory might be enough to lock things down, especially if we
put the socket in a new subdirectory that only we can read/write to. 
Should I persue that in my patch?

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Yet another failure mode in pg_upgrade

2012-09-01 Thread Bruce Momjian
On Sat, Sep  1, 2012 at 02:43:35PM -0400, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  Well, you only want the unix_socket* if sockdir is defined, but you want
  LISTEN_STR unconditionally, even if there is no sockdir.
 
 Really?  What will happen when the installation's default is to not
 listen on any Unix socket?  (unix_socket_directories = '' in
 postgresql.conf.)

Well, don't do that then.  Locking out TCP seems like a big win.

 I'm inclined to think that the no sockdir case is broken and you
 should get rid of it.  If you're starting a postmaster, you can and
 should tell it a sockdir, period.  If you're running a live check this
 code is all moot anyway.

I don't think you understand.  The no sockdir case is only for live
checks of pre-9.1 old servers, because we can't find the socket
directory being used.  Everything else uses the local directory for the
socket.  If we remove that case, we can't do live checks on pre-9.1
servers.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Yet another failure mode in pg_upgrade

2012-09-01 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 My point is that we are still going to need traditional connections for
 live checks.

Yes, but that's not terribly relevant, IMO.  All it means is that we
don't want to invent some solution that doesn't go through libpq.

 If we could find a solution for Windows, the socket in
 current directory might be enough to lock things down, especially if we
 put the socket in a new subdirectory that only we can read/write to. 

Who is we?  Somebody else logged in under the postgres userid could
still connect.

 Should I persue that in my patch?

I think this is just a band-aid, and we shouldn't be putting more
effort into it than needed to ensure that unexpected configuration
settings won't break it.  The right fix is a better form of
standalone-backend mode.  Maybe I will go pursue that, since nobody
else seems to want to.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Yet another failure mode in pg_upgrade

2012-09-01 Thread Tom Lane
Bruce Momjian br...@momjian.us writes:
 On Sat, Sep  1, 2012 at 02:43:35PM -0400, Tom Lane wrote:
 I'm inclined to think that the no sockdir case is broken and you
 should get rid of it.  If you're starting a postmaster, you can and
 should tell it a sockdir, period.  If you're running a live check this
 code is all moot anyway.

 I don't think you understand.  The no sockdir case is only for live
 checks of pre-9.1 old servers, because we can't find the socket
 directory being used.  Everything else uses the local directory for the
 socket.  If we remove that case, we can't do live checks on pre-9.1
 servers.

If it's a live check, then (a) you aren't restarting the postmaster,
and (b) you wouldn't want to lock out TCP anyway.  So adding
--listen-addresses to the string seems pointless and/or wrong.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Yet another failure mode in pg_upgrade

2012-09-01 Thread Bruce Momjian
On Sat, Sep  1, 2012 at 03:06:57PM -0400, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  On Sat, Sep  1, 2012 at 02:43:35PM -0400, Tom Lane wrote:
  I'm inclined to think that the no sockdir case is broken and you
  should get rid of it.  If you're starting a postmaster, you can and
  should tell it a sockdir, period.  If you're running a live check this
  code is all moot anyway.
 
  I don't think you understand.  The no sockdir case is only for live
  checks of pre-9.1 old servers, because we can't find the socket
  directory being used.  Everything else uses the local directory for the
  socket.  If we remove that case, we can't do live checks on pre-9.1
  servers.
 
 If it's a live check, then (a) you aren't restarting the postmaster,
 and (b) you wouldn't want to lock out TCP anyway.  So adding
 --listen-addresses to the string seems pointless and/or wrong.

What about the new server?  That is still started and stopped.  You are
right that this code is never going to be called for the check of a
running old server.

Let's walk through the options:

non-live check:
uses current directory, start/stop old/new servers

live check, old server = 9.1:
only new server started/stopped, new server uses old server's
socket directory and PGHOST set so clients use the same directory

live check, old server  9.1:
only new server started/stopped, old/new servers use their
default/configured socket directory

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Yet another failure mode in pg_upgrade

2012-09-01 Thread Bruce Momjian
On Sat, Sep  1, 2012 at 03:05:01PM -0400, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  My point is that we are still going to need traditional connections for
  live checks.
 
 Yes, but that's not terribly relevant, IMO.  All it means is that we
 don't want to invent some solution that doesn't go through libpq.
 
  If we could find a solution for Windows, the socket in
  current directory might be enough to lock things down, especially if we
  put the socket in a new subdirectory that only we can read/write to. 
 
 Who is we?  Somebody else logged in under the postgres userid could
 still connect.

But they have to find the current directory to do that;  seems unlikely.
They could kill -9 pg_upgrade too if they are the same user id.

  Should I persue that in my patch?
 
 I think this is just a band-aid, and we shouldn't be putting more
 effort into it than needed to ensure that unexpected configuration
 settings won't break it.  The right fix is a better form of
 standalone-backend mode.  Maybe I will go pursue that, since nobody
 else seems to want to.

I am worried that is going to be a complex solution to a very minor
problem.  Also, how is that going to get backpatched?

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] too much pgbench init output

2012-09-01 Thread Tomas Vondra
On 1 Září 2012, 12:30, Robert Haas wrote:
 On Sat, Sep 1, 2012 at 12:00 AM, Peter Eisentraut pete...@gmx.net wrote:
 When initializing a large database, pgbench writes tons of %d tuples
 done lines.  I propose to change this to a sort of progress counter
 that stays on the same line, as in the attached patch.

 I'm not sure I like this - what if the output is being saved off to a
 file?

What about using istty(stdout) to handle this situation? Although I find
it usually confusing, because it prints one thing when executed directly
and something else when the output is redirected to a file.

I see two other options:

(1) removing this output altogether (I can't imagine a situation when this
really matters) and replace it with a simple inserted 23% of rows,
estimated remaining time 14:23 (863 sec), updated each 1%

(2) adding a switch (--verbose) that enables these lines, don't print them
by default

Another option might be updating the process title so that the top shows
current progress more precisely than (1).

Tomas



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Date conversion using day of week

2012-09-01 Thread Bruce Momjian
[Properly posted to hackers list]

On Fri, Apr  1, 2011 at 02:27:02AM +1100, Brendan Jurd wrote:
 On 1 April 2011 02:00, Adrian Klaver adrian.kla...@gmail.com wrote:
  On Wednesday, March 30, 2011 8:39:25 pm Brendan Jurd wrote:
  If we wanted to make it work, then I think the thing to do would be
  to add a new set of formatting tokens IDY, IDAY etc.  I don't like the
  idea of interpreting DY and co. differently depending on whether the
  other tokens happen to be ISO week or Gregorian.
 
  Just to play Devils advocate here, but why not? The day name is the same 
  either
  way, it is the index that changes. I am not sure why that could not be 
  context
  specific?
 
 
 To be perfectly honest, it's mostly because I was hoping not to spend
 very much more of my time in formatting.c.  Every time I go in there I
 come out a little bit less sane.  I'm concerned that if I do anything
  ---

Agreed!

 further to it, I might inadvertently summon Chattur'gha or something.
 But since you went to the trouble of calling me on my laziness, let's
 take a look at the problem.
 
 At the time when the day-of-week token gets converted into a numeric
 value and put into the TmFromChar.d field, the code has no knowledge
 of whether the overall pattern is Gregorian or ISO (the DY field could
 well be at the front of the pattern, for example).
 
 Later on, in do_to_timestamp, the code expects the 'd' value to make
 sense given the mode (it should be zero-based on Sunday for Gregorian,
 or one-based on Monday for ISO).  That's all well and good *except* in
 the totally bizarre case raised by the OP.
 
 To resolve it, we could make TmFromChar.d always stored using the ISO
 convention (because zero then has the useful property of meaning not
 set) and converted to the Gregorian convention as necessary in
 do_to_timestamp.

I did quite a bit if study on this and have a fix in the attached patch.
Brendan above is correct about the cause of the problems.  Basically,
'd' was sometimes numbered 1-7 with Monday as week start, and 'd' was at
other times 0-6 with Sunday as start.  Plus, zero was used to designate
not supplied in ISO tests.  Obviously the number and the start value
both caused problems.

The attached patch fixes this by using Gregorian 1-7 (Sunday=7) format
throughout, allowing any mix of Gregorian and ISO week designations.  It
is converted to ISO (or Unix format 0-6, Sunday=0) as needed.

Sample output:

test= select to_date('2011-13-MON', 'IYYY-IW-DY');
  to_date

 2011-03-28
(1 row)

test= select to_date('2011-13-SUN', 'IYYY-IW-DY');
  to_date

 2011-04-03
(1 row)

test= select to_date('2011-13-SAT', 'IYYY-IW-DY');
  to_date

 2011-04-02
(1 row)

test= select to_date('2011-13-1', 'IYYY-IW-ID');
  to_date

 2011-03-28
(1 row)

test= select to_date('2011-13-7', 'IYYY-IW-ID');
  to_date

 2011-04-03
(1 row)

test= select to_date('2011-13-0', 'IYYY-IW-ID');
  to_date

 2011-04-03
(1 row)

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
new file mode 100644
index 25af8a2..2aa6df1
*** a/src/backend/utils/adt/formatting.c
--- b/src/backend/utils/adt/formatting.c
*** typedef struct
*** 412,418 
  mi,
  ss,
  ,
! d,
  dd,
  ddd,
  mm,
--- 412,418 
  mi,
  ss,
  ,
! d,/* stored as 1-7, Sunday = 1, 0 means missing */
  dd,
  ddd,
  mm,
*** DCH_from_char(FormatNode *node, char *in
*** 2897,2902 
--- 2897,2903 
  from_char_seq_search(value, s, days, ONE_UPPER,
  	 MAX_DAY_LEN, n);
  from_char_set_int(out-d, value, n);
+ out-d++;
  break;
  			case DCH_DY:
  			case DCH_Dy:
*** DCH_from_char(FormatNode *node, char *in
*** 2904,2909 
--- 2905,2911 
  from_char_seq_search(value, s, days, ONE_UPPER,
  	 MAX_DY_LEN, n);
  from_char_set_int(out-d, value, n);
+ out-d++;
  break;
  			case DCH_DDD:
  from_char_parse_int(out-ddd, s, n);
*** DCH_from_char(FormatNode *node, char *in
*** 2919,2929 
  break;
  			case DCH_D:
  from_char_parse_int(out-d, s, n);
- out-d--;
  s += SKIP_THth(n-suffix);
  break;
  			case DCH_ID:
  from_char_parse_int_len(out-d, s, 1, n);
  s += SKIP_THth(n-suffix);
  break;
  			case DCH_WW:
--- 2921,2933 
  break;
  			case DCH_D:
  from_char_parse_int(out-d, s, n);
  s += SKIP_THth(n-suffix);
  break;
  			case DCH_ID:
 

Re: [HACKERS] Getting rid of cheap-startup-cost paths earlier

2012-09-01 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Tue, May 22, 2012 at 1:50 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Currently, the planner keeps paths that appear to win on the grounds of
 either cheapest startup cost or cheapest total cost.  It suddenly struck
 me that in many simple cases (viz, those with no LIMIT, EXISTS, cursor
 fast-start preference, etc) we could know a-priori that cheapest startup
 cost is not going to be interesting, and hence immediately discard any
 path that doesn't win on total cost.
 
 This would require some additional logic to detect whether the case
 applies, as well as extra complexity in add_path.  So it's possible
 that it wouldn't be worthwhile overall.  Still, it seems like it might
 be a useful idea to investigate.
 
 Thoughts?

 Yeah, I think we should investigate that.  Presumably you could easily
 have a situation where one part of the tree is under a LIMIT or EXISTS
 and therefore needs to preserve fast-start plans but the rest of the
 (potentially large) tree isn't, so we need something fairly
 fine-grained, I think.  Maybe we could add a flag to each RelOptInfo
 indicating whether fast-start plans should be kept, or something like
 that.

I got around to looking at this finally.  It turns out to be a big win,
at least for queries without any LIMIT or other reason to worry about
fast-start plans.

As things currently stand, there isn't any reason to control the
decision at finer than per-subquery level.  I did it the way you suggest
above anyway, with a per-RelOptInfo flag, because add_path() is passed
only a RelOptInfo and not the PlannerInfo root structure.  We could
have changed that of course, but it would have meant changing an API
used by FDWs, which would be annoying.  It seems possible that in future
somebody might think of a way to determine this on a per-relation level,
so I thought this design might be a bit more future-proof anyway.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Getting rid of cheap-startup-cost paths earlier

2012-09-01 Thread Bruce Momjian
On Sat, Sep  1, 2012 at 06:23:59PM -0400, Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
  On Tue, May 22, 2012 at 1:50 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  Currently, the planner keeps paths that appear to win on the grounds of
  either cheapest startup cost or cheapest total cost.  It suddenly struck
  me that in many simple cases (viz, those with no LIMIT, EXISTS, cursor
  fast-start preference, etc) we could know a-priori that cheapest startup
  cost is not going to be interesting, and hence immediately discard any
  path that doesn't win on total cost.
  
  This would require some additional logic to detect whether the case
  applies, as well as extra complexity in add_path.  So it's possible
  that it wouldn't be worthwhile overall.  Still, it seems like it might
  be a useful idea to investigate.
  
  Thoughts?
 
  Yeah, I think we should investigate that.  Presumably you could easily
  have a situation where one part of the tree is under a LIMIT or EXISTS
  and therefore needs to preserve fast-start plans but the rest of the
  (potentially large) tree isn't, so we need something fairly
  fine-grained, I think.  Maybe we could add a flag to each RelOptInfo
  indicating whether fast-start plans should be kept, or something like
  that.
 
 I got around to looking at this finally.  It turns out to be a big win,
 at least for queries without any LIMIT or other reason to worry about
 fast-start plans.

Yes, I remember from the early days how quickly the number of considred
paths can grow.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Yet another failure mode in pg_upgrade

2012-09-01 Thread Robert Haas
On Sat, Sep 1, 2012 at 11:45 AM, Bruce Momjian br...@momjian.us wrote:
 On Mon, Aug 13, 2012 at 12:46:43PM +0200, Magnus Hagander wrote:
 On Mon, Aug 13, 2012 at 4:34 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  I've been experimenting with moving the Unix socket directory to
  /var/run/postgresql for the Fedora distribution (don't ask :-().
  It's mostly working, but I found out yet another way that pg_upgrade
  can crash and burn: it doesn't consider the possibility that the
  old or new postmaster is compiled with a different default
  unix_socket_directory than what is compiled into the libpq it's using
  or that pg_dump is using.
 
  This is another hazard that we could forget about if we had some way for
  pg_upgrade to run standalone backends instead of starting a postmaster.

 Yeah, that would be nice.


  But in the meantime, I suggest it'd be a good idea for pg_upgrade to
  explicitly set unix_socket_directory (or unix_socket_directories in
  HEAD) when starting the postmasters, and also explicitly set PGHOST
  to ensure that the client-side code plays along.

 That sounds like a good idea for other reasons as well - manual
 connections attempting to get in during an upgrade will just fail with
 a no connection error, which makes sense...

 So, +1.

 OK, I looked this over, and I have a patch, attached.

 Because we are already playing with socket directories, this patch creates
 the socket files in the current directory for upgrades and non-live
 checks, but not live checks.  This eliminates the someone accidentally
 connects problem, at least on Unix, plus we are using port 50432
 already.  This also turns off TCP connections for unix domain socket
 systems.

 For live check operation, you are checking a running server, so
 assuming the socket is in the current directory is not going to work.
 What the code does is to read the 5th line from the running server's
 postmaster.pid file, which has the socket directory in PG = 9.1.  For
 pre-9.1, pg_upgrade uses the compiled-in defaults for socket directory.
 If the defaults are different between the two servers, the new binaries,
 e.g. pg_dump, will not work.  The fix is for the user to set pg_upgrade
 -O to match the old socket directory, and set PGHOST before running
 pg_upgrade.  I could not find a good way to generate a proper error
 message because we are blind to the socket directory in pre-9.1.
 Frankly, this is a problem if the old pre-9.1 server is running in a
 user-configured socket directory too, so a documentation addition seems
 right here.

 So, in summary, this patch moves the socket directory to the current
 directory all but live check operation, and handles different socket
 directories for old cluster = 9.1.  I have added a documentation
 mention of how to make this work for for pre-9.1 old servers.

I don't think this is reducing the number of failure modes; it's just
changing it from one set of obscure cases to a slightly different set
of obscure cases.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Fwd: PATCH: psql boolean display

2012-09-01 Thread Pavel Stehule
-- Forwarded message --
From: Pavel Stehule pavel.steh...@gmail.com
Date: 2012/9/1
Subject: PATCH: psql boolean display
To: Phil Sorber p...@omniti.com


Hello

I am looking to your patch:

I have only one note. I don't think so using any text for values
true and false is good idea. I don't see a sense of any special
texts like foo, bar from your example.

More strict design is better - I wrote simple modification - it is
based on psql psets - and it add new configuration settings boolstyle
[char|word]. A advantage of this design is consistency  and possible
autocomplete support.

Regards

Pavel



postgres= select true, false;
 bool │ bool
──┼──
 t│ f
(1 row)

postgres= \pset boolstyle word
Bool output style is word.
postgres= select true, false;
 bool │ bool
──┼───
 true │ false
(1 row)


boolstyle.diff
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] bitmap scan much slower than index scan, hash_search_with_hash_value

2012-09-01 Thread Sergey Koposov

Hi,

I'm experiencing the case when bitmap scan is ~ 70 times slower than 
index scan which seems to be caused by 1) very big table 2) some hash 
search logic (hash_search_with_hash_value )


Here is the explain analyze of the query with bitmap scans allowed:

wsdb= explain analyze select * from test as t, crts.data as d1
where d1.objid=t.objid and d1.mjd=t.mjd limit 1;
  QUERY PLAN
---
 Limit  (cost=11514.04..115493165.44 rows=1 width=68) (actual 
time=27.512..66620.231 rows=1 loops=1)
   -  Nested Loop  (cost=11514.04..1799585184.18 rows=155832 width=68) (actual 
time=27.511..66616.807 rows=1 loops=1)
 -  Seq Scan on test t  (cost=0.00..2678.40 rows=156240 width=28) 
(actual time=0.010..4.685 rows=11456 loops=1)
 -  Bitmap Heap Scan on data d1  (cost=11514.04..11518.05 rows=1 
width=40) (actual time=5.807..5.807 rows=1 loops=11456)
   Recheck Cond: ((mjd = t.mjd) AND (objid = t.objid))
   -  BitmapAnd  (cost=11514.04..11514.04 rows=1 width=0) (actual 
time=5.777..5.777 rows=0 loops=11456)
 -  Bitmap Index Scan on data_mjd_idx  (cost=0.00..2501.40 
rows=42872 width=0) (actual time=3.920..3.920 rows=22241 loops=11456)
   Index Cond: (mjd = t.mjd)
 -  Bitmap Index Scan on data_objid_idx  
(cost=0.00..8897.90 rows=415080 width=0) (actual time=0.025..0.025 rows=248 
loops=11456)
   Index Cond: (objid = t.objid)
 Total runtime: 66622.026 ms
(11 rows)

Here is the output when bitmap scans are disabled:  
  QUERY PLAN
 QUERY PLAN 


 Limit  (cost=0.00..329631941.65 rows=1 width=68) (actual 
time=0.082..906.876 rows=1 loops=1)
   -  Nested Loop  (cost=0.00..4979486036.95 rows=151062 width=68) (actual 
time=0.081..905.683 rows=1 loops=1)
 Join Filter: (t.mjd = d1.mjd)
 -  Seq Scan on test t  (cost=0.00..2632.77 rows=151677 width=28) 
(actual time=0.009..1.679 rows=11456 loops=1)
 -  Index Scan using data_objid_idx on data d1  (cost=0.00..26603.32 
rows=415080 width=40) (actual time=0.010..0.050 rows=248 loops=11456)
   Index Cond: (objid = t.objid)
 Total runtime: 907.462 ms

When the bitmap scans are enabled the prof of postgres shows
47.10%  postmaster  postgres   [.] hash_search_with_hash_value
|
--- hash_search_with_hash_value

11.06%  postmaster  postgres   [.] hash_seq_search
|
--- hash_seq_search

 6.95%  postmaster  postgres   [.] hash_any
|
--- hash_any

 5.17%  postmaster  postgres   [.] _bt_checkkeys
|
--- _bt_checkkeys

 4.07%  postmaster  postgres   [.] tbm_add_tuples
|
--- tbm_add_tuples

 3.41%  postmaster  postgres   [.] hash_search
|
--- hash_search


And the last note is that the crts.data table which is being bitmap scanned is 
a 1.1Tb table with ~ 20e9 rows. My feeling is that the bitmap index scan code

is somehow unprepared to combine two bitmaps for such a big table, and this
leads to the terrible performance.

Regards,
Sergey

PS Here are the schemas of the tables, just in case:
wsdb= \d test
  Table koposov.test
 Column  |   Type   | Modifiers
-+--+---
 mjd | double precision |
 fieldid | bigint   |
 intmag  | integer  |
 objid   | bigint   |

wsdb= \d crts.data
   Table crts.data
 Column |   Type   | Modifiers
+--+---
 objid  | bigint   |
 mjd| double precision |
 mag| real |
 emag   | real |
 ra | double precision |
 dec| double precision |
Indexes:
data_mjd_idx btree (mjd) WITH (fillfactor=100)
data_objid_idx btree (objid) WITH (fillfactor=100)
data_q3c_ang2ipix_idx btree (q3c_ang2ipix(ra, dec)) WITH 
(fillfactor=100)

PPS shared_buffers=10GB, work_mem=1GB
All the test shown here were don in fully cached regime.

PPS I can believe that what I'm seeing is a feature, not a bug of bitmap scans,
and I can live with disabling them, but I still thought it's worth reporting.


*
Sergey E. Koposov, PhD, Research Associate
Institute of Astronomy, University of Cambridge
Madingley road, CB3 0HA, Cambridge, UK
Tel: +44-1223-337-551


--