Re: [HACKERS] Add socket dir to pg_config..?

2011-10-28 Thread Cédric Villemain
2011/10/28 Tom Lane :
> Stephen Frost  writes:
>>   Was just wondering if we might want to include the default socket
>>   directory that was compiled in as part of the pg_config output..?
>
> [ shrug... ]  We don't report the compiled-in port number, which is
> considerably more critical.  And we don't report changes in any of the
> other stuff in pg_config_manual.h.
>
> MHO is that changing the socket directory is only marginally supported,
> and we shouldn't encourage it unless we're prepared to fully support it
> (which we can't really).

There is a TODO about that.

   Allow simpler reporting of the unix domain socket directory and allow
   easier configuration of its default location

   * http://archives.postgresql.org/pgsql-hackers/2010-10/msg01555.php

Last time the subject came in, the result was that pg_config may
output it, but the solution that most people seems to agree at this
time was to add a configure option. Except that we didn't want to
encourage people to change the default_socket_dir, so a documentation
update in this direction was suggested to be done at the same time the
switch is added to PostgreSQL.

-- 
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

-- 
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] Add socket dir to pg_config..?

2011-10-28 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> [ shrug... ]  We don't report the compiled-in port number, which is
> considerably more critical.  And we don't report changes in any of the
> other stuff in pg_config_manual.h.

True.

> MHO is that changing the socket directory is only marginally supported,
> and we shouldn't encourage it unless we're prepared to fully support it
> (which we can't really).

This concerns me a bit, as most distros change it..  What would you
expect to break when the socket dir is changed from the default?

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Bruce Momjian
Tom Lane wrote:
> Bruce Momjian  writes:
> > Robert Haas wrote:
> >> that if you're doing something to the database that someone might
> >> object to, you oughtn't be doing it to the postgres database either.
> >> You should create a database just for pg_upgrade's use and install its
> >> crap in there.
> 
> > It installs crap in all databases to set oids on system tables,
> 
> It seems like you're both confusing the source and target clusters.
> None of that stuff gets installed in the source, does it?

Right, only in the target.

Let me summarize:

The postgres database is required in the source because pg_upgrade likes
to have a 1-1 database mapping of old and new clusters.  This can be
changed, but it makes pg_upgrade slightly more complex.

The postgres database is required on the target because pg_upgrade
creates the support functions first in that database.  That can be
changed, but pg_dumpall restores roles in the postgres database by
default, not template1.  Again, this can be changed.

Because of this 'postgres' new cluster requirement, you can't just
delete the postgres database from the new cluster and run pg_upgrade.

Tom wants pg_upgrade to work if the old cluster doesn't have a postgres
database.  I see two solutions --- either remove the 1-1 mapping of
old/new databases, or remove the pg_upgrade and pg_dumpall dependence on
the postgres database and tell users to remove postgres from the new
cluster before the upgrade.  

They already get a clear error message about the problem, which I think
is why we haven't seen more problem reports.  My guess is they are just
creating the postgres database on the old cluster before the upgrade
after they get the error.

I have applied the attached patch to at least clarify that they need the
postgres database in the old cluster, rather than them trying to remove
the postgres database from the new cluster.

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

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
new file mode 100644
index 5b9b4cd..e400814
*** a/contrib/pg_upgrade/check.c
--- b/contrib/pg_upgrade/check.c
*** check_old_cluster_has_new_cluster_dbs(vo
*** 403,410 
  	   new_cluster.dbarr.dbs[new_dbnum].db_name) == 0)
  break;
  		if (old_dbnum == old_cluster.dbarr.ndbs)
! 			pg_log(PG_FATAL, "New cluster database \"%s\" does not exist in the old cluster\n",
!    new_cluster.dbarr.dbs[new_dbnum].db_name);
  	}
  }
  
--- 403,415 
  	   new_cluster.dbarr.dbs[new_dbnum].db_name) == 0)
  break;
  		if (old_dbnum == old_cluster.dbarr.ndbs)
! 		{
! 			if (strcmp(new_cluster.dbarr.dbs[new_dbnum].db_name, "postgres") == 0)
! pg_log(PG_FATAL, "The \"postgres\" database must exist in the old cluster\n");
! 			else
! pg_log(PG_FATAL, "New cluster database \"%s\" does not exist in the old cluster\n",
! 	   new_cluster.dbarr.dbs[new_dbnum].db_name);
! 		}
  	}
  }
  

-- 
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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Bruce Momjian
Magnus Hagander wrote:
> On Oct 28, 2011 5:22 AM, "Tom Lane"  wrote:
> >
> > Bruce Momjian  writes:
> > > Stephen Frost wrote:
> > >> Yes, they would have removed it because they didn't want it.  As I
> > >> recall, part of the agreement to create an extra database by default
> was
> > >> that it could be removed if users didn't want it.  Turning around and
> > >> then saying "but things won't work if it's not there" isn't exactly
> > >> supporting users who decide to remove it.
> >
> > > Well, you would have to remove it _after_ you did the pg_upgrade.
> >
> > As far as the *target* cluster is concerned, I have no sympathy for
> > someone who messes with its contents before running pg_upgrade.
> > That's an RTFM matter: you're supposed to upgrade into a virgin
> > just-initdb'd cluster.
> >
> > However, it would be nice if pg_upgrade supported transferring from a
> > *source* cluster that didn't have the postgres DB.
> >
> > What about creating a new, single-purpose database in the source
> > cluster and then removing it again after we're done?
> 
> How about naming this newly created database "postgres"? That would make the
> code simple enough - always use the postgres database, just drop it at the
> end if it didn't exist in the source cluster.

Yes, that would work, but see my summarization email on this.  Using
template1 is not a problem for pg_upgrade, it is the modifications to
pg_dumpall that are an issue.

-- 
  Bruce Momjian  http://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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Bruce Momjian
Magnus Hagander wrote:
> On Oct 28, 2011 5:19 AM, "Bruce Momjian"  wrote:
> >
> > Stephen Frost wrote:
> >
> > > > > Regarding pg_dumpall and pg_restore, I'm pretty sure both of those
> can
> > > > > be configured to connect to other databases instead, including for
> > > > > globals.
> > > >
> > > > Well, please show me the code, because the C code I showed you had the
> > > > '\connect postgres' string hardcoded in there.
> > >
> > > I guess there's a difference between "can be used and will work
> > > correctly, but might create some extra garbage" and "can't be used at
> > > all".  pg_dumpall has a -l option for connecting to whatever *existing*
> > > database you have to pull the global data, and then it'll restore into a
> > > clean initdb'd cluster, after which you could remove postgres.
> >
> > Keep in mind -l might connect to a specified database to do the dump,
> > but it will still connect to the 'postgres' database to recreate them.
> >
> > > Admittedly, if you initdb the cluster, drop postgres, and then try a
> > > restore, it would fail.  Personally, I'm not a big fan of that (why
> >
> > Right, same with pg_upgrade.
> >
> > > don't we use what was passed in to -l for that..?), but, practically,
> >
> > No idea.
> 
> Chicken/egg? If we did that, the pg_dumpall dump could no longer be loaded
> into an empty cluster since the db it wanted to talk to didn't exist yet.
> And restoring into an empty cluster has to be the main use for pg_dumpall
> after all

True.  My assumption was that they had created some special database
before they did the pg_dumpall restore, but it would be odd because the
database would have been hard-coded into the dump, which isn't good.

-- 
  Bruce Momjian  http://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] So, is COUNT(*) fast now?

2011-10-28 Thread Bruce Momjian
Tom Lane wrote:
> I wonder how trustworthy the measure of the visibilitymap_test call site
> as a consumer of cycles really is.  I've frequently noticed that
> oprofile blames remarkably large fractions of the runtime on individual
> statements that appear to be quite trivial.  I'm not sure if that

Are these statements perhaps near kernel calls?

-- 
  Bruce Momjian  http://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] Add socket dir to pg_config..?

2011-10-28 Thread Andrew Dunstan



On 10/28/2011 08:01 AM, Stephen Frost wrote:




