Re: [HACKERS] Re: [COMMITTERS] pgsql: Force strings passed to and from plperl to be in UTF8 encoding.

2011-10-03 Thread Heikki Linnakangas

On 04.10.2011 08:35, Amit Khandekar wrote:

On 3 October 2011 22:37, Alex Hunsaker  wrote:

It might be worth adding a regression test also...


I could not find any basic pl/perl tests in the regression
serial_schedule. I am not sure if we want to add just this scenario
without any basic tests for pl/perl ?


See src/pl/plperl/[sql|expected]

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.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] bug of recovery?

2011-10-03 Thread Fujii Masao
On Mon, Oct 3, 2011 at 4:32 PM, Simon Riggs  wrote:
>> I don't think this should use the rm_safe_restartpoint machinery. As you
>> said, it's not tied to any specific resource manager. And I've actually been
>> thinking that we will get rid of rm_safe_restartpoint altogether in the
>> future. The two things that still use it are the b-tree and gin, and I'd
>> like to change both of those to not require any post-recovery cleanup step
>> to finish multi-page operations, similar to what I did with GiST in 9.1.
>
> I thought that was quite neat doing it that way, but there's no
> specific reason to do it that way I guess. If you're happy to rewrite
> the patch then I guess we're OK.
>
> I certainly would like to get rid of rm_safe_restartpoint in the
> longer term, hopefully sooner.

Though Heikki might be already working on that,... anyway,
the attached patch is the version which doesn't use rm_safe_restartpoint
machinery.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***
*** 557,563  static TimeLineID lastPageTLI = 0;
  static XLogRecPtr minRecoveryPoint;		/* local copy of
  		 * ControlFile->minRecoveryPoint */
  static bool updateMinRecoveryPoint = true;
! static bool reachedMinRecoveryPoint = false;
  
  static bool InRedo = false;
  
--- 557,563 
  static XLogRecPtr minRecoveryPoint;		/* local copy of
  		 * ControlFile->minRecoveryPoint */
  static bool updateMinRecoveryPoint = true;
! bool reachedMinRecoveryPoint = false;
  
  static bool InRedo = false;
  