MHO is that changing the socket directory is only marginally supported,
and we shouldn't encourage it unless we're prepared to fully support it
(which we can't really).

This concerns me a bit, as most distros change it..  What would you
expect to break when the socket dir is changed from the default?




Er, which distros other than debian/ubuntu?

cheers

andrew

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


[HACKERS] ecpg-related build failure with make 3.82

2011-10-28 Thread Robert Haas
On my Fedora 14 box, make -j3 fails.  I think the below is the
relevant portion of the output.  This has a passing similarity to bug
5665, but I can't for the life of me see what the problem is here.
parer.o depends on preproc.h, which depends on preproc.c; therefore,
parser.o should not be built until preproc.c has been built, but
that's exactly what make is doing.

make[4]: Entering directory `/home/rhaas/pgsql/src/interfaces/ecpg/preproc'
make -C ../../../../src/port all
'/usr/bin/perl' ./parse.pl . < ../../../backend/parser/gram.y > preproc.y
make[4]: Entering directory `/home/rhaas/pgsql/src/interfaces/ecpg/pgtypeslib'
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -g -pthread  -D_REENTRANT -D_THREAD_SAFE
-D_POSIX_PTHREAD_SEMANTICS -fpic -I../include
-I../../../../src/interfaces/ecpg/include
-I../../../../src/include/utils -I../../../../src/interfaces/libpq
-I../../../../src/include -D_GNU_SOURCE  -DSO_MAJOR_VERSION=3  -c -o
numeric.o numeric.c -MMD -MP -MF .deps/numeric.Po
make[5]: Entering directory `/home/rhaas/pgsql/src/port'
make -C ../backend submake-errcodes
make[6]: Entering directory `/home/rhaas/pgsql/src/backend'
make[6]: Nothing to be done for `submake-errcodes'.
make[6]: Leaving directory `/home/rhaas/pgsql/src/backend'
make[5]: Leaving directory `/home/rhaas/pgsql/src/port'
/usr/bin/flex  -o'pgc.c' pgc.l
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -g -pthread  -D_REENTRANT -D_THREAD_SAFE
-D_POSIX_PTHREAD_SEMANTICS -DECPG_COMPILE -I../include
-I../../../../src/interfaces/ecpg/include -I. -I. -DMAJOR_VERSION=4
-DMINOR_VERSION=7 -DPATCHLEVEL=0 -I../../../../src/include
-D_GNU_SOURCE   -c -o type.o type.c -MMD -MP -MF .deps/type.Po
'/usr/bin/perl' ./check_rules.pl . ../../../backend/parser/gram.y
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -g -pthread  -D_REENTRANT -D_THREAD_SAFE
-D_POSIX_PTHREAD_SEMANTICS -DECPG_COMPILE -I../include
-I../../../../src/interfaces/ecpg/include -I. -I. -DMAJOR_VERSION=4
-DMINOR_VERSION=7 -DPATCHLEVEL=0 -I../../../../src/include
-D_GNU_SOURCE   -c -o ecpg.o ecpg.c -MMD -MP -MF .deps/ecpg.Po
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -g -pthread  -D_REENTRANT -D_THREAD_SAFE
-D_POSIX_PTHREAD_SEMANTICS -DECPG_COMPILE -I../include
-I../../../../src/interfaces/ecpg/include -I. -I. -DMAJOR_VERSION=4
-DMINOR_VERSION=7 -DPATCHLEVEL=0 -I../../../../src/include
-D_GNU_SOURCE   -c -o output.o output.c -MMD -MP -MF .deps/output.Po
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -g -pthread  -D_REENTRANT -D_THREAD_SAFE
-D_POSIX_PTHREAD_SEMANTICS -fpic -I../include
-I../../../../src/interfaces/ecpg/include
-I../../../../src/include/utils -I../../../../src/interfaces/libpq
-I../../../../src/include -D_GNU_SOURCE  -DSO_MAJOR_VERSION=3  -c -o
datetime.o datetime.c -MMD -MP -MF .deps/datetime.Po
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -g -pthread  -D_REENTRANT -D_THREAD_SAFE
-D_POSIX_PTHREAD_SEMANTICS -DECPG_COMPILE -I../include
-I../../../../src/interfaces/ecpg/include -I. -I. -DMAJOR_VERSION=4
-DMINOR_VERSION=7 -DPATCHLEVEL=0 -I../../../../src/include
-D_GNU_SOURCE   -c -o parser.o parser.c -MMD -MP -MF .deps/parser.Po
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -g -pthread  -D_REENTRANT -D_THREAD_SAFE
-D_POSIX_PTHREAD_SEMANTICS -fpic -I../include
-I../../../../src/interfaces/ecpg/include
-I../../../../src/include/utils -I../../../../src/interfaces/libpq
-I../../../../src/include -D_GNU_SOURCE  -DSO_MAJOR_VERSION=3  -c -o
common.o common.c -MMD -MP -MF .deps/common.Po
parser.c:25:21: fatal error: preproc.h: No such file or directory
compilation terminated.
make[4]: *** [parser.o] Error 1
make[4]: Leaving directory `/home/rhaas/pgsql/src/interfaces/ecpg/preproc'
make[3]: *** [all-preproc-recurse] Error 2
make[3]: *** Waiting for unfinished jobs

-- 
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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 8:12 AM, Bruce Momjian  wrote:
> Yes, that would work, but see my summarization email on this.  Using
> template1 is not a problem for pg_upgrade, it is the modifications to
> pg_dumpall that are an issue.

I just did a bit of testing on this.  It appears that pg_dumpall, if
given a cluster containing no postgres database, will happily try to
connect to template1 instead.  If template1 isn't available either,
you can use "-l SOMEDBNAME" to specify the name of another database to
connect to instead.  So there is infinite flexibility there.

But regardless of which database it uses to *generate* the dump, the
dump itself will *always* contain this, right at the very beginning:

\connect postgres

That line is in fact hard-coded as a literal string in pg_dumpall.c.
It seems like the easiest fix here might be to just remove that line
from the dump, because AFAICS it's completely pointless.  During the
time for which that setting is in effect, we're just restoring
globals, so it shouldn't matter which database we're connected to;
only that we have a valid connection.  So trying to switch the
connection from whatever the user is connected to currently to
postgres doesn't accomplish anything useful, but it does make it
possible for dump restoration to unnecessarily fail.

-- 
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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Thom Brown
On 28 October 2011 14:28, Robert Haas  wrote:
> On Fri, Oct 28, 2011 at 8:12 AM, Bruce Momjian  wrote:
>> Yes, that would work, but see my summarization email on this.  Using
>> template1 is not a problem for pg_upgrade, it is the modifications to
>> pg_dumpall that are an issue.
>
> I just did a bit of testing on this.  It appears that pg_dumpall, if
> given a cluster containing no postgres database, will happily try to
> connect to template1 instead.  If template1 isn't available either,
> you can use "-l SOMEDBNAME" to specify the name of another database to
> connect to instead.  So there is infinite flexibility there.
>
> But regardless of which database it uses to *generate* the dump, the
> dump itself will *always* contain this, right at the very beginning:
>
> \connect postgres
>
> That line is in fact hard-coded as a literal string in pg_dumpall.c.
> It seems like the easiest fix here might be to just remove that line
> from the dump, because AFAICS it's completely pointless.  During the
> time for which that setting is in effect, we're just restoring
> globals, so it shouldn't matter which database we're connected to;
> only that we have a valid connection.  So trying to switch the
> connection from whatever the user is connected to currently to
> postgres doesn't accomplish anything useful, but it does make it
> possible for dump restoration to unnecessarily fail.

This was the kind of qualm I had with createdb, which I submitted a
patch for earlier this year:
http://archives.postgresql.org/pgsql-hackers/2011-03/msg00999.php

-- 
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: 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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Bruce Momjian
Robert Haas wrote:
> On Fri, Oct 28, 2011 at 8:12 AM, Bruce Momjian  wrote:
> > Yes, that would work, but see my summarization email on this. ?Using
> > template1 is not a problem for pg_upgrade, it is the modifications to
> > pg_dumpall that are an issue.
> 
> I just did a bit of testing on this.  It appears that pg_dumpall, if
> given a cluster containing no postgres database, will happily try to
> connect to template1 instead.  If template1 isn't available either,
> you can use "-l SOMEDBNAME" to specify the name of another database to
> connect to instead.  So there is infinite flexibility there.
> 
> But regardless of which database it uses to *generate* the dump, the
> dump itself will *always* contain this, right at the very beginning:
> 
> \connect postgres
> 
> That line is in fact hard-coded as a literal string in pg_dumpall.c.
> It seems like the easiest fix here might be to just remove that line
> from the dump, because AFAICS it's completely pointless.  During the
> time for which that setting is in effect, we're just restoring
> globals, so it shouldn't matter which database we're connected to;
> only that we have a valid connection.  So trying to switch the
> connection from whatever the user is connected to currently to
> postgres doesn't accomplish anything useful, but it does make it
> possible for dump restoration to unnecessarily fail.

If you remove that line, I can modify pg_upgrade to use template1
instead of postgres, and then the user should just remove the postgres
database from the new cluster before the upgrade --- we can give them a
clear error message on that.

-- 
  Bruce Momjian  http://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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 9:55 AM, Bruce Momjian  wrote:
> Robert Haas wrote:
>> On Fri, Oct 28, 2011 at 8:12 AM, Bruce Momjian  wrote:
>> > Yes, that would work, but see my summarization email on this. ?Using
>> > template1 is not a problem for pg_upgrade, it is the modifications to
>> > pg_dumpall that are an issue.
>>
>> I just did a bit of testing on this.  It appears that pg_dumpall, if
>> given a cluster containing no postgres database, will happily try to
>> connect to template1 instead.  If template1 isn't available either,
>> you can use "-l SOMEDBNAME" to specify the name of another database to
>> connect to instead.  So there is infinite flexibility there.
>>
>> But regardless of which database it uses to *generate* the dump, the
>> dump itself will *always* contain this, right at the very beginning:
>>
>> \connect postgres
>>
>> That line is in fact hard-coded as a literal string in pg_dumpall.c.
>> It seems like the easiest fix here might be to just remove that line
>> from the dump, because AFAICS it's completely pointless.  During the
>> time for which that setting is in effect, we're just restoring
>> globals, so it shouldn't matter which database we're connected to;
>> only that we have a valid connection.  So trying to switch the
>> connection from whatever the user is connected to currently to
>> postgres doesn't accomplish anything useful, but it does make it
>> possible for dump restoration to unnecessarily fail.
>
> If you remove that line,

I'm happy to do that, unless someone can see a hole in my logic.

> I can modify pg_upgrade to use template1
> instead of postgres, and then the user should just remove the postgres
> database from the new cluster before the upgrade --- we can give them a
> clear error message on that.

What I would prefer is to have the upgrade succeed, and just ignore
the existence of a postgres database in the new cluster.  Maybe give
the user a notice and let them decide whether they wish to take any
action.  I understand that failing is probably less code, but IMHO one
of the biggest problems with pg_upgrade is that it's too fragile:
there are too many seemingly innocent things that can make it croak
(which isn't good, when you consider that anyone using pg_upgrade is
probably in a hurry to get the upgrade done and the database back
on-line).  It seems like this is an opportunity to get rid of one of
those unnecessary failure 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


Re: [HACKERS] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Bruce Momjian
Robert Haas wrote:
> >> But regardless of which database it uses to *generate* the dump, the
> >> dump itself will *always* contain this, right at the very beginning:
> >>
> >> \connect postgres
> >>
> >> That line is in fact hard-coded as a literal string in pg_dumpall.c.
> >> It seems like the easiest fix here might be to just remove that line
> >> from the dump, because AFAICS it's completely pointless. ?During the
> >> time for which that setting is in effect, we're just restoring
> >> globals, so it shouldn't matter which database we're connected to;
> >> only that we have a valid connection. ?So trying to switch the
> >> connection from whatever the user is connected to currently to
> >> postgres doesn't accomplish anything useful, but it does make it
> >> possible for dump restoration to unnecessarily fail.
> >
> > If you remove that line,
> 
> I'm happy to do that, unless someone can see a hole in my logic.
> 
> > I can modify pg_upgrade to use template1
> > instead of postgres, and then the user should just remove the postgres
> > database from the new cluster before the upgrade --- we can give them a
> > clear error message on that.
> 
> What I would prefer is to have the upgrade succeed, and just ignore
> the existence of a postgres database in the new cluster.  Maybe give
> the user a notice and let them decide whether they wish to take any
> action.  I understand that failing is probably less code, but IMHO one
> of the biggest problems with pg_upgrade is that it's too fragile:
> there are too many seemingly innocent things that can make it croak
> (which isn't good, when you consider that anyone using pg_upgrade is
> probably in a hurry to get the upgrade done and the database back
> on-line).  It seems like this is an opportunity to get rid of one of
> those unnecessary failure cases.

OK, then the simplest fix, once you modify pg_dumpall, would be to
modify pg_upgrade to remove reference to the postgres database in the
new cluster if it doesn't exist in the old one.  That would allow
pg_upgrade to maintain a 1-1 matching of databases in the old and new
cluster --- it allows the change to be locallized without affecting much
code.

-- 
  Bruce Momjian  http://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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Bruce Momjian
Robert Haas wrote:
> action.  I understand that failing is probably less code, but IMHO one
> of the biggest problems with pg_upgrade is that it's too fragile:
> there are too many seemingly innocent things that can make it croak
> (which isn't good, when you consider that anyone using pg_upgrade is
> probably in a hurry to get the upgrade done and the database back
> on-line).  It seems like this is an opportunity to get rid of one of
> those unnecessary failure cases.

FYI, the original design goal of pg_upgrade was to be do reliable
upgrades and fail at the hint of any inconsistency.  Seems it is time to
adjust its goals.

-- 
  Bruce Momjian  http://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] Unreproducible bug in snapshot import code

2011-10-28 Thread Bruce Momjian
Gurjeet Singh wrote:
> > > I have tried reproducing the bug starting from 1 and 2 transactions
> > before
> > > the one shown in snippet, and I used tab-completion to get the same
> > > screen-output as termonal1.txt and yet it's not reproducible.
> >
> > I could reproduce it when I typed TAB just after typing "set" in "set
> > transaction snapshot".
> > As Tom and Alvaro pointed out, the tab-completion issues a query and which
> > prevents the "set transaction snapshot" command.
> >
> 
> Great! That settles it then. Reproducible, but not a bug.

Yes, it is only tabs that query the database for completion that cause
this.  Should this be documented somehow?  (No idea how.)

-- 
  Bruce Momjian  http://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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 10:07 AM, Bruce Momjian  wrote:
> OK, then the simplest fix, once you modify pg_dumpall, would be to
> modify pg_upgrade to remove reference to the postgres database in the
> new cluster if it doesn't exist in the old one.  That would allow
> pg_upgrade to maintain a 1-1 matching of databases in the old and new
> cluster --- it allows the change to be locallized without affecting much
> code.

That sounds just fine.  +1.

-- 
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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 10:09 AM, Bruce Momjian  wrote:
> Robert Haas wrote:
>> action.  I understand that failing is probably less code, but IMHO one
>> of the biggest problems with pg_upgrade is that it's too fragile:
>> there are too many seemingly innocent things that can make it croak
>> (which isn't good, when you consider that anyone using pg_upgrade is
>> probably in a hurry to get the upgrade done and the database back
>> on-line).  It seems like this is an opportunity to get rid of one of
>> those unnecessary failure cases.
>
> FYI, the original design goal of pg_upgrade was to be do reliable
> upgrades and fail at the hint of any inconsistency.  Seems it is time to
> adjust its goals.

We definitely don't want it to do anything that could compromise data
integrity.  But in this case there seems no risk of that, so it seems
we can have our cake and eat it, too.

-- 
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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Bruce Momjian
Robert Haas wrote:
> On Fri, Oct 28, 2011 at 10:07 AM, Bruce Momjian  wrote:
> > OK, then the simplest fix, once you modify pg_dumpall, would be to
> > modify pg_upgrade to remove reference to the postgres database in the
> > new cluster if it doesn't exist in the old one. ?That would allow
> > pg_upgrade to maintain a 1-1 matching of databases in the old and new
> > cluster --- it allows the change to be locallized without affecting much
> > code.
> 
> That sounds just fine.  +1.

FYI, I don't want to modify pg_dumpall myself because I didn't want to
have pg_upgrade forcing a pg_dumpall change that applies to
non-binary-upgrade dumps.  pg_dumpall is too important.  I am fine if
someone else does it, though.  :-)

-- 
  Bruce Momjian  http://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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Bruce Momjian
Robert Haas wrote:
> On Fri, Oct 28, 2011 at 10:09 AM, Bruce Momjian  wrote:
> > Robert Haas wrote:
> >> action. ?I understand that failing is probably less code, but IMHO one
> >> of the biggest problems with pg_upgrade is that it's too fragile:
> >> there are too many seemingly innocent things that can make it croak
> >> (which isn't good, when you consider that anyone using pg_upgrade is
> >> probably in a hurry to get the upgrade done and the database back
> >> on-line). ?It seems like this is an opportunity to get rid of one of
> >> those unnecessary failure cases.
> >
> > FYI, the original design goal of pg_upgrade was to be do reliable
> > upgrades and fail at the hint of any inconsistency. ?Seems it is time to
> > adjust its goals.
> 
> We definitely don't want it to do anything that could compromise data
> integrity.  But in this case there seems no risk of that, so it seems
> we can have our cake and eat it, too.

Agreed.  I was extra cautious.

-- 
  Bruce Momjian  http://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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Bruce Momjian
Bruce Momjian wrote:
> Robert Haas wrote:
> > On Fri, Oct 28, 2011 at 10:07 AM, Bruce Momjian  wrote:
> > > OK, then the simplest fix, once you modify pg_dumpall, would be to
> > > modify pg_upgrade to remove reference to the postgres database in the
> > > new cluster if it doesn't exist in the old one. ?That would allow
> > > pg_upgrade to maintain a 1-1 matching of databases in the old and new
> > > cluster --- it allows the change to be locallized without affecting much
> > > code.
> > 
> > That sounds just fine.  +1.
> 
> FYI, I don't want to modify pg_dumpall myself because I didn't want to
> have pg_upgrade forcing a pg_dumpall change that applies to
> non-binary-upgrade dumps.  pg_dumpall is too important.  I am fine if
> someone else does it, though.  :-)

If you want crazy, you could suppress the "\connect postgres" line only
in --binary-upgrade dumps, but that seems to error-prone because then
only --binary-upgrade dumps are exercising that behavior.

-- 
  Bruce Momjian  http://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


[HACKERS] Documentation mistake

2011-10-28 Thread Vik Reykja
in Section 13.2.3 of the 9.1 docs [1], the follow sentence fragment can be
found:

"using Serializable transactions will allow one transaction to commit and
and will roll the other back"

Note the double "and" towards the end. (Is this the right list for this kind
of report?)

[1]
http://www.postgresql.org/docs/9.1/static/transaction-iso.html#XACT-SERIALIZABLE


Re: [HACKERS] out-of-order caution

2011-10-28 Thread Kevin Grittner
Simon Riggs  wrote:
> On Thu, Oct 27, 2011 at 4:41 PM, Kevin Grittner
>  wrote:
>> On the docs page for the SELECT statement, there is a caution
>> which starts with:
>>
>> | It is possible for a SELECT command using ORDER BY and FOR
>> | UPDATE/SHARE to return rows out of order. This is because ORDER
>> | BY is applied first.
>>
>> Is this risk limited to queries running in READ COMMITTED
>> transactions?  If so, I think that should be mentioned in the
>> caution.
> 
> I think it should say that if this occurs with SERIALIZED
> transactions it will result in a serialisation error.
> 
> Just to say there is no effect in serializable mode wouldn't be
> helpful.
 
OK, doc patch attached.
 
-Kevin