***
*** 6841,6852  StartupXLOG(void)
  		LocalXLogInsertAllowed = -1;
  
  		/*
- 		 * Check to see if the XLOG sequence contained any unresolved
- 		 * references to uninitialized pages.
- 		 */
- 		XLogCheckInvalidPages();
- 
- 		/*
  		 * Perform a checkpoint to update all our recovery activity to disk.
  		 *
  		 * Note that we write a shutdown checkpoint rather than an on-line
--- 6841,6846 
***
*** 6983,6988  CheckRecoveryConsistency(void)
--- 6977,6988 
  		XLByteLE(minRecoveryPoint, EndRecPtr) &&
  		XLogRecPtrIsInvalid(ControlFile->backupStartPoint))
  	{
+ 		/*
+ 		 * Check to see if the XLOG sequence contained any unresolved
+ 		 * references to uninitialized pages.
+ 		 */
+ 		XLogCheckInvalidPages();
+ 
  		reachedMinRecoveryPoint = true;
  		ereport(LOG,
  (errmsg("consistent recovery state reached at %X/%X",
***
*** 7974,7980  RecoveryRestartPoint(const CheckPoint *checkPoint)
  	volatile XLogCtlData *xlogctl = XLogCtl;
  
  	/*
! 	 * Is it safe to checkpoint?  We must ask each of the resource managers
  	 * whether they have any partial state information that might prevent a
  	 * correct restart from this point.  If so, we skip this opportunity, but
  	 * return at the next checkpoint record for another try.
--- 7974,7980 
  	volatile XLogCtlData *xlogctl = XLogCtl;
  
  	/*
! 	 * Is it safe to restartpoint?  We must ask each of the resource managers
  	 * whether they have any partial state information that might prevent a
  	 * correct restart from this point.  If so, we skip this opportunity, but
  	 * return at the next checkpoint record for another try.
***
*** 7994,7999  RecoveryRestartPoint(const CheckPoint *checkPoint)
--- 7994,8015 
  	}
  
  	/*
+ 	 * Is it safe to restartpoint?  We must check whether there are any
+ 	 * unresolved references to invalid pages that might prevent
+ 	 * a correct restart from this point.  If so, we skip this opportunity,
+ 	 * but return at the next checkpoint record for another try.
+ 	 */
+ 	if (have_invalid_pages())
+ 	{
+ 		elog(trace_recovery(DEBUG2),
+ 			 "could not record restart point at %X/%X because there "
+ 			 "are unresolved references to invalid pages",
+ 			 checkPoint->redo.xlogid,
+ 			 checkPoint->redo.xrecoff);
+ 		return;
+ 	}
+ 
+ 	/*
  	 * Copy the checkpoint record to shared memory, so that bgwriter can use
  	 * it the next time it wants to perform a restartpoint.
  	 */
*** a/src/backend/access/transam/xlogutils.c
--- b/src/backend/access/transam/xlogutils.c
***
*** 52,57  typedef struct xl_invalid_page
--- 52,73 
  static HTAB *invalid_page_tab = NULL;
  
  
+ /* Report a reference to an invalid page */
+ static void
+ report_invalid_page(int elevel, RelFileNode node, ForkNumber forkno,
+ 	BlockNumber blkno, bool present)
+ {
+ 	char	   *path = relpathperm(node, forkno);
+ 
+ 	if (present)
+ 		elog(elevel, "page %u of relation %s is uninitialized",
+ 			 blkno, path);
+ 	else
+ 		elog(elevel, "page %u of relation %s does not exist",
+ 			 blkno, path);
+ 	pfree(path);
+ }
+ 
  /* Log a reference to an invalid page */
  static void
  log_invalid_page(RelFileNode node, ForkNumber forkno, BlockNumber blkno,
***
*** 62,83  log_invalid_page(RelFileNode node, ForkNumber forkno, B

Re: [HACKERS] Re: [COMMITTERS] pgsql: Force strings passed to and from plperl to be in UTF8 encoding.

2011-10-03 Thread Amit Khandekar
On 3 October 2011 22:37, Alex Hunsaker  wrote:
> On Mon, Oct 3, 2011 at 04:20, Amit Khandekar
>  wrote:
>
>> Is there a plan to commit this issue? I am still seeing this issue on
>> PG 9.1 STABLE branch. Attached is a small patch that targets only the
>> specific issue in the described testcase :
>>
>> create or replace function zerob() returns text as $$ return
>> "abcd\0efg"; $$ language plperl;
>> SELECT zerob();
>>
>> The patch does the perl data validation in the function utf_u2e() itself.
>
> I think thats fine, but as coded it will verify the string twice in
> the GetDatabaseEncoding() != PG_UTF8 case (once for
> pg_do_encoding_conversion() and again with the added
> pg_verify_mbstr_len), which seems a bit wasteful.

WHen GetDatabaseEncoding() != PG_UTF8 case, ret will not be equal to
utf8_str, so pg_verify_mbstr_len() will not get called. That's the
reason, pg_verify_mbstr_len() is under the ( ret == utf8_str )
condition.


>
> It might be worth adding a regression test also...

I could not find any basic pl/perl tests in the regression
serial_schedule. I am not sure if we want to add just this scenario
without any basic tests for pl/perl ?

>

-- 
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] Tracking latest timeline in standby mode

2011-10-03 Thread Fujii Masao
On Mon, Oct 3, 2011 at 3:18 PM, senthilnathan  wrote:
> Whether this feature is available in version 9.1.0. ??

Yes, it's available in 9.1.x.

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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Bruce Momjian wrote:
> Alvaro Herrera wrote:
> > 
> > Excerpts from Bruce Momjian's message of lun oct 03 17:28:53 -0300 2011:
> > > 
> > > Alvaro Herrera wrote:
> > 
> > > > Well, we have the Gentoo developer in this very thread.  I'm sure they
> > > > would fix their command line if we gave them a pg_ctl that worked.
> > > > Surely the package that contains the init script also contains pg_ctl,
> > > > so they would both be upgraded simultaneously.
> > > 
> > > What is the fix?  If they started the server by using --data-directory,
> > > pg_ctl stop has no way to find the postmaster.pid file, and hence stop
> > > the server.  Are you suggesting we remove this ability?
> > 
> > I am suggesting they don't start it by using --data-directory in the
> > first place.
> 
> Agreed.  If you remove that, the logical problem goes away and it
> becomes a simple problem of dumping the contents of postgresql.conf and
> having pg_ctl (and pg_upgrade) use that.  Let me look at how much code
> that would take.

OK, here is a patch that adds a -C option to the postmaster so any
config variable can be dumped, even while the server is running (there
is no security check because we don't have a user name at this point),
e.g.:

postgres -D /pg_upgrade/tmp -C data_directory
/u/pg/data

It also modifies pg_ctl to use this feature.  It works fine for pg_ctl
-w start/stop with a config-only directory, so this is certainly in the
right direction.  You can also use pg_ctl -o '--data_directory=/abc' and
it will be understood:

pg_ctl -o '--data_directory=/u/pg/data' -D tmp start

If you used --data_directory to start the server, you will need to use
--data_directory to stop it, which seems reasonable.

Patch attached.  This was much simpler than I thought.  :-)

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

  + It's impossible for everything to be true. +
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
new file mode 100644
index 0a84d97..660458e
*** a/src/backend/postmaster/postmaster.c
--- b/src/backend/postmaster/postmaster.c
*** bool		enable_bonjour = false;
*** 203,208 
--- 203,210 
  char	   *bonjour_name;
  bool		restart_after_crash = true;
  
+ char 		dump_config_variable[MAXPGPATH] = "";
+ 
  /* PIDs of special child processes; 0 when not running */
  static pid_t StartupPID = 0,
  			BgWriterPID = 0,
*** PostmasterMain(int argc, char *argv[])
*** 537,543 
  	 * tcop/postgres.c (the option sets should not conflict) and with the
  	 * common help() function in main/main.c.
  	 */
! 	while ((opt = getopt(argc, argv, "A:B:bc:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1)
  	{
  		switch (opt)
  		{
--- 539,545 
  	 * tcop/postgres.c (the option sets should not conflict) and with the
  	 * common help() function in main/main.c.
  	 */
! 	while ((opt = getopt(argc, argv, "A:B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1)
  	{
  		switch (opt)
  		{
*** PostmasterMain(int argc, char *argv[])
*** 554,559 
--- 556,565 
  IsBinaryUpgrade = true;
  break;
  
+ 			case 'C':
+ strlcpy(dump_config_variable, optarg, MAXPGPATH);
+ break;
+ 
  			case 'D':
  userDoption = optarg;
  break;
*** PostmasterMain(int argc, char *argv[])
*** 728,733 
--- 734,746 
  	if (!SelectConfigFiles(userDoption, progname))
  		ExitPostmaster(2);
  
+ 	if (dump_config_variable[0] != '\0')
+ 	{
+ 		/* This allows anyone to read super-user config values. */
+ 		printf("%s\n", GetConfigOption(dump_config_variable, false, false));
+ 		ExitPostmaster(0);
+ 	}
+ 	
  	/* Verify that DataDir looks reasonable */
  	checkDataDir();
  
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
new file mode 100644
index c7eac71..a5eae49
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
*** process_postgres_switches(int argc, char
*** 3170,3176 
  	 * postmaster/postmaster.c (the option sets should not conflict) and with
  	 * the common help() function in main/main.c.
  	 */
! 	while ((flag = getopt(argc, argv, "A:B:bc:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:v:W:-:")) != -1)
  	{
  		switch (flag)
  		{
--- 3170,3176 
  	 * postmaster/postmaster.c (the option sets should not conflict) and with
  	 * the common help() function in main/main.c.
  	 */
! 	while ((flag = getopt(argc, argv, "A:B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:v:W:-:")) != -1)
  	{
  		switch (flag)
  		{
*** process_postgres_switches(int argc, char
*** 3187,3192 
--- 3187,3196 
  IsBinaryUpgrade = true;
  break;
  
+ 			case 'C':
+ /* ignored for consistency with postmaster */
+ break;
+ 
  			case 'D':
  if (secure)
  	userDoption = strdup(optarg);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
new file mode 100644
index 0dbdfe7..18a02ad
*** a/src/bin/pg_ctl/pg

Re: [HACKERS] [v9.2] make_greater_string() does not return a string in some cases

2011-10-03 Thread Robert Haas
On Mon, Oct 3, 2011 at 2:13 PM, Robert Haas  wrote:
> On Thu, Sep 29, 2011 at 6:24 AM, Kyotaro HORIGUCHI
>  wrote:
>> This is new version of make_greater_string patch.
>
> According to the comments in the original source code, the purpose of
> savelastchar is to avoid confusing pg_mbcliplen().  You've preserved
> savelastchar only for the case where datatype == BYTEAOID, while
> making it the increment function's job not to do anything permanent
> unless it also returns true.  But it seems to me that if the datatype
> is BYTEAOID then there's no need to restore anything at all, because
> we're not going to call pg_mbcliplen() in that case anyway.  So I
> think the logic here can be simplified.
>
> Also, you haven't completely fixed the style issues.  Function
> definitions should look like this:
>
> static void
> thingy()
> {
> }
>
> Not like this:
>
> static void thingy()
> {
> }
>
> Opening curly braces should be on a line by themselves, not at the end
> of the preceding if, while, etc. line.
>
> "finnaly" is spelled incorrectly.

Oh, and there's this:

wchar.c: In function ‘pg_utf8_increment’:
wchar.c:1376: warning: unused variable ‘success’
wchar.c: In function ‘pg_eucjp_increment’:
wchar.c:1433: warning: unused variable ‘success’

-- 
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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Andrew Dunstan



On 10/03/2011 06:45 PM, Bruce Momjian wrote:

Alvaro Herrera wrote:

Excerpts from Bruce Momjian's message of lun oct 03 17:28:53 -0300 2011:

Alvaro Herrera wrote:

Well, we have the Gentoo developer in this very thread.  I'm sure they
would fix their command line if we gave them a pg_ctl that worked.
Surely the package that contains the init script also contains pg_ctl,
so they would both be upgraded simultaneously.

What is the fix?  If they started the server by using --data-directory,
pg_ctl stop has no way to find the postmaster.pid file, and hence stop
the server.  Are you suggesting we remove this ability?

I am suggesting they don't start it by using --data-directory in the
first place.

Agreed.  If you remove that, the logical problem goes away and it
becomes a simple problem of dumping the contents of postgresql.conf and
having pg_ctl (and pg_upgrade) use that.  Let me look at how much code
that would take.



Yeah, this pattern can be changed to have a config file that reads:

   data_directory = '/path/to/data'
   include '/path/to/common/config'

and I presume (or hope) that would meet your need, and not upset the FHS 
purists.



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] restoring an object to a different name

2011-10-03 Thread Andrew Dunstan


This is a subject that has come up recently, and I can think of a number 
of use cases for it.


However, there are lots of wrinkles. For example, the names of objects 
appear in LOTS of places, and making sure we caught them all might be 
quite tricky. Say you have a table x that inherits a,b, and c, and you 
decide to restore with b renamed. Now x will have a dependency on b 
recorded, but finding b in the opaque sql string that is stored for the 
creation of x is not going to be easy (don't anyone mention regexes here 
- this is not a good case for their use IMNSHO, much as I love them).


One idea I came up with was to set up the SQL using OIDS instead of 
names as placeholders, and then replacing the OIDS with the right name 
at run time. So if we want to restore something with a different name, 
we'd just change the stored name in the node where it's defined and the 
new name would then be picked up everywhere it's used (might need a 
 pair, but the idea would be the same).


Does anyone else have anything better? I don't think this is something 
that can be achieved cleanly with a small patch.


cheers

andrew

PS, if you want to see what info pg_restore actually has available in a 
dump file, you might like to use my little utility at 
.




--
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] Separating bgwriter and checkpointer

2011-10-03 Thread Dickson S. Guedes
2011/10/3 Simon Riggs :
> On Sun, Oct 2, 2011 at 11:45 PM, Dickson S. Guedes  
> wrote:
>> I'm trying your patch, it was applied cleanly to master and compiled
>> ok. But since I started postgres I'm seeing a  99% of CPU usage:
>
> Oh, thanks. I see what happened. I was toying with the idea of going
> straight to a WaitLatch implementation for the loop but decided to
> leave it out for a later patch, and then skipped the sleep as well.
>
> New version attached.

Working now but even passing all tests for make check, the
regress_database's postmaster doesn't shutdown properly.

$ make check
...
...
== creating temporary installation==
== initializing database system   ==
== starting postmaster==
running on port 57432 with PID 20094
== creating database "regression" ==
...
== shutting down postmaster   ==
pg_ctl: server does not shut down
pg_regress: could not stop postmaster: exit code was 256

$ uname -a
Linux betelgeuse 2.6.38-11-generic-pae #50-Ubuntu SMP Mon Sep 12
22:21:04 UTC 2011 i686 i686 i386 GNU/Linux

$ grep "$ ./configure" config.log
  $ ./configure --enable-debug --enable-cassert
--prefix=/srv/postgres/bgwriter_split

Best regards,
-- 
Dickson S. Guedes
mail/xmpp: gue...@guedesoft.net - skype: guediz
http://guedesoft.net - http://www.postgresql.org.br

-- 
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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Andrew Dunstan wrote:
> 
> 
> On 10/03/2011 06:45 PM, Bruce Momjian wrote:
> > Alvaro Herrera wrote:
> >> Excerpts from Bruce Momjian's message of lun oct 03 17:28:53 -0300 2011:
> >>> Alvaro Herrera wrote:
>  Well, we have the Gentoo developer in this very thread.  I'm sure they
>  would fix their command line if we gave them a pg_ctl that worked.
>  Surely the package that contains the init script also contains pg_ctl,
>  so they would both be upgraded simultaneously.
> >>> What is the fix?  If they started the server by using --data-directory,
> >>> pg_ctl stop has no way to find the postmaster.pid file, and hence stop
> >>> the server.  Are you suggesting we remove this ability?
> >> I am suggesting they don't start it by using --data-directory in the
> >> first place.
> > Agreed.  If you remove that, the logical problem goes away and it
> > becomes a simple problem of dumping the contents of postgresql.conf and
> > having pg_ctl (and pg_upgrade) use that.  Let me look at how much code
> > that would take.
> >
> 
> Yeah, this pattern can be changed to have a config file that reads:
> 
> data_directory = '/path/to/data'
> include '/path/to/common/config'
> 
> and I presume (or hope) that would meet your need, and not upset the FHS 
> purists.

Actually, the existing setup is fine as long as there is something that
tell us where to find the data directory.

-- 
  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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Aidan Van Dyk
On Mon, Oct 3, 2011 at 7:10 PM, Andrew Dunstan  wrote:

>> Agreed.  If you remove that, the logical problem goes away and it
>> becomes a simple problem of dumping the contents of postgresql.conf and
>> having pg_ctl (and pg_upgrade) use that.  Let me look at how much code
>> that would take.
>>
>
> Yeah, this pattern can be changed to have a config file that reads:
>
>   data_directory = '/path/to/data'
>   include '/path/to/common/config'
>
> and I presume (or hope) that would meet your need, and not upset the FHS
> purists.

I kinda like the way the debian (and ubuntu) packages do it...

They start pg_ctl/postgres like:
... -D /path/to/real-data/data-dir -c
config_file=/etc/postgresql/$INSTANCE/postgresql.conf

In /etc/postgresql/$INSTANCE/postgresql.conf, these are explictly set:
  data_directory=/path/to/real-data/data-dir
  hba_file=/etc/postgresql/$INSTANCE/pg_hba.conf
  ident_file=/etc/postgresql/$INSTANCE/pg_ident.conf
  external_pid_file=/var/run/postgresql/$INSTANCE.pid

It actually looks in /etc/postgresql/$INSTANCE/postgresql.conf to find
data_directory to use when invoking pg_ctl/postgres.

But, in my opinion, there is enough flexibility with postgresql's
config (and ability to pass un"recorded" options to postmaster at
startup too) that pg_upgrade can't guarantee it's going to figure out
every thing "automatically given a single $pgdata location to start
from".  That's simply not realistic.  Distros who do stranger things
than debian (and probably even Debian) are going to have to give their
users guidance on how to call pg_upgrade with their specific setup of
paths/configs/invocations.  It's simply that simple.

I'ld be happy enough if pg_upgrade could easily upgrade given a
datadir that had a postgresql.conf in it, or possibly a
postgresql.conf that had data_directory set in it.

Anything else, and I say it's responsibility of whoever scripted the
startup to be able to provide all the necessary information to
pg_upgrade (be it by extra command line options, or crafting a special
pg_data with symlinks that is more "normal").

a.
-- 
Aidan Van Dyk                                             Create like a god,
ai...@highrise.ca                                       command like a king,
http://www.highrise.ca/                                   work like a slave.

-- 
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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Alvaro Herrera wrote:
> 
> Excerpts from Bruce Momjian's message of lun oct 03 17:28:53 -0300 2011:
> > 
> > Alvaro Herrera wrote:
> 
> > > Well, we have the Gentoo developer in this very thread.  I'm sure they
> > > would fix their command line if we gave them a pg_ctl that worked.
> > > Surely the package that contains the init script also contains pg_ctl,
> > > so they would both be upgraded simultaneously.
> > 
> > What is the fix?  If they started the server by using --data-directory,
> > pg_ctl stop has no way to find the postmaster.pid file, and hence stop
> > the server.  Are you suggesting we remove this ability?
> 
> I am suggesting they don't start it by using --data-directory in the
> first place.

Agreed.  If you remove that, the logical problem goes away and it
becomes a simple problem of dumping the contents of postgresql.conf and
having pg_ctl (and pg_upgrade) use that.  Let me look at how much code
that would take.

-- 
  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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Andrew Dunstan wrote:
> 
> 
> On 10/03/2011 04:41 PM, Peter Eisentraut wrote:
> > On m?n, 2011-10-03 at 15:09 -0400, Bruce Momjian wrote:
> >> Why were people not using pg_ctl?  Because of the limitations which
> >> were fixed in PG 9.1?  As Dave already said, windows already has to
> >> use pg_ctl.
> > Historically, pg_ctl has had a lot of limitations.  Just off the top of
> > my head, nonstandard ports used to break it, nonstandard socket
> > directories used to break it, nonstandard authentication setups used to
> > break it, the waiting business was unreliable, the stop modes were weird
> > and not flexible enough, the behavior in error cases does not conform to
> > LSB init script conventions, there were some race conditions that I
> > don't recall the details of right now.  And you had to keep a list of
> > exactly which of these bugs were addressed in which version.
> 
> 
> I'm not sure ancient history helps us much here.  Many of these went 
> away long ago.

Agreed.  You could argue that pg_ctl 9.1 is much better than anything
anyone would be able to craft in a script.

-- 
  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] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Tom Lane
Dimitri Fontaine  writes:
> Tom Lane  writes:
>> No, because there are people who do intentionally use placeholder
>> variables as session-local storage, and that would be taking away
>> that capability.

> Or do you want to open SET typo.wrogn TO 'foobar' to just work silently?

Well, right at the moment it *does* work silently, as long as the prefix
is one you listed in custom_variable_classes.  I don't think we want to
take that away, and in particular I don't want to assume that every
variable will be declared in advance.  It's a fairly safe bet that there
are some apps out there that would be broken by such a requirement.

At the same time, I'd kind of like to see a facility for declaring such
variables, if only so you could define them to be bool/int/real not just
strings.  But this is getting far afield from the immediate proposal,
and no I'm not volunteering to do 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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Andrew Dunstan



On 10/03/2011 04:41 PM, Peter Eisentraut wrote:

On mån, 2011-10-03 at 15:09 -0400, Bruce Momjian wrote:

Why were people not using pg_ctl?  Because of the limitations which
were fixed in PG 9.1?  As Dave already said, windows already has to
use pg_ctl.

Historically, pg_ctl has had a lot of limitations.  Just off the top of
my head, nonstandard ports used to break it, nonstandard socket
directories used to break it, nonstandard authentication setups used to
break it, the waiting business was unreliable, the stop modes were weird
and not flexible enough, the behavior in error cases does not conform to
LSB init script conventions, there were some race conditions that I
don't recall the details of right now.  And you had to keep a list of
exactly which of these bugs were addressed in which version.



I'm not sure ancient history helps us much here.  Many of these went 
away long ago.




Basically, pg_ctl is a neat convenience for interactive use for people
who don't want to write advanced shell constructs, but for writing a
robust init script, you can and should do better.  For me personally,
pg_ctl is somewhere between a toy, and annoyance, and a dangerous
instrument.

Obviously, pg_ctl is now a lot better than when it was started, but
that's the reason why it is not used in certain places.




Our job should be to make it better.

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


Re: [HACKERS] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Dimitri Fontaine
Tom Lane  writes:
> Dimitri Fontaine  writes:
>> Another compromise might be to allow for defining variable in any class
>> from the configuration files but restrict that to existing classes from
>> the SET command.  Wait, that's exactly what happens as soon as there's
>> no explicit custom_variable_classes, right?
>
> No, because there are people who do intentionally use placeholder
> variables as session-local storage, and that would be taking away
> that capability.

They would have to set the variable to its default value in some
configuration file and reload, just as now.  They wouldn't have to also
edit custom_variable_classes, that's about it.

Or do you want to open SET typo.wrogn TO 'foobar' to just work silently?

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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Peter Eisentraut
On mån, 2011-10-03 at 15:09 -0400, Bruce Momjian wrote:
> Why were people not using pg_ctl?  Because of the limitations which
> were fixed in PG 9.1?  As Dave already said, windows already has to
> use pg_ctl. 

Historically, pg_ctl has had a lot of limitations.  Just off the top of
my head, nonstandard ports used to break it, nonstandard socket
directories used to break it, nonstandard authentication setups used to
break it, the waiting business was unreliable, the stop modes were weird
and not flexible enough, the behavior in error cases does not conform to
LSB init script conventions, there were some race conditions that I
don't recall the details of right now.  And you had to keep a list of
exactly which of these bugs were addressed in which version.

Basically, pg_ctl is a neat convenience for interactive use for people
who don't want to write advanced shell constructs, but for writing a
robust init script, you can and should do better.  For me personally,
pg_ctl is somewhere between a toy, and annoyance, and a dangerous
instrument.

Obviously, pg_ctl is now a lot better than when it was started, but
that's the reason why it is not used in certain places.



-- 
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] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Tom Lane
Dimitri Fontaine  writes:
> Another compromise might be to allow for defining variable in any class
> from the configuration files but restrict that to existing classes from
> the SET command.  Wait, that's exactly what happens as soon as there's
> no explicit custom_variable_classes, right?

No, because there are people who do intentionally use placeholder
variables as session-local storage, and that would be taking away
that capability.

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] [REVIEW] pg_last_xact_insert_timestamp

2011-10-03 Thread Robert Haas
On Mon, Oct 3, 2011 at 4:25 PM, Simon Riggs  wrote:
> On Mon, Oct 3, 2011 at 8:07 PM, Robert Haas  wrote:
>
>> Sorry, but I still don't really think it's fair to say that you've
>> proposed a solution to this problem.  Or if you have, neither I nor
>> Fujii Masao understand that proposal well enough to decide whether we
>> like it.
>
> Arguing between trenches doesn't really get us anywhere.
>
> As ever, when someone claims to have a better solution then it is up
> to them to prove that is the case.

So... are you going to do that?

-- 
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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Alvaro Herrera

Excerpts from Bruce Momjian's message of lun oct 03 17:28:53 -0300 2011:
> 
> Alvaro Herrera wrote:

> > Well, we have the Gentoo developer in this very thread.  I'm sure they
> > would fix their command line if we gave them a pg_ctl that worked.
> > Surely the package that contains the init script also contains pg_ctl,
> > so they would both be upgraded simultaneously.
> 
> What is the fix?  If they started the server by using --data-directory,
> pg_ctl stop has no way to find the postmaster.pid file, and hence stop
> the server.  Are you suggesting we remove this ability?

I am suggesting they don't start it by using --data-directory in the
first place.

-- 
Á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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Robert Haas
On Mon, Oct 3, 2011 at 3:59 PM, Bruce Momjian  wrote:
> pg_ctl would have to do some detective work to see if PG_VERSION existed
> in that directory and adjust its behavior --- the pg_upgrade patch I
> posted does this kind of detection.  The goal is the change would happen
> only for people using config-only directories, and when a config-only
> directory is specified.

Exactly.  That sounds like a good improvement for master even if
pg_upgrade were not at issue, and we should do it.  We can also
consider whether it makes sense to back-patch it so that pg_upgrade
can benefit from 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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Peter Eisentraut
On mån, 2011-10-03 at 19:11 +0100, Dave Page wrote:
> On Mon, Oct 3, 2011 at 7:07 PM, Peter Eisentraut  wrote:
> > On mån, 2011-10-03 at 11:27 -0400, Bruce Momjian wrote:
> >> Frankly, I am confused how this breakage has gone unreported for so
> >> long.
> >
> > Well, nobody is required to use pg_ctl,
> 
> You are if you wish to run as a service on Windows.

OK, some people are more prone to use pg_ctl than others. ;-)


-- 
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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Alvaro Herrera wrote:
> 
> Excerpts from Bruce Momjian's message of lun oct 03 17:06:16 -0300 2011:
> > 
> > Magnus Hagander wrote:
> > > Well, how does the server get from the config file to where the state
> > > file is? Can we do it the same way, or even expose it to the tools
> > > using a commandline parameter or something?
> > 
> > In that case (the Gentoo example), they use --data-directory
> > 
> > su -l postgres \
> > -c "env PGPORT=\"${PGPORT}\" ${PG_EXTRA_ENV} \
> > /usr/lib/postgresql-9.0/bin/pg_ctl \
> > start ${WAIT_FOR_START} -t ${START_TIMEOUT} -s -D ${DATA_DIR} \
> > -o '-D ${PGDATA} --data-directory=${DATA_DIR} \
> > --silent-mode=true ${PGOPTS}'"
> > 
> > We could have pg_ctl read that information from the command line for
> > pg_ctl start, but for pg_ctl stop, we have no way of getting to that
> > value.  :-(  It is not like something is missing from the code.  The
> > user can start multiple clusters from a single config dir and the
> > information they give gives us no way to know which cluster they want,
> > or where is it located.  
> 
> Well, we have the Gentoo developer in this very thread.  I'm sure they
> would fix their command line if we gave them a pg_ctl that worked.
> Surely the package that contains the init script also contains pg_ctl,
> so they would both be upgraded simultaneously.

What is the fix?  If they started the server by using --data-directory,
pg_ctl stop has no way to find the postmaster.pid file, and hence stop
the server.  Are you suggesting we remove this ability?  We could
require the --data-directory to be specified for pg_ctl stop.  Of
course, just specifying the real data directory for pg_ctl stop works
just fine so what is their motivation to specify both the configuration
and real data directories?

-- 
  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] [REVIEW] pg_last_xact_insert_timestamp

2011-10-03 Thread Simon Riggs
On Mon, Oct 3, 2011 at 8:07 PM, Robert Haas  wrote:

> Sorry, but I still don't really think it's fair to say that you've
> proposed a solution to this problem.  Or if you have, neither I nor
> Fujii Masao understand that proposal well enough to decide whether we
> like it.

Arguing between trenches doesn't really get us anywhere.

As ever, when someone claims to have a better solution then it is up
to them to prove that is the case.

-- 
 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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Alvaro Herrera

Excerpts from Bruce Momjian's message of lun oct 03 17:06:16 -0300 2011:
> 
> Magnus Hagander wrote:
> > Well, how does the server get from the config file to where the state
> > file is? Can we do it the same way, or even expose it to the tools
> > using a commandline parameter or something?
> 
> In that case (the Gentoo example), they use --data-directory
> 
> su -l postgres \
> -c "env PGPORT=\"${PGPORT}\" ${PG_EXTRA_ENV} \
> /usr/lib/postgresql-9.0/bin/pg_ctl \
> start ${WAIT_FOR_START} -t ${START_TIMEOUT} -s -D ${DATA_DIR} \
> -o '-D ${PGDATA} --data-directory=${DATA_DIR} \
> --silent-mode=true ${PGOPTS}'"
> 
> We could have pg_ctl read that information from the command line for
> pg_ctl start, but for pg_ctl stop, we have no way of getting to that
> value.  :-(  It is not like something is missing from the code.  The
> user can start multiple clusters from a single config dir and the
> information they give gives us no way to know which cluster they want,
> or where is it located.  

Well, we have the Gentoo developer in this very thread.  I'm sure they
would fix their command line if we gave them a pg_ctl that worked.
Surely the package that contains the init script also contains pg_ctl,
so they would both be upgraded simultaneously.

-- 
Á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] Bug with pg_ctl -w/wait and config-only directoriesf

2011-10-03 Thread Bruce Momjian
Alvaro Herrera wrote:
> > The problem is pg_ctl has to read server _state_ which cannot be put
> > in a configuration directory, and we don't even require the real data
> > directory to be recorded in the config file.
> 
> How so?  It certainly is in postgresql.conf.

See my other email, e.g. -o 'data_directory=/abc'

-- 
  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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Alvaro Herrera

Excerpts from Bruce Momjian's message of lun oct 03 16:55:54 -0300 2011:
> 
> Alvaro Herrera wrote:
> > 
> > Excerpts from Bruce Momjian's message of lun oct 03 16:09:08 -0300 2011:
> > 
> > > Alvaro Herrera wrote:
> > 
> > > > My guess is that we could fix the simple case (the one that doesn't
> > > > involve a "-o datadir" option) with the parse-and-report option that has
> > > > been mentioned, and dictate that the other one doesn't work.  That's
> > > > much less likely to cause a problem in practice.
> > > 
> > > Well, we are unlikely to backpatch that parse-and-report option so it
> > > would be +2 years before it could be expected to work for even
> > > single-major-version upgrades.  That just seems unworkable.  Yeah. :-(
> > 
> > If we don't do anything, then it's never going to work.  If we do it
> > today, we can have it working in the next release (9.2, right?).
> 
> No, old and new have to support this in both the postgres and pg_ctl
> binaries, which is why I said 2+ years, e.g. going from 9.1 to 9.3 is
> not going to work, unless we backpatch, and then we have to make sure
> users are on later minor versions.

Well, so 2 releases.  Same argument.  I hope you're not trying to imply
that the world will end in 2013.  (Note that I don't necessarily
disagree with Robert Haas' opinion that we might be able to backpatch
the postmaster option).


> > > Yes, auto-creation of symlinks would be useful, but at that point pg_ctl
> > > and pg_upgrade would have to use the real data directory, so I again
> > > wonder what the config-only directory is getting us.
> > 
> > Not mixing config stuff (in /etc per FHS) with server data (/var/lib,
> > again per FHS).  It's Debian policy anyway.  I don't judge whether this
> > is sane or not.  See
> > http://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard
> 
> Yes, but why not do this via symlinks?

It doesn't matter now, because we have the functionality already.

> The problem is pg_ctl has to read server _state_ which cannot be put
> in a configuration directory, and we don't even require the real data
> directory to be recorded in the config file.

How so?  It certainly is in postgresql.conf.

-- 
Á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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Magnus Hagander wrote:
> Well, how does the server get from the config file to where the state
> file is? Can we do it the same way, or even expose it to the tools
> using a commandline parameter or something?

In that case (the Gentoo example), they use --data-directory

su -l postgres \
-c "env PGPORT=\"${PGPORT}\" ${PG_EXTRA_ENV} \
/usr/lib/postgresql-9.0/bin/pg_ctl \
start ${WAIT_FOR_START} -t ${START_TIMEOUT} -s -D ${DATA_DIR} \
-o '-D ${PGDATA} --data-directory=${DATA_DIR} \
--silent-mode=true ${PGOPTS}'"

We could have pg_ctl read that information from the command line for
pg_ctl start, but for pg_ctl stop, we have no way of getting to that
value.  :-(  It is not like something is missing from the code.  The
user can start multiple clusters from a single config dir and the
information they give gives us no way to know which cluster they want,
or where is it located.  

Yes, this is where the system seems logically broken for our purposes.
It took me a while to understand this problem.

> Or looking just from the pg_upgrade perspective, can we get enough
> info out of a running backend before sending signals, or do we need it
> on startup as well?

pg_upgrade starts with all clusters stopped so there is no way to query
it --- it is a chicken and egg in that we don't know where the data
directory is to start it.


-- 
  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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Robert Haas wrote:
> On Mon, Oct 3, 2011 at 3:09 PM, Bruce Momjian  wrote:
> > Well, we are unlikely to backpatch that parse-and-report option so it
> > would be +2 years before it could be expected to work for even
> > single-major-version upgrades. ?That just seems unworkable. ?Yeah. :-(
> 
> I'd like to see the patch first, but I am not convinced that we
> couldn't back-patch this.  I am not a big fan of back-patching things
> that are not bug fixes, but I think you can make a fairly reasonable
> argument that this is a bug in pg_ctl, and therefore in pg_upgrade,
> and that we should therefore fix it.  Frankly, I think the
> parse-and-report option is the least of our troubles.  Implementing
> that much without breaking anything seems like it should be quite
> straightforward.  If that's all we need to get ourselves out of this
> mess, then let's just go do it (carefully).

We can't work on a patch until we have the defined behavior we want and
it should be understandable.

> The trickier part is that you then have to make sure that - in the
> course of fixing the cases where pg_ctl behaves properly today - you
> don't make any backward-incompatible behavior changes.  Just for
> example, we can't make a unilateral decision now that - in
> split-config scenarios - pg_ctl should always be invoked with a -D
> argument that points to the postgresql.conf directory rather than the
> data directory, because per your email upthread there are cases where
> that doesn't work today, and therefore people are probably pointing at
> the data directory.  But we probably *could* get away with making
> cases work that are currently broken - e.g. allow pg_ctl stop -D $FOO
> to work if $FOO is *either* the config dir or the real data dir.  Now,
> is that too much to back-patch?  Without having looked at the code,
> I'm not sure, but it might turn out it's not that bad.  We've
> certainly back-patched scarier stuff before when it's been necessary
> to fix bugs - see, for example, commit
> ceaf5052c6a7bee794211f5d4c503639bdf3dff0.

pg_ctl would have to do some detective work to see if PG_VERSION existed
in that directory and adjust its behavior --- the pg_upgrade patch I
posted does this kind of detection.  The goal is the change would happen
only for people using config-only directories, and when a config-only
directory is specified.

> Furthermore, if we look at this and ultimately conclude that it's too
> invasive to back-patch, all is not lost.  We have a recommended layout
> for our tree, and the Ubuntu and Gentoo folks have decided not to use
> it (which is perfectly fine), and they have installed various
> workarounds for problems like "pg_ctl doesn't work well with that
> directory layout".  This will be another scenario that they will need
> to work around, and I'm guessing that they are more than capable of
> doing that (if they aren't, perhaps they shouldn't have insisted on a
> different layout in the first place... but I don't think that's the
> case).  We can also document the workarounds for other users who have

Yes, they are using symlinks now to work around the pg_upgrade/pg_ctl
problem.

> this problem, and we can fix it for real in 9.2.  Sure, that will mean
> it's 2+ years before people really start being able to take advantage
> of the new features, but I don't think that makes it not worth doing.
> Rome wasn't built in a day, and this didn't get broken in a day.  I'm
> not abandoning all hope of a short-term fix, but even if we do give up
> on that, I don't think that a long-term fix plus some documentation of
> what to do meanwhile is a crazy approach to the problem.

I can't figure out what a non-crazy solution looks like.

-- 
  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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Magnus Hagander
On Mon, Oct 3, 2011 at 21:55, Bruce Momjian  wrote:
> Alvaro Herrera wrote:
>>
>> Excerpts from Bruce Momjian's message of lun oct 03 16:09:08 -0300 2011:
>>
>> > Alvaro Herrera wrote:
>>
>> > > My guess is that we could fix the simple case (the one that doesn't
>> > > involve a "-o datadir" option) with the parse-and-report option that has
>> > > been mentioned, and dictate that the other one doesn't work.  That's
>> > > much less likely to cause a problem in practice.
>> >
>> > Well, we are unlikely to backpatch that parse-and-report option so it
>> > would be +2 years before it could be expected to work for even
>> > single-major-version upgrades.  That just seems unworkable.  Yeah. :-(
>>
>> If we don't do anything, then it's never going to work.  If we do it
>> today, we can have it working in the next release (9.2, right?).
>
> No, old and new have to support this in both the postgres and pg_ctl
> binaries, which is why I said 2+ years, e.g. going from 9.1 to 9.3 is
> not going to work, unless we backpatch, and then we have to make sure
> users are on later minor versions.
>
>> "It doesn't work now but will work in the next release; and here's a
>> workaround that can get you out for now" is a useful if painful answer;
>> "it's never going to work" is a lot worse.
>>
>> We've been in that sort of situation before, and the answer has always
>> been to fix the issue for future users.  Assuming the world doesn't end
>> next year (a safe bet if you ask me), those are going to be more common
>> that current users, so it's worth the hassle.
>>
>> > Yes, auto-creation of symlinks would be useful, but at that point pg_ctl
>> > and pg_upgrade would have to use the real data directory, so I again
>> > wonder what the config-only directory is getting us.
>>
>> Not mixing config stuff (in /etc per FHS) with server data (/var/lib,
>> again per FHS).  It's Debian policy anyway.  I don't judge whether this
>> is sane or not.  See
>> http://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard
>
> Yes, but why not do this via symlinks?  The problem is pg_ctl has to
> read server _state_ which cannot be put in a configuration directory,
> and we don't even require the real data directory to be recorded in the
> config file.

Well, how does the server get from the config file to where the state
file is? Can we do it the same way, or even expose it to the tools
using a commandline parameter or something?

Or looking just from the pg_upgrade perspective, can we get enough
info out of a running backend before sending signals, or do we need it
on startup as well?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Alvaro Herrera wrote:
> 
> Excerpts from Bruce Momjian's message of lun oct 03 16:09:08 -0300 2011:
> 
> > Alvaro Herrera wrote:
> 
> > > My guess is that we could fix the simple case (the one that doesn't
> > > involve a "-o datadir" option) with the parse-and-report option that has
> > > been mentioned, and dictate that the other one doesn't work.  That's
> > > much less likely to cause a problem in practice.
> > 
> > Well, we are unlikely to backpatch that parse-and-report option so it
> > would be +2 years before it could be expected to work for even
> > single-major-version upgrades.  That just seems unworkable.  Yeah. :-(
> 
> If we don't do anything, then it's never going to work.  If we do it
> today, we can have it working in the next release (9.2, right?).

No, old and new have to support this in both the postgres and pg_ctl
binaries, which is why I said 2+ years, e.g. going from 9.1 to 9.3 is
not going to work, unless we backpatch, and then we have to make sure
users are on later minor versions.

> "It doesn't work now but will work in the next release; and here's a
> workaround that can get you out for now" is a useful if painful answer;
> "it's never going to work" is a lot worse.
> 
> We've been in that sort of situation before, and the answer has always
> been to fix the issue for future users.  Assuming the world doesn't end
> next year (a safe bet if you ask me), those are going to be more common
> that current users, so it's worth the hassle.
> 
> > Yes, auto-creation of symlinks would be useful, but at that point pg_ctl
> > and pg_upgrade would have to use the real data directory, so I again
> > wonder what the config-only directory is getting us.
> 
> Not mixing config stuff (in /etc per FHS) with server data (/var/lib,
> again per FHS).  It's Debian policy anyway.  I don't judge whether this
> is sane or not.  See
> http://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard

Yes, but why not do this via symlinks?  The problem is pg_ctl has to
read server _state_ which cannot be put in a configuration directory,
and we don't even require the real data directory to be recorded in the
config file.

-- 
  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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Alvaro Herrera

Excerpts from Bruce Momjian's message of lun oct 03 16:09:08 -0300 2011:

> Alvaro Herrera wrote:

> > My guess is that we could fix the simple case (the one that doesn't
> > involve a "-o datadir" option) with the parse-and-report option that has
> > been mentioned, and dictate that the other one doesn't work.  That's
> > much less likely to cause a problem in practice.
> 
> Well, we are unlikely to backpatch that parse-and-report option so it
> would be +2 years before it could be expected to work for even
> single-major-version upgrades.  That just seems unworkable.  Yeah. :-(

If we don't do anything, then it's never going to work.  If we do it
today, we can have it working in the next release (9.2, right?).

"It doesn't work now but will work in the next release; and here's a
workaround that can get you out for now" is a useful if painful answer;
"it's never going to work" is a lot worse.

We've been in that sort of situation before, and the answer has always
been to fix the issue for future users.  Assuming the world doesn't end
next year (a safe bet if you ask me), those are going to be more common
that current users, so it's worth the hassle.

> Yes, auto-creation of symlinks would be useful, but at that point pg_ctl
> and pg_upgrade would have to use the real data directory, so I again
> wonder what the config-only directory is getting us.

Not mixing config stuff (in /etc per FHS) with server data (/var/lib,
again per FHS).  It's Debian policy anyway.  I don't judge whether this
is sane or not.  See
http://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard

> Why were people not using pg_ctl?  Because of the limitations which were
> fixed in PG 9.1?  As Dave already said, windows already has to use pg_ctl.

As I said, Debian has their own version pg_ctlcluster because of their
work to allow multiple major versions to work simultaneously in the same
server.  I dunno what about Gentoo.

-- 
Á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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Robert Haas
On Mon, Oct 3, 2011 at 3:09 PM, Bruce Momjian  wrote:
> Well, we are unlikely to backpatch that parse-and-report option so it
> would be +2 years before it could be expected to work for even
> single-major-version upgrades.  That just seems unworkable.  Yeah. :-(

I'd like to see the patch first, but I am not convinced that we
couldn't back-patch this.  I am not a big fan of back-patching things
that are not bug fixes, but I think you can make a fairly reasonable
argument that this is a bug in pg_ctl, and therefore in pg_upgrade,
and that we should therefore fix it.  Frankly, I think the
parse-and-report option is the least of our troubles.  Implementing
that much without breaking anything seems like it should be quite
straightforward.  If that's all we need to get ourselves out of this
mess, then let's just go do it (carefully).

The trickier part is that you then have to make sure that - in the
course of fixing the cases where pg_ctl behaves properly today - you
don't make any backward-incompatible behavior changes.  Just for
example, we can't make a unilateral decision now that - in
split-config scenarios - pg_ctl should always be invoked with a -D
argument that points to the postgresql.conf directory rather than the
data directory, because per your email upthread there are cases where
that doesn't work today, and therefore people are probably pointing at
the data directory.  But we probably *could* get away with making
cases work that are currently broken - e.g. allow pg_ctl stop -D $FOO
to work if $FOO is *either* the config dir or the real data dir.  Now,
is that too much to back-patch?  Without having looked at the code,
I'm not sure, but it might turn out it's not that bad.  We've
certainly back-patched scarier stuff before when it's been necessary
to fix bugs - see, for example, commit
ceaf5052c6a7bee794211f5d4c503639bdf3dff0.

Furthermore, if we look at this and ultimately conclude that it's too
invasive to back-patch, all is not lost.  We have a recommended layout
for our tree, and the Ubuntu and Gentoo folks have decided not to use
it (which is perfectly fine), and they have installed various
workarounds for problems like "pg_ctl doesn't work well with that
directory layout".  This will be another scenario that they will need
to work around, and I'm guessing that they are more than capable of
doing that (if they aren't, perhaps they shouldn't have insisted on a
different layout in the first place... but I don't think that's the
case).  We can also document the workarounds for other users who have
this problem, and we can fix it for real in 9.2.  Sure, that will mean
it's 2+ years before people really start being able to take advantage
of the new features, but I don't think that makes it not worth doing.
Rome wasn't built in a day, and this didn't get broken in a day.  I'm
not abandoning all hope of a short-term fix, but even if we do give up
on that, I don't think that a long-term fix plus some documentation of
what to do meanwhile is a crazy approach to the problem.

-- 
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] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Dimitri Fontaine
David Fetter  writes:
> Perhaps it's best to document this usage and include the warning for
> those less "bright," as you term them.   I'd be less tempted to call
> them "not bright" and more tempted to think they might assume
> PostgreSQL already takes care of cleaning this up, but whatever.

Who's that dim?  D'oh.

Another compromise might be to allow for defining variable in any class
from the configuration files but restrict that to existing classes from
the SET command.  Wait, that's exactly what happens as soon as there's
no explicit custom_variable_classes, right?

So we're talking about people with configuration file editing and reload
powers, not about anyone who can connect.  I think that's ok.

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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Alvaro Herrera wrote:
> 
> Excerpts from Bruce Momjian's message of lun oct 03 16:03:47 -0300 2011:
> 
> > > I'm not sure how big the overlap is - would it be easier if you moved
> > > the required functionality into pg_upgrade itself, as you mentioned at
> > > some point? As in, would it be easier to fix the config-only directory
> > > case for the limited subset of functionality that pg_upgrade needs?
> > 
> > Not really --- it is the -w/wait mode pg_upgrade needs. There is a lot
> > of new code in pg_ctl that reads the postmaster.pid file for socket
> > location, port number, etc, that doesn't make sense to duplicate. 
> > Frankly, there is the huge problem that they might specify the data
> > directory on the command line --- that would be a bear to support.
> 
> How about creating a library with the controlling stuff that's shared by
> pg_ctl and pg_upgrade?

Fine, but again, unlikely to be backpatched, which means +2 years.

-- 
  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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Alvaro Herrera wrote:
> 
> Excerpts from Bruce Momjian's message of lun oct 03 15:23:47 -0300 2011:
> > Peter Eisentraut wrote:
> > > On m?n, 2011-10-03 at 11:27 -0400, Bruce Momjian wrote:
> > > > Frankly, I am confused how this breakage has gone unreported for so
> > > > long.
> > > 
> > > Well, nobody is required to use pg_ctl, and for the longest time, it was
> > > pg_ctl that was considered to be broken (for various other reasons) and
> > > avoided in packaged init scripts.
> > 
> > Yes, but I am now seeing that pg_ctl is really unfixable.  Is the
> > config-only directory really a valuable feature if pg_ctl does not work?
> > 
> > If we could document that pg_ctl (and pg_upgrade) doesn't work with
> > config-only directories, at least we would have a consistent API.  The
> > question is whether the config-only directory is useful with this
> > restriction.
> 
> Evidently people that use config-only dirs don't care all that much
> about pg_ctl; we'd have a lot of bugs about it otherwise.  But I don't
> think that's the case for pg_upgrade.  I think that simply dictating the
> combination of conf-only dirs and pg_upgrade doesn't work is not going
> to be a very popular choice, particularly if there's a simple workaround
> such as adding a symlink.  (This makes me wonder, though, we don't we
> require that said symlink is always in place; maybe have postmaster
> create it automatically if it's not present?)
> 
> My guess is that we could fix the simple case (the one that doesn't
> involve a "-o datadir" option) with the parse-and-report option that has
> been mentioned, and dictate that the other one doesn't work.  That's
> much less likely to cause a problem in practice.

Well, we are unlikely to backpatch that parse-and-report option so it
would be +2 years before it could be expected to work for even
single-major-version upgrades.  That just seems unworkable.  Yeah. :-(

Yes, auto-creation of symlinks would be useful, but at that point pg_ctl
and pg_upgrade would have to use the real data directory, so I again
wonder what the config-only directory is getting us.

Why were people not using pg_ctl?  Because of the limitations which were
fixed in PG 9.1?  As Dave already said, windows already has to use pg_ctl.

-- 
  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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Alvaro Herrera

Excerpts from Bruce Momjian's message of lun oct 03 16:03:47 -0300 2011:

> > I'm not sure how big the overlap is - would it be easier if you moved
> > the required functionality into pg_upgrade itself, as you mentioned at
> > some point? As in, would it be easier to fix the config-only directory
> > case for the limited subset of functionality that pg_upgrade needs?
> 
> Not really --- it is the -w/wait mode pg_upgrade needs. There is a lot
> of new code in pg_ctl that reads the postmaster.pid file for socket
> location, port number, etc, that doesn't make sense to duplicate. 
> Frankly, there is the huge problem that they might specify the data
> directory on the command line --- that would be a bear to support.

How about creating a library with the controlling stuff that's shared by
pg_ctl and pg_upgrade?

-- 
Á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] [REVIEW] pg_last_xact_insert_timestamp

2011-10-03 Thread Robert Haas
On Sun, Oct 2, 2011 at 8:21 AM, Simon Riggs  wrote:
> The problem is to find the replication delay, even when the system is quiet.
>
> What I have proposed finds the replication delay more accurately even
> than looking at the last commit, since often there are writes but no
> commits.
>
> If we focus on the problem, rather than the first suggested solution
> to that problem, we'll come out on top.

Sorry, but I still don't really think it's fair to say that you've
proposed a solution to this problem.  Or if you have, neither I nor
Fujii Masao understand that proposal well enough to decide whether we
like it.  You said "maybe we could WAL log something once per
checkpoint cycle" or "maybe we could add a new protocol message".
We've both replied with various emails saying "we don't understand how
that would solve the problem".  If you want to add some detail to your
proposal, then we can weigh the pros and cons as compared with what
the patch does - but right now all you've provided is a theory that
there might be a better solution to this problem out there, not any
details about how it would work.  Or if you have, then please post a
link to the message where those details are written out, because I
cannot find them on the thread.

I do, however, agree that that the case where the system is quiet is
the problem case for computing replication delay.  It seems to me
that, even without this patch, if the system has a continuous stream
of commits, you can easily find the replication delay by differencing
the current time on the master with the value returned by
pg_last_xact_replay_timestamp().  However, if the master goes quiet,
then the slave will appear to be progressively farther behind.  With
the addition of this patch, that problem goes away: you can now
difference the return value of pg_last_xact_insert_timestamp() on the
master with the return value of pg_last_xact_replay_timestamp() on the
slave.  If the master goes quiet, then pg_last_xact_insert_timestamp()
will stop advancing, and so the two values you are comparing will be
equal once the slave has caught up, and remain equal until activity
resumes on the master.

Now, there is a more subtle remaining problem, which is that when
activity *does* resume on the master, there will be a (probably quite
short) window of time during which the slave will have a much earlier
timestamp than the one on the master.  When the master has a commit
after a long idle period but the slave has not yet replayed the commit
record, the replication delay will appear to be equal to the length of
the idle period.  But that doesn't seem like a sufficient reason to
reject the whole approach, because there are several ways around it.
First, you could simply decide that the large computed lag value,
although counterintuitive, is accurate under some definition, because,
well, that really is the lag between the last transaction committed on
the master and the last transaction committed on the standby, and if
you don't like the fact that timestamps behave that way, you should
compare using WAL positions instead.  If you don't like that approach,
then a second, also viable approach is to teach your monitoring
software that the replication delay can never increase faster than the
rate at which clock time is passing.  So if you were caught up a
minute ago, then you can't be more than a minute behind now.

Another point I want to make here is that there's probably more than
one useful definition of replication delay.  The previous question
presupposes that you're trying to answer the question "if I have a
transaction that committed N seconds ago on the master, will it be
visible on the standby?".  It's also a reasonable time-based
substitute for measuring the difference in master and standby WAL
positions, although certainly it's going to work better if the rate of
WAL generation is relatively even.  But for a lot of people, it may be
that what they really want to know is "what is the expected time for
the standby to replay all generated but not yet applied WAL?" - or
maybe some third thing that I'm not thinking of - and this function
won't provide that.  I think we can ultimately afford to provide more
than one mechanism here, so I don't see doing this as foreclosing any
other also-useful calculation that someone may wish to add in the
future.

-- 
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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Magnus Hagander wrote:
> >> So, you are saying that people who want config-only directories are just
> >> not people who normally use pg_ctl, because if they were, they would
> >> have reported the bug? ?That seems unlikely. ?I will admit the Gentoo
> >> case is exactly that.
> >
> > As Dave has pointed out there are many more people that use it, probably
> > most notably Debian/Ubuntu users.
> >
> >> So we just document that config-only directories don't work for pg_ctl
> >> and pg_upgrade?
> >>
> >
> > I'd rather not if it can be avoided.
> 
> I think we "can live" with pg_ctl not working - since that problem has
> already been solved by these people - at least partially.
> 
> Getting pg_upgrade to work would be a *lot* more important.

Well, the users are currently symlinking the config files into the real
data directory and running pg_upgrade that way --- we can document that
work-around.

> I'm not sure how big the overlap is - would it be easier if you moved
> the required functionality into pg_upgrade itself, as you mentioned at
> some point? As in, would it be easier to fix the config-only directory
> case for the limited subset of functionality that pg_upgrade needs?

Not really --- it is the -w/wait mode pg_upgrade needs. There is a lot
of new code in pg_ctl that reads the postmaster.pid file for socket
location, port number, etc, that doesn't make sense to duplicate. 
Frankly, there is the huge problem that they might specify the data
directory on the command line --- that would be a bear to support.

I think the only sane fix is to require pg_ctl and pg_upgrade to specify
the config _and_ real data directory.  The fact that PGDATA/-D currently
can point to a config-only directory means this will lead to a host of
confusion.

What would make more sense would be to add a PGCONFIG/-C parameter that
points to the config directory and make PGDATA/-D only point to the real
data directory.  Yes, it is more work for simple config-only installs,
but it allows pg_ctl and pg_upgrade to work with some sanity.

-- 
  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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Alvaro Herrera

Excerpts from Bruce Momjian's message of lun oct 03 15:23:47 -0300 2011:
> Peter Eisentraut wrote:
> > On m?n, 2011-10-03 at 11:27 -0400, Bruce Momjian wrote:
> > > Frankly, I am confused how this breakage has gone unreported for so
> > > long.
> > 
> > Well, nobody is required to use pg_ctl, and for the longest time, it was
> > pg_ctl that was considered to be broken (for various other reasons) and
> > avoided in packaged init scripts.
> 
> Yes, but I am now seeing that pg_ctl is really unfixable.  Is the
> config-only directory really a valuable feature if pg_ctl does not work?
> 
> If we could document that pg_ctl (and pg_upgrade) doesn't work with
> config-only directories, at least we would have a consistent API.  The
> question is whether the config-only directory is useful with this
> restriction.

Evidently people that use config-only dirs don't care all that much
about pg_ctl; we'd have a lot of bugs about it otherwise.  But I don't
think that's the case for pg_upgrade.  I think that simply dictating the
combination of conf-only dirs and pg_upgrade doesn't work is not going
to be a very popular choice, particularly if there's a simple workaround
such as adding a symlink.  (This makes me wonder, though, we don't we
require that said symlink is always in place; maybe have postmaster
create it automatically if it's not present?)

My guess is that we could fix the simple case (the one that doesn't
involve a "-o datadir" option) with the parse-and-report option that has
been mentioned, and dictate that the other one doesn't work.  That's
much less likely to cause a problem in practice.

-- 
Á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] PostgreSQL X/Open Socket / BSD Socket Issue on HP-UX

2011-10-03 Thread Heikki Linnakangas

On 22.09.2011 13:51, MUHAMMAD ASIF wrote:

You are right, _xpg_ socket functionality is not available in older systems, it 
is available in hp-ux 11.23 version through patch HCO_35744 . HPUX 10.20 is 
very old machine (1996). I am using latest HPUX B.11.31 machine, I don't have 
access to older systems. -D_XOPEN_SOURCE_EXTENDED make the postgres build 
X/Open Socket enabled including connector's i.e libpq. Now if system default 
64bit perl (BSD Socket) try to use libpq (X/Open Socket) it will end up in 
unexpected results or errors . HP-UX don't allow mixing of X/Open Socket 
objects and BSD Socket objects in the same 64bit binary, HP tried to fix this 
issue through -D_HPUX_ALT_XOPEN_SOCKET_API on later version of OS. It seems 
nice that if postgres adopt this fix at least for connectors (PFA patch, minor 
change in src/interfaces/libpq/Makefile) and so that users on later hp-ux boxes 
don't trouble with these socket issues  and connect their applications to 
database server with the help of libpq without the fear of X/Open So

cket or BSD Socket complexity. On older system defining 
_HPUX_ALT_XOPEN_SOCKET_API should do no effects or issues.

You're right that defining _HPUX_ALT_XOPEN_SOCKET_API should have no 
effect on older systems that don't have that. But removing -lxnet and 
-D_XOPEN_SOURCE_EXTENDED *is* clearly going to cause problems on older 
systems.


According to 
http://docstore.mik.ua/manuals/hp-ux/en/B2355-60130/xopen_networking.7.html, 
-D_XOPEN_SOURCE_EXTENDED should still be defined, even if you use 
-D_HPUX_ALT_XOPEN_SOCKET_API. So removing that was bogus. But -lxnet 
should indeed not be used with _HPUX_ALT_XOPEN_SOCKET_API, so I think we 
need a configure test to see whether that option is available, and use 
it only if it is.


Looking at the headers, it seems pretty hard to detect whether 
_HPUX_ALT_XOPEN_SOCKET_API is available. The best I can think of is to 
check whether the _xpg_* functions exist. That's a bit ugly because a 
program is not supposed to call those functions directly, but it should 
work fine in practice, so attached is a patch to do that.


I did some experiments on my HP-UX box ("HP-UX guest2 B.11.31 U ia64 
HP-UX", according to uname -a). I built a small test program that uses 
libpq, and also calls socket() and getsockopt() on an unrelated socket. 
I also tested a little perl function in the database, that calls 
getsockopt(). Without this patch, the perl function fails, and the test 
program fails unless compiled with "-lxnet -D_XOPEN_SOURCE_EXTENDED" 
(ie. unless it uses X/Open sockets). With the patch, the perl function 
works, and the test program works, whether it's compiled with X/Open or not.


In the patch, I had to move the logic into configure.in, because the 
autoconf AC_* macros can't be used in the template, which is a plain 
shell script.


Unforunately I don't have access to any older HP-UX boxes that don't 
have _HPUX_ALT_XOPEN_SOCKET_API. Tom, can you test this on that old 
HP-UX box of yours?


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
diff --git a/configure b/configure
index 58fea90..b209e0f 100755
--- a/configure
+++ b/configure
@@ -4654,6 +4654,131 @@ if test "$PORTNAME" = "win32"; then
   CPPFLAGS="$CPPFLAGS -I$srcdir/src/include/port/win32 -DEXEC_BACKEND"
 fi
 
+# On HP-UX, we need to use the X/Open Networking Interfaces. Otherwise bind(),
+# getpeername() and so on don't work correctly in the LP64 data model.
+#
+# There are two ways to use X/Open Networking Interfaces, as described by
+# xopen_networking(7) man page. Method A is to define -D_XOPEN_SOURCE_EXTENDED
+# and link with -lxnet. libxnet contains the X/Open versions of the socket
+# functions. In method B, we define -D_XOPEN_SOURCE_EXTENDED and
+# _HPUX_ALT_XOPEN_SOCKET_API, and do *not* link with libxnet. In this method,
+# sys/socket.h maps the socket functions to variants in libc with prefix
+# _xpg_*, which have the right interface. Method B is preferred, as it allows
+# linking with other libraries whether they use BSD or X/Open sockets, but
+# it's not available on older versions of HP-UX. Detect whether method B can
+# be used, by checking whether libc has function _xpg_socket().
+if test "$PORTNAME" = "hpux"; then
+
+for ac_func in _xpg_socket
+do
+as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
+{ $as_echo "$as_me:$LINENO: checking for $ac_func" >&5
+$as_echo_n "checking for $ac_func... " >&6; }
+if { as_var=$as_ac_var; eval "test \"\${$as_var+set}\" = set"; }; then
+  $as_echo_n "(cached) " >&6
+else
+  cat >conftest.$ac_ext <<_ACEOF
+/* confdefs.h.  */
+_ACEOF
+cat confdefs.h >>conftest.$ac_ext
+cat >>conftest.$ac_ext <<_ACEOF
+/* end confdefs.h.  */
+/* Define $ac_func to an innocuous variant, in case  declares $ac_func.
+   For example, HP-UX 11i  declares gettimeofday.  */
+#define $ac_func innocuous_$ac_func
+
+/* System header to define __stub macros and hopefully few prototypes,
+which can conflict with char $ac_func (); b

Re: [HACKERS] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Magnus Hagander
On Mon, Oct 3, 2011 at 20:39, Andrew Dunstan  wrote:
>
> On 10/03/2011 02:25 PM, Bruce Momjian wrote:
>>
>> Andrew Dunstan wrote:
>>>
>>> On 10/03/2011 02:15 PM, Bruce Momjian wrote:

 Andrew Dunstan wrote:
>
> On 10/03/2011 12:54 PM, Tom Lane wrote:
>>
>> I was never exactly thrilled with the separate-config-directory design
>> to start with, so I'm probably not the person to opine on whether we
>> could get away with removing it.
>>
>>
>
> The horse has well and truly bolted. We'd have a major row if anyone
> tried to remove it. Let's not rehash old battles. Our only option is to
> make it work as best we can.

 I disagree.  If people were using it we would have had many more bug
 reports about pg_ctl not working.

>>> No, that's an indication people aren't using pg_ctl, not that they
>>> aren't using separate config dirs.
>>
>> So, you are saying that people who want config-only directories are just
>> not people who normally use pg_ctl, because if they were, they would
>> have reported the bug?  That seems unlikely.  I will admit the Gentoo
>> case is exactly that.
>
> As Dave has pointed out there are many more people that use it, probably
> most notably Debian/Ubuntu users.
>
>> So we just document that config-only directories don't work for pg_ctl
>> and pg_upgrade?
>>
>
> I'd rather not if it can be avoided.

I think we "can live" with pg_ctl not working - since that problem has
already been solved by these people - at least partially.

Getting pg_upgrade to work would be a *lot* more important.

I'm not sure how big the overlap is - would it be easier if you moved
the required functionality into pg_upgrade itself, as you mentioned at
some point? As in, would it be easier to fix the config-only directory
case for the limited subset of functionality that pg_upgrade needs?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Andrew Dunstan wrote:
> 
> 
> On 10/03/2011 02:25 PM, Bruce Momjian wrote:
> > Andrew Dunstan wrote:
> >>
> >> On 10/03/2011 02:15 PM, Bruce Momjian wrote:
> >>> Andrew Dunstan wrote:
>  On 10/03/2011 12:54 PM, Tom Lane wrote:
> > I was never exactly thrilled with the separate-config-directory design
> > to start with, so I'm probably not the person to opine on whether we
> > could get away with removing it.
> >
> > 
>  The horse has well and truly bolted. We'd have a major row if anyone
>  tried to remove it. Let's not rehash old battles. Our only option is to
>  make it work as best we can.
> >>> I disagree.  If people were using it we would have had many more bug
> >>> reports about pg_ctl not working.
> >>>
> >> No, that's an indication people aren't using pg_ctl, not that they
> >> aren't using separate config dirs.
> > So, you are saying that people who want config-only directories are just
> > not people who normally use pg_ctl, because if they were, they would
> > have reported the bug?  That seems unlikely.  I will admit the Gentoo
> > case is exactly that.
> 
> As Dave has pointed out there are many more people that use it, probably 
> most notably Debian/Ubuntu users.
> 
> > So we just document that config-only directories don't work for pg_ctl
> > and pg_upgrade?
> >
> 
> I'd rather not if it can be avoided.

OK, please propose and "avoid" plan?  I can't come up with one that makes
any sense.

-- 
  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] Unexpected collation error in 9.1.1

2011-10-03 Thread Christian Ullrich

* Tom Lane wrote:


Christian Ullrich  writes:



I tried adding a not-null column in one step and got a collation
error for a different column.



itd=>  alter table livedata add column pricechanged timestamp not null default 
current_timestamp;
ERROR:  no collation was derived for column "whois_b" with collatable type 
citext
TIP:  Use the COLLATE clause to set the collation explicitly.


That's pretty bizarre, but I can't reproduce it on the basis of the
supplied example:

I tried adding UNIQUE and CHECK constraints too, and still no luck.
Are you sure you're using 9.1.1?


Yes, the EDB x64 Windows build. But I can't reproduce it now, either. I 
got that error twice today (out of only two attempts), while doing 
basically this:


- Dump database A
- Clear out database B by doing DROP SCHEMA CASCADE; CREATE SCHEMA
- Load dump into database B
- Replace column in B by DROPping it (it was BOOLEAN before) and then
  ADDing the new one as a TIMESTAMP

There was no other activity on B while I was doing it.

I just tried doing that again, but it worked several times in a row. 
That may be because I changed the type in A in the meantime, so (among 
other things) the heap layout before the DROP COLUMN is different now. 
I'll give it another try with the original dump tomorrow when I'm back 
at work.


--
Christian

--
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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Andrew Dunstan



On 10/03/2011 02:25 PM, Bruce Momjian wrote:

Andrew Dunstan wrote:


On 10/03/2011 02:15 PM, Bruce Momjian wrote:

Andrew Dunstan wrote:

On 10/03/2011 12:54 PM, Tom Lane wrote:

I was never exactly thrilled with the separate-config-directory design
to start with, so I'm probably not the person to opine on whether we
could get away with removing it.



The horse has well and truly bolted. We'd have a major row if anyone
tried to remove it. Let's not rehash old battles. Our only option is to
make it work as best we can.

I disagree.  If people were using it we would have had many more bug
reports about pg_ctl not working.


No, that's an indication people aren't using pg_ctl, not that they
aren't using separate config dirs.

So, you are saying that people who want config-only directories are just
not people who normally use pg_ctl, because if they were, they would
have reported the bug?  That seems unlikely.  I will admit the Gentoo
case is exactly that.


As Dave has pointed out there are many more people that use it, probably 
most notably Debian/Ubuntu users.



So we just document that config-only directories don't work for pg_ctl
and pg_upgrade?



I'd rather not if it can be avoided.

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


Re: [HACKERS] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Andrew Dunstan wrote:
> 
> 
> On 10/03/2011 02:15 PM, Bruce Momjian wrote:
> > Andrew Dunstan wrote:
> >>
> >> On 10/03/2011 12:54 PM, Tom Lane wrote:
> >>> I was never exactly thrilled with the separate-config-directory design
> >>> to start with, so I'm probably not the person to opine on whether we
> >>> could get away with removing it.
> >>>
> >>>   
> >> The horse has well and truly bolted. We'd have a major row if anyone
> >> tried to remove it. Let's not rehash old battles. Our only option is to
> >> make it work as best we can.
> > I disagree.  If people were using it we would have had many more bug
> > reports about pg_ctl not working.
> >
> 
> No, that's an indication people aren't using pg_ctl, not that they 
> aren't using separate config dirs.

So, you are saying that people who want config-only directories are just
not people who normally use pg_ctl, because if they were, they would
have reported the bug?  That seems unlikely.  I will admit the Gentoo
case is exactly that.

So we just document that config-only directories don't work for pg_ctl
and pg_upgrade?

-- 
  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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Dave Page
On Mon, Oct 3, 2011 at 7:15 PM, Bruce Momjian  wrote:
> Andrew Dunstan wrote:
>>
>>
>> On 10/03/2011 12:54 PM, Tom Lane wrote:
>> >
>> > I was never exactly thrilled with the separate-config-directory design
>> > to start with, so I'm probably not the person to opine on whether we
>> > could get away with removing it.
>> >
>> >
>>
>> The horse has well and truly bolted. We'd have a major row if anyone
>> tried to remove it. Let's not rehash old battles. Our only option is to
>> make it work as best we can.
>
> I disagree.  If people were using it we would have had many more bug
> reports about pg_ctl not working.

Debian/ubuntu packages and our own project infrastructure use it.
Though, there is a non-trivial script wrapping it, presumably to try
to make it work properly, and handle side-by-side installations of
different major versions.

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Peter Eisentraut wrote:
> On m?n, 2011-10-03 at 11:27 -0400, Bruce Momjian wrote:
> > Frankly, I am confused how this breakage has gone unreported for so
> > long.
> 
> Well, nobody is required to use pg_ctl, and for the longest time, it was
> pg_ctl that was considered to be broken (for various other reasons) and
> avoided in packaged init scripts.

Yes, but I am now seeing that pg_ctl is really unfixable.  Is the
config-only directory really a valuable feature if pg_ctl does not work?

If we could document that pg_ctl (and pg_upgrade) doesn't work with
config-only directories, at least we would have a consistent API.  The
question is whether the config-only directory is useful with this
restriction.  Are people recording the postmaster pid somewhere when
they start it?  I doubt they are parsing the connection information we
added to postmaster.pid in 9.1.  Are they manually going into the
postmaster.pdi file and grabbing the first line?

> Arguably, if push came to shove, pg_upgrade wouldn't really need to use
> pg_ctl either.

It would have to implement the 'wait' mode inside pg_upgrade, and in
other applications that needs 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


Re: [HACKERS] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Andrew Dunstan



On 10/03/2011 02:15 PM, Bruce Momjian wrote:

Andrew Dunstan wrote:


On 10/03/2011 12:54 PM, Tom Lane wrote:

I was never exactly thrilled with the separate-config-directory design
to start with, so I'm probably not the person to opine on whether we
could get away with removing it.



The horse has well and truly bolted. We'd have a major row if anyone
tried to remove it. Let's not rehash old battles. Our only option is to
make it work as best we can.

I disagree.  If people were using it we would have had many more bug
reports about pg_ctl not working.



No, that's an indication people aren't using pg_ctl, not that they 
aren't using separate config dirs.


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


Re: [HACKERS] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Tom Lane wrote:
> Bruce Momjian  writes:
> > Config-only directories seem to be only adding confusion.  All possible
> > solutions seem to be adding more code and user requirements, which the
> > creation of symlinks avoids.
> 
> > Is it time for me to ask on 'general' if removal of this feature is
> > warranted?
> 
> Well, the way we could fix it is to invent the parse-the-config-files
> option that was alluded to recently.  Then pg_ctl would continue to
> take the -D switch or PGDATA environment variable with the same meaning
> that the postmaster attaches to it, and would do something like
> 
>   postgres --print-config-value=data_directory -D $PGDATA
> 
> to extract the actual location of the data directory.

That works, assuming the server was not started with -o
'data_directory=/abc'.  The only workaround there would be to have
pg_ctl supply the -o, even on pg_ctl stop, and parse that in pg_ctl.

> Whether this is worth the trouble is highly debatable IMO.  One obvious
> risk factor for pg_ctl stop/restart is that the current contents of
> postgresql.conf might not match what they were when the postmaster was
> started.
> 
> I was never exactly thrilled with the separate-config-directory design
> to start with, so I'm probably not the person to opine on whether we
> could get away with removing it.

The entire thing seems logically broken, to the point where even if we
did get code working, few users would even understand it.

-- 
  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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Andrew Dunstan wrote:
> 
> 
> On 10/03/2011 12:54 PM, Tom Lane wrote:
> >
> > I was never exactly thrilled with the separate-config-directory design
> > to start with, so I'm probably not the person to opine on whether we
> > could get away with removing it.
> >
> > 
> 
> The horse has well and truly bolted. We'd have a major row if anyone 
> tried to remove it. Let's not rehash old battles. Our only option is to 
> make it work as best we can.

I disagree.  If people were using it we would have had many more bug
reports about pg_ctl not working.

-- 
  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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Andrew Dunstan



On 10/03/2011 12:54 PM, Tom Lane wrote:


I was never exactly thrilled with the separate-config-directory design
to start with, so I'm probably not the person to opine on whether we
could get away with removing it.




The horse has well and truly bolted. We'd have a major row if anyone 
tried to remove it. Let's not rehash old battles. Our only option is to 
make it work as best we can.


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


Re: [HACKERS] [v9.2] make_greater_string() does not return a string in some cases

2011-10-03 Thread Robert Haas
On Thu, Sep 29, 2011 at 6:24 AM, Kyotaro HORIGUCHI
 wrote:
> This is new version of make_greater_string patch.

According to the comments in the original source code, the purpose of
savelastchar is to avoid confusing pg_mbcliplen().  You've preserved
savelastchar only for the case where datatype == BYTEAOID, while
making it the increment function's job not to do anything permanent
unless it also returns true.  But it seems to me that if the datatype
is BYTEAOID then there's no need to restore anything at all, because
we're not going to call pg_mbcliplen() in that case anyway.  So I
think the logic here can be simplified.

Also, you haven't completely fixed the style issues.  Function
definitions should look like this:

static void
thingy()
{
}

Not like this:

static void thingy()
{
}

Opening curly braces should be on a line by themselves, not at the end
of the preceding if, while, etc. line.

"finnaly" is spelled incorrectly.

-- 
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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Dave Page
On Mon, Oct 3, 2011 at 7:07 PM, Peter Eisentraut  wrote:
> On mån, 2011-10-03 at 11:27 -0400, Bruce Momjian wrote:
>> Frankly, I am confused how this breakage has gone unreported for so
>> long.
>
> Well, nobody is required to use pg_ctl,

You are if you wish to run as a service on Windows.


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Peter Eisentraut
On mån, 2011-10-03 at 11:27 -0400, Bruce Momjian wrote:
> Frankly, I am confused how this breakage has gone unreported for so
> long.

Well, nobody is required to use pg_ctl, and for the longest time, it was
pg_ctl that was considered to be broken (for various other reasons) and
avoided in packaged init scripts.

Arguably, if push came to shove, pg_upgrade wouldn't really need to use
pg_ctl either.


-- 
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] Re: [COMMITTERS] pgsql: Force strings passed to and from plperl to be in UTF8 encoding.

2011-10-03 Thread Alex Hunsaker
On Mon, Oct 3, 2011 at 04:20, Amit Khandekar
 wrote:

> Is there a plan to commit this issue? I am still seeing this issue on
> PG 9.1 STABLE branch. Attached is a small patch that targets only the
> specific issue in the described testcase :
>
> create or replace function zerob() returns text as $$ return
> "abcd\0efg"; $$ language plperl;
> SELECT zerob();
>
> The patch does the perl data validation in the function utf_u2e() itself.

I think thats fine, but as coded it will verify the string twice in
the GetDatabaseEncoding() != PG_UTF8 case (once for
pg_do_encoding_conversion() and again with the added
pg_verify_mbstr_len), which seems a bit wasteful.

It might be worth adding a regression test also...

-- 
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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Tom Lane
Bruce Momjian  writes:
> Config-only directories seem to be only adding confusion.  All possible
> solutions seem to be adding more code and user requirements, which the
> creation of symlinks avoids.

> Is it time for me to ask on 'general' if removal of this feature is
> warranted?

Well, the way we could fix it is to invent the parse-the-config-files
option that was alluded to recently.  Then pg_ctl would continue to
take the -D switch or PGDATA environment variable with the same meaning
that the postmaster attaches to it, and would do something like

postgres --print-config-value=data_directory -D $PGDATA

to extract the actual location of the data directory.

Whether this is worth the trouble is highly debatable IMO.  One obvious
risk factor for pg_ctl stop/restart is that the current contents of
postgresql.conf might not match what they were when the postmaster was
started.

I was never exactly thrilled with the separate-config-directory design
to start with, so I'm probably not the person to opine on whether we
could get away with removing 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] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Robert Haas
On Mon, Oct 3, 2011 at 12:25 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Mon, Oct 3, 2011 at 11:16 AM, Tom Lane  wrote:
>>> Robert Haas  writes:
 Yeah.  custom_variable_classes is a pretty annoying wart, but if it's
 set to the default value (namely, empty) then it actually does prevent
 people from setting bajillions of completely pointless settings, which
 seems like it has some merit.
>
>>> Well, that argument was essentially why we put it in to begin with.
>>> But I think pretty much everybody agrees that it's more trouble than
>>> it's worth (in fact, weren't you one of the people complaining about
>>> it?)
>
>> Well, yes.  But I was arguing that we should replace the leaky dam
>> with one that's watertight, rather than demolishing the dam.
>
> If we had some idea how to do that, I'd probably agree.  But we don't.
> In any case, custom_variable_classes as currently defined is not the
> basis for a solution to that desire, and removing it won't create an
> impediment to solving the problem properly, should we come up with
> a solution.

Yeah, that's why I'm not complaining too loudly.  :-)

> (This is, however, a good reason for continuing to not document that
> you can create random GUC variables --- we might someday shut that
> off again.)

Or maybe better still would be to explicitly document the fact that
behavior in this area should not be relied upon.

-- 
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] Unexpected collation error in 9.1.1

2011-10-03 Thread Tom Lane
Christian Ullrich  writes:
> I tried adding a not-null column in one step and got a collation
> error for a different column.

> itd=> alter table livedata add column pricechanged timestamp not null default 
> current_timestamp;
> ERROR:  no collation was derived for column "whois_b" with collatable type 
> citext
> TIP:  Use the COLLATE clause to set the collation explicitly.

That's pretty bizarre, but I can't reproduce it on the basis of the
supplied example:

regression=# create extension citext;
CREATE EXTENSION
regression=# create table foo (f1 int, f2 citext  default '  
'::character varying);
CREATE TABLE
regression=# insert into foo values (1, 'one');
INSERT 0 1
regression=# insert into foo values (2, 'two');
INSERT 0 1
regression=# alter table foo add column pricechanged timestamp not null default 
current_timestamp;
ALTER TABLE

I tried adding UNIQUE and CHECK constraints too, and still no luck.
Are you sure you're using 9.1.1?

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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Alvaro Herrera wrote:
> 
> Excerpts from Tom Lane's message of lun oct 03 12:34:22 -0300 2011:
> > Bruce Momjian  writes:
> > > I am starting to question the value of config-only directories if pg_ctl
> > > stop doesn't work, or you have to specify a different directory for
> > > start and stop.
> > 
> > Yup.
> > 
> > > Did we not think of these things when we designed config-only
> > > directories?  I don't even see this problem mentioned in our
> > > documentation.
> > 
> > Yeah, we did.  The people who were lobbying for the feature didn't care,
> > or possibly thought that somebody would fix it for them later.
> 
> I think the main proponents are the Debian guys, and they don't use
> pg_ctl because they have their own pg_ctlcluster.

OK, so it is as messed up as I thought.

I am all fine for people lobbying for features, but not if they don't
work with our tools.  pg_upgrade is certainly not going to use the
Debian start/stop tools unless Debian patches pg_upgrade.

So someone thought we would eventually fix the tools?  I am unclear
exactly how to fix much of this.  Even documenting some workarounds
seems impossible, e.g. pg_ctl restart.

I can't see any feature config-only directories adds that can't be
accomplished by symlinks.  Even the ability to use a single
configuration file for multiple clusters can be done.

In summary, here is what I have found that works or is impossible with
config-only directories:

pg_ctl startspecify config directory
pg_ctl -w start impossible
pg_ctl restart  impossible
pg_ctl stop specify real data dir
pg_ctl -w stop  specify real data dir
pg_ctl reload   specify real data dir

Config-only directories seem to be only adding confusion.  All possible
solutions seem to be adding more code and user requirements, which the
creation of symlinks avoids.

Is it time for me to ask on 'general' if removal of this feature is
warranted?

-- 
  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] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Tom Lane
Robert Haas  writes:
> On Mon, Oct 3, 2011 at 11:16 AM, Tom Lane  wrote:
>> Robert Haas  writes:
>>> Yeah.  custom_variable_classes is a pretty annoying wart, but if it's
>>> set to the default value (namely, empty) then it actually does prevent
>>> people from setting bajillions of completely pointless settings, which
>>> seems like it has some merit.

>> Well, that argument was essentially why we put it in to begin with.
>> But I think pretty much everybody agrees that it's more trouble than
>> it's worth (in fact, weren't you one of the people complaining about
>> it?)

> Well, yes.  But I was arguing that we should replace the leaky dam
> with one that's watertight, rather than demolishing the dam.

If we had some idea how to do that, I'd probably agree.  But we don't.
In any case, custom_variable_classes as currently defined is not the
basis for a solution to that desire, and removing it won't create an
impediment to solving the problem properly, should we come up with
a solution.

(This is, however, a good reason for continuing to not document that
you can create random GUC variables --- we might someday shut that
off again.)

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] [v9.2] DROP statement reworks

2011-10-03 Thread Robert Haas
On Mon, Oct 3, 2011 at 11:28 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Mon, Oct 3, 2011 at 10:54 AM, Dimitri Fontaine
>>  wrote:
>>> Robert Haas  writes:
 I think that new versions of patch can handle unified diffs without a
 problem, but older versions choke on them.  My Mac has 2.5.8 and
 handles unidiffs no problem.
>
>>> Even containing git headers?
>
>> Yeah, it just skips right over them.  I've never had even a minor
>> problem on that account, which is why I was surprised to see it giving
>> you so much trouble.
>
> I haven't observed any such problems even with the rather ancient copy
> of GNU patch on my HPUX box (seems to be 2.5.4, released in 1999).
> I vaguely recall having had to replace the even older vendor-supplied
> patch because that one didn't do unidiffs ...

I have seen unified diffs blow up when using patch on a fairly new
HP-UX box, but I'm not sure whether I'm using an OS-supplied copy of
patch or something someone installed along the line somewhere.

-- 
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] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Robert Haas
On Mon, Oct 3, 2011 at 11:16 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> Yeah.  custom_variable_classes is a pretty annoying wart, but if it's
>> set to the default value (namely, empty) then it actually does prevent
>> people from setting bajillions of completely pointless settings, which
>> seems like it has some merit.  I'm not sure it has enough merit to
>> justify keeping it around, but it has more than none.  We could allow
>> entering a date of February 31st, too, but we don't.
>
> Well, that argument was essentially why we put it in to begin with.
> But I think pretty much everybody agrees that it's more trouble than
> it's worth (in fact, weren't you one of the people complaining about
> it?)

Well, yes.  But I was arguing that we should replace the leaky dam
with one that's watertight, rather than demolishing the dam.

-- 
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_dump issues

2011-10-03 Thread Tom Lane
Alvaro Herrera  writes:
> Excerpts from Tom Lane's message of lun oct 03 01:47:18 -0300 2011:
>> (Without cassert, it looks like LockReassignCurrentOwner is the next
>> biggest time sink; I'm wondering if there's some sort of O(N^2) behavior
>> in there.)

> That seems fishy.  Even if there weren't quadratic behavior, should this
> be called at all?  AFAIK it should only be used on cases where there are
> subtransactions at work, and I don't think pg_dump uses them.

I wondered that too, but the calls are legit --- they're coming from
PortalDrop.

It appears that most of the calls don't actually have anything to do,
but they're iterating through a rather large local lock table to find
that out.  We probably ought to think of a way to avoid that.  The trick
is to not make performance worse for typical small transactions that
aren't holding many locks (which I think was the design center for this
to begin with).

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] How can i get record by data block not by sql?

2011-10-03 Thread Kevin Grittner
Please don't cross-post.  Responding on -hackers because it seems a
better fit here than on -performance.

"姜头" <104186...@qq.com> wrote:
 
> How can i get record by data block not by sql?
> 
> I want to read and write lots of data by data blocks and write
> record to a appointed data block and read it. so i can form a
> disk-resident tree by recording the block address. But i don't
> know  how to implement in postgresql.
> Is there system function can do this?  
 
It's not really clear what you want to do, why you want parts of
PostgreSQL involved, or why you want to bypass other parts of
PostgreSQL.
 
That said, I think you might want to start reading source code and
README files.  You might find something in the contrib area which
could make a good example.  An FDW might be exactly what you want,
or the exact opposite of what you want -- I really can't tell from
the sketchy information provided.
 
-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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Alvaro Herrera

Excerpts from Tom Lane's message of lun oct 03 12:34:22 -0300 2011:
> Bruce Momjian  writes:
> > I am starting to question the value of config-only directories if pg_ctl
> > stop doesn't work, or you have to specify a different directory for
> > start and stop.
> 
> Yup.
> 
> > Did we not think of these things when we designed config-only
> > directories?  I don't even see this problem mentioned in our
> > documentation.
> 
> Yeah, we did.  The people who were lobbying for the feature didn't care,
> or possibly thought that somebody would fix it for them later.

I think the main proponents are the Debian guys, and they don't use
pg_ctl because they have their own pg_ctlcluster.

-- 
Á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] [v9.2] DROP statement reworks

2011-10-03 Thread Dimitri Fontaine
Robert Haas  writes:
> Yeah, it just skips right over them.  I've never had even a minor
> problem on that account, which is why I was surprised to see it giving
> you so much trouble.

Ok then, I'll try some more next time.  Been trying not to spend too
much time here…  on the other hand git apply was happy to apply it…

Sorry for the noise,

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] pg_dump issues

2011-10-03 Thread Andrew Dunstan



On 10/03/2011 12:47 AM, Tom Lane wrote:

Andrew Dunstan  writes:

While investigating a client problem I just observed that pg_dump takes
a surprisingly large amount of time to dump a schema with a large number
of views. The client's hardware is quite spiffy, and yet pg_dump is
taking many minutes to dump a schema with some 35,000 views. Here's a
simple test case:
 create schema views;
 do 'begin for i in 1 .. 1 loop execute $$create view views.v_$$
 || i ||$$ as select current_date as d, current_timestamp as ts,
 $_$a$_$::text || n as t, n from generate_series(1,5) as n$$; end
 loop; end;';
On my modest hardware this database took 4m18.864s for pg_dump to run.

It takes about that on my machine too ... with --enable-cassert.
oprofile said that 90% of the runtime was going into AllocSetCheck,
so I rebuilt without cassert, and the runtime dropped to 16 seconds.
What were you testing?


Yeah, you're right, that must have been it. That's a big hit, I didn't 
realise cassert was so heavy. (Note to self: test with production build 
settings). I don't seem to be batting 1000 ...


I still need to get to the bottom of why the client's machine is taking 
so long.


I do notice that we seem to be doing a lot of repetitive tasks, e.g. 
calling pg_format_type() over and over for the same arguments. Would we 
be better off cacheing that?


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


Re: [HACKERS] pg_dump issues

2011-10-03 Thread Alvaro Herrera

Excerpts from Tom Lane's message of lun oct 03 01:47:18 -0300 2011:

> (Without cassert, it looks like LockReassignCurrentOwner is the next
> biggest time sink; I'm wondering if there's some sort of O(N^2) behavior
> in there.)

That seems fishy.  Even if there weren't quadratic behavior, should this
be called at all?  AFAIK it should only be used on cases where there are
subtransactions at work, and I don't think pg_dump uses them.

-- 
Á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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Tom Lane
Bruce Momjian  writes:
> I am starting to question the value of config-only directories if pg_ctl
> stop doesn't work, or you have to specify a different directory for
> start and stop.

Yup.

> Did we not think of these things when we designed config-only
> directories?  I don't even see this problem mentioned in our
> documentation.

Yeah, we did.  The people who were lobbying for the feature didn't care,
or possibly thought that somebody would fix it for them later.

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] [v9.2] DROP statement reworks

2011-10-03 Thread Alvaro Herrera

Excerpts from Dimitri Fontaine's message of lun oct 03 11:54:36 -0300 2011:
> Robert Haas  writes:
> > I think that new versions of patch can handle unified diffs without a
> > problem, but older versions choke on them.  My Mac has 2.5.8 and
> > handles unidiffs no problem.
> 
> Even containing git headers?

I remember redirecting whole emails to "patch", and it worked just fine.
And this wasn't recently.

-- 
Á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] [v9.2] DROP statement reworks

2011-10-03 Thread Tom Lane
Robert Haas  writes:
> On Mon, Oct 3, 2011 at 10:54 AM, Dimitri Fontaine
>  wrote:
>> Robert Haas  writes:
>>> I think that new versions of patch can handle unified diffs without a
>>> problem, but older versions choke on them.  My Mac has 2.5.8 and
>>> handles unidiffs no problem.

>> Even containing git headers?

> Yeah, it just skips right over them.  I've never had even a minor
> problem on that account, which is why I was surprised to see it giving
> you so much trouble.

I haven't observed any such problems even with the rather ancient copy
of GNU patch on my HPUX box (seems to be 2.5.4, released in 1999).
I vaguely recall having had to replace the even older vendor-supplied
patch because that one didn't do unidiffs ...

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] Mismatch of relation names: pg_toast.pg_toast_nnn during pg_upgrade from 8.4 to 9.1

2011-10-03 Thread Bruce Momjian
Jamie Fox wrote:
> I regret that as a part-timer recently brought back on here I didn't
> get an opportunity to test this earlier.  The upgrade with the patch
> worked fine on my first attempt.

Great.  Thanks for the report, and sorry for the bug.

---


> 
> Thanks again,
> 
> Jamie
> 
> On Wed, Sep 28, 2011 at 7:32 PM, Bruce Momjian  wrote:
> > Jamie Fox wrote:
> >> Thanks, I'm following the thread "pg_upgrade automatic testing" and
> >> will try the patch just detailed there.
> >
> > I have applied the patch to head and 9.1.X. ?We still have a win32 bug
> > to fix. ?It is a shame I was not able to fix these before 9.1.1 was
> > released. ?:-(
> >
> > ---
> >
> >>
> >>
> >> On Wed, Sep 28, 2011 at 12:50 AM, Peter Eisentraut  wrote:
> >> > On tis, 2011-09-27 at 16:19 -0700, Jamie Fox wrote:
> >> >>
> >> >> It fails at this stage:
> >> >>
> >> >> ? ? Restoring user relation files
> >> >> ? ? linking /data/pgsql/prod-84/base/11564/2613 to
> >> >> /data/pgsql/prod-91/base/12698/12570
> >> >> ? ? linking /data/pgsql/prod-84/base/11564/2683 to
> >> >> /data/pgsql/prod-91/base/12698/12572
> >> >> ? ? Mismatch of relation names: database "prod1", old rel
> >> >> pg_toast.pg_toast_54542379, new rel pg_toast.pg_toast_16735
> >> >> ? ? Failure, exiting
> >> >
> >> > This issue is known and a fix is currently being discussed.
> >> >
> >> >
> >>
> >> --
> >> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> >> To make changes to your subscription:
> >> http://www.postgresql.org/mailpref/pgsql-hackers
> >
> > --
> > ?Bruce Momjian ? ? ? ? ?http://momjian.us
> > ?EnterpriseDB ? ? ? ? ? ? ? ? ? ? ? ? ? ? http://enterprisedb.com
> >
> > ?+ It's impossible for everything to be true. +
> >

-- 
  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] Bug with pg_ctl -w/wait and config-only directories

2011-10-03 Thread Bruce Momjian
Fujii Masao wrote:
> On Sun, Oct 2, 2011 at 7:54 AM, Bruce Momjian  wrote:
> > What exactly is your question? ?You are not using a config-only
> > directory but the real data directory, so it should work fine.
> 
> No. He is using PGDATA (= /etc/postgresql-9.0) as a config-only
> directory, and DATA_DIR (= /var/lib/postgresql/9.0/data) as a
> real data directory.

Wow, I see what you mean now!  So the user already figured out it was
broken and used the workaround I recently discovered?  Was this ever
reported to the community?  If so, I never saw it.

So, in testing, I see it is even more broken than I thought.  Not only
is pg_ctl -w broken for start/stop for config-only installs, but pg_ctl
stop (no -w) is also broken because it can't find the postmaster.pid
file to check or use to get the pid to send the signal.  pg_ctl reload
and restart are similarly broken.  :-(

And it gets worse.  The example supplied by the Gentoo developer shows a
use case where the data directory is not even specified in the
configuration file but rather on the command line:

su -l postgres \
-c "env PGPORT=\"${PGPORT}\" ${PG_EXTRA_ENV} \
/usr/lib/postgresql-9.0/bin/pg_ctl \
start ${WAIT_FOR_START} -t ${START_TIMEOUT} -s -D ${DATA_DIR} \
-o '-D ${PGDATA} --data-directory=${DATA_DIR} \
--silent-mode=true ${PGOPTS}'"

In this case, dumping the postgresql.conf file settings is not going to
help --- there is nothing in the config directory that is going to point
us to the data directory --- it exists only in the process arguments.

Frankly, I am confused how this breakage has gone unreported for so long.

Our current TODO item is:

Allow pg_ctl to work properly with configuration files located outside
the PGDATA directory

pg_ctl can not read the pid file because it isn't located in the
config directory but in the PGDATA directory. The solution is to allow
pg_ctl to read and understand postgresql.conf to find the data_directory
value.

BUG #5103: "pg_ctl -w (re)start" fails with custom
unix_socket_directory 

While this is accurate, it certainly is missing much of the breakage. 
Finding a non-standard socket directory is the least of our problems
with config-only directories (even standard settings don't work), and
reading the config file is not enough of a solution because of the
possible passing of parameters on the command line.

To add even more complexity, imagine someone using the same config
directory for several data/cluster directories, and just passing a
unique --data-directory for each one on start --- in that case,
specifying the config directory is not sufficiently unique to specify
which data directory.  It seems we would need some way to pass the data
directory to pg_ctl, perhaps via -o, but parsing that was something we
have tried to avoid (there may be no other choice), and it would have to
be supplied for start and stop.

The only conclusion I can come up with is that we need to be able to
dump postgresql.conf's data_directory, but also to read it from the
command line.

I am starting to question the value of config-only directories if pg_ctl
stop doesn't work, or you have to specify a different directory for
start and stop.  Writing a second postmaster.pid file into the config
directory would help, but it would break with shared-config setups and I
don't think we can assume we have write permission on the config
directory.

What are config-only directories buying us that we can't get from
telling users to use symlinks and point to the data directory directly?

Did we not think of these things when we designed config-only
directories?  I don't even see this problem mentioned in our
documentation.

-- 
  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_dump issues

2011-10-03 Thread Tom Lane
Robert Haas  writes:
> On Sat, Oct 1, 2011 at 9:46 PM, Andrew Dunstan  wrote:
>> How would that help? This isn't a lock failure.

> It is, rather, a failure to lock.  Currently, LOCK TABLE only works on
> tables, and pg_dump only applies it to tables.  If the offending
> object had been a table rather than a view, pg_dump would (I believe)
> have blocked trying to obtain an AccessShareLock against the existing
> AccessExclusiveLock.

Yeah, and it would still have failed once the lock was released.

Short of providing some sort of global DDL-blocking lock (with attendant
performance consequences) it's not clear how to create an entirely
bulletproof solution here.  This isn't a new problem --- we've been
aware of pg_dump's limitations in this area for many years.

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] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Tom Lane
Robert Haas  writes:
> Yeah.  custom_variable_classes is a pretty annoying wart, but if it's
> set to the default value (namely, empty) then it actually does prevent
> people from setting bajillions of completely pointless settings, which
> seems like it has some merit.  I'm not sure it has enough merit to
> justify keeping it around, but it has more than none.  We could allow
> entering a date of February 31st, too, but we don't.

Well, that argument was essentially why we put it in to begin with.
But I think pretty much everybody agrees that it's more trouble than
it's worth (in fact, weren't you one of the people complaining about
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] [v9.2] DROP statement reworks

2011-10-03 Thread Robert Haas
On Mon, Oct 3, 2011 at 10:54 AM, Dimitri Fontaine
 wrote:
> Robert Haas  writes:
>> I think that new versions of patch can handle unified diffs without a
>> problem, but older versions choke on them.  My Mac has 2.5.8 and
>> handles unidiffs no problem.
>
> Even containing git headers?

Yeah, it just skips right over them.  I've never had even a minor
problem on that account, which is why I was surprised to see it giving
you so much trouble.

-- 
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] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Robert Haas
On Mon, Oct 3, 2011 at 10:55 AM, David Fetter  wrote:
> Perhaps it's best to document this usage and include the warning for
> those less "bright," as you term them.   I'd be less tempted to call
> them "not bright" and more tempted to think they might assume
> PostgreSQL already takes care of cleaning this up, but whatever.

Yeah.  custom_variable_classes is a pretty annoying wart, but if it's
set to the default value (namely, empty) then it actually does prevent
people from setting bajillions of completely pointless settings, which
seems like it has some merit.  I'm not sure it has enough merit to
justify keeping it around, but it has more than none.  We could allow
entering a date of February 31st, too, but we don't.

-- 
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_dump issues

2011-10-03 Thread Robert Haas
On Sat, Oct 1, 2011 at 9:46 PM, Andrew Dunstan  wrote:
> How would that help? This isn't a lock failure.

It is, rather, a failure to lock.  Currently, LOCK TABLE only works on
tables, and pg_dump only applies it to tables.  If the offending
object had been a table rather than a view, pg_dump would (I believe)
have blocked trying to obtain an AccessShareLock against the existing
AccessExclusiveLock.  We talked about allowing locks on other types of
relations, but due to some bad syntax choices in the past it's not
completely obvious how to shoehorn that in.

http://archives.postgresql.org/pgsql-hackers/2011-06/msg00119.php

-- 
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] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread David Fetter
On Mon, Oct 03, 2011 at 10:41:48AM -0400, Tom Lane wrote:
> Andrew Dunstan  writes:
> > On 10/03/2011 10:17 AM, Tom Lane wrote:
> >> Right.  Getting rid of custom_variable_classes should actually
> >> make those use-cases easier, since it will eliminate a required
> >> setup step.
> 
> > So are we going to sanction using this as a poor man's session
> > variable mechanism?
> 
> People already are doing that, sanctioned or not.
> 
> > If so maybe we should at least warn that anything set will be
> > accessible by all roles, so security definer functions for example
> > should be wary of trusting such values.
> 
> Since it's not documented anywhere, I'm not sure where we'd put such
> a warning.  I think anyone bright enough to think of such a hack
> should be able to see the potential downsides, anyway.

Perhaps it's best to document this usage and include the warning for
those less "bright," as you term them.   I'd be less tempted to call
them "not bright" and more tempted to think they might assume
PostgreSQL already takes care of cleaning this up, but whatever.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

-- 
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] [v9.2] DROP statement reworks

2011-10-03 Thread Dimitri Fontaine
Robert Haas  writes:
> I think that new versions of patch can handle unified diffs without a
> problem, but older versions choke on them.  My Mac has 2.5.8 and
> handles unidiffs no problem.

Even containing git headers?

Here's what I'm talking about here:

 src/backend/catalog/objectaddress.c |  653 ++-
 src/include/catalog/objectaddress.h |   13 +
 src/include/nodes/parsenodes.h  |2 +-
 3 files changed, 575 insertions(+), 93 deletions(-)

diff --git a/src/backend/catalog/objectaddress.c 
b/src/backend/catalog/objectaddress.c
index 8feb601..6094146 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -82,6 +82,463 @@ static ObjectAddress get_object_address_opcf(ObjectType 
objtype, List *objname,
List *objargs, bool missing_ok);
 static bool object_exists(ObjectAddress address);


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] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Tom Lane
Andrew Dunstan  writes:
> On 10/03/2011 10:17 AM, Tom Lane wrote:
>> Right.  Getting rid of custom_variable_classes should actually make
>> those use-cases easier, since it will eliminate a required setup step.

> So are we going to sanction using this as a poor man's session variable 
> mechanism?

People already are doing that, sanctioned or not.

> If so maybe we should at least warn that anything set will be accessible 
> by all roles, so security definer functions for example should be wary 
> of trusting such values.

Since it's not documented anywhere, I'm not sure where we'd put such
a warning.  I think anyone bright enough to think of such a hack should
be able to see the potential downsides, anyway.

regards, tom lane

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


Re: [HACKERS] SPI_processed is not set for COPY statement

2011-10-03 Thread Andrew Dunstan



On 10/03/2011 10:34 AM, Pavel Stehule wrote:

Hello

is there some possibility to get a processed rows from COPY statement
from PL/pgSQL?

I searched any ways, but there are no command tag.




You mean something like a RETURNING clause?

My worry would be about possible speed effects, although that would 
probably be slight when it's not used.


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] SPI_processed is not set for COPY statement

2011-10-03 Thread Pavel Stehule
Hello

is there some possibility to get a processed rows from COPY statement
from PL/pgSQL?

I searched any ways, but there are no command tag.

Regards

Pavel Stehule

-- 
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] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Andrew Dunstan



On 10/03/2011 10:17 AM, Tom Lane wrote:

Magnus Hagander  writes:

Don't forget that there are usecases for variables under
custom_variable_classes that aren't actually associated with
extensions (as general session-shared-variables). Though I guess if it
was somehow restricted to extensions, those who needed that could just
rewrap all their code as extensions - though that would make it less
convenient.

Right.  Getting rid of custom_variable_classes should actually make
those use-cases easier, since it will eliminate a required setup step.

I tried to think of a security argument for keeping the setting, but
couldn't really.  Yeah, not having it will let people clutter their
individual backend's GUC array with lots of useless stuff, but so what?
There's plenty of other ways to run your session out of memory if you're
so inclined.





So are we going to sanction using this as a poor man's session variable 
mechanism?


If so maybe we should at least warn that anything set will be accessible 
by all roles, so security definer functions for example should be wary 
of trusting such values.


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


Re: [HACKERS] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Tom Lane
Magnus Hagander  writes:
> Don't forget that there are usecases for variables under
> custom_variable_classes that aren't actually associated with
> extensions (as general session-shared-variables). Though I guess if it
> was somehow restricted to extensions, those who needed that could just
> rewrap all their code as extensions - though that would make it less
> convenient.

Right.  Getting rid of custom_variable_classes should actually make
those use-cases easier, since it will eliminate a required setup step.

I tried to think of a security argument for keeping the setting, but
couldn't really.  Yeah, not having it will let people clutter their
individual backend's GUC array with lots of useless stuff, but so what?
There's plenty of other ways to run your session out of memory if you're
so inclined.

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


[HACKERS] How can i get record by data block not by sql?

2011-10-03 Thread 姜头
How can i get record by data block not by sql?
 I want to read and write lots of data by data blocks and write a record to a 
appointed data blocks,so i can form a disk-resident tree by recording the block 
address. But i don't know  how to implement in postgresql. 
 Is there system function can do this?  
 Can someone help me?? Thank you very very much1

Re: [HACKERS] [v9.2] DROP statement reworks

2011-10-03 Thread Robert Haas
On Sun, Oct 2, 2011 at 4:08 PM, Dimitri Fontaine  wrote:
> Ok I needed `git apply' to apply the patches, now that I used that I can
> confirm that the 3 patches apply, compile, pass tests, and that I could
> play with them a little.  I think I'm going to mark that ready for
> commiter.  I don't have enough time for a more deep review but at the
> same time patch reading and testing both passed :)
>
> You might need to post a version that patch will be happy with, though,
> using e.g.
>
>  git diff |filterdiff --format=context > /tmp/foo.patch
>
> Then diffstat reports:
>  35 files changed, 932 insertions(+), 1913 deletions(-), 345 modifications(!)

I think that new versions of patch can handle unified diffs without a
problem, but older versions choke on them.  My Mac has 2.5.8 and
handles unidiffs no problem.

-- 
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] [PATCH] Unremovable tuple monitoring (was: Addition of some trivial auto vacuum logging)

2011-10-03 Thread Royce Ausburn
On 28/09/2011, at 11:17 AM, Tom Lane wrote:

> Alvaro Herrera  writes:
>> Excerpts from Royce Ausburn's message of mar sep 27 21:28:26 -0300 2011:
>>> Tom's suggestion looks like it's less trivial that I can do just yet, but 
>>> I'll take a look and ask for help if I need it.
> 
>> It's not that difficult.  Just do a search on "git log
>> src/backend/postmaster/pgstat.c" for patches that add a new column
>> somewhere.
> 
> Yeah, I was just about to say the same thing.  Find a previous patch
> that adds a feature similar to what you have in mind, and crib like mad.
> We've added enough stats counters over time that you should have several
> models to work from.


This patch does as Tom suggested, adding a column to the pg_stat_all_tables 
view which exposes the number of unremovable tuples in the last vacuum.  This 
patch does not include my previous work to log this information as part of 
auto_vacuum's logging.

User visible additions:
New column pg_stat_all_tables.n_unremovable_tup
New function bigint pg_stat_get_unremovable_tuples(oid)

A few notes / questions:

- I'm not sure if I'm supposed to update CATALOG_VERSION_NO in catalog.h.  In 
this patch I have.

- I'm not sure about how I should be selecting an OID for my new stats 
function.  I used the unused_oids script and picked one that seemed reasonable.

- The VACUUM FULL implementation in cluster.c doesn't do any stats updating 
similar to vacuumlazy.c, so I haven't don't anything there… (is this right?  A 
vacuum full may also encounter unremovable tuples, right?)

- I'm not usually a C developer, so peeps reviewing please watch for noob 
mistakes.

Cheers,

--Royce



diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index a19e3f0..af7b235 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -328,7 +328,8 @@ postgres: user database 
host FULL vacuumed manually,
   the last time it was vacuumed by the autovacuum daemon,
   the last time it was analyzed manually,
@@ -764,6 +765,14 @@ postgres: user database 
host 
  
+ 
+ 
+  
pg_stat_get_unremovable_tuples(oid)
+  bigint
+  
+   Number of dead rows not removed in the table's last vacuum
+  
+ 
 
  
   
pg_stat_get_blocks_fetched(oid)
diff --git a/src/backend/catalog/system_views.sql 
b/src/backend/catalog/system_views.sql
index 2253ca8..9c18dc7 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -353,6 +353,7 @@ CREATE VIEW pg_stat_all_tables AS
 pg_stat_get_tuples_hot_updated(C.oid) AS n_tup_hot_upd,
 pg_stat_get_live_tuples(C.oid) AS n_live_tup,
 pg_stat_get_dead_tuples(C.oid) AS n_dead_tup,
+pg_stat_get_unremovable_tuples(C.oid) AS n_unremovable_tup,
 pg_stat_get_last_vacuum_time(C.oid) as last_vacuum,
 pg_stat_get_last_autovacuum_time(C.oid) as last_autovacuum,
 pg_stat_get_last_analyze_time(C.oid) as last_analyze,
diff --git a/src/backend/commands/vacuumlazy.c 
b/src/backend/commands/vacuumlazy.c
index cf8337b..140fe92 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -91,6 +91,7 @@ typedef struct LVRelStats
double  scanned_tuples; /* counts only tuples on scanned pages 
*/
double  old_rel_tuples; /* previous value of pg_class.reltuples 
*/
double  new_rel_tuples; /* new estimated total # of tuples */
+   double  unremovable_tuples; /* count of dead tuples not yet 
removable */
BlockNumber pages_removed;
double  tuples_deleted;
BlockNumber nonempty_pages; /* actually, last nonempty page + 1 */
@@ -245,7 +246,8 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
/* report results to the stats collector, too */
pgstat_report_vacuum(RelationGetRelid(onerel),
 onerel->rd_rel->relisshared,
-new_rel_tuples);
+new_rel_tuples,
+
vacrelstats->unremovable_tuples);
 
/* and log the action if appropriate */
if (IsAutoVacuumWorkerProcess() && Log_autovacuum_min_duration >= 0)
@@ -829,6 +831,7 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
/* save stats for use later */
vacrelstats->scanned_tuples = num_tuples;
vacrelstats->tuples_deleted = tups_vacuumed;
+   vacrelstats->unremovable_tuples = nkeep;
 
/* now we can compute the new value for pg_class.reltuples */
vacrelstats->new_rel_tuples = vac_estimate_reltuples(onerel, false,
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 9132db7..40d1107 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -1251,7 +1251,7 @@ pgstat_report_autovac(Oid dboid)
  * 

[HACKERS] Unexpected collation error in 9.1.1

2011-10-03 Thread Christian Ullrich

I tried adding a not-null column in one step and got a collation
error for a different column. Adding the column in several steps
works:


itd=> alter table livedata add column pricechanged timestamp not null default 
current_timestamp;
ERROR:  no collation was derived for column "whois_b" with collatable type 
citext
TIP:  Use the COLLATE clause to set the collation explicitly.

itd=> \d livedata
   Table "public.livedata"
Column|Type |Modifiers
--+-+-
 accessid | integer | not null
 sp_b | double precision| default 0
 csh_b| double precision| default 0
 sp_a | double precision| default 0
 csh_a| double precision| default 0
 asw_b| double precision| default 0
 asw_a| double precision| default 0
 amount   | character varying(25)   | default ' '::character varying
 bench| character varying(25)   | default NULL::character varying
 updated_b| date|
 updated_a| date|
 whois_b  | citext  | default '  '::character 
varying
 whois_a  | citext  | default '  '::character 
varying
 b_orig   | double precision| default 0
 a_orig   | double precision| default 0
 lcontrol | integer | not null default 0
 rcontrol | integer | not null default 0
 hlcleared| boolean | not null default false
 yield_b  | double precision| default 0
 yield_a  | double precision| default 0

itd=> alter table livedata add column pricechanged timestamp;
ALTER TABLE
itd=> alter table livedata alter column pricechanged set default 
current_timestamp;
ALTER TABLE
itd=> update livedata set pricechanged = default;
UPDATE 6000
itd=> alter table livedata alter column pricechanged set not null;
ALTER TABLE

--
Christian


--
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] Tracking latest timeline in standby mode

2011-10-03 Thread senthilnathan
Whether this feature is available in version 9.1.0. ??


--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/Tracking-latest-timeline-in-standby-mode-tp3238829p4863900.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Re: [COMMITTERS] pgsql: Force strings passed to and from plperl to be in UTF8 encoding.

2011-10-03 Thread Amit Khandekar
On 12 February 2011 14:48, Alex Hunsaker  wrote:
> On Sun, Feb 6, 2011 at 15:31, Andrew Dunstan  wrote:
>> Force strings passed to and from plperl to be in UTF8 encoding.
>>
>> String are converted to UTF8 on the way into perl and to the
>> database encoding on the way back. This avoids a number of
>> observed anomalies, and ensures Perl a consistent view of the
>> world.
>
> So I noticed a problem while playing with this in my discussion with
> David Wheeler. pg_do_encoding() does nothing when the src encoding ==
> the dest encoding. That means on a UTF-8 database we fail make sure
> our strings are valid utf8.
>
> An easy way to see this is to embed a null in the middle of a string:
> => create or replace function zerob() returns text as $$ return
> "abcd\0efg"; $$ language plperl;
> => SELECT zerob();
> abcd
>
> Also It seems bogus to bogus to do any encoding conversion when we are
> SQL_ASCII, and its really trivial to fix.
>
> With the attached:
> - when we are on a utf8 database make sure to verify our output string
> in sv2cstr (we assume database strings coming in are already valid)
>
> - Do no string conversion when we are SQL_ASCII in or out
>
> - add plperl_helpers.h as a dep to plperl.o in our makefile
>
> - remove some redundant calls to pg_verify_mbstr()
>
> - as utf_e2u only as one caller dont pstrdup() instead have the caller
> check (saves some cycles and memory)
>

Is there a plan to commit this issue? I am still seeing this issue on
PG 9.1 STABLE branch. Attached is a small patch that targets only the
specific issue in the described testcase :

create or replace function zerob() returns text as $$ return
"abcd\0efg"; $$ language plperl;
SELECT zerob();

The patch does the perl data validation in the function utf_u2e() itself.

>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>
diff --git a/src/pl/plperl/plperl_helpers.h b/src/pl/plperl/plperl_helpers.h
index 81c177b..3afe2f5 100644
--- a/src/pl/plperl/plperl_helpers.h
+++ b/src/pl/plperl/plperl_helpers.h
@@ -10,7 +10,10 @@ utf_u2e(const char *utf8_str, size_t len)
 	char	   *ret = (char *) pg_do_encoding_conversion((unsigned char *) utf8_str, len, PG_UTF8, GetDatabaseEncoding());
 
 	if (ret == utf8_str)
+	{
+		pg_verify_mbstr_len(PG_UTF8, utf8_str, len, false);
 		ret = pstrdup(ret);
+	}
 	return ret;
 }
 

-- 
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] pg_last_xact_insert_timestamp

2011-10-03 Thread Fujii Masao
On Fri, Sep 30, 2011 at 4:18 PM, Simon Riggs  wrote:
> If we want to measure times, we can easily send regular messages into
> WAL to provide this function. Using checkpoint records would seem
> frequent enough to me. We don't always send checkpoint records but we
> can send an info message instead if we are streaming. If
> archive_timeout is not set and max_wal_senders > 0 then we can send an
> info WAL message with timestamp on a regular cycle.

What timestamp are you thinking the walsender should send? What we need
is the timestamp which is comparable with the result of
pg_last_xact_replay_timestamp() which returns the timestamp of the
transaction commit or abort record. So, even if we adopt your proposal,
ISTM that we still need to collect the timestamp for each commit. No?

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] [REVIEW] pg_last_xact_insert_timestamp

2011-10-03 Thread Fujii Masao
On Sun, Oct 2, 2011 at 8:19 PM, Robert Haas  wrote:
> It occurs to me that pgstat_report_xact_end_timestamp doesn't really
> need to follow the protocol of bumping the change count before and
> after bumping the timestamp.  We elsewhere assume that four-byte reads
> and writes are atomic, so there's no harm in assuming the same thing
> here (and if they're not... then the change-count thing is pretty
> dubious anyway).  I think it's sufficient to just set the value, full
> stop.

I agree with Tom here. It seems to be safer to follow the protocol even if
that's not required for now.

> Also, in pg_last_xact_insert_timestamp, the errhint() seems a little
> strange - this is not exactly a WAL *control* function, is it?

Not only "control" but also "WAL" might be confusing. What about
"transaction information functions"?

BTW, pg_current_xlog_location() and pg_current_xlog_insert_location()
use the same HINT message as I used for pg_last_xact_insert_timestamp(),
but they are also not WAL *control* function. And, in the document,
they are categorized as "Backup Control Functions", but which sounds also
strange. We should call them "WAL information functions" in both
HINT message and the document?

> In the documentation, for the short description of
> pg_last_xact_insert_timestamp(), how about something like "returns the
> time at which a transaction commit or transaction about record was
> last inserted into the transaction log"?  Or maybe that's too long.
> But the current description doesn't seem to do much other than
> recapitulate the function name, so I'm wondering if we can do any
> better than that.

Agreed. I will change the description per your suggestion.

> I think that instead of hacking up the backend-status copying code to
> have a mode where it copies everything, you should just have a
> special-purpose function that computes the value you need directly off
> the backend status entries themselves.  This approach seems like it
> both clutters the code and adds lots of extra data copying.

Agreed. Will change.

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_dump issues

2011-10-03 Thread Florian Pflug
On Oct2, 2011, at 23:15 , Joe Abbate wrote:
> I'm
> somewhat surprised there appears to be no ability to lock a database
> exclusively for something like pg_dump/pg_restore, so you're not
> surprised by concurrent activity against the catalogs.  I'm guessing the
> assumption is that MVCC will take care of that?

I think the hope is more that it will, one day. Currently, postgres internally
accesses the catalog with SnapshotNow, not with a MVCC snapshot. This is
necessary to ensure, for example, that rows inserted into a table also get
inserted into a newly created index. This wouldn't affects pg_dump if it only
access the catalog via SQL, but it doesn't. pg_dump also depends on some 
server-side
functions to do its work, and since these functions in turn use 
SnapshotNow-based
internal backend functions, pg_dump essentially uses a mix of SnapshotNow and
its transaction's MVCC snapshot.

There has been talk about reducing the use of of SnapshotNow for catalog access,
but AFAIK there's no concrete proposal, and certainly no patch, available.

best regards,
Florian Pflug


-- 
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] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Magnus Hagander
On Sun, Oct 2, 2011 at 23:05, Tom Lane  wrote:
> During the discussion of Alexey Klyukin's rewrite of ParseConfigFile,
> considerable unhappiness was expressed by various people about the
> complexity and relative uselessness of the custom_variable_classes GUC.
> While working over his patch just now, I've come around to the side that
> was saying that this variable isn't worth its keep.  We don't have any
> way to validate whether the second part of a qualified GUC name is
> correct, if its associated extension module isn't loaded, so how much
> point is there in validating the first part?  And the variable is
> certainly a pain in the rear both to DBAs and to the GUC code itself.

Don't forget that there are usecases for variables under
custom_variable_classes that aren't actually associated with
extensions (as general session-shared-variables). Though I guess if it
was somehow restricted to extensions, those who needed that could just
rewrap all their code as extensions - though that would make it less
convenient.

The point being that even if you *could* validate them somehow against
a static list, requiring that might not be a good idea.


> So at this point I'd vote for just dropping it and always allowing
> custom (that is, qualified) GUC names to be set, whether the prefix
> corresponds to any loaded module or not.

Seems reasonable to me.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] [v9.2] Object access hooks with arguments support (v1)

2011-10-03 Thread Kohei KaiGai
BTW, I remember that I was suggested the object-access-hooks to acquire
controls around changes of system catalogs are also useful to implement
clustering features, not only enhanced security features, when I had a talk
at PGcon2001.

It might be my mistake that I categorized this patch at the "security" topic.
If someone volunteers to review this patch from the different viewpoint, not
only security features, it is quite helpful for us.

Thanks,

2011/9/29 Kohei KaiGai :
> I noticed that the previous revision does not provide any way to inform
> the modules name of foreign server, even if foreign table was created,
> on the OAT_POST_CREATE hook.
> So, I modified the invocation at heap_create_with_catalog to deliver
> this information to the modules.
>
> Rest of parts were unchanged, except for rebasing to the latest git
> master.
>
> 2011/8/28 Kohei KaiGai :
>> The attached patch is a draft to support arguments in addition to
>> OAT_* enum and object identifiers.
>>
>> The existing object_access_hook enables loadable modules to acquire
>> control when objects are referenced. The first guest of this hook is
>> contrib/sepgsql for assignment of default security label on newly
>> created objects. Right now, OAT_POST_CREATE is the all supported
>> object access type. However, we plan to enhance this mechanism onto
>> other widespread purpose; such as comprehensive DDL permissions
>> supported by loadable modules.
>>
>> This patch is a groundwork to utilize this hook for object creation
>> permission checks, not only default labeling.
>> At the v9.1 development cycle, I proposed an idea that defines both
>> OAT_CREATE hook prior to system catalog modification and
>> OAT_POST_CREATE hook as currently we have. This design enables to
>> check permission next to the existing pg_xxx_aclcheck() or
>> pg_xxx_ownercheck(), and raise an error before system catalog updates.
>> However, it was painful to deliver private datum set on OAT_CREATE to
>> the OAT_POST_CREATE due to the code complexity.
>>
>> The other idea tried to do all the necessary things in OAT_POST_CREATE
>> hook, and it had been merged, because loadable modules can pull
>> properties of the new object from system catalogs by the supplied
>> object identifiers. Thus, contrib/sepgsql assigns a default security
>> label on new object using OAT_POST_CREATE hook.
>> However, I have two concern on the existing hook to implement
>> permission check for object creation. The first one is the entry of
>> system catalog is not visible using SnaphotNow, so we had to open and
>> scan system catalog again, instead of syscache mechanism. The second
>> one is more significant. A part of information to make access control
>> decision is not available within contents of the system catalog
>> entries. For example, we hope to skip permission check when
>> heap_create_with_catalog() was launched by make_new_heap() because the
>> new relation is just used to swap later.
>>
>> Thus, I'd like to propose to support argument of object_access_hook to
>> inform the loadable modules additional contextual information on its
>> invocations; to solve these concerns.
>>
>> Regarding to the first concern, fortunately, most of existing
>> OAT_POST_CREATE hook is deployed just after insert or update of system
>> catalogs, but before CCI. So, it is helpful for the loadable modules
>> to deliver Form_pg_ data to reference properties of the new
>> object, instead of open and scan the catalog again.
>> In the draft patch, I enhanced OAT_POST_CREATE hook commonly to take
>> an argument that points to the Form_pg_ data of the new object.
>>
>> Regarding to the second concern, I added a few contextual information
>> as second or later arguments in a part of object classes. Right now, I
>> hope the following contextual information shall be provided to
>> OAT_POST_CREATE hook to implement permission checks of object
>> creation.
>>
>> * pg_class - TupleDesc structure of the new relation
>> I want to reference of pg_attribute, not only pg_class.
>>
>> * pg_class - A flag to show whether the relation is defined for
>> rebuilding, or not.
>> I want not to apply permission check in the case when it is invoked
>> from make_new_heap(), because it just create a dummy table as a part
>> of internal process. All the necessary permission checks should be
>> done at ALTER TABLE or CLUSTER.
>>
> And, name of the foreign server being used by the foreign table
> being created just a moment before.
>
>> * pg_class - A flag to show whether the relation is created with
>> SELECT INTO, or not.
>> I want to check "insert" permission of the new table, created by
>> SELECT INTO, because DML hook is not available to check this case.
>>
>> * pg_type - A flag to show whether the type is defined as implicit
>> array, or not.
>> I want not to apply permission check on creation of implicit array type.
>>
>> * pg_database - Oid of the source (template) database.
>> I want to fetch security label of the source databas

Re: [HACKERS] Should we get rid of custom_variable_classes altogether?

2011-10-03 Thread Dimitri Fontaine
Tom Lane  writes:
> Simon Riggs  writes:
>> On Sun, Oct 2, 2011 at 10:05 PM, Tom Lane  wrote:
>>> So at this point I'd vote for just dropping it and always allowing
>>> custom (that is, qualified) GUC names to be set, whether the prefix
>>> corresponds to any loaded module or not.
>
>> Sounds sensible. One less thing to configure is a good thing.
>
> Attached is a draft patch for that.

I fiddled with custom_variable_classes for the extension's patch, the
idea was to be able to append to it from the control file.  Removing it
entirely makes it even simpler.

I think we should load any qualified entry in the control file as a
custom GUC, or allow a new extension.conf file to be given containing
the default values.

> While working on this I got annoyed at our cheesy handling of the
> situation where a "placeholder" value has to be turned into a real
> setting, which happens when the corresponding extension gets loaded.
> There are several issues:
>
> 1. If it's a SUSET variable, a preceding attempt to set the value via
> SET will fail even if you're a superuser, for example
>
> regression=# set plpgsql.variable_conflict = use_variable;
> SET
> regression=# load 'plpgsql';
> ERROR:  permission denied to set parameter "plpgsql.variable_conflict"
>
> The reason for that is that define_custom_variable doesn't know whether
> the pre-existing value was set by a superuser, so it must assume the
> worst.  Seems like we could easily fix that by having set_config_option
> set a flag in the GUC variable noting whether a SET was done by a
> superuser or not.

I managed to do that by having another specific GUC array so that I
could call the GUC validation code (assign hooks) at module loading
time.  I guess a new flag would provide same capabilities.

> 2. If you do get an error while re-assigning the pre-existing value of
> the variable, it's thrown as an ERROR.  This is really pretty nasty
> because it'll abort execution of the extension module's init function;
> for example, a likely consequence is that other custom variables of
> the module don't set set up correctly, and it could easily be a lot
> worse if there are other things the init function hasn't done yet.
>
> I think we need to arrange that set_config_option only reports failures
> to apply such values as WARNING, not ERROR.  There isn't anything in its
> present API that could be used for that, but perhaps we could add a new
> enum variant for "action" that commands it.

I think this behavior only makes sense when we had a previous default
value before loading the module (set in the main postgresql.conf file),
and that we should still ERROR out otherwise (default provided by the
extension's code itself).  Or maybe I'm confused now.

> 3. We aren't very careful about preserving the reset value of the
> variable, in case it's different from the active value (which could
> happen if the user did a SET and there's also a value from the
> postgresql.conf file).
>
> This seems like it just requires working a bit harder in
> define_custom_variable, to reapply the reset value as well as the
> current value of the variable.
>
> Any objections to fixing that stuff, while I'm looking at it?

Please do :)

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] WIP: Join push-down for foreign tables

2011-10-03 Thread Kohei KaiGai
Hanada-san,

I applied your patch and run a few test cases. while this test, I
noticed a few points.

At first, I tried to use file_fdw, however, it was crashed of course.
It seems to me this logic should be modified to confirm whether the target FDW
support join push down, or not.

+   if (enable_foreignjoin &&
+   joinrel->serverid != InvalidOid &&
+   (IsA(outerpath, ForeignPath) || IsA(outerpath, ForeignJoinPath)) &&
+   (IsA(inner_cheapest_total, ForeignPath) ||
+IsA(inner_cheapest_total, ForeignJoinPath)))
+
+   {
+   ForeignJoinPath*path;
+   path = create_foreignjoin_path(root,
+  joinrel,
+  jointype,
+  sjinfo,
+  outerpath,
+  inner_cheapest_total,
+  restrictlist,
+  merge_pathkeys);
+   if (path != NULL)
+   add_path(joinrel, (Path *) path);
+   }
+

In my opinion, FdwRoutine should have an additional API to inform the core its
supported features; such as inner-join, outer-join, order-by,
group-by, aggregate
functions, insert, update, delete, etc... in the future version.

Obviously, it is not hard to implement inner/outer-join feature for
pgsql_fdw module,
but it may be a tough work for memcached_fdw module.

> *) SELECT * FROM A JOIN B (...) doesn't work.  Specifying columns in
> SELECT clause explicitly like "SELECT A.col1, A.col2, ..." seems to work.
> *) ORDER BY causes error if no column is specified in SELECT clause from
> sort key's table.
>
I doubt these issues are in pgsql_fdw side, not the proposed patch itself.

In the case when the table and column names/types are compatible between
local-side and remote-side, the problem was not reproduced in my environment.
I'd like to suggest you to add a functionality to map remote column names to
the local ones in pgsql_fdw.

See below:

* I set up three foreign tables on the local side.
CREATE FOREIGN TABLE ft_1 (a int, b text) SERVER local_db;
CREATE FOREIGN TABLE ft_2 (x int, y text) SERVER local_db;
CREATE FOREIGN TABLE ft_3 (s int, t text) SERVER local_db;

* I also set up related tables on the remote side.
CREATE TABLE ft_1 (a int, b text);
CREATE TABLE ft_2 (x int, y text);
CREATE TABLE ft_3 (ss int, tt text);
Please note that column name of ft_3 is not compatible

* JOIN ft_1 and ft_2 works collectly.
postgres=# SELECT * FROM ft_1 JOIN ft_2 ON a = x;
 a |  b  | x |  y
---+-+---+-
 2 | bbb | 2 | bbb
 3 | ccc | 3 | ccc
 4 | ddd | 4 | ddd
(3 rows)

postgres=# EXPLAIN SELECT * FROM ft_1 JOIN ft_2 ON a = x;
 QUERY PLAN
-
 Foreign Scan on multiple foreign tables  (cost=0.00..0.00 rows=5000 width=72)
   Remote SQL: SELECT ft_1.a, ft_1.b, ft_2.x, ft_2.y FROM public.ft_1
ft_1, public.ft_2 ft_2 WHERE (ft_1.a = ft_2.x)
(2 rows)

* JOIN ft_1 and ft_3 does not works. Error message says ft_3.s does
not exist. Probably, it means ft_3.s does not exist "on the remote
host".

postgres=# SELECT * FROM ft_1 JOIN ft_3 ON a = s;
ERROR:  could not execute foreign query
DETAIL:  ERROR:  column ft_3.s does not exist
LINE 1: SELECT ft_1.a, ft_1.b, ft_3.s, ft_3.t FROM public.ft_1 ft_1,...
   ^

HINT:  SELECT ft_1.a, ft_1.b, ft_3.s, ft_3.t FROM public.ft_1 ft_1,
public.ft_3 ft_3 WHERE (ft_1.a = ft_3.s)
postgres=# EXPLAIN SELECT * FROM ft_1 JOIN ft_3 ON a = s;
 QUERY PLAN
-
 Foreign Scan on multiple foreign tables  (cost=0.00..0.00 rows=5000 width=72)
   Remote SQL: SELECT ft_1.a, ft_1.b, ft_3.s, ft_3.t FROM public.ft_1
ft_1, public.ft_3 ft_3 WHERE (ft_1.a = ft_3.s)
(2 rows)

In fact, EXPLAIN shows us the remote SQL tries to reference ft_3.s,
instead of ft_3.ss.

Thanks,

2011年9月14日10:24 Shigeru Hanada :
> Hi all,
>
> I'd like to propose $SUBJECT for further foreign query optimization.
> I've not finished development, but I'd appreciate it if I got someone's
> review on my WIP code and its design.
>
> Changes I made
> ==
>
> (1) Add foreign server OID to RelOptInfo
> I think it would be nice to know whether a join comes from one foreign
> server or not without digging into child nodes during considering paths
> for a query.  So I added serverid field to RelOptInfo, which defaults to
> InvalidOid ,and is set to OID of the server if the node and all of its
> children are from same foreign server.  This also avoids looking catalog
> up for foreign table entry to determine FDW routine.
>
> (2) Add new planner node, ForeignJoinPat

Re: [HACKERS] bug of recovery?

2011-10-03 Thread Simon Riggs
On Mon, Oct 3, 2011 at 6:23 AM, Fujii Masao  wrote:

> So I think that the idea should be implemented separately from
> the patch I've posted.

Agreed. I'll do a final review and commit today.

-- 
 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


  1   2   >