*** a/doc/src/sgml/ref/select.sgml
--- b/doc/src/sgml/ref/select.sgml
***
*** 1281,1287  ROLLBACK TO s;
  

 
! It is possible for a SELECT command using ORDER
  BY and FOR UPDATE/SHARE to return rows out of
  order.  This is because ORDER BY is applied first.
  The command sorts the result, but might then block trying to obtain a lock
--- 1281,1288 
  

 
! It is possible for a SELECT command running at the 
READ
! COMMITTED transaction isolation level and using ORDER
  BY and FOR UPDATE/SHARE to return rows out of
  order.  This is because ORDER BY is applied first.
  The command sorts the result, but might then block trying to obtain a lock
***
*** 1302,1307  SELECT * FROM (SELECT * FROM mytable FOR UPDATE) ss ORDER BY 
column1;
--- 1303,1315 
  only if concurrent updates of the ordering columns are expected and a
  strictly sorted result is required.
 
+ 
+
+ At the REPEATABLE READ or 
SERIALIZABLE
+ transaction isolation level this would cause a serialization failure (with
+ a SQLSTATE of '40001'), so there is
+ no possibility of receiving rows out of order under these isolation 
levels.
+


  

-- 
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] Documentation mistake

2011-10-28 Thread Kevin Grittner
Vik Reykja  wrote:
> in Section 13.2.3 of the 9.1 docs [1], the follow sentence
> fragment can be found:
> 
> "using Serializable transactions will allow one transaction to
> commit and and will roll the other back"
> 
> Note the double "and" towards the end. (Is this the right list for
> this kind of report?)
 
When it's strictly documentation and doesn't involve questions of
how the software works, pgsql-docs is probably better, but this
works.
 
Trivial patch attached.  Thanks!
 
-Kevin

*** a/doc/src/sgml/mvcc.sgml
--- b/doc/src/sgml/mvcc.sgml
***
*** 547,553  SELECT SUM(value) FROM mytab WHERE class = 2;
  If either transaction were running at the Repeatable Read isolation level,
  both would be allowed to commit; but since there is no serial order of 
execution
  consistent with the result, using Serializable transactions will allow one
! transaction to commit and and will roll the other back with this message:
  
  
  ERROR:  could not serialize access due to read/write dependencies among 
transactions
--- 547,553 
  If either transaction were running at the Repeatable Read isolation level,
  both would be allowed to commit; but since there is no serial order of 
execution
  consistent with the result, using Serializable transactions will allow one
! transaction to commit and will roll the other back with this message:
  
  
  ERROR:  could not serialize access due to read/write dependencies among 
transactions

-- 
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] Unreproducible bug in snapshot import code

2011-10-28 Thread Gurjeet Singh
On Fri, Oct 28, 2011 at 10:11 AM, Bruce Momjian  wrote:

> Gurjeet Singh wrote:
> > > > I have tried reproducing the bug starting from 1 and 2 transactions
> > > before
> > > > the one shown in snippet, and I used tab-completion to get the same
> > > > screen-output as termonal1.txt and yet it's not reproducible.
> > >
> > > I could reproduce it when I typed TAB just after typing "set" in "set
> > > transaction snapshot".
> > > As Tom and Alvaro pointed out, the tab-completion issues a query and
> which
> > > prevents the "set transaction snapshot" command.
> > >
> >
> > Great! That settles it then. Reproducible, but not a bug.
>
> Yes, it is only tabs that query the database for completion that cause
> this.  Should this be documented somehow?  (No idea how.)
>

If we have a doc section on psql's tab-completion, I think this needs to go
there. A note like:

"Trying to tab-complete on psql may send queries to the server, and
depending on the transaction state, execution of these queries may lead to
non-default/unexpected behaviour by the queries executed after
tab-completion. For example, ..."

Regards,
-- 
Gurjeet Singh
EnterpriseDB Corporation
The Enterprise PostgreSQL Company


Re: [HACKERS] debug query execution

2011-10-28 Thread Kevin Grittner
vadym nikolaiev  wrote:
 
> I would like to ask you which sources are responsible for execute
> queries in PostgreSQL?
 
http://wiki.postgresql.org/wiki/Developer_FAQ#How_is_the_source_code_organized.3F
 
> ( i would like to run a simple query and debug it execution on
> PostgreSql server for understanding how PostgeSql does query
> processing internally)
 
Pick any function you know will be executed, set a breakpoint, get a
stacktrace from there, and pick where you want to get in next time
you run the query.
 
-Kevin

-- 
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] Documentation mistake

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 11:01 AM, Kevin Grittner
 wrote:
> Vik Reykja  wrote:
>> in Section 13.2.3 of the 9.1 docs [1], the follow sentence
>> fragment can be found:
>>
>> "using Serializable transactions will allow one transaction to
>> commit and and will roll the other back"
>>
>> Note the double "and" towards the end. (Is this the right list for
>> this kind of report?)
>
> When it's strictly documentation and doesn't involve questions of
> how the software works, pgsql-docs is probably better, but this
> works.
>
> Trivial patch attached.  Thanks!

Trivial commit performed.  Thanks.

-- 
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] out-of-order caution

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 10:44 AM, Kevin Grittner
 wrote:
> OK, doc patch 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] Add socket dir to pg_config..?

2011-10-28 Thread Dimitri Fontaine
Andrew Dunstan  writes:
> Er, which distros other than debian/ubuntu?

Well, any and all derivatives I guess, to begin with.

  http://distrowatch.com/dwres.php?resource=independence#debian
  Based on Debian GNU/Linux: 129 Distributions

More seriously, I'm not sure how to understand why some people will both
frown upon distribution allowing themselves to patch the version of
PostgreSQL they are packaging, and vote against making their life
easier.

If /tmp is the only decent place where to put the socket file on Unix
when security and other concerns are considered, then sure, making
distro life difficult is a good thing to do. But then let's take it to
the FHS that debian and ubuntu are implementing, AFAIUI.

I'm puzzled, maybe I'm not understanding a key point here though.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

-- 
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] Review: [PL/pgSQL] %TYPE and array declaration - second patch

2011-10-28 Thread Tom Lane
Pavel Stehule  writes:
> this is just small note about length of this patch. This patch was
> significantly smaller then he solved problem with derivate types for
> compound types - it should to solve problem described in this thread

> http://stackoverflow.com/questions/7634704/declare-variable-of-composite-type-in-postgresql-using-type

Well, I think what that example shows is that there's a good reason for
plpgsql_parse_wordtype and plpgsql_parse_cwordtype to handle the
PLPGSQL_NSTYPE_ROW case, which they could do based on the tdtypeid from
the row's tupdesc.  Still isn't going to run to anything like 500 lines
of new code, nor justify a grammar rewrite that risks introducing new
bugs.  The existing code doesn't need to special-case type names that
are also plpgsql keywords, and I'd just as soon not introduce an
assumption that there's no overlap there.

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] ecpg-related build failure with make 3.82

2011-10-28 Thread Tom Lane
Robert Haas  writes:
> On my Fedora 14 box, make -j3 fails.  I think the below is the
> relevant portion of the output.  This has a passing similarity to bug
> 5665, but I can't for the life of me see what the problem is here.
> parer.o depends on preproc.h, which depends on preproc.c; therefore,
> parser.o should not be built until preproc.c has been built, but
> that's exactly what make is doing.

I tried to reproduce this on my own Fedora 14 box, and couldn't.
I cd'd to src/interfaces/ecpg/preproc and did several repetitions of

make maintainer-clean
make -j

and every time, make carefully waited until bison was done before
launching the compiles of preproc.o, parser.o, and the other files
that are declared to depend on preproc.h.

I *can* reproduce failures if I do the same thing one directory level
up, in src/interfaces/ecpg.  But those are caused by the other sub-makes
not waiting for include/ecpg_config.h to get built.  Fixing that is
beyond my level of make-fu.

This is with make-3.82-3.fc14.x86_64 ...

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] So, is COUNT(*) fast now?

2011-10-28 Thread Robert Haas
On Mon, Oct 24, 2011 at 4:23 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Mon, Oct 24, 2011 at 3:35 PM, Tom Lane  wrote:
>>> I wonder how trustworthy the measure of the visibilitymap_test call site
>>> as a consumer of cycles really is.
>
>> I'm not sure either.  I guess we could try short-circuiting
>> visibilitymap_test and see what that does to performance (let's leave
>> correct answers out of it).
>
> That would conflate the cost of the call with the cost of the function.
> Maybe you could try manually inlining the visibility test?

I fooled around with this some more on my Fedora 14 desktop machine.
pgbench database, scale factor 20, shared_buffers=400MB.  I ran the
query "select count(*) from pgbench_accounts".  I'm having a hard time
getting completely reproducible results, but it appears that warming
the cache makes the sequential scan go faster drop from maybe 390 ms
to 245 ms, and an index-only scan takes about 350 ms, so which one is
better depends a lot on your assumptions about what is going on on the
system at the same time (which means maybe we ought not to sweat it).

If I wipe out the whole if-block that calls visibilitymap_test(), the
index-only scan drops down to about 300 ms (and delivers potentially
wrong answers, of course).  Inlining visibilitymap_test (see attached
vismap-inline.patch) causes the index-only scan to drop to about 330
ms.

I also tried changing the BufferIsValid() tests in
visibilitymap_test() to use BufferIsInvalid() instead, with the sense
of the tests reversed (see attached vismap-test-invalid.patch).  Since
BufferIsInvalid() just checks for InvalidBuffer instead of also doing
the sanity checks, it's significantly cheaper.  This also reduced the
time to about 330 ms, so seems clearly worth doing.  Apparently these
changes don't stack, because doing both things only gets me down to
about 320 ms, which is fairly unexciting for the amount of ugliness
that inlining entails.

I tried sprinkling a little branch-prediction magic on this code using
GCC's __builtin_expect().  That initially seemed to help, but once I
changed the BufferIsValid() test to !BufferIsInvalid() essentially all
of the savings disappeared.

I also spent some time poking through the opannotate -s -a output,
which shows where time is being spent by individual assembler
instruction, but also annotates the assembler listing with the
original source code.  At least according to oprofile, the time that
is being spent in IndexOnlyNext() is mostly being spent on seemingly
innocuous operations like saving and restoring registers.  For
example, much of the time being attributed to the visibilitymap_test()
call in IndexOnlyNext() is actually attributable to the instructions
that are calculating what address to pass for scandesc->heapRelation.
Many but not all of the pointer deferences at the top of
IndexOnlyNext() have a chunk cycles attributed to them, and while
they're not that significant individually, they add up.  Similarly,
the long series of instructions to which index_getattr() resolves
bleeds cycles at just about every step.  There's not much to optimize
there, though, unless we want to add some code that avoids decoding
the tuple altogether in the particular case of a zero-argument
aggregate, or maybe something more general that only pulls out the
columns that are actually needed.

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


vismap-inline.patch
Description: Binary data


vismap-test-invalid.patch
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


Re: [HACKERS] ecpg-related build failure with make 3.82

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 1:24 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On my Fedora 14 box, make -j3 fails.  I think the below is the
>> relevant portion of the output.  This has a passing similarity to bug
>> 5665, but I can't for the life of me see what the problem is here.
>> parer.o depends on preproc.h, which depends on preproc.c; therefore,
>> parser.o should not be built until preproc.c has been built, but
>> that's exactly what make is doing.
>
> I tried to reproduce this on my own Fedora 14 box, and couldn't.
> I cd'd to src/interfaces/ecpg/preproc and did several repetitions of
>
>        make maintainer-clean
>        make -j
>
> and every time, make carefully waited until bison was done before
> launching the compiles of preproc.o, parser.o, and the other files
> that are declared to depend on preproc.h.

I see the same thing.  But when I do make -j3 at the toplevel, it
bombs out as described before.

> I *can* reproduce failures if I do the same thing one directory level
> up, in src/interfaces/ecpg.  But those are caused by the other sub-makes
> not waiting for include/ecpg_config.h to get built.  Fixing that is
> beyond my level of make-fu.

I can also reproduce this problem.  I think this one does not occur on
a fresh build because ecpg_config.h is created by configure and is
only removed by make maintainer-clean.  If I make maintainer-clean in
src/interfaces/ecpg, then rerun config.status, and then do make -j it
succeeds.  However, if I repeat those steps from one level further up,
in src/interfaces, then it bombs out as described in my OP.

> This is with make-3.82-3.fc14.x86_64 ...

That's the same make I'm using here.

-- 
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] TOAST versus VACUUM, or "missing chunk number 0 for toast value" identified

2011-10-28 Thread Tom Lane
Robert Haas  writes:
> Wait a minute: I'm confused.  What's at issue here, at least AIUI, is
> taking a TOAST pointer and fetching the corresponding value.  But that
> must involve reading from the TOAST table, and our usual paradigm for
> reading from system catalogs is (1) take AccessShareLock, (2) read the
> relevant rows, (3) release AccessShareLock.  If we're doing that here,
> then AcceptInvalidationMessages() is already getting called.  If we're
> not, this seems horribly unsafe; aside from the current bug, somebody
> could rewrite the table in the interim.

Yes, the TOAST fetch will call AcceptInvalidationMessages().  But
(1) we are starting from a TOAST pointer that is within a now-stale
syscache entry, and even if we get an sinval message telling us it's
stale when we open the toast table, we're still going to try to fetch
that toast OID.  Worse, (2) there is no guarantee that the inval message
has even been sent yet, because there is no lock keeping us from trying
to use the syscache entry before the inval has been sent.

After further reflection I believe that pg_statistic entries invalidated
by ANALYZE are not the only problem.  Consider a pg_proc row whose
prosrc field is toasted (not too unlikely), and suppose that somebody
executes CREATE OR REPLACE FUNCTION to update it to a different toasted
value.  Meanwhile, somebody else is about to use the function, and they
have a copy of its pg_proc row in syscache.  There is no lock that will
block that second backend from fetching the toast tuples before it's
seen the sinval message from the CREATE OR REPLACE FUNCTION.  So the
exact same type of race can occur, if the first backend gets delayed
between committing and broadcasting its sinval messages, and an
overeager vacuum comes along to clean up the dead toast tuples.

I am currently thinking that we will have to go with Heikki's solution
of detoasting any out-of-line fields before we put a tuple into
syscache.  That does fix the problem, so long as we hold a transaction
snapshot at the instant we fetch any catalog tuple that needs this
treatment, because at the time we fetch the tuple we make a SnapshotNow
test that shows the tuple is good (and its dependent toast tuples are
too).  Even if somebody has outdated the tuple and commits immediately
afterward, vacuum cannot remove the toast tuples before we fetch them,
because the outdater is beyond our advertised MyProc->xmin.  The risk
factor comes in when we hold a syscache entry across transactions; then
this guarantee is lost.  (In both the original example and the pg_proc
case above, the second backend is only at risk when it starts its
transaction after the first one's commit, and in between VACUUM was able
to compute an OldestXmin greater than the first one's XID.)

I guess an alternative approach would be to discard syscache entries
at transaction end if HeapTupleHasExternal is true, or equivalently
mark them as to which transaction read them and not use them in later
transactions.  But that seems more fragile, because it would have to
be tied into SnapshotResetXmin --- any time we discard our last snapshot
intra-transaction, toasted syscache entries would have to be invalidated
since our advertised xmin is no longer protecting them.

The main concern I had about detoast before caching is the risk of
circularity, ie, needing detoastable cache entries in order to figure
out how to detoast.  But I think it's probably okay.  The current list
of catalogs with toast tables is

 pg_attrdef
 pg_constraint
 pg_database
 pg_db_role_setting
 pg_description
 pg_proc
 pg_rewrite
 pg_seclabel
 pg_shdescription
 pg_statistic
 pg_trigger

Of these, only pg_proc is even conceivably consulted during a toast
table fetch, and we can be sure that functions needed for such a fetch
don't have toasted entries.  But we will have to be very wary of any
future proposal for adding a toast table to pg_class, pg_index, etc.

Detoast-before-caching also seems sufficiently safe and localized to
be a back-patchable fix, whereas I'd be very scared of making any
significant overhaul of the sinval mechanism in the back branches.

Barring objections, I'll see about making this happen.

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] Include commit identifier in version() function

2011-10-28 Thread Robert Haas
2011/10/28 pasman pasmański :
> I think, it be useful to include in version() function a hexadecimal
> identifier of commit, for fast checkout to it in git.

It's sort of impossible to make this accurate, though.  You may have
patched your tree but not created a commit with those changes before
you build; or if you have created a commit it may very well not be one
that's in the public repo, but just something on our system.
Moreover, there's no real guarantee that you're compiling from a git
tree at all; you could well be compiling from a tarball.

All in all I feel like this is a solution in search of a problem.
People shouldn't be using anything other than a released version in
production, or if they do, they should take it upon themselves to
version-stamp it appropriately.  An automated solution will probably
not be reliable, and probably will create all sorts of build-system
headaches.

-- 
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] So, is COUNT(*) fast now?

2011-10-28 Thread Tom Lane
Robert Haas  writes:
> I also tried changing the BufferIsValid() tests in
> visibilitymap_test() to use BufferIsInvalid() instead, with the sense
> of the tests reversed (see attached vismap-test-invalid.patch).  Since
> BufferIsInvalid() just checks for InvalidBuffer instead of also doing
> the sanity checks, it's significantly cheaper.  This also reduced the
> time to about 330 ms, so seems clearly worth doing.

Hmm.  I wonder whether it wouldn't be better to get rid of the range
checks in BufferIsValid, or better convert them into Asserts.  It seems
less than intuitive that BufferIsValid and BufferIsInvalid aren't simple
inverses.

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] So, is COUNT(*) fast now?

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 2:48 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> I also tried changing the BufferIsValid() tests in
>> visibilitymap_test() to use BufferIsInvalid() instead, with the sense
>> of the tests reversed (see attached vismap-test-invalid.patch).  Since
>> BufferIsInvalid() just checks for InvalidBuffer instead of also doing
>> the sanity checks, it's significantly cheaper.  This also reduced the
>> time to about 330 ms, so seems clearly worth doing.
>
> Hmm.  I wonder whether it wouldn't be better to get rid of the range
> checks in BufferIsValid, or better convert them into Asserts.  It seems
> less than intuitive that BufferIsValid and BufferIsInvalid aren't simple
> inverses.

Seems reasonable.  It would break if anyone is using an out-of-range
buffer number in lieu of InvalidBuffer, but I doubt that anyone is.

-- 
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] ecpg-related build failure with make 3.82

2011-10-28 Thread Tom Lane
Robert Haas  writes:
> On Fri, Oct 28, 2011 at 1:24 PM, Tom Lane  wrote:
>> I tried to reproduce this on my own Fedora 14 box, and couldn't.
>> I cd'd to src/interfaces/ecpg/preproc and did several repetitions of
>> 
>>make maintainer-clean
>>make -j
>> 
>> and every time, make carefully waited until bison was done before
>> launching the compiles of preproc.o, parser.o, and the other files
>> that are declared to depend on preproc.h.

> I see the same thing.  But when I do make -j3 at the toplevel, it
> bombs out as described before.

In that case it's a make bug, which you should file with the proper
authorities.  I could believe it was our problem if it manifested when
starting from src/interfaces/ecpg, but none of the higher-level
makefiles know anything about subdirectories of ecpg.

>> I *can* reproduce failures if I do the same thing one directory level
>> up, in src/interfaces/ecpg.  But those are caused by the other sub-makes
>> not waiting for include/ecpg_config.h to get built.  Fixing that is
>> beyond my level of make-fu.

> I can also reproduce this problem.  I think this one does not occur on
> a fresh build because ecpg_config.h is created by configure and is
> only removed by make maintainer-clean.

Well, on closer inspection, running ecpg/include/Makefile is sufficient
to rebuild ecpg_config.h, apparently because of rules in
Makefile.global.  Now that I look at it, I bet we could fix that part
with some additions to the dependency rules in ecpg/Makefile.  But it
doesn't seem related at all to the preproc problem.

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] So, is COUNT(*) fast now?

2011-10-28 Thread Tom Lane
Robert Haas  writes:
> On Fri, Oct 28, 2011 at 2:48 PM, Tom Lane  wrote:
>> Hmm.  I wonder whether it wouldn't be better to get rid of the range
>> checks in BufferIsValid, or better convert them into Asserts.  It seems
>> less than intuitive that BufferIsValid and BufferIsInvalid aren't simple
>> inverses.

> Seems reasonable.  It would break if anyone is using an out-of-range
> buffer number in lieu of InvalidBuffer, but I doubt that anyone is.

Yeah, I find that unlikely as well.  But leaving Asserts in place would
tell us.

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] So, is COUNT(*) fast now?

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 3:27 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Fri, Oct 28, 2011 at 2:48 PM, Tom Lane  wrote:
>>> Hmm.  I wonder whether it wouldn't be better to get rid of the range
>>> checks in BufferIsValid, or better convert them into Asserts.  It seems
>>> less than intuitive that BufferIsValid and BufferIsInvalid aren't simple
>>> inverses.
>
>> Seems reasonable.  It would break if anyone is using an out-of-range
>> buffer number in lieu of InvalidBuffer, but I doubt that anyone is.
>
> Yeah, I find that unlikely as well.  But leaving Asserts in place would
> tell us.

OK.  Should I go do that, or are you all over it?

-- 
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] fstat vs. lseek

2011-10-28 Thread Andres Freund
Hi All,

The lseek patches just got included in Linus tree.

Andres

-- 
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] fstat vs. lseek

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 3:33 PM, Andres Freund  wrote:
> The lseek patches just got included in Linus tree.

Excellent, thanks for the update!

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=ef3d0fd27e90f67e35da516dafc1482c82939a60

So I guess this will be in Linux 3.2?

-- 
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] TOAST versus VACUUM, or "missing chunk number 0 for toast value" identified

2011-10-28 Thread Alvaro Herrera

Excerpts from Tom Lane's message of vie oct 28 15:37:43 -0300 2011:

> The main concern I had about detoast before caching is the risk of
> circularity, ie, needing detoastable cache entries in order to figure
> out how to detoast.  But I think it's probably okay.  The current list
> of catalogs with toast tables is
> 
>  pg_attrdef
>  pg_constraint
>  pg_database
>  pg_db_role_setting
>  pg_description
>  pg_proc
>  pg_rewrite
>  pg_seclabel
>  pg_shdescription
>  pg_statistic
>  pg_trigger
> 
> Of these, only pg_proc is even conceivably consulted during a toast
> table fetch, and we can be sure that functions needed for such a fetch
> don't have toasted entries.  But we will have to be very wary of any
> future proposal for adding a toast table to pg_class, pg_index, etc.

BTW we had previous discussions about dropping pg_database's toast
table.  Maybe this is a good time to do it, even if there's no risk of
this bug (or the hypothetical circularity detoasting problem) showing up
there.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] TOAST versus VACUUM, or "missing chunk number 0 for toast value" identified

2011-10-28 Thread Alvaro Herrera

Excerpts from Alvaro Herrera's message of vie oct 28 16:47:13 -0300 2011:

> BTW we had previous discussions about dropping pg_database's toast
> table.  Maybe this is a good time to do it, even if there's no risk of
> this bug (or the hypothetical circularity detoasting problem) showing up
> there.

Oh, something unrelated I just remembered: we have
pg_database.datlastsysoid which seems unused.  Perhaps we should remove
that column for cleanliness.

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


[HACKERS] So where are we on the open commitfest?

2011-10-28 Thread Tom Lane
The September commitfest has been drifting sideways for most of this
month.  I think it's about time to put it out of its misery, especially
since the next one is due to start in barely more than 2 weeks.

The remaining open items:

* Allow encoding specific character incrementer

This has certainly gotten reviewed.  I'm unclear on whether it's
committable or not.  Let's either commit it or mark it Returned With
Feedback (Robert?).

* Separating bgwriter and checkpointer

Same for this one.

* pg_last_xact_insert_timestamp

This one is stuck because we don't have consensus on whether it should
be applied.  I suggest pushing it forward to the next 'fest to give
Simon a reasonable amount of time to come up with a counterproposal.
(At some point, though, we should commit it if he doesn't provide one.)

* Non-inheritable check constraints

Greg Stark claimed this one for committing a few weeks ago, but has
not done anything visible since then.  Greg?

* Range Types

This has certainly had plenty of work done too.  If it's not committable
yet, I think we should mark it Returned With Feedback for now.

* WIP: SP-GiST, Space-Partitioned GiST

I was willing to review this as soon as Oleg and Teodor provided more
than no documentation; but none has been forthcoming, and I think Oleg
is on vacation in the Himalayas again.  Suggest pushing it to next fest.

* %TYPE and array declaration

Reviewed, don't have any problem marking this as Returned With Feedback.

* prepare plans of embedded sql on function start

This was reviewed and more or less rejected in September.  There is a
new patch there that is completely different, hasn't been reviewed,
but was submitted in October.  I think we should mark the original patch
as RWF or even Rejected, and put the new patch in as a brand new item
(new title at least) in the next fest.

* unite recovery.conf and postgresql.conf

This one also seems to be lacking consensus more than anything else.
What do we do about that?

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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 10:16 AM, Bruce Momjian  wrote:
> Robert Haas wrote:
>> On Fri, Oct 28, 2011 at 10:07 AM, Bruce Momjian  wrote:
>> > OK, then the simplest fix, once you modify pg_dumpall, would be to
>> > modify pg_upgrade to remove reference to the postgres database in the
>> > new cluster if it doesn't exist in the old one. ?That would allow
>> > pg_upgrade to maintain a 1-1 matching of databases in the old and new
>> > cluster --- it allows the change to be locallized without affecting much
>> > code.
>>
>> That sounds just fine.  +1.
>
> FYI, I don't want to modify pg_dumpall myself because I didn't want to
> have pg_upgrade forcing a pg_dumpall change that applies to
> non-binary-upgrade dumps.  pg_dumpall is too important.  I am fine if
> someone else does it, though.  :-)

OK, done.

-- 
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] So, is COUNT(*) fast now?

2011-10-28 Thread Tom Lane
Robert Haas  writes:
> On Fri, Oct 28, 2011 at 3:27 PM, Tom Lane  wrote:
>> Yeah, I find that unlikely as well.  But leaving Asserts in place would
>> tell us.

> OK.  Should I go do that, or are you all over it?

Go for it.

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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 10:18 AM, Bruce Momjian  wrote:
> Bruce Momjian wrote:
>> Robert Haas wrote:
>> > On Fri, Oct 28, 2011 at 10:07 AM, Bruce Momjian  wrote:
>> > > OK, then the simplest fix, once you modify pg_dumpall, would be to
>> > > modify pg_upgrade to remove reference to the postgres database in the
>> > > new cluster if it doesn't exist in the old one. ?That would allow
>> > > pg_upgrade to maintain a 1-1 matching of databases in the old and new
>> > > cluster --- it allows the change to be locallized without affecting much
>> > > code.
>> >
>> > That sounds just fine.  +1.
>>
>> FYI, I don't want to modify pg_dumpall myself because I didn't want to
>> have pg_upgrade forcing a pg_dumpall change that applies to
>> non-binary-upgrade dumps.  pg_dumpall is too important.  I am fine if
>> someone else does it, though.  :-)
>
> If you want crazy, you could suppress the "\connect postgres" line only
> in --binary-upgrade dumps, but that seems to error-prone because then
> only --binary-upgrade dumps are exercising that behavior.

Also, then it would still be doing something silly in the non
--binary-upgrade case.

-- 
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] TOAST versus VACUUM, or "missing chunk number 0 for toast value" identified

2011-10-28 Thread Kevin Grittner
Tom Lane  wrote:
 
> The risk factor comes in when we hold a syscache entry across
> transactions; then this guarantee is lost.  (In both the original
> example and the pg_proc case above, the second backend is only at
> risk when it starts its transaction after the first one's commit,
> and in between VACUUM was able to compute an OldestXmin greater
> than the first one's XID.)
 
If we made the commit sequence number more generally available,
incrementing it at the point of visibility change under cover of
ProcArrayLock, and including the then-current value in a Snapshot
object when built, would that help with this at all?  Since it's
something we might need to do to optimize SSI for 9.2, and the
issues sound vaguely echoic of the reasons this is maintained for
SSI, it seems worth asking the question, anyway.
 
-Kevin

-- 
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] TOAST versus VACUUM, or "missing chunk number 0 for toast value" identified

2011-10-28 Thread Tom Lane
"Kevin Grittner"  writes:
> If we made the commit sequence number more generally available,
> incrementing it at the point of visibility change under cover of
> ProcArrayLock, and including the then-current value in a Snapshot
> object when built, would that help with this at all?

No, because we need a back-patchable fix.  Even going forward,
I don't find the idea of flushing syscache entries at transaction
end to be especially appealing.

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] So where are we on the open commitfest?

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 3:50 PM, Tom Lane  wrote:
> * Allow encoding specific character incrementer
>
> This has certainly gotten reviewed.  I'm unclear on whether it's
> committable or not.  Let's either commit it or mark it Returned With
> Feedback (Robert?).

I think it's committable.  Let me drum up a round tuit or two.

> * Separating bgwriter and checkpointer
>
> Same for this one.

Possibly some discussion here is warranted about the cost of pushing
fsync requests from the background writer to the checkpointer.  I
guess I feel like that's probably not a major concern; it can likely
be optimized further if it turns out to be an issue.

> * pg_last_xact_insert_timestamp
>
> This one is stuck because we don't have consensus on whether it should
> be applied.  I suggest pushing it forward to the next 'fest to give
> Simon a reasonable amount of time to come up with a counterproposal.
> (At some point, though, we should commit it if he doesn't provide one.)

+1 to all of that, including the parenthetical note.

> * Non-inheritable check constraints
>
> Greg Stark claimed this one for committing a few weeks ago, but has
> not done anything visible since then.  Greg?

No comment.

> * Range Types
>
> This has certainly had plenty of work done too.  If it's not committable
> yet, I think we should mark it Returned With Feedback for now.

I have been thinking about looking at committing at least part of
this, but thought Heikki might be planning to pick it up.

> * WIP: SP-GiST, Space-Partitioned GiST
>
> I was willing to review this as soon as Oleg and Teodor provided more
> than no documentation; but none has been forthcoming, and I think Oleg
> is on vacation in the Himalayas again.  Suggest pushing it to next fest.

+1.

> * %TYPE and array declaration
>
> Reviewed, don't have any problem marking this as Returned With Feedback.

+1.

> * prepare plans of embedded sql on function start
>
> This was reviewed and more or less rejected in September.  There is a
> new patch there that is completely different, hasn't been reviewed,
> but was submitted in October.  I think we should mark the original patch
> as RWF or even Rejected, and put the new patch in as a brand new item
> (new title at least) in the next fest.

+1.

> * unite recovery.conf and postgresql.conf
>
> This one also seems to be lacking consensus more than anything else.
> What do we do about that?

AFAIR, the only person objecting is Simon.  I'm not necessarily saying
that means we should drive it in over his objections, but OTOH there
were quite a few people who spoke in favor of it and we shouldn't
ignore those voices either.

-- 
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] So where are we on the open commitfest?

2011-10-28 Thread Joshua D. Drake




* unite recovery.conf and postgresql.conf

This one also seems to be lacking consensus more than anything else.
What do we do about that?


AFAIR, the only person objecting is Simon.  I'm not necessarily saying
that means we should drive it in over his objections, but OTOH there
were quite a few people who spoke in favor of it and we shouldn't
ignore those voices either.


The problem is, it will break tools. I was one of the people that 
supported Simon in his argument against. Not going to cause a huge stink 
but it is something to consider.


JD






--
Command Prompt, Inc. - http://www.commandprompt.com/
PostgreSQL Support, Training, Professional Services and Development
The PostgreSQL Conference - http://www.postgresqlconference.org/
@cmdpromptinc - @postgresconf - 509-416-6579

--
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] TOAST versus VACUUM, or "missing chunk number 0 for toast value" identified

2011-10-28 Thread Tom Lane
Alvaro Herrera  writes:
> Excerpts from Alvaro Herrera's message of vie oct 28 16:47:13 -0300 2011:
>> BTW we had previous discussions about dropping pg_database's toast
>> table.  Maybe this is a good time to do it, even if there's no risk of
>> this bug (or the hypothetical circularity detoasting problem) showing up
>> there.

No objection from me.

> Oh, something unrelated I just remembered: we have
> pg_database.datlastsysoid which seems unused.  Perhaps we should remove
> that column for cleanliness.

I have a vague recollection that some client-side code uses this to
figure out what's the dividing line between user and system OIDs.
pg_dump used to need to know that number, and it can still be useful
with casts and other things that are hard to tell whether they're built-in.
While in principle people could use FirstNormalObjectId instead, that
could come back to bite us if we ever have to increase that constant
in future releases.  I'm inclined to leave this one alone.

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] fstat vs. lseek

2011-10-28 Thread Andres Freund
Hi,

On Friday, October 28, 2011 09:40:51 PM Robert Haas wrote:
> On Fri, Oct 28, 2011 at 3:33 PM, Andres Freund  wrote:
> > The lseek patches just got included in Linus tree.
> 
> Excellent, thanks for the update!
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=ef3
> d0fd27e90f67e35da516dafc1482c82939a60
> 
> So I guess this will be in Linux 3.2?
Unless they get reverted for some reason, yes.


Andres

-- 
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] So where are we on the open commitfest?

2011-10-28 Thread Tom Lane
"Joshua D. Drake"  writes:
>>> * unite recovery.conf and postgresql.conf
>>> 
>>> This one also seems to be lacking consensus more than anything else.
>>> What do we do about that?

>> AFAIR, the only person objecting is Simon.  I'm not necessarily saying
>> that means we should drive it in over his objections, but OTOH there
>> were quite a few people who spoke in favor of it and we shouldn't
>> ignore those voices either.

> The problem is, it will break tools. I was one of the people that 
> supported Simon in his argument against. Not going to cause a huge stink 
> but it is something to consider.

Yeah, but it's those same tools that will benefit from a cleaner
definition.  Sometimes it doesn't pay to be slaves to backwards
compatibility.

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] So where are we on the open commitfest?

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 4:07 PM, Joshua D. Drake  wrote:
>>> * unite recovery.conf and postgresql.conf
>>>
>>> This one also seems to be lacking consensus more than anything else.
>>> What do we do about that?
>>
>> AFAIR, the only person objecting is Simon.  I'm not necessarily saying
>> that means we should drive it in over his objections, but OTOH there
>> were quite a few people who spoke in favor of it and we shouldn't
>> ignore those voices either.
>
> The problem is, it will break tools. I was one of the people that supported
> Simon in his argument against. Not going to cause a huge stink but it is
> something to consider.

Oh, sorry, JD.  I had forgotten that you also spoke in favor of leaving it be.

Anyway, we just need to make up our mind on what we're going to do and
do it.  Not talking about it, or talking about it but not making a
decision, leads to frustration all around.

-- 
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] CASE w/out ELSE hides typmod (was: How define a view that use a case operator for geometry field)

2011-10-28 Thread Sandro Santilli
On Fri, Oct 28, 2011 at 10:33:45PM +0200, aperi2007 wrote:

> Seem that postgis try to define it an else condition assigning it a
> unknown "geometry" ?

It's PostgreSQL, not PostGIS.

The same happens with any type, can be reproduced with something like this:

=# CREATE VIEW test1 AS SELECT
  CASE WHEN random() < 0.5 THEN 1::numeric(10, 0)
  END;

=# \d test1
   View "test.test1"
 Column |  Type   | Modifiers
+-+---
 case   | numeric |

=# CREATE VIEW test2 AS SELECT
  CASE WHEN random() < 0.5 THEN 1::numeric(10, 0)
  ELSE 0::numeric(10, 0)
  END;

=# \d test2
 View "test.test2"
 Column | Type  | Modifiers
+---+---
 case   | numeric(10,0) |


Maybe someone on pgsql-hackers can tell us more about this.

--strk;

  ()   Free GIS & Flash consultant/developer
  /\   http://strk.keybit.net/services.html

-- 
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] So where are we on the open commitfest?

2011-10-28 Thread Simon Riggs
On Fri, Oct 28, 2011 at 8:50 PM, Tom Lane  wrote:

> * Separating bgwriter and checkpointer
>
> Same for this one.

Will commit by end of Monday


> * pg_last_xact_insert_timestamp
>
> This one is stuck because we don't have consensus on whether it should
> be applied.  I suggest pushing it forward to the next 'fest to give
> Simon a reasonable amount of time to come up with a counterproposal.
> (At some point, though, we should commit it if he doesn't provide one.)

+1

> * unite recovery.conf and postgresql.conf
>
> This one also seems to be lacking consensus more than anything else.
> What do we do about that?

I'll re-read the thread in detail to see if I can break impasse.


-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

-- 
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] CASE w/out ELSE hides typmod (was: How define a view that use a case operator for geometry field)

2011-10-28 Thread Tom Lane
Sandro Santilli  writes:
> The same happens with any type, can be reproduced with something like this:

> =# CREATE VIEW test1 AS SELECT
>   CASE WHEN random() < 0.5 THEN 1::numeric(10, 0)
>   END;

> =# \d test1
>View "test.test1"
>  Column |  Type   | Modifiers
> +-+---
>  case   | numeric |

> =# CREATE VIEW test2 AS SELECT
>   CASE WHEN random() < 0.5 THEN 1::numeric(10, 0)
>   ELSE 0::numeric(10, 0)
>   END;

> =# \d test2
>  View "test.test2"
>  Column | Type  | Modifiers
> +---+---
>  case   | numeric(10,0) |

> Maybe someone on pgsql-hackers can tell us more about this.

IIRC, the result of a CASE is only considered to have a defined typmod
if all arms of the CASE ... including ELSE ... have the identical
typmod.

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] So, is COUNT(*) fast now?

2011-10-28 Thread Robert Haas
On Fri, Oct 28, 2011 at 3:53 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Fri, Oct 28, 2011 at 3:27 PM, Tom Lane  wrote:
>>> Yeah, I find that unlikely as well.  But leaving Asserts in place would
>>> tell us.
>
>> OK.  Should I go do that, or are you all over it?
>
> Go for it.

OK, done.  Any other thoughts on the rest of what I wrote?

-- 
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] TOAST versus VACUUM, or "missing chunk number 0 for toast value" identified

2011-10-28 Thread Merlin Moncure
On Fri, Oct 28, 2011 at 3:11 PM, Tom Lane  wrote:
> Alvaro Herrera  writes:
>> Excerpts from Alvaro Herrera's message of vie oct 28 16:47:13 -0300 2011:
>>> BTW we had previous discussions about dropping pg_database's toast
>>> table.  Maybe this is a good time to do it, even if there's no risk of
>>> this bug (or the hypothetical circularity detoasting problem) showing up
>>> there.
>
> No objection from me.
>
>> Oh, something unrelated I just remembered: we have
>> pg_database.datlastsysoid which seems unused.  Perhaps we should remove
>> that column for cleanliness.
>
> I have a vague recollection that some client-side code uses this to
> figure out what's the dividing line between user and system OIDs.
> pg_dump used to need to know that number, and it can still be useful
> with casts and other things that are hard to tell whether they're built-in.
> While in principle people could use FirstNormalObjectId instead, that
> could come back to bite us if we ever have to increase that constant
> in future releases.  I'm inclined to leave this one alone.

A cursory search of the archives turned up that pgadmin is explicitly
querying it out (or at least did in 2009).  Also, there are some
fairly ancient references of discussion about using it via pg_dump to
deal with the 'not dumping user casts wrapping system objects' issue,
but that was deemed unreliable and is probably not relevant.

merlin

-- 
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] So where are we on the open commitfest?

2011-10-28 Thread Josh Berkus

>> This one also seems to be lacking consensus more than anything else.
>> What do we do about that?
> 
> I'll re-read the thread in detail to see if I can break impasse.

When we last left it, I pointed out to you that 100% backwards
compatibility is impossible: we simply can't maintain recovery.conf as a
trigger file and also clean up the broken API.  You had no response for
how to resolve that.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

-- 
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] So where are we on the open commitfest?

2011-10-28 Thread Fujii Masao
On Sat, Oct 29, 2011 at 5:50 AM, Simon Riggs  wrote:
> On Fri, Oct 28, 2011 at 8:50 PM, Tom Lane  wrote:
>
>> * Separating bgwriter and checkpointer
>>
>> Same for this one.
>
> Will commit by end of Monday

There are plenty of source comments (and probably documents) describing that
checkpoint is performed by bgwriter, but the patch that you posted
didn't correct
them. Are you going to include the change of them in the patch? Or commit
separately?

>> * pg_last_xact_insert_timestamp
>>
>> This one is stuck because we don't have consensus on whether it should
>> be applied.  I suggest pushing it forward to the next 'fest to give
>> Simon a reasonable amount of time to come up with a counterproposal.
>> (At some point, though, we should commit it if he doesn't provide one.)
>
> +1

+1

>> * unite recovery.conf and postgresql.conf
>>
>> This one also seems to be lacking consensus more than anything else.
>> What do we do about that?
>
> I'll re-read the thread in detail to see if I can break impasse.

That's very helpful. I'd like to hear what you think we should not change
for the backward compatibility, and what we can do. AFAIR you agreed
to rename recovery.conf, so I don't guess that you want 100% compatibility.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] pg_upgrade if 'postgres' database is dropped

2011-10-28 Thread Bruce Momjian
Robert Haas wrote:
> On Fri, Oct 28, 2011 at 10:16 AM, Bruce Momjian  wrote:
> > Robert Haas wrote:
> >> On Fri, Oct 28, 2011 at 10:07 AM, Bruce Momjian  wrote:
> >> > OK, then the simplest fix, once you modify pg_dumpall, would be to
> >> > modify pg_upgrade to remove reference to the postgres database in the
> >> > new cluster if it doesn't exist in the old one. ?That would allow
> >> > pg_upgrade to maintain a 1-1 matching of databases in the old and new
> >> > cluster --- it allows the change to be locallized without affecting much
> >> > code.
> >>
> >> That sounds just fine. ?+1.
> >
> > FYI, I don't want to modify pg_dumpall myself because I didn't want to
> > have pg_upgrade forcing a pg_dumpall change that applies to
> > non-binary-upgrade dumps. ?pg_dumpall is too important. ?I am fine if
> > someone else does it, though. ?:-)
> 
> OK, done.

OK, the attached, applied patch removes the pg_upgrade dependency on the
'postgres' database existing in the new cluster.  However, vacuumdb,
used by pg_upgrade, still has this dependency:

conn = connectDatabase("postgres", host, port, username,
prompt_password, progname);

In fact, all the /scripts binaries use the postgres database, except for
createdb/dropdb, which has this heuristic:

/*
 * Connect to the 'postgres' database by default, except have the
 * 'postgres' user use 'template1' so he can create the 'postgres'
 * database.
 */
conn = connectDatabase(strcmp(dbname, "postgres") == 0 ? "template1" : 
"postgres",
   host, port, username, prompt_password, progname);

This makes sense because you might be creating or dropping the postgres
database.  Do we want these to have smarter database selection code?

I will now work on code to allow the old cluster to optionally not have
a postgres database.

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

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c
new file mode 100644
index 273561e..12df463
*** a/contrib/pg_upgrade/pg_upgrade.c
--- b/contrib/pg_upgrade/pg_upgrade.c
*** static void set_frozenxids(void);
*** 52,60 
  static void setup(char *argv0, bool live_check);
  static void cleanup(void);
  
- /* This is the database used by pg_dumpall to restore global tables */
- #define GLOBAL_DUMP_DB	"postgres"
- 
  ClusterInfo old_cluster,
  			new_cluster;
  OSInfo		os_info;
--- 52,57 
*** prepare_new_databases(void)
*** 233,242 
  	prep_status("Creating databases in the new cluster");
  
  	/*
! 	 * Install support functions in the global-restore database to preserve
! 	 * pg_authid.oid.
  	 */
! 	install_support_functions_in_new_db(GLOBAL_DUMP_DB);
  
  	/*
  	 * We have to create the databases first so we can install support
--- 230,241 
  	prep_status("Creating databases in the new cluster");
  
  	/*
! 	 * Install support functions in the global-object restore database to
! 	 * preserve pg_authid.oid.  pg_dumpall uses 'template0' as its template
! 	 * database so objects we add into 'template1' are not propogated.  They
! 	 * are removed on pg_upgrade exit.
  	 */
! 	install_support_functions_in_new_db("template1");
  
  	/*
  	 * We have to create the databases first so we can install support
*** create_new_objects(void)
*** 270,276 
  		DbInfo	   *new_db = &new_cluster.dbarr.dbs[dbnum];
  
  		/* skip db we already installed */
! 		if (strcmp(new_db->db_name, GLOBAL_DUMP_DB) != 0)
  			install_support_functions_in_new_db(new_db->db_name);
  	}
  	check_ok();
--- 269,275 
  		DbInfo	   *new_db = &new_cluster.dbarr.dbs[dbnum];
  
  		/* skip db we already installed */
! 		if (strcmp(new_db->db_name, "template1") != 0)
  			install_support_functions_in_new_db(new_db->db_name);
  	}
  	check_ok();

-- 
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] So where are we on the open commitfest?

2011-10-28 Thread Tom Lane
Robert Haas  writes:
> On Fri, Oct 28, 2011 at 3:50 PM, Tom Lane  wrote:
>> * Range Types
>> 
>> This has certainly had plenty of work done too.  If it's not committable
>> yet, I think we should mark it Returned With Feedback for now.

> I have been thinking about looking at committing at least part of
> this, but thought Heikki might be planning to pick it up.

Yeah, this one is in Heikki's court AFAIK.

I've updated the commitfest items for which there seemed to be no
disagreement.  We still have

>> * Allow encoding specific character incrementer (on your head)
>> * Separating bgwriter and checkpointer (on Simon's)
>> * Non-inheritable check constraints (on Greg Stark's)
>> * Range Types (on Heikki's)
>> * unite recovery.conf and postgresql.conf (Simon's on the hook to advance 
>> this discussion)

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