Re: [HACKERS] Typos

2011-03-30 Thread Heikki Linnakangas

On 30.03.2011 08:34, Fujii Masao wrote:

The attached patch fixes two typos.


Thanks, applied.

--
  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] Problem with streaming replication, backups, and recovery (9.0.x)

2011-03-30 Thread Fujii Masao
On Wed, Mar 30, 2011 at 11:39 AM, Fujii Masao masao.fu...@gmail.com wrote:
 On Wed, Mar 30, 2011 at 12:54 AM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 Hmm, why did we change that?

 I'm not sure, but I guess that's because I missed the case where crash
 recovery starts from the backup :(

 It seems like a mistake, the database is not
 consistent until you reach the backup stop location, whether or not you're
 doing archive recovery. +1 for reverting that, and backpatching it as well.

 Agreed.

Attached patch reverts that. Comments?

Regards,

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


backup_stop_location_v1.patch
Description: Binary data

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


Re: [HACKERS] Replication server timeout patch

2011-03-30 Thread Heikki Linnakangas

On 29.03.2011 07:55, Fujii Masao wrote:

On Mon, Mar 28, 2011 at 7:49 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com  wrote:

pq_flush_if_writable() calls internal_flush() without using PG_TRY block.
This seems unsafe because for example pgwin32_waitforsinglesocket()
called by secure_write() can throw ERROR.


Perhaps it's time to give up on the assumption that the socket is in
blocking mode except within those two functions. Attached patch adds the
pq_set_nonblocking() function from your patch, and adds calls to it before
all secure_read/write operations to put the socket in the right mode.
There's only a few of those operations.


Sounds good.

+   pq_set_nonblocking(false); /* XXX: Is this required? */

No. Since secure_close and close_SSL don't use MyProcPort-sock and
MyProcPort-noblock which can be changed in pq_set_nonblocking,
I don't think that is required.


Ok, I took that out.


+   pq_putmessage_noblock('d', msgbuf, 1 + sizeof(WalDataMessageHeader) + 
nbytes);

Don't we need to check the return value of pq_putmessage_noblock? That
can return EOF when trouble happens (for example the send system call fails).


No, pq_putmessage_noblock doesn't call send() because it enlarges the 
buffer to make sure the message fits, and it doesn't anything else that 
could fail else. I changed its return type to void, and added an 
Assert() to check that the pq_putmessage() call it does internally 
indeed doesn't fail.



Should we use COMMERROR instead of ERROR if we fail to put the socket in the
right mode?


Maybe.


I made it COMMERROR. ERRORs are sent to the client, and you could get 
into infinite recursion if sending the ERROR requires setting the 
blocking mode again.


Committed with those changes. I also reworded the docs a bit.

--
  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] pg_last_xlog_receive_location()

2011-03-30 Thread Tatsuo Ishii
 Ok. It seems I need to patch PostgreSQL.
 
 Why is that feature required? It's for pgpool-II?

Yes. And probably any automated failover management tool will require
the functionality.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

-- 
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] Replication server timeout patch

2011-03-30 Thread Fujii Masao
On Wed, Mar 30, 2011 at 4:24 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 +       pq_putmessage_noblock('d', msgbuf, 1 +
 sizeof(WalDataMessageHeader) + nbytes);

 Don't we need to check the return value of pq_putmessage_noblock? That
 can return EOF when trouble happens (for example the send system call
 fails).

 No, pq_putmessage_noblock doesn't call send() because it enlarges the buffer
 to make sure the message fits, and it doesn't anything else that could fail
 else. I changed its return type to void, and added an Assert() to check that
 the pq_putmessage() call it does internally indeed doesn't fail.

Oh, you're right.

 Committed with those changes. I also reworded the docs a bit.

Thanks a lot!

+A value of zero means wait forever.  This parameter can only be set in

The first sentence sounds misleading. Even if you set the parameter to zero,
replication connections can be terminated because of keepalive or socket error.

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] Additional options for Sync Replication

2011-03-30 Thread Simon Riggs
On Wed, Mar 30, 2011 at 5:29 AM, Fujii Masao masao.fu...@gmail.com wrote:

 I'm very excited about new options, especially recv. But I agree with
 Robert and Heikki because what the patch provides looks like new
 feature rather than bug fix. And I think that we still require some
 discussions of the design; how far transactions must wait for sync
 rep in recv mode? In the patch, they wait for WAL to be written in
 the standby, but I think that they should wait until walreceiver has
 recieved WAL instead. That would increase the performance of sync
 rep. Anyway, I don't think now is time to discuss about such a design
 except for bug fix.

Not waiting for write would just be much less safe and would not have
any purpose as a sync rep option.

The difference in time would be very marginal also.

-- 
 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] Replication server timeout patch

2011-03-30 Thread Heikki Linnakangas

On 30.03.2011 10:58, Fujii Masao wrote:

On Wed, Mar 30, 2011 at 4:24 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com  wrote:
+A value of zero means wait forever.  This parameter can only be set in

The first sentence sounds misleading. Even if you set the parameter to zero,
replication connections can be terminated because of keepalive or socket error.


Hmm, should I change it back to A value of zero disables the timeout ? 
Any better suggestions?


--
  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] Problem with streaming replication, backups, and recovery (9.0.x)

2011-03-30 Thread Heikki Linnakangas

On 30.03.2011 09:25, Fujii Masao wrote:

On Wed, Mar 30, 2011 at 11:39 AM, Fujii Masaomasao.fu...@gmail.com  wrote:

On Wed, Mar 30, 2011 at 12:54 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com  wrote:

Hmm, why did we change that?


I'm not sure, but I guess that's because I missed the case where crash
recovery starts from the backup :(


It seems like a mistake, the database is not
consistent until you reach the backup stop location, whether or not you're
doing archive recovery. +1 for reverting that, and backpatching it as well.


Agreed.


Attached patch reverts that. Comments?


Looks good, committed.

We could also improve the error message. If we haven't reached the 
end-of-backup location, we could say something along the lines of:


ERROR: WAL ends before the end of online backup
HINT: Online backup must be ended with pg_stop_backup(), and all the WAL 
up to that point must be available at recovery.


--
  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] Replication server timeout patch

2011-03-30 Thread Fujii Masao
On Wed, Mar 30, 2011 at 5:03 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 On 30.03.2011 10:58, Fujii Masao wrote:

 On Wed, Mar 30, 2011 at 4:24 PM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com  wrote:
 +        A value of zero means wait forever.  This parameter can only be
 set in

 The first sentence sounds misleading. Even if you set the parameter to
 zero,
 replication connections can be terminated because of keepalive or socket
 error.

 Hmm, should I change it back to A value of zero disables the timeout ? Any
 better suggestions?

I like that. But I appreciate if anyone suggests the better.

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] Problem with streaming replication, backups, and recovery (9.0.x)

2011-03-30 Thread Fujii Masao
On Wed, Mar 30, 2011 at 4:56 PM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 Attached patch reverts that. Comments?

 Looks good, committed.

Thanks!

 We could also improve the error message. If we haven't reached the
 end-of-backup location, we could say something along the lines of:

 ERROR: WAL ends before the end of online backup
 HINT: Online backup must be ended with pg_stop_backup(), and all the WAL up
 to that point must be available at recovery.

+1

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] Another swing at JSON

2011-03-30 Thread Dimitri Fontaine
Alvaro Herrera alvhe...@commandprompt.com writes:
 Why are you worrying with the non-PGXS build chain anyway?  Just assume
 that the module is going to be built with PGXS and things should just
 work.

 We've gone over this a dozen times in the past.

+1

I'm not sure why we still support the pre-PGXS build recipe in the
contrib Makefiles, and didn't want to change that as part as the
extension patch series, but I think we should reconsider.

This and removing module_pathname in the control files to just use
$libdir/contrib in the .sql files.  That would set a better example to
people who want to make their own extensions, as the general case is
that those will not get into contrib.

I think we should lower the differences between contrib and external
extensions, so that contrib is only about who maintains the code and
distribute the extension.

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] PostgreSQL sourcecode

2011-03-30 Thread Nicolas Barbier
2011/3/30 aaronenabs aaronen...@btconnect.com:

 Hi all i have been trying to compile the sourcecode for postgresql but keep
 getting an error message when running it in cygwin.

 it states:

 dllwrap: gcc exited with status 1
 make[3]: *** [cygpq.dll] Error 1
 make[3]: Leaving directory
 `/postgresql-9.0.3/postgresql-9.0.3/src/interfaces/li
 bpq'
 make[2]: *** [all] Error 2
 make[2]: Leaving directory
 `/postgresql-9.0.3/postgresql-9.0.3/src/interfaces'
 make[1]: *** [all] Error 2
 make[1]: Leaving directory `/postgresql-9.0.3/postgresql-9.0.3/src'
 make: *** [all] Error 2

FYI, you left out the error message stating the real problem that
probably came right before the part you pasted here. The Make[n]
things are just the submakes that return an error recursively.

Nicolas

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

2011-03-30 Thread aaronenabs
Sorry about that, here is the section before the make.

dllwrap -o cygpq.dll --dllname cygpq.dll  --def libpqdll.def fe-auth.o
fe-connec
t.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o fe-protocol2.o fe-protocol3.o
pqexp
buffer.o pqsignal.o fe-secure.o libpq-events.o md5.o ip.o wchar.o encnames.o
nob
lock.o pgstrcasecmp.o thread.o  -L../../../src/port
-Wl,--allow-multiple-definit
ion -Wl,--enable-auto-import  -L/usr/local/lib -Wl,--as-needed   -lcrypt 
-lpthr
ead
thread.o:thread.c:(.text+0x5c): undefined reference to `__xpg_strerror_r'
collect2: ld returned 1 exit status
dllwrap: gcc exited with status 1
make[3]: *** [cygpq.dll] Error 1
make[3]: Leaving directory
`/postgresql-9.0.3/postgresql-9.0.3/src/interfaces/li
bpq'
make[2]: *** [all] Error 2
make[2]: Leaving directory
`/postgresql-9.0.3/postgresql-9.0.3/src/interfaces'
make[1]: *** [all] Error 2
make[1]: Leaving directory `/postgresql-9.0.3/postgresql-9.0.3/src'
make: *** [all] Error 2

Can you alos advise how i change the the HeapTupleSatisfiesVisibility() to
true within the source code:
/*
 * HeapTupleSatisfiesVisibility
 *  True iff heap tuple satisfies a time qual.
 *
 * Notes:
 *  Assumes heap tuple is valid.
 *  Beware of multiple evaluations of snapshot argument.
 *  Hint bits in the HeapTuple's t_infomask may be updated as a side effect;
 *  if so, the indicated buffer is marked dirty.
 */
#define HeapTupleSatisfiesVisibility(tuple, snapshot, buffer) \
((*(snapshot)-satisfies) ((tuple)-t_data, snapshot, buffer))

--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/PostgreSQL-sourcecode-tp4270534p4271359.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] PostgreSQL sourcecode

2011-03-30 Thread Nicolas Barbier
2011/3/30 aaronenabs aaronen...@btconnect.com:

 Can you alos advise how i change the the HeapTupleSatisfiesVisibility() to
 true within the source code:

[..]

 #define HeapTupleSatisfiesVisibility(tuple, snapshot, buffer) \
        ((*(snapshot)-satisfies) ((tuple)-t_data, snapshot, buffer))

As someone totally not familiar with the PostgreSQL source code, I
would guess something like:

#define HeapTupleSatisfiesVisibility(tuple, snapshot, buffer) (1)

You might want to check the return type of the satisfies function
pointer though.

Nicolas

-- 
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] crash-safe visibility map, take four

2011-03-30 Thread Heikki Linnakangas

On 30.03.2011 06:24, 高增琦 wrote:

Should we do full-page write for visibilitymap all the time?
Now, when clear visiblitymap, there is no full-page write for vm
since we don't save buffer info in insert/update/delete's log.

The full-page write is used to protect pages from disk failure. Without it,
1) set vm: the vm bits that should be set to 1 may still be 0
2) clear vm: the vm bits that should be set to 0 may still be 1
Are these true? Or the page is totally unpredictable?


Not quite. The WAL replay will set or clear vm bits, regardless of full 
page writes. Full page writes protect from torn pages, ie. the problem 
where some operations on a page have made it to disk while others have 
not. That's not a problem for VM pages, as each bit on the page can be 
set or cleared individually. But for something like a heap page where 
you have an offset in the beginning of the page that points to the tuple 
elsewhere on the page, you have to ensure that they stay in sync, even 
if you don't otherwise care if the update makes it to disk or not.



Another question:
To address the problem in
http://archives.postgresql.org/pgsql-hackers/2010-02/msg02097.php
, should we just clear the vm before the log of insert/update/delete?
This may reduce the performance, is there another solution?


Yeah, that's a straightforward way to fix it. I don't think the 
performance hit will be too bad. But we need to be careful not to hold 
locks while doing I/O, which might require some rearrangement of the 
code. We might want to do a similar dance that we do in vacuum, and call 
visibilitymap_pin first, then lock and update the heap page, and then 
set the VM bit while holding the lock on the heap page.


--
  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] Another swing at JSON

2011-03-30 Thread Alvaro Herrera
Excerpts from Dimitri Fontaine's message of mié mar 30 05:27:07 -0300 2011:
 Alvaro Herrera alvhe...@commandprompt.com writes:
  Why are you worrying with the non-PGXS build chain anyway?  Just assume
  that the module is going to be built with PGXS and things should just
  work.
 
  We've gone over this a dozen times in the past.
 
 +1
 
 I'm not sure why we still support the pre-PGXS build recipe in the
 contrib Makefiles, and didn't want to change that as part as the
 extension patch series, but I think we should reconsider.

This is in the archives somewhere.  I think it's something to do with
the fact that make check doesn't work under PGXS; you have to use
make installcheck.  Or maybe it was something else.  I don't really
remember the details.  I also pushed for this, a long time ago.

-- 
Álvaro Herrera alvhe...@commandprompt.com
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] Another swing at JSON

2011-03-30 Thread David Fetter
On Wed, Mar 30, 2011 at 10:27:07AM +0200, Dimitri Fontaine wrote:

 I think we should lower the differences between contrib and external
 extensions, so that contrib is only about who maintains the code and
 distribute the extension.

+10 :)

Cheers,
David.
-- 
David Fetter da...@fetter.org 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] Another swing at JSON

2011-03-30 Thread Alvaro Herrera
Excerpts from Alvaro Herrera's message of mié mar 30 10:27:39 -0300 2011:
 Excerpts from Dimitri Fontaine's message of mié mar 30 05:27:07 -0300 2011:

  I'm not sure why we still support the pre-PGXS build recipe in the
  contrib Makefiles, and didn't want to change that as part as the
  extension patch series, but I think we should reconsider.
 
 This is in the archives somewhere.  I think it's something to do with
 the fact that make check doesn't work under PGXS; you have to use
 make installcheck.  Or maybe it was something else.  I don't really
 remember the details.  I also pushed for this, a long time ago.

In http://archives.postgresql.org/pgsql-hackers/2009-07/msg00245.php
Tom writes:

 The main reason contrib still has the alternate method is that PGXS
 doesn't really work until after you've installed the core build.

Maybe we could have a look and try to fix that problem.

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

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


[HACKERS] 9.0.3 SIGFAULT on FreeBSD with dtrace

2011-03-30 Thread Luca Ferrari
HI all,
I'm trying to compile PostgreSQL 9.0.3 on FreeBSD 8.1-stable, and I can make 
it working if I compile without dtrace. However when I compile with --enable-
dtrace I'm unable to use the cluster and even initdb.
In particular initdb claims that:

fgets failure: No such file or directory
The program postgres is needed by initdb but was not found in the
same directory as /usr/local/pgsql/bin/initdb.
Check your installation

but digging a little I found that the problem is when a pipe to postgres -V 
process is created, since the latter goes in SIGFAULT. The following is the 
truss of the postgres command:

__sysctl(0xbfbfe744,0x2,0xbfbfe74c,0xbfbfe750,0x0,0x0) = 0 (0x0)
mmap(0x0,336,PROT_READ|PROT_WRITE,MAP_ANON,-1,0x0) = 675897344 (0x28496000)
munmap(0x28496000,336)   = 0 (0x0)
__sysctl(0xbfbfe7a8,0x2,0x2848ce3c,0xbfbfe7b0,0x0,0x0) = 0 (0x0)
mmap(0x0,32768,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANON,-1,0x0) = 675897344 
(0x28496000)
issetugid(0x28485a47,0xbfbfe870,0x104,0x0,0x0,0x0) = 0 (0x0)
open(/etc/libmap.conf,O_RDONLY,0666)   ERR#2 'No such file or 
directory'
access(/usr/local/pgsql/lib/libm.so.5,0)   ERR#2 'No such file or 
directory'
open(/var/run/ld-elf.so.hints,O_RDONLY,00) = 2 (0x2)
read(2,Ehnt\^A\0\0\0\M^@\0\0\0-\0\0\0\0...,128) = 128 (0x80)
lseek(2,0x80,SEEK_SET)   = 128 (0x80)
read(2,/lib:/usr/lib:/usr/lib/compat:/u...,45) = 45 (0x2d)
close(2) = 0 (0x0)
access(/lib/libm.so.5,0)   = 0 (0x0)
open(/lib/libm.so.5,O_RDONLY,00)   = 2 (0x2)
fstat(2,{ mode=-r--r--r-- ,inode=1813507,size=146183,blksize=16384 }) = 0 
(0x0)
pread(0x2,0x2848bd80,0x1000,0x0,0x0,0x0) = 4096 (0x1000)
mmap(0x0,106496,PROT_NONE,MAP_PRIVATE|MAP_ANON|MAP_NOCORE,-1,0x0) = 675930112 
(0x2849e000)
mmap(0x2849e000,102400,PROT_READ|PROT_EXEC,MAP_PRIVATE|MAP_FIXED|
MAP_NOCORE,2,0x0) = 675930112 (0x2849e000)
mmap(0x284b7000,4096,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_FIXED,2,0x18000) = 
676032512 (0x284b7000)
close(2) = 0 (0x0)
access(/usr/local/pgsql/lib/libc.so.7,0)   ERR#2 'No such file or 
directory'
access(/lib/libc.so.7,0)   = 0 (0x0)
open(/lib/libc.so.7,O_RDONLY,027757763634) = 2 (0x2)
fstat(2,{ mode=-r--r--r-- ,inode=1813528,size=1346561,blksize=16384 }) = 0 
(0x0)
pread(0x2,0x2848bd80,0x1000,0x0,0x0,0x0) = 4096 (0x1000)
mmap(0x0,1155072,PROT_NONE,MAP_PRIVATE|MAP_ANON|MAP_NOCORE,-1,0x0) = 676036608 
(0x284b8000)
mmap(0x284b8000,1036288,PROT_READ|PROT_EXEC,MAP_PRIVATE|MAP_FIXED|
MAP_NOCORE,2,0x0) = 676036608 (0x284b8000)
mmap(0x285b5000,24576,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_FIXED,2,0xfd000) = 
677072896 (0x285b5000)
mprotect(0x285bb000,94208,PROT_READ|PROT_WRITE)  = 0 (0x0)
close(2) = 0 (0x0)
sysarch(0xa,0xbfbfe810,0x2845c39b,0x2848b2f8,0x2846ef09,0x2848b2f8) = 0 (0x0)
mmap(0x0,48744,PROT_READ|PROT_WRITE,MAP_ANON,-1,0x0) = 677191680 (0x285d2000)
munmap(0x285d2000,48744) = 0 (0x0)
mmap(0x0,2072,PROT_READ|PROT_WRITE,MAP_ANON,-1,0x0) = 677191680 (0x285d2000)
munmap(0x285d2000,2072)  = 0 (0x0)
mmap(0x0,21904,PROT_READ|PROT_WRITE,MAP_ANON,-1,0x0) = 677191680 (0x285d2000)
munmap(0x285d2000,21904) = 0 (0x0)
sigprocmask(SIG_BLOCK,SIGHUP|SIGINT|SIGQUIT|SIGKILL|SIGPIPE|SIGALRM|SIGTERM|
SIGURG|SIGSTOP|SIGTSTP|SIGCONT|SIGCHLD|SIGTTIN|SIGTTOU|SIGIO|SIGXCPU|SIGXFSZ|
SIGVTALRM|SIGPROF|SIGWINCH|SIGINFO|SIGUSR1|SIGUSR2,0x0) = 0 (0x0)
sigprocmask(SIG_SETMASK,0x0,0x0) = 0 (0x0)
__sysctl(0xbfbfe7c4,0x2,0x8460670,0xbfbfe7cc,0x0,0x0) = 0 (0x0)
sigprocmask(SIG_BLOCK,SIGHUP|SIGINT|SIGQUIT|SIGKILL|SIGPIPE|SIGALRM|SIGTERM|
SIGURG|SIGSTOP|SIGTSTP|SIGCONT|SIGCHLD|SIGTTIN|SIGTTOU|SIGIO|SIGXCPU|SIGXFSZ|
SIGVTALRM|SIGPROF|SIGWINCH|SIGINFO|SIGUSR1|SIGUSR2,0x0) = 0 (0x0)
sigprocmask(SIG_SETMASK,0x0,0x0) = 0 (0x0)
sigprocmask(SIG_BLOCK,SIGHUP|SIGINT|SIGQUIT|SIGKILL|SIGPIPE|SIGALRM|SIGTERM|
SIGURG|SIGSTOP|SIGTSTP|SIGCONT|SIGCHLD|SIGTTIN|SIGTTOU|SIGIO|SIGXCPU|SIGXFSZ|
SIGVTALRM|SIGPROF|SIGWINCH|SIGINFO|SIGUSR1|SIGUSR2,0x0) = 0 (0x0)
sigprocmask(SIG_SETMASK,0x0,0x0) = 0 (0x0)
SIGNAL 11 (SIGSEGV)
process exit, rval = 0


and this is the output of ldd:

/usr/local/pgsql/bin/postgres:
libm.so.5 = /lib/libm.so.5 (0x2849e000)
libc.so.7 = /lib/libc.so.7 (0x284b8000)

I checked that dtrace -l gives me output, so it seems to work. Am I doing 
something wrong?

Thanks,
Luca

-- 
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] Another swing at JSON

2011-03-30 Thread David Fetter
On Wed, Mar 30, 2011 at 10:32:55AM -0300, Alvaro Herrera wrote:
 Excerpts from Alvaro Herrera's message of mié mar 30 10:27:39 -0300 2011:
  Excerpts from Dimitri Fontaine's message of mié mar 30 05:27:07 -0300 2011:
 
   I'm not sure why we still support the pre-PGXS build recipe in the
   contrib Makefiles, and didn't want to change that as part as the
   extension patch series, but I think we should reconsider.
  
  This is in the archives somewhere.  I think it's something to do with
  the fact that make check doesn't work under PGXS; you have to use
  make installcheck.  Or maybe it was something else.  I don't really
  remember the details.  I also pushed for this, a long time ago.
 
 In http://archives.postgresql.org/pgsql-hackers/2009-07/msg00245.php
 Tom writes:
 
  The main reason contrib still has the alternate method is that PGXS
  doesn't really work until after you've installed the core build.
 
 Maybe we could have a look and try to fix that problem.

+1 :)

Cheers,
David.
-- 
David Fetter da...@fetter.org 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] Triggers on system catalog

2011-03-30 Thread Robert Haas
On Tue, Mar 29, 2011 at 9:40 PM, Stephen Frost sfr...@snowman.net wrote:
 * Robert Haas (robertmh...@gmail.com) wrote:
 I do think we need some kind way of capturing DDL events, though. I wonder 
 if the object-access-hook stuff KaiGai and I did to support SE-PostgreSQL 
 could be extended to meet this need...

 My inclination would be 'probably', but it's not likely to really be the
 way we'd want to provide generalized DDL triggers..

I guess I was imagining that DDL triggers would be primarily important
for things like Slony, that are already writing C code anyway, but
maybe that's overly optimistic...

-- 
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] Replication server timeout patch

2011-03-30 Thread Robert Haas
On Wed, Mar 30, 2011 at 4:08 AM, Fujii Masao masao.fu...@gmail.com wrote:
 On Wed, Mar 30, 2011 at 5:03 PM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 On 30.03.2011 10:58, Fujii Masao wrote:

 On Wed, Mar 30, 2011 at 4:24 PM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com  wrote:
 +        A value of zero means wait forever.  This parameter can only be
 set in

 The first sentence sounds misleading. Even if you set the parameter to
 zero,
 replication connections can be terminated because of keepalive or socket
 error.

 Hmm, should I change it back to A value of zero disables the timeout ? Any
 better suggestions?

 I like that. But I appreciate if anyone suggests the better.

Maybe sticking the word mechanism in there would be a bit better.
A value of zero disables the timeout mechanism?

-- 
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] Triggers on system catalog

2011-03-30 Thread David Fetter
On Tue, Mar 29, 2011 at 08:21:04PM -0400, Robert Haas wrote:
 On Mar 29, 2011, at 2:17 PM, Christopher Browne cbbro...@gmail.com wrote:
  A proposal to adding triggers to system catalog tables won't be
  terribly popular.
 
 Well, I'd support it if I thought it had any chance of actually
 working, but I don't.
 
 I do think we need some kind way of capturing DDL events, though. I
 wonder if the object-access-hook stuff KaiGai and I did to support
 SE-PostgreSQL could be extended to meet this need...

Jan Wieck sketched out a proposal, which I'll post in more detail when
I've transcribed it and checked that with him.  Part of it is to fire
triggers on the command completion code, and there are some code
cleanups that that would depend on.

Cheers,
David.
-- 
David Fetter da...@fetter.org 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] Process local hint bit cache

2011-03-30 Thread Merlin Moncure
On Tue, Mar 29, 2011 at 4:34 PM, Merlin Moncure mmonc...@gmail.com wrote:
 In a previous thread
 (http://postgresql.1045698.n5.nabble.com/Set-hint-bits-upon-eviction-from-BufMgr-td4264323.html)
 I was playing with the idea of granting the bgwriter the ability to
 due last chance hint bit setting before evicting the page out. I still
 think might be a good idea, and it might be an especially good idea,
 especially in scenarios where you get set the PD_ALL_VISIBLE bit when
 writing out the page, a huge win in some cases.  That said, it bugged
 me that it didn't really fix the general case of large data insertions
 and the subsequent i/o problems setting out the bits, which is my
 primary objective in the short term.

 So I went back to the drawing board, reviewed the archives, and came
 up with a new proposal.  I'd like to see a process local clog page
 cache of around 1-4 pages (8-32kb typically) that would replace the
 current TransactionLogFetch last xid cache. It should be small,
 because I doubt more would really help much (see below) and we want to
 keep this data in the tight cpu caches since it's going to be
 constantly scanned.

 The cache itself will be the clog pages and a small header per page
 which will contain the minimum information necessary to match an xid
 against a page to determine a hit, and a count of hits.  Additionally
 we keep a separate small array (say 100) of type xid (int) that we
 insert write into in a cache miss.

 So, cache lookup algorithm basically is:
 scan clog cache headers and check for hit
 if found (xid in range covered in clog page),
  header.hit_count++;
 else
  miss_array[miss_count++] = xid;

 A cache hit is defined about getting useful information from the page,
 that is a transaction being committed or invalid.

 When the miss count array fills,  we sort it and determine the most
 commonly hit clog page that is not in the cache and use that
 information to replace pages in the cache if necessary, then reset the
 counts. Maybe we can add a minimum threshold of hits, say 5-10% if
 miss_array size for a page to be deemed interesting enough to be
 loaded into the cache.

 Interaction w/set hint bits:
 *) If a clog lookup faults through the cache, we basically keep the
 current behavior.  That is, the hint bits are set and the page is
 marked BM_DIRTY and the hint bits get written back

 *) If we get a clog cache hit, that is the hint bits are not set but
 we pulled the transaction status from the cache, the hint bits are
 recorded on the page *but the page is not written back*, at least on
 hint bit basis alone.  This behavior branch is more or less the
 BM_UNTIDY as suggested by haas (see archives), except it's only seen
 in 'cache hit' scenarios.  We are not writing pages back because the
 cache is suggesting there is little/not benefit to write them back.

 Thus, if a single backend is scanning a lot of pages with transactions
 touching a very small number of clog pages, hint bits are generally
 not written back because they are not needed and in fact not helping.
 However, if the xid are spread around a large number of clog pages, we
 get the current behavior more or less (plus the overhead of cache
 maintenance).

 With the current code base, hint bits are very beneficial when the xid
 entropy is high and the number of repeated scan is high, and not so
 good when the xid entropy is low and the number of repeated scans is
 low.  The process local cache attempts to redress this without
 disadvantaging the already good cases.  Furthermore, if it can be
 proven that the cache overhead is epsilon, it's pretty unlikely to
 negatively impact anyone negatively, at lest, that's my hope.  Traffic
 to clog will reduce (although not much, since i'd wager the current
 'last xid' cache works pretty well), but i/o should be reduced, in
 some cases quite significantly for a tiny cpu cost (although that
 remains to be proven).


A couple of details I missed:
*) clog lookups that return no cacheable information will not have
their xid inserted into the 'missed' array -- this will prevent a clog
page returning 'in progress' type states for transactions from pushing
out pages that are returning useful information.  In other words, an
in progress transaction is neither a hit or miss from the point of
view of the cache -- it's nothing.

*) If we fault to the clog and get useful information
(commit/invalid), and the clog page is already cached -- either the
particular bit is set or the entire cache page is refreshed (not sure
which is the better way to go yet)

*) The clog cache might be useful in other places like during page
eviction hint bet setting scenarios I mentioned earlier.  In non
bgwriter scenarios it's almost certainly a win to at least check the
cache and set hint bits for BM_HEAP pages since you are leveraging the
work already paid during scans.  In the bgwriter case, you would have
to build the cache by checking clog pages.  I'm somewhat skeptical if
this would 

Re: [HACKERS] Triggers on system catalog

2011-03-30 Thread Christopher Browne
On Wed, Mar 30, 2011 at 9:59 AM, David Fetter da...@fetter.org wrote:
 On Tue, Mar 29, 2011 at 08:21:04PM -0400, Robert Haas wrote:
 On Mar 29, 2011, at 2:17 PM, Christopher Browne cbbro...@gmail.com wrote:
  A proposal to adding triggers to system catalog tables won't be
  terribly popular.

 Well, I'd support it if I thought it had any chance of actually
 working, but I don't.

 I do think we need some kind way of capturing DDL events, though. I
 wonder if the object-access-hook stuff KaiGai and I did to support
 SE-PostgreSQL could be extended to meet this need...

 Jan Wieck sketched out a proposal, which I'll post in more detail when
 I've transcribed it and checked that with him.  Part of it is to fire
 triggers on the command completion code, and there are some code
 cleanups that that would depend on.

See http://wiki.postgresql.org/wiki/DDL_Triggers

That already captured an understanding of this, which might already be
nearly adequate.
-- 
http://linuxfinances.info/info/linuxdistributions.html

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


Re: [HACKERS] pg_dump --binary-upgrade vs. ALTER TYPE ... DROP ATTRIBUTE

2011-03-30 Thread Robert Haas
On Tue, Mar 29, 2011 at 5:50 PM, Noah Misch n...@leadboat.com wrote:
 I took a look at the open item concerning typed tables and pg_upgrade:
 http://archives.postgresql.org/pgsql-hackers/2011-03/msg00767.php

Thanks!

 [ helpful summary of problem clipped ]

 To reproduce that catalog state, the dump would need to create the type, 
 create
 all typed tables predating the DROP ATTRIBUTE, and finally create typed tables
 postdating the DROP ATTRIBUTE.  That implies an extra dump entry for the DROP
 ATTRIBUTE with the appropriate dependencies to compel that order of events.  
 Is
 there a better way?

I think so.  We have this same problem with regular table inheritance,
and the way we fix it is to jigger the tuple descriptor for the child
table so that it matches what we need, and THEN attach it to the
parent:

CREATE TABLE child (
a integer,
pg.dropped.2 INTEGER /* dummy */
);

-- For binary upgrade, recreate inherited column.
UPDATE pg_catalog.pg_attribute
SET attislocal = false
WHERE attname = 'a'
  AND attrelid = 'child'::pg_catalog.regclass;

-- For binary upgrade, recreate dropped column.
UPDATE pg_catalog.pg_attribute
SET attlen = 4, attalign = 'i', attbyval = false
WHERE attname = 'pg.dropped.2'
  AND attrelid = 'child'::pg_catalog.regclass;
ALTER TABLE ONLY child DROP COLUMN pg.dropped.2;

-- For binary upgrade, set up inheritance this way.
ALTER TABLE ONLY child INHERIT parent;

I think we need to do something similar here -- use the same hack
shown above to get the dropped column into the right state, and then
jigger things so that the child is a typed table associated with the
parent.  Perhaps it would be reasonable to extend ALTER TABLE .. [NO]
INHERIT to accept a type name as the final argument.  If used in this
way, it converts a typed table into a regular table or visca versa.
We could also do it with a direct catalog change, but there are some
dependencies that would need to be frobbed, which makes me a bit
reluctant to go that way.

-- 
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] Process local hint bit cache

2011-03-30 Thread Heikki Linnakangas

On 30.03.2011 17:05, Merlin Moncure wrote:

*) Maybe the shared buffer cache currently being maintained over the
clog can be scrapped. I'm going to leave it alone for now, but I'm
quite skeptical it provides much benefit even without local process
cache.  clog page have a very nice property that you don't have to
worry about what else is going on from other processes and thus no
complicated locking or invalidation issues when considering cache
structure.  IMNSHO -- this makes a local cache a much better fit even
if you have to keep it smaller for memory usage reasons.


A related idea I've sometimes pondered is to mmap() the clog in every 
process.


--
  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] Process local hint bit cache

2011-03-30 Thread Greg Stark
On Tue, Mar 29, 2011 at 10:34 PM, Merlin Moncure mmonc...@gmail.com wrote:
 So I went back to the drawing board, reviewed the archives, and came
 up with a new proposal.  I'd like to see a process local clog page
 cache of around 1-4 pages (8-32kb typically) that would replace the
 current TransactionLogFetch last xid cache. It should be small,
 because I doubt more would really help much (see below) and we want to
 keep this data in the tight cpu caches since it's going to be
 constantly scanned.


How is this different from the existing clog SLRU? It seems like the
fundamental difference is that you plan to defer updating the hint
bits rather than update them every time the row is accessed. That
doesn't seem like a net win, it'll just defer the i/o, not eliminate
it. I suppose the existing clog SLRU is page-based whereas this could
cache individual xids in a btree so that it could have a higher hit
rate. Or we could just increase the clog SLRU size if there's any
evidence there are often cache misses on it. I suggested having the
SLRU share memory pages with the buffer cache so it would
automatically size itself rather than having to be statically sized.

There is something to be gained by trying to update *all* the hint
bits on a page whenever any row is updated. And there might be
something to be gained by not dirtying the page so we only update the
hint bits on disk if the page is dirtied for some other reason.

But one way or another the hint bits have to get set sometime. The
sooner that happens the less clog i/o has to happen in the meantime.

-- 
greg

-- 
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] gcc 4.6 warnings -Wunused-but-set-variable

2011-03-30 Thread Robert Haas
On Tue, Mar 29, 2011 at 4:48 PM, Peter Eisentraut pete...@gmx.net wrote:
 As you might have heard, GCC 4.6 was released the other day.  It
 generates a bunch of new warnings with the PostgreSQL source code, most
 of which belong to the new warning scenario -Wunused-but-set-variable,
 which is included in -Wall.

 Attached is a patch that gets rid of most of these.  As you can see,
 most of these remove real leftover garbage.  The line I marked in
 pg_basebackup.c might be an actual problem: It goes through a whole lot
 to figure out the timeline and then doesn't do anything with it.  In
 some other cases, however, one might argue that the changes lose some
 clarity, such as when dropping the return value of strtoul() or
 va_arg().  How should we proceed?  In any case, my patch should be
 re-reviewed for any possible side effects that I might have hastily
 removed.

In the case of variable.c, it is entirely unclear that there's any
point in calling strtoul() at all.  Maybe we should just remove that
and the following Assert() as well.

In parse_utilcmd.c, do we need to look up the collation OID if we're
just discarding it anyway?

In the case of the va_arg() calls, maybe something like /* advance arg
position, but ignore result */?

-- 
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] Problem with pg_upgrade?

2011-03-30 Thread Bruce Momjian
Alvaro Herrera wrote:
 Excerpts from Jeff Davis's message of mar mar 29 21:27:34 -0300 2011:
  On Tue, 2011-03-29 at 15:52 -0400, Bruce Momjian wrote:
   Does anyone have any other suggestions on how to make sure autovacuum
   does not run in freeze mode?
  
  Can you run in single user mode?
 
 I asked the same thing.  Apparently the problem is that it would make
 error handling a lot more difficult.  I think it would be better to have
 some sort of option to disable autovacuum completely which would be used
 only during pg_upgrade.

Yes, also consider that pg_dumpall assumes psql with its use of
\connect, and I would have to start/stop the single-user backend for
every database change, plus I use psql with ON_ERROR_STOP=on.

I think we have three options:

o  find if the use of autovacuum_freeze_max_age is safe, or make
   it safe
o  document that autovacuum_naptime always happens before
   autovacuum does anything and set it high
o  modify autovacuum to be an enum, with values on/off/disabled

I think the last one makes more sense, and is safer if we need to
backpatch this.  Creating a new variable for this would be confusing
because it could conflict with the 'autovacuum' setting.

Also, I am unclear if this is really our bug.  At least one of the
systems was on Ubuntu/Debian, and they might both have been, and I know
Debian changes our source code.  Where can I find a copy of the diffs
they have made?

I am hearing only second-hand reports of this problem through
Rhodiumtoad on IRC.  I don't have IRC access this week so if someone can
get details from him that would help.  I think the fix he found was to
pull the clog files off of an old file system backup.

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

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

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


Re: [HACKERS] Process local hint bit cache

2011-03-30 Thread Robert Haas
On Wed, Mar 30, 2011 at 10:40 AM, Greg Stark gsst...@mit.edu wrote:
 But one way or another the hint bits have to get set sometime. The
 sooner that happens the less clog i/o has to happen in the meantime.

I talked about this with Merlin a bit yesterday.  I think that his
thought is that most transactions will access a small enough number of
distinct CLOG pages, and the cache accesses might be fast enough, that
we really wouldn't need to get the hint bits set, or at least that
vacuum/freeze time would be soon enough.  I'm not sure if that's
actually true, though - I think the overhead of the cache might be
higher than he's imagining.  However, there's a sure-fire way to find
out... code it up and see how it plays.

-- 
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] Process local hint bit cache

2011-03-30 Thread Merlin Moncure
On Wed, Mar 30, 2011 at 9:40 AM, Greg Stark gsst...@mit.edu wrote:
 On Tue, Mar 29, 2011 at 10:34 PM, Merlin Moncure mmonc...@gmail.com wrote:
 So I went back to the drawing board, reviewed the archives, and came
 up with a new proposal.  I'd like to see a process local clog page
 cache of around 1-4 pages (8-32kb typically) that would replace the
 current TransactionLogFetch last xid cache. It should be small,
 because I doubt more would really help much (see below) and we want to
 keep this data in the tight cpu caches since it's going to be
 constantly scanned.


 How is this different from the existing clog SLRU? It seems like the
 fundamental difference is that you plan to defer updating the hint
 bits rather than update them every time the row is accessed. That
 doesn't seem like a net win, it'll just defer the i/o, not eliminate


It is very different -- the slru layer is in shared memory and
requires locks to access.   The entire point is trying to avoid
accessing this structure in tight code paths.  I'm actually very
skeptical the slru layer provides any benefit at all.  I bet it would
be cheaper just mmap in the pages directly (as Heikki is also
speculating).

Regarding deferring i/o, you have good chance of eliminating i/o if
the tuples are deleted or touched before a particular backend gets to
a page without already having seen the transaction (very high chance
under certain workloads).  Worst case scenario, you get the old
behavior plus the overhead of maintaining the cache (which is why i'm
keen on bucketing at the page level -- so it's ultra cheap to scan and
memory efficient).

merlin

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


Re: [HACKERS] Another swing at JSON

2011-03-30 Thread Andrew Dunstan



On 03/30/2011 09:42 AM, David Fetter wrote:


In http://archives.postgresql.org/pgsql-hackers/2009-07/msg00245.php
Tom writes:


The main reason contrib still has the alternate method is that PGXS
doesn't really work until after you've installed the core build.

Maybe we could have a look and try to fix that problem.

+1 :)





Maybe we could teach pg_config to report appropriate settings for 
uninstalled source via an environment setting. Without changing 
pg_config it looks like we have a chicken and egg problem.


(If my suggestion is right, this would probably be a good beginner TODO.)

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] Another swing at JSON

2011-03-30 Thread Dimitri Fontaine
Andrew Dunstan and...@dunslane.net writes:
 Maybe we could teach pg_config to report appropriate settings for
 uninstalled source via an environment setting. Without changing pg_config it
 looks like we have a chicken and egg problem.

 (If my suggestion is right, this would probably be a good beginner TODO.)

Is it still 9.1 material?  I think it should be considered UI
improvements on what we already have (contribs and extensions), and
allowed to be fixed while in beta.

-- 
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] Process local hint bit cache

2011-03-30 Thread Alvaro Herrera
Excerpts from Merlin Moncure's message of mié mar 30 12:14:20 -0300 2011:

 It is very different -- the slru layer is in shared memory and
 requires locks to access.   The entire point is trying to avoid
 accessing this structure in tight code paths.  I'm actually very
 skeptical the slru layer provides any benefit at all.  I bet it would
 be cheaper just mmap in the pages directly (as Heikki is also
 speculating).

Maybe it would be useful to distinguish the last SLRU page(s) (the one
where clog writing actually takes place) from the older ones (which only
ever see reading).  You definitely need locks to be able to access the
active pages, but all the rest could be held as mmapped and accessed
without locks because they never change (except to be truncated away).
You know that any page behind RecentXmin is not going to be written
anymore, so why go through all the locking hoops?

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

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


[HACKERS] proposal: a validator for configuration files

2011-03-30 Thread Alexey Klyukin
Hi,

I'd like to add an ability to validate the corectness of PostgreSQL
configuration files, i.e. postgresql.conf, pg_hba.conf, pg_ident.conf without
actually applying them. The idea is that it would be a command-line option to
postgres for a stand-alone case, or a user-callable function when postmaster
is running.

Per the former discussion of a validator for PostgreSQL configuration files
(see http://archives.postgresql.org/pgsql-hackers/2008-08/msg00048.php),
here's an implementation proposal. The development plan consists of 2 parts.
The first one is to add new code that would allow running the checks in both a
stand-alone process, when postmaster is not running, and as a function call
from a backend postgres process. Initially the code would simply loads
configuration files, without performing any of the validation checks. The
second part consists of adding specific checks.

I think most of the code related to this feature should be put into the
auxiliary process. The rationale is that we already have a stand-alone
CheckerProcess, which nowadays only parses postgresql.conf, runs BaseInit and
exists. We can easily load pg_hba.conf and pg_ident.conf and run all the
necessary checks there. Moreover, the same process can be used when checks are
launched from a built-in function. In that case, it would save the postgres
backend from reloading postgresql.conf, pg_hba.conf and pg_ident.conf
internally and restoring the previous configuration options when the function
exists. Below is a more detailed description of implementation steps:

1.1. Stand-alone process (postmaster is not running):

- Add a new option (--check-config) to main.c. Run AuxiliaryProcessMain with
  auxType= CheckerProcess if this option is specified.
- In CheckerModeMain load pg_hba.conf, pg_ident.conf


1.2. Built-in function, called from a backend process.
- Add a new built-in function
- Add LastCheckState shared flag to denote whether the check has been
  successful or failed
- Add a new PMSignalReason 
- From the function call SendPostmasterSignal with the reason added on the
  previous step.
- In the postmaster add a check for this reason in sigusr1_handler, spawning
  a checker process. Set LastCheckStatus to InProgress.
- Store current configuration options in AuxillaryProcessMain before reloading
  configuration files to avoid checking for options that haven't changed.
- In AuxillaryProcessMain, modify SelectConfigFiles invocation to call it
  if IsUnderPostmaster = true if auxType process type is CheckerProcess.
- In the CheckerModeMain, set LastCheckState to either success or failure at the
  end of all checks.
- The built-in function would wait until LastCheckState changes its value from
  InProgress to either Succcess or Failure, or until some predefined timeout
  expires, in which case it would emit an error.

2. Add following configuration checks:

postgresql.conf:

Check that:
  1. postgres can bind to listen address:port
  2. unix_socket_directory has proper permissions (n/a on Windows).
  3. unix_socket_group group exists on a target system (n/a Windows).
  4. unix_socket_permissions are valid (write permission is not revoked from
the owner, or a group, if group is different from the user).
  5. server.crt and server.key exist in the data directory if ssl is
set to true (and server is compiled with openssl)
  6. krb_server_keyfile is readable by the server (if  set)
  7. server is not requesting more shared memory than it's available in the 
system.
  8. shared_preload_libraries and local_preload_libraries exist.
  9. synchronous_standby_names are not empty and max_wal_senders
are  0 if synchronous_replication = true
  10. all enable_*  query planner parameters are on.
  11. seq_page_cost = random_page_cost, and cpu_ costs are lower than page_ 
costs.
  12. effective_cache_size is less than the amount of physical memory available.
  13. geqo is turned on
  14. default_statistics_target is = 100
  15. logging collector is enabled if log destination is stderr
  16. log directory either exists and writable or that the parent
 directory allows creating subdris
  17. track counts is on if autovacuum is enabled
  18. stats_temp_directory is writeable
  19. default tablespace exists (if set)
  20. temp_tablespace exists (if set)
  21. statement_timeout is 0 (disabled)
  22. dynamic_library_path exists
  23. sql_inheritance is off
  24. zero_damaged_pages is off
 
pg_hba.conf:
Check that:
  1. the server is compiled with ssl if hostssl is specified
  2. ssl is not required if hostnossl is specified

- Add a Warning value to LastCheckState for cases when configuration
files were loaded successfully, but one or more validation checks have failed.

Note that these are basic checks don't require the checker process to have
access to the database, Also, with this implementation, a client won't receive
an exact error message in case validation is unsuccessful, however, it would
receive a status 

Re: [HACKERS] Another swing at JSON

2011-03-30 Thread Andrew Dunstan



On 03/30/2011 11:37 AM, Dimitri Fontaine wrote:

Andrew Dunstanand...@dunslane.net  writes:

Maybe we could teach pg_config to report appropriate settings for
uninstalled source via an environment setting. Without changing pg_config it
looks like we have a chicken and egg problem.

(If my suggestion is right, this would probably be a good beginner TODO.)

Is it still 9.1 material?  I think it should be considered UI
improvements on what we already have (contribs and extensions), and
allowed to be fixed while in beta.



I think we're pretty much down to only fixing bugs now, for 9.1, and 
this isn't a bug, however inconvenient it might be.


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] crash-safe visibility map, take four

2011-03-30 Thread Robert Haas
On Wed, Mar 30, 2011 at 8:52 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 Yeah, that's a straightforward way to fix it. I don't think the performance
 hit will be too bad. But we need to be careful not to hold locks while doing
 I/O, which might require some rearrangement of the code. We might want to do
 a similar dance that we do in vacuum, and call visibilitymap_pin first, then
 lock and update the heap page, and then set the VM bit while holding the
 lock on the heap page.

Looking at heap_insert(), for example, we get the target buffer by
calling RelationGetBufferForTuple(), which kicks out an already
x-locked buffer.  Until we have an x-lock on the buffer, we don't
actually know that there's enough free space for the new tuple.  But
if we release the x-lock to pin the visibility map page, then, one,
we're adding an additional lock acquisition and release cycle, and,
two, by the time we reacquire the x-lock the amount of free space may
no longer be sufficient.  Maybe we could check PD_ALL_VISIBLE before
taking the buffer lock - if it appears to be set, then we pin the
visibility map page before taking the buffer lock.  Otherwise, we take
the buffer lock at once.  Either way, once we have the lock, we
recheck the bit.  Only if it's set and we haven't got a pin do we need
to do the drop-lock-pin-reacquire-lock dance.  Is that at all
sensible?

I also wonder if we should be thinking about holding on to the
visibility map pin longer, rather than potentially reacquiring it for
every tuple inserted.  For bulk loading it doesn't matter; we'll
usually be extending the relation anyway, so the PD_ALL_VISIBLE bits
won't be set and we needn't ever hit the map.  But for a table where
there's a large amount of internal free space, it might matter.  Then
again maybe we don't clear all-visible bits often enough for it to be
an issue.

-- 
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] Process local hint bit cache

2011-03-30 Thread Merlin Moncure
On Wed, Mar 30, 2011 at 10:43 AM, Alvaro Herrera
alvhe...@commandprompt.com wrote:
 Excerpts from Merlin Moncure's message of mié mar 30 12:14:20 -0300 2011:

 It is very different -- the slru layer is in shared memory and
 requires locks to access.   The entire point is trying to avoid
 accessing this structure in tight code paths.  I'm actually very
 skeptical the slru layer provides any benefit at all.  I bet it would
 be cheaper just mmap in the pages directly (as Heikki is also
 speculating).

 Maybe it would be useful to distinguish the last SLRU page(s) (the one
 where clog writing actually takes place) from the older ones (which only
 ever see reading).  You definitely need locks to be able to access the
 active pages, but all the rest could be held as mmapped and accessed
 without locks because they never change (except to be truncated away).
 You know that any page behind RecentXmin is not going to be written
 anymore, so why go through all the locking hoops?

hm, that's a good thought -- cheap and easy -- and good to implement
irrespective of other changes.  any improvement helps here, although
from my perspective  the largest hobgoblins are shared memory access
generally and the extra i/o.

merlin

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


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

2011-03-30 Thread Steve Crawford

On 03/29/2011 04:24 PM, Adrian Klaver wrote:

...
Well the strange part is only fails for SUN:...
test(5432)aklaver=select to_date('2011-13-SUN', 'IYYY-IW-DY');
   to_date

  2011-03-28

...
You specified Sunday as the day but the date returned is a Monday. I 
would categorize that as a bug. (Hackers cc'd). Since Sunday is the last 
day of an ISO week, it should have returned 2011-04-03.


My first inclination without consulting source or morning coffee is that 
PostgreSQL is seeing Sunday as day zero. Note that while:

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

 2011-03-28

So does:
steve=# select to_date('2011-13-0', 'IYYY-IW-ID');
  to_date

 2011-03-28

So something isn't right. All sorts of other stuff is allowed as well - 
I don't know if that's by design or not:


steve=# select to_date('2011-13--23', 'IYYY-IW-ID');
  to_date

 2011-03-04


steve=# select to_date('2011-13-56', 'IYYY-IW-ID');
  to_date

 2011-05-22



Agreed, maintaining ISO arguments across the board is the way to go:

Monday
select to_date('2011-13-1', 'IYYY-IW-ID');...
We have to distinguish Gregorian and ISO days when represented as an 
integer since they define the start-of-week differently. Same with year. 
I don't think I've ever seen and ISO-week-date written as 2011-13-SUN 
but it *does* define a distinct date (which is not Monday). And even if 
PostgreSQL were updated to throw an error on that mix of formats it 
still leaves the problem of ISO day-of-week equal to zero.


Cheers,
Steve


--
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] Process local hint bit cache

2011-03-30 Thread Heikki Linnakangas

On 30.03.2011 18:02, Robert Haas wrote:

On Wed, Mar 30, 2011 at 10:40 AM, Greg Starkgsst...@mit.edu  wrote:

But one way or another the hint bits have to get set sometime. The
sooner that happens the less clog i/o has to happen in the meantime.


I talked about this with Merlin a bit yesterday.  I think that his
thought is that most transactions will access a small enough number of
distinct CLOG pages, and the cache accesses might be fast enough, that
we really wouldn't need to get the hint bits set, or at least that
vacuum/freeze time would be soon enough.  I'm not sure if that's
actually true, though - I think the overhead of the cache might be
higher than he's imagining.  However, there's a sure-fire way to find
out... code it up and see how it plays.


I did a little experiment: I hacked SetHintBits() to do nothing, so that 
hint bits are never set. I then created a table with 10 rows in it:


postgres=# \d foo
  Table public.foo
 Column |  Type   | Modifiers
+-+---
 a  | integer |

postgres=# INSERT INTO foo SELECT generate_series(1, 10);
INSERT 0 10

And ran queries on the table:

postgres=# do $$
declare
  i int4;
begin
  loop
perform COUNT(*) FROM foo;
  end loop;
end;
$$;

This test case is designed to exercise the visibility tests as much as 
possible. However, all the tuples are loaded in one transaction, so the 
one-element cache in TransactionLogFetch is 100% effective.


I ran oprofile on that. It shows that about 15% of the time is spent in 
HeapTupleSatisfiesMVCC and its subroutines. 6.6% is spent in 
HeapTupleSatisfiesMVCC itself. Here's the breakdown of that:


$ opreport  -c --global-percent

CPU: Intel Architectural Perfmon, speed 2266 MHz (estimated)
Counted CPU_CLK_UNHALTED events (Clock cycles when not halted) with a 
unit mask of 0x00 (No unit mask) count 10

samples  %app name symbol name
...
---
  2143  0.4419  postgres postgres 
heapgettup_pagemode
  7327715.1091  postgres postgres 
heapgetpage
31858 6.5688  postgres postgres 
HeapTupleSatisfiesMVCC
  31858 6.5688  postgres postgres 
HeapTupleSatisfiesMVCC [self]
  12809 2.6411  postgres postgres 
TransactionIdIsInProgress
  12091 2.4931  postgres postgres 
XidInMVCCSnapshot
  7150  1.4743  postgres postgres 
TransactionIdIsCurrentTransactionId
  7056  1.4549  postgres postgres 
TransactionIdDidCommit
  1839  0.3792  postgres postgres 
TransactionIdPrecedes
  1467  0.3025  postgres postgres 
SetHintBits
  1155  0.2382  postgres postgres 
TransactionLogFetch

---
...

I then ran the same test again with an unpatched version, to set the 
hint bits. After the hint bits were set, I ran oprofile again:


---
  275   0.4986  postgres heapgettup_pagemode
  4459  8.0851  postgres heapgetpage
3005  5.4487  postgres HeapTupleSatisfiesMVCC
  3005  5.4487  postgres HeapTupleSatisfiesMVCC [self]
  1620  2.9374  postgres XidInMVCCSnapshot
  110   0.1995  postgres TransactionIdPrecedes
---

So with hint bits set, HeapTupleSatisfiesMVCC accounts for 8% of the 
total CPU time, and without hint bits, 15%.


Even if clog access was free, hint bits still give a significant speedup 
thanks to skipping all the other overhead like 
TransactionIdIsInProgress() and TransactionIdIsCurrentTransactionId(). 
Speeding up clog access is important; when the one-element cache isn't 
saving you the clog access becomes a dominant factor. But you have to 
address that other overhead too before you can get rid of hint bits.


--
  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] [GENERAL] Date conversion using day of week

2011-03-30 Thread Adrian Klaver

On 03/30/2011 09:15 AM, Steve Crawford wrote:

On 03/29/2011 04:24 PM, Adrian Klaver wrote:

...
Well the strange part is only fails for SUN:...
test(5432)aklaver=select to_date('2011-13-SUN', 'IYYY-IW-DY');
to_date

2011-03-28

...

You specified Sunday as the day but the date returned is a Monday. I
would categorize that as a bug. (Hackers cc'd). Since Sunday is the last
day of an ISO week, it should have returned 2011-04-03.

My first inclination without consulting source or morning coffee is that
PostgreSQL is seeing Sunday as day zero. Note that while:


I started going through the source (formatting.c,timestamp.c), got as 
far as the Julian date functions before the brain imploded and I had to 
take a break:) I would agree it has to do with the difference in the 
week rotating around either Sunday or Monday.



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

2011-03-28

So does:
steve=# select to_date('2011-13-0', 'IYYY-IW-ID');
to_date

2011-03-28

So something isn't right. All sorts of other stuff is allowed as well -
I don't know if that's by design or not:


Well I can see how this is possible and indeed likely. The permutations 
of all the possible date/time representations is immense. It just 
emphasizes that when dealing with time consistency is good.




steve=# select to_date('2011-13--23', 'IYYY-IW-ID');
to_date

2011-03-04


steve=# select to_date('2011-13-56', 'IYYY-IW-ID');
to_date

2011-05-22






Cheers,
Steve




--
Adrian Klaver
adrian.kla...@gmail.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] Another swing at JSON

2011-03-30 Thread Dimitri Fontaine
Andrew Dunstan and...@dunslane.net writes:
 I think we're pretty much down to only fixing bugs now, for 9.1, and this
 isn't a bug, however inconvenient it might be.

It's not just inconvenient, it's setting a bad example for people to
work on their own extensions.  It's more than unfortunate.  I will
prepare a doc patch, but copying from contrib is the usual way to go
creating your own extension, right?

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] Another swing at JSON

2011-03-30 Thread Andrew Dunstan



On 03/30/2011 12:29 PM, Dimitri Fontaine wrote:

Andrew Dunstanand...@dunslane.net  writes:

I think we're pretty much down to only fixing bugs now, for 9.1, and this
isn't a bug, however inconvenient it might be.

It's not just inconvenient, it's setting a bad example for people to
work on their own extensions.  It's more than unfortunate.  I will
prepare a doc patch, but copying from contrib is the usual way to go
creating your own extension, right?




None of that makes it a bug.

I don't have any objection to putting some comments in the contrib 
Makefiles telling people to use PGXS, but I don't think that at this 
stage of the cycle we can start work on something that so far is just an 
idea from the top of my head.


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] Process local hint bit cache

2011-03-30 Thread Merlin Moncure
On Wed, Mar 30, 2011 at 11:23 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 Even if clog access was free, hint bits still give a significant speedup
 thanks to skipping all the other overhead like TransactionIdIsInProgress()
 and TransactionIdIsCurrentTransactionId(). Speeding up clog access is
 important; when the one-element cache isn't saving you the clog access
 becomes a dominant factor. But you have to address that other overhead too
 before you can get rid of hint bits.

Yes, note I am not getting rid of the hint bits -- either you get them
directly from the tuple or the cache.  The clog access layers are:

1. hint bit
2. backend local clog cache (my proposal)
shared memory layer 
3. slru
4. os file cache
5. clog file

My idea working hinges on rigging a cache (#2) that is not materially
slower than the raw hint bit check.  If you think out all the cases
around what i'm suggesting, there is no way you hit #3 that you
wouldn't ottherwise with the old behavior, since when you fault
through #2 you mark the page dirty, but there are cases were full
faults to clog are avoided.  I'm not optimizing clog accesses, I'm
avoiding them.

Basically, i'm extending the last xid cache check in
TransactionLogFetch so it holds 1 xid, and not setting BM_DIRTY *if
and only if* you got xid status from that cache.

merlin

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


Re: [HACKERS] Problem with pg_upgrade?

2011-03-30 Thread Peter Eisentraut
On ons, 2011-03-30 at 10:57 -0400, Bruce Momjian wrote:
 Also, I am unclear if this is really our bug.  At least one of the
 systems was on Ubuntu/Debian, and they might both have been, and I know
 Debian changes our source code.  Where can I find a copy of the diffs
 they have made?

http://bazaar.launchpad.net/~pitti/postgresql/debian-9.0/files/head:/debian/patches/



-- 
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] Typed-tables patch broke pg_upgrade

2011-03-30 Thread Peter Eisentraut
On tor, 2011-02-10 at 06:31 +0200, Peter Eisentraut wrote:
  ERROR:  cannot drop column from typed table
  
  which probably is because test_type2 has a dropped column.
 
 It should call
 
 ALTER TYPE test_type2 DROP ATTRIBUTE xyz CASCADE;
 
 instead.  That will propagate to the table.

Here is a patch that addresses this problem.

It looks like Noah Misch might have found another problem in this area.
We'll have to investigate that.
diff --git i/src/bin/pg_dump/pg_dump.c w/src/bin/pg_dump/pg_dump.c
index 5561295..4cea954 100644
--- i/src/bin/pg_dump/pg_dump.c
+++ w/src/bin/pg_dump/pg_dump.c
@@ -7889,6 +7889,7 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
 	int			ntups;
 	int			i_attname;
 	int			i_atttypdefn;
+	int			i_attisdropped;
 	int			i_typrelid;
 	int			i;
 
@@ -7900,11 +7901,11 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
 
 	appendPQExpBuffer(query, SELECT a.attname, 
 			pg_catalog.format_type(a.atttypid, a.atttypmod) AS atttypdefn, 
+	  a.attisdropped, 
 	  typrelid 
 	  FROM pg_catalog.pg_type t, pg_catalog.pg_attribute a 
 	  WHERE t.oid = '%u'::pg_catalog.oid 
 	  AND a.attrelid = t.typrelid 
-	  AND NOT a.attisdropped 
 	  ORDER BY a.attnum ,
 	  tyinfo-dobj.catId.oid);
 
@@ -7915,6 +7916,7 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
 
 	i_attname = PQfnumber(res, attname);
 	i_atttypdefn = PQfnumber(res, atttypdefn);
+	i_attisdropped = PQfnumber(res, attisdropped);
 	i_typrelid = PQfnumber(res, typrelid);
 
 	if (binary_upgrade)
@@ -7932,11 +7934,20 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
 	{
 		char	   *attname;
 		char	   *atttypdefn;
+		bool		attisdropped;
 
 		attname = PQgetvalue(res, i, i_attname);
 		atttypdefn = PQgetvalue(res, i, i_atttypdefn);
+		attisdropped = (PQgetvalue(res, i, i_attisdropped)[0] == 't');
 
-		appendPQExpBuffer(q, \n\t%s %s, fmtId(attname), atttypdefn);
+		if (attisdropped)
+		{
+			if (binary_upgrade)
+/* see under dumpTableSchema() */
+appendPQExpBuffer(q, \n\t%s INTEGER /* dummy */, fmtId(attname));
+		}
+		else
+			appendPQExpBuffer(q, \n\t%s %s, fmtId(attname), atttypdefn);
 		if (i  ntups - 1)
 			appendPQExpBuffer(q, ,);
 	}
@@ -12105,14 +12116,26 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 	  tbinfo-attlen[j],
 	  tbinfo-attalign[j]);
 	appendStringLiteralAH(q, tbinfo-attnames[j], fout);
-	appendPQExpBuffer(q, \n  AND attrelid = );
+	appendPQExpBuffer(q, \n  AND attrelid IN ();
 	appendStringLiteralAH(q, fmtId(tbinfo-dobj.name), fout);
-	appendPQExpBuffer(q, ::pg_catalog.regclass;\n);
+	appendPQExpBuffer(q, ::pg_catalog.regclass);
+	if (tbinfo-reloftype)
+		appendPQExpBuffer(q, , '%s'::pg_catalog.regclass, tbinfo-reloftype);
+	appendPQExpBuffer(q, );\n);
 
-	appendPQExpBuffer(q, ALTER TABLE ONLY %s ,
-	  fmtId(tbinfo-dobj.name));
-	appendPQExpBuffer(q, DROP COLUMN %s;\n,
-	  fmtId(tbinfo-attnames[j]));
+	if (tbinfo-reloftype)
+	{
+		appendPQExpBuffer(q, ALTER TYPE %s ,
+		  tbinfo-reloftype);
+		appendPQExpBuffer(q, DROP ATTRIBUTE %s CASCADE;\n,
+		  fmtId(tbinfo-attnames[j]));
+	}
+	else {
+		appendPQExpBuffer(q, ALTER TABLE ONLY %s ,
+		  fmtId(tbinfo-dobj.name));
+		appendPQExpBuffer(q, DROP COLUMN %s;\n,
+		  fmtId(tbinfo-attnames[j]));
+	}
 }
 else if (!tbinfo-attislocal[j])
 {

-- 
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 --binary-upgrade vs. ALTER TYPE ... DROP ATTRIBUTE

2011-03-30 Thread Peter Eisentraut
On tis, 2011-03-29 at 17:50 -0400, Noah Misch wrote:
 Fixing that looks clear enough, but the right fix for the typed table
 issue is less clear to me.  The pg_attribute tuples for a typed table
 will include any attributes dropped from the parent type after the
 table's creation, but not those attributes dropped before the table's
 creation.  Example:
 
 create type t as (x int, y int);
 create table is_a of t;
 alter type t drop attribute y cascade;
 create table is_a2 of t;
 select * from pg_attribute where attrelid = 'is_a'::regclass;
 select * from pg_attribute where attrelid = 'is_a2'::regclass;
 
 To reproduce that catalog state, the dump would need to create the
 type, create all typed tables predating the DROP ATTRIBUTE, and
 finally create typed tables postdating the DROP ATTRIBUTE.  That
 implies an extra dump entry for the DROP ATTRIBUTE with the
 appropriate dependencies to compel that order of events.  Is
 there a better way? 

Maybe we could just copy the dropped attributes from the type when the
table is created.  That might be as simple as removing the

if (attr-attisdropped)
continue;

in transformOfType().



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


[HACKERS] pg_upgrade and PGCLIENTENCODING

2011-03-30 Thread Peter Eisentraut
pg_upgrade complains if any of the libpq connection variables are set.
That's probably reasonable by default, but it seems to me that we ought
to allow keeping PGCLIENTENCODING set, otherwise various values and
error messages that are coming from the servers will not be in the
encoding appropriate for the terminal.


-- 
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] Another swing at JSON

2011-03-30 Thread Dimitri Fontaine
Andrew Dunstan and...@dunslane.net writes:
 I don't have any objection to putting some comments in the contrib Makefiles
 telling people to use PGXS, but I don't think that at this stage of the
 cycle we can start work on something that so far is just an idea from the
 top of my head.

I might be mistaken on how much work is hidden there, ok, you have the
point.  I will try to clarify this in the doc patch I intend to submit
soon'ish:  we still accept those while getting to beta, right?

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] Re: [COMMITTERS] pgsql: Fix plpgsql to release SPI plans when a function or DO block is

2011-03-30 Thread Jan Urbański
On 28/03/11 17:25, Stephen Frost wrote:
 * Jan Urbański (wulc...@wulczer.org) wrote:
 On 28/03/11 04:31, Tom Lane wrote:
 Do the other PLs we ship need similar fixes?

 Offhand I think the other PLs leave management of prepared plans to the
 user.  If there are any places where they cache plans behind the scenes,
 maybe a similar fix would be appropriate.

 FWIW I executed

 do $$ plpy.execute(select 1 from pg_class) $$ language plpythonu;

 10k times in a session and the backend grew a lot in memory and never
 released it. I can't offhand see where the memory went, I'll try to
 investigate in the evening.
 
 I'm about 90% sure that they all have this problem..  I havn't had a
 chance to look at how Tom fixed pl/pgsql (I didn't think it'd be easy to
 do w/o coming up with a way to explicitly tell the PL to release
 something) so perhaps I'm mistaken, but they all share very similar
 code..

Valgrind showed me the way. PFA a trivial patch to avoid leaking a
PLyProcedure struct in inline blocks.

Prepared plans get cleaned up currectly, the SPI plan is deallocated
when the Python plan object is garbage collected.

Cheers,
Jan
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index dd2b919..58d070f 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -626,6 +626,7 @@ plpython_inline_handler(PG_FUNCTION_ARGS)
 	PG_CATCH();
 	{
 		PLy_procedure_delete(proc);
+		PLy_free(proc);
 		PLy_curr_procedure = save_curr_proc;
 		PyErr_Clear();
 		PG_RE_THROW();
@@ -633,6 +634,7 @@ plpython_inline_handler(PG_FUNCTION_ARGS)
 	PG_END_TRY();
 
 	PLy_procedure_delete(proc);
+	PLy_free(proc);
 
 	/* Pop the error context stack */
 	error_context_stack = plerrcontext.previous;

-- 
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 --binary-upgrade vs. ALTER TYPE ... DROP ATTRIBUTE

2011-03-30 Thread Robert Haas
On Wed, Mar 30, 2011 at 12:55 PM, Peter Eisentraut pete...@gmx.net wrote:
 Maybe we could just copy the dropped attributes from the type when the
 table is created.  That might be as simple as removing the

        if (attr-attisdropped)
            continue;

 in transformOfType().

Seems a bit fragile...

-- 
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] Process local hint bit cache

2011-03-30 Thread Merlin Moncure
On Wed, Mar 30, 2011 at 11:23 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 On 30.03.2011 18:02, Robert Haas wrote:

 On Wed, Mar 30, 2011 at 10:40 AM, Greg Starkgsst...@mit.edu  wrote:

 But one way or another the hint bits have to get set sometime. The
 sooner that happens the less clog i/o has to happen in the meantime.

 I talked about this with Merlin a bit yesterday.  I think that his
 thought is that most transactions will access a small enough number of
 distinct CLOG pages, and the cache accesses might be fast enough, that
 we really wouldn't need to get the hint bits set, or at least that
 vacuum/freeze time would be soon enough.  I'm not sure if that's
 actually true, though - I think the overhead of the cache might be
 higher than he's imagining.  However, there's a sure-fire way to find
 out... code it up and see how it plays.

 I did a little experiment: I hacked SetHintBits() to do nothing, so that
 hint bits are never set. I then created a table with 10 rows in it:

 postgres=# \d foo
      Table public.foo
  Column |  Type   | Modifiers
 +-+---
  a      | integer |

 postgres=# INSERT INTO foo SELECT generate_series(1, 10);
 INSERT 0 10

 And ran queries on the table:

 postgres=# do $$
 declare
  i int4;
 begin
  loop
    perform COUNT(*) FROM foo;
  end loop;
 end;
 $$;

 This test case is designed to exercise the visibility tests as much as
 possible. However, all the tuples are loaded in one transaction, so the
 one-element cache in TransactionLogFetch is 100% effective.

 I ran oprofile on that. It shows that about 15% of the time is spent in
 HeapTupleSatisfiesMVCC and its subroutines. 6.6% is spent in
 HeapTupleSatisfiesMVCC itself. Here's the breakdown of that:

 $ opreport  -c --global-percent

 CPU: Intel Architectural Perfmon, speed 2266 MHz (estimated)
 Counted CPU_CLK_UNHALTED events (Clock cycles when not halted) with a unit
 mask of 0x00 (No unit mask) count 10
 samples  %        app name                 symbol name
 ...
 ---
  2143      0.4419  postgres                 postgres heapgettup_pagemode
  73277    15.1091  postgres                 postgres heapgetpage
 31858 6.5688  postgres                 postgres HeapTupleSatisfiesMVCC
  31858 6.5688  postgres                 postgres HeapTupleSatisfiesMVCC
 [self]
  12809     2.6411  postgres                 postgres
 TransactionIdIsInProgress
  12091     2.4931  postgres                 postgres XidInMVCCSnapshot
  7150      1.4743  postgres                 postgres
 TransactionIdIsCurrentTransactionId
  7056      1.4549  postgres                 postgres TransactionIdDidCommit
  1839      0.3792  postgres                 postgres TransactionIdPrecedes
  1467      0.3025  postgres                 postgres SetHintBits
  1155      0.2382  postgres                 postgres TransactionLogFetch
 ---
 ...

 I then ran the same test again with an unpatched version, to set the hint
 bits. After the hint bits were set, I ran oprofile again:

 ---
  275       0.4986  postgres                 heapgettup_pagemode
  4459      8.0851  postgres                 heapgetpage
 3005      5.4487  postgres                 HeapTupleSatisfiesMVCC
  3005      5.4487  postgres                 HeapTupleSatisfiesMVCC [self]
  1620      2.9374  postgres                 XidInMVCCSnapshot
  110       0.1995  postgres                 TransactionIdPrecedes
 ---

 So with hint bits set, HeapTupleSatisfiesMVCC accounts for 8% of the total
 CPU time, and without hint bits, 15%.

 Even if clog access was free, hint bits still give a significant speedup
 thanks to skipping all the other overhead like TransactionIdIsInProgress()
 and TransactionIdIsCurrentTransactionId(). Speeding up clog access is
 important; when the one-element cache isn't saving you the clog access
 becomes a dominant factor. But you have to address that other overhead too
 before you can get rid of hint bits.

Here is a patch demonstrating the caching action, but without the
cache table, which isn't done yet -- It only works using the very last
transaction id fetched.  I used macros so I could keep the changes
quite localized.

The payoff is obvious:

stock postgres:
postgres=# create table v as select generate_series(1,5000) v;
select count(*) from v;
SELECT 5000
Time: 70010.160 ms

select count(*) from v;
run 1: 64.5 seconds -- !
run 2: 21.3 seconds
run 3: 19.3 seconds

hint bit patch:
run 1: 19.2 seconds -- the 'money shot'
run 2: 20.7 seconds
run 3: 19.3 seconds

Of course, until I get the cache table mechanism finished, you only
see real benefit if you have significant # of pages all the same
transaction.  otoh, 

Re: [HACKERS] Problem with pg_upgrade?

2011-03-30 Thread Bruce Momjian
Peter Eisentraut wrote:
 On ons, 2011-03-30 at 10:57 -0400, Bruce Momjian wrote:
  Also, I am unclear if this is really our bug.  At least one of the
  systems was on Ubuntu/Debian, and they might both have been, and I know
  Debian changes our source code.  Where can I find a copy of the diffs
  they have made?
 
 http://bazaar.launchpad.net/~pitti/postgresql/debian-9.0/files/head:/debian/patches/

These all seem reasonable, but I am confused by this.  Someone reported
last week that the equals sign is not optional in postgreql.conf on
Debian but I don't see that patch in here.  Also I thought they modified
pg_hba.conf in some odd way.  Are these changes no longer made in 9.0?

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

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

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


Re: [HACKERS] pg_upgrade and PGCLIENTENCODING

2011-03-30 Thread Bruce Momjian
Peter Eisentraut wrote:
 pg_upgrade complains if any of the libpq connection variables are set.
 That's probably reasonable by default, but it seems to me that we ought
 to allow keeping PGCLIENTENCODING set, otherwise various values and
 error messages that are coming from the servers will not be in the
 encoding appropriate for the terminal.

What is the error message?  I don't see pg_upgrade checking
PGCLIENTENCODING.  I do check the cluster encodings.

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

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

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


Re: [HACKERS] Another swing at JSON

2011-03-30 Thread Alvaro Herrera
Excerpts from Dimitri Fontaine's message of mié mar 30 15:05:36 -0300 2011:
 Andrew Dunstan and...@dunslane.net writes:
  I don't have any objection to putting some comments in the contrib Makefiles
  telling people to use PGXS, but I don't think that at this stage of the
  cycle we can start work on something that so far is just an idea from the
  top of my head.
 
 I might be mistaken on how much work is hidden there, ok, you have the
 point.  I will try to clarify this in the doc patch I intend to submit
 soon'ish:  we still accept those while getting to beta, right?

Sure.

-- 
Álvaro Herrera alvhe...@commandprompt.com
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] proposal: a validator for configuration files

2011-03-30 Thread Selena Deckelmann
Hi!

On Wed, Mar 30, 2011 at 8:40 AM, Alexey Klyukin al...@commandprompt.com wrote:

 I'd like to add an ability to validate the corectness of PostgreSQL
 configuration files, i.e. postgresql.conf, pg_hba.conf, pg_ident.conf without
 actually applying them. The idea is that it would be a command-line option to
 postgres for a stand-alone case, or a user-callable function when postmaster
 is running.

 Per the former discussion of a validator for PostgreSQL configuration files
 (see http://archives.postgresql.org/pgsql-hackers/2008-08/msg00048.php),
 here's an implementation proposal.

I did a little bit of work on this, and we discussed it here:

http://archives.postgresql.org/pgsql-hackers/2009-03/msg00345.php
http://archives.postgresql.org/pgsql-hackers/2009-03/msg01142.php

Probably there's a bit of bitrot in there.

 The development plan consists of 2 parts.
 The first one is to add new code that would allow running the checks in both a
 stand-alone process, when postmaster is not running, and as a function call
 from a backend postgres process. Initially the code would simply loads
 configuration files, without performing any of the validation checks. The
 second part consists of adding specific checks.

Cool!  Mine was only going to work if the system started up or was reloaded.

 I think most of the code related to this feature should be put into the
 auxiliary process. The rationale is that we already have a stand-alone
 CheckerProcess, which nowadays only parses postgresql.conf, runs BaseInit and
 exists. We can easily load pg_hba.conf and pg_ident.conf and run all the
 necessary checks there. Moreover, the same process can be used when checks are
 launched from a built-in function. In that case, it would save the postgres
 backend from reloading postgresql.conf, pg_hba.conf and pg_ident.conf
 internally and restoring the previous configuration options when the function
 exists. Below is a more detailed description of implementation steps:

 1.1. Stand-alone process (postmaster is not running):

 - Add a new option (--check-config) to main.c. Run AuxiliaryProcessMain with
  auxType= CheckerProcess if this option is specified.
 - In CheckerModeMain load pg_hba.conf, pg_ident.conf


 1.2. Built-in function, called from a backend process.
 - Add a new built-in function
 - Add LastCheckState shared flag to denote whether the check has been
  successful or failed
 - Add a new PMSignalReason
 - From the function call SendPostmasterSignal with the reason added on the
  previous step.
 - In the postmaster add a check for this reason in sigusr1_handler, spawning
  a checker process. Set LastCheckStatus to InProgress.
 - Store current configuration options in AuxillaryProcessMain before reloading
  configuration files to avoid checking for options that haven't changed.
 - In AuxillaryProcessMain, modify SelectConfigFiles invocation to call it
  if IsUnderPostmaster = true if auxType process type is CheckerProcess.
 - In the CheckerModeMain, set LastCheckState to either success or failure at 
 the
  end of all checks.
 - The built-in function would wait until LastCheckState changes its value from
  InProgress to either Succcess or Failure, or until some predefined timeout
  expires, in which case it would emit an error.

 2. Add following configuration checks:

 postgresql.conf:

 Check that:
  1. postgres can bind to listen address:port
  2. unix_socket_directory has proper permissions (n/a on Windows).
  3. unix_socket_group group exists on a target system (n/a Windows).
  4. unix_socket_permissions are valid (write permission is not revoked from
        the owner, or a group, if group is different from the user).
  5. server.crt and server.key exist in the data directory if ssl is
        set to true (and server is compiled with openssl)
  6. krb_server_keyfile is readable by the server (if  set)
  7. server is not requesting more shared memory than it's available in the 
 system.
  8. shared_preload_libraries and local_preload_libraries exist.
  9. synchronous_standby_names are not empty and max_wal_senders
        are  0 if synchronous_replication = true
  10. all enable_*  query planner parameters are on.
  11. seq_page_cost = random_page_cost, and cpu_ costs are lower than page_ 
 costs.
  12. effective_cache_size is less than the amount of physical memory 
 available.
  13. geqo is turned on
  14. default_statistics_target is = 100
  15. logging collector is enabled if log destination is stderr
  16. log directory either exists and writable or that the parent
         directory allows creating subdris
  17. track counts is on if autovacuum is enabled
  18. stats_temp_directory is writeable
  19. default tablespace exists (if set)
  20. temp_tablespace exists (if set)
  21. statement_timeout is 0 (disabled)
  22. dynamic_library_path exists
  23. sql_inheritance is off
  24. zero_damaged_pages is off

Some of this seems not like things that postgres itself should be
doing, but more something that an 

Re: [HACKERS] Triggers on system catalog

2011-03-30 Thread Jan Wieck

On 3/30/2011 9:49 AM, Robert Haas wrote:

On Tue, Mar 29, 2011 at 9:40 PM, Stephen Frostsfr...@snowman.net  wrote:

 * Robert Haas (robertmh...@gmail.com) wrote:

 I do think we need some kind way of capturing DDL events, though. I wonder if 
the object-access-hook stuff KaiGai and I did to support SE-PostgreSQL could be 
extended to meet this need...


 My inclination would be 'probably', but it's not likely to really be the
 way we'd want to provide generalized DDL triggers..


I guess I was imagining that DDL triggers would be primarily important
for things like Slony, that are already writing C code anyway, but
maybe that's overly optimistic...



Slony is using C code in every performance critical path. Other than 
that, we are perfectly happy with PL/pgSQL code.


What I would envision for DDL triggers is that they first don't fire on 
an object type, but rather on a command completion code, like CREATE 
TABLE or DROP SCHEMA.


To do anything useful with that of course would require that all DDL 
does go through tcop's ProcessUtility and actually synthesizes a proper 
Utility parsetree. That isn't the case today, so there would be some 
previous clean up work to be done.



Jan

--
Anyone who trades liberty for security deserves neither
liberty nor security. -- Benjamin Franklin

--
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] deadlock_timeout at PGC_SIGHUP?

2011-03-30 Thread Robert Haas
On Tue, Mar 29, 2011 at 2:29 PM, Noah Misch n...@leadboat.com wrote:
 It's actually not
 clear to me what the user could usefully do other than trying to
 preserve his transaction by setting a high deadlock_timeout - what is
 the use case, other than that?

 The other major use case is reducing latency in deadlock-prone transactions.  
 By
 reducing deadlock_timeout for some or all involved transactions, the error 
 will
 arrive earlier.

Good point.

 Is it worth thinking about having an explicit setting for deadlock
 priority?  That'd be more work, of course, and I'm not sure it it's
 worth it, but it'd also provide stronger guarantees than you can get
 with this proposal.

 That is a better UI for the first use case.  I have only twice wished to tweak
 deadlock_timeout: once for the use case you mention, another time for that
 second use case.  Given that, I wouldn't have minded a rough UI.  If you'd use
 this often and assign more than two or three distinct priorities, you'd 
 probably
 appreciate the richer UI.  Not sure how many shops fall in that group.

Me neither.  If making the deadlock timeout PGC_SUSET is independently
useful, I don't object to doing that first, and then we can wait and
see if anyone feels motivated to do more.

(Of course, we're speaking of 9.2.)

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

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


Re: [HACKERS] Problem with pg_upgrade?

2011-03-30 Thread Robert Haas
On Wed, Mar 30, 2011 at 10:57 AM, Bruce Momjian br...@momjian.us wrote:
 I think we have three options:

        o  find if the use of autovacuum_freeze_max_age is safe, or make
           it safe
        o  document that autovacuum_naptime always happens before
           autovacuum does anything and set it high
        o  modify autovacuum to be an enum, with values on/off/disabled

 I think the last one makes more sense, and is safer if we need to
 backpatch this.  Creating a new variable for this would be confusing
 because it could conflict with the 'autovacuum' setting.

I have to admit the prospect of abuse is slightly frightening to me
here.  I guess we can't be held responsible for users who do dumb
things, but it might not be too clear to someone what the difference
is between autovacuum=off and autovacuum=disabled.  I don't really
understand why this is an issue in the first place, though.  Surely we
must be setting the XID counter on the new cluster to match the one on
the old cluster, and migrating the relfrozenxid and datfrozenxid
settings, so why does it matter if someone runs vacuum freeze?

-- 
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] Problem with pg_upgrade?

2011-03-30 Thread Jeff Davis
On Wed, 2011-03-30 at 16:46 -0400, Robert Haas wrote:
 I don't really
 understand why this is an issue in the first place, though.  Surely we
 must be setting the XID counter on the new cluster to match the one on
 the old cluster, and migrating the relfrozenxid and datfrozenxid
 settings, so why does it matter if someone runs vacuum freeze?

Because autovacuum may run before those things are properly set, as
Bruce said in the original email:

I am concerned that somehow autovaccum is running
in frozen mode before I have restored the frozen xids for the table or
database.

I think some kind of hidden GUC might be the best option. I tend to
agree that a third option to the autovacuum setting would be
confusing.

Regards,
Jeff Davis


-- 
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] Problem with pg_upgrade?

2011-03-30 Thread Peter Eisentraut
On ons, 2011-03-30 at 15:39 -0400, Bruce Momjian wrote:
 Peter Eisentraut wrote:
  On ons, 2011-03-30 at 10:57 -0400, Bruce Momjian wrote:
   Also, I am unclear if this is really our bug.  At least one of the
   systems was on Ubuntu/Debian, and they might both have been, and I know
   Debian changes our source code.  Where can I find a copy of the diffs
   they have made?
  
  http://bazaar.launchpad.net/~pitti/postgresql/debian-9.0/files/head:/debian/patches/
 
 These all seem reasonable, but I am confused by this.  Someone reported
 last week that the equals sign is not optional in postgreql.conf on
 Debian but I don't see that patch in here.

That's probably because some other tool processes to the configuration
file to find the data directory or something.

 Also I thought they modified
 pg_hba.conf in some odd way.  Are these changes no longer made in 9.0?

I think that was about 10 years ago.



-- 
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] Problem with pg_upgrade?

2011-03-30 Thread Bruce Momjian
Robert Haas wrote:
 On Wed, Mar 30, 2011 at 10:57 AM, Bruce Momjian br...@momjian.us wrote:
  I think we have three options:
 
  ? ? ? ?o ?find if the use of autovacuum_freeze_max_age is safe, or make
  ? ? ? ? ? it safe
  ? ? ? ?o ?document that autovacuum_naptime always happens before
  ? ? ? ? ? autovacuum does anything and set it high
  ? ? ? ?o ?modify autovacuum to be an enum, with values on/off/disabled
 
  I think the last one makes more sense, and is safer if we need to
  backpatch this. ?Creating a new variable for this would be confusing
  because it could conflict with the 'autovacuum' setting.
 
 I have to admit the prospect of abuse is slightly frightening to me
 here.  I guess we can't be held responsible for users who do dumb
 things, but it might not be too clear to someone what the difference
 is between autovacuum=off and autovacuum=disabled.  I don't really
 understand why this is an issue in the first place, though.  Surely we
 must be setting the XID counter on the new cluster to match the one on
 the old cluster, and migrating the relfrozenxid and datfrozenxid
 settings, so why does it matter if someone runs vacuum freeze?

First, I am not sure it is a problem, but based on the IRC reports I
felt I should ask here for confirmation.  Here is a sample pg_dump
output:

CREATE TABLE sample (
x integer
);

-- For binary upgrade, set relfrozenxid.
UPDATE pg_catalog.pg_class
SET relfrozenxid = '703'
WHERE oid = 'sample'::pg_catalog.regclass;

So, we set the cluster xid while we do this schema-only restore.  I
belive it might be possible for autovacuum to run while the schema is
restored, see an empty table, and set the relfrozenxid to be the current
xid, when in fact we are about to put a heap file in place of the
current empty file.  I thought the autovacuum_freeze_max_age=20
would prevent this but now I am not sure.  I assumed that since the gap
between the restored relfrozenxid and the current counter would
certainly be  20 that autovacuum would not touch it.  It is
possible these users had drastically modified autovacuum_freeze_max_age
to cause 3-billion gaps --- again, I have no direct contact with the
reporters, but I figured being paranoid is a good thing where pg_upgrade
is involved.

I wonder if the fact that these people never reported the bug means
there were doing something odd with their servers.

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

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

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


Re: [HACKERS] pg_upgrade and PGCLIENTENCODING

2011-03-30 Thread Peter Eisentraut
On ons, 2011-03-30 at 15:42 -0400, Bruce Momjian wrote:
 Peter Eisentraut wrote:
  pg_upgrade complains if any of the libpq connection variables are set.
  That's probably reasonable by default, but it seems to me that we ought
  to allow keeping PGCLIENTENCODING set, otherwise various values and
  error messages that are coming from the servers will not be in the
  encoding appropriate for the terminal.
 
 What is the error message?  I don't see pg_upgrade checking
 PGCLIENTENCODING.  I do check the cluster encodings.

It's done via check_for_libpq_envvars().

Actually, this is particular issue is new in 9.1, because the client
encoding wasn't a connection option before.



-- 
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] Problem with pg_upgrade?

2011-03-30 Thread Bruce Momjian
Bruce Momjian wrote:
 First, I am not sure it is a problem, but based on the IRC reports I
 felt I should ask here for confirmation.  Here is a sample pg_dump
 output:
 
   CREATE TABLE sample (
   x integer
   );
   
   -- For binary upgrade, set relfrozenxid.
   UPDATE pg_catalog.pg_class
   SET relfrozenxid = '703'
   WHERE oid = 'sample'::pg_catalog.regclass;
 
 So, we set the cluster xid while we do this schema-only restore.  I
 belive it might be possible for autovacuum to run while the schema is
 restored, see an empty table, and set the relfrozenxid to be the current
 xid, when in fact we are about to put a heap file in place of the
 current empty file.  I thought the autovacuum_freeze_max_age=20
 would prevent this but now I am not sure.  I assumed that since the gap
 between the restored relfrozenxid and the current counter would
 certainly be  20 that autovacuum would not touch it.  It is
 possible these users had drastically modified autovacuum_freeze_max_age
 to cause 3-billion gaps --- again, I have no direct contact with the
 reporters, but I figured being paranoid is a good thing where pg_upgrade
 is involved.
 
 I wonder if the fact that these people never reported the bug means
 there were doing something odd with their servers.

I just updated the C comment about what we are doing:

 * Using autovacuum=off disables cleanup vacuum and analyze, but
 * freeze vacuums can still happen, so we set
 * autovacuum_freeze_max_age very high.  We assume all datfrozenxid and
 * relfrozen values are less than a gap of 20 from the current
 * xid counter, so autovacuum will not touch them.

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

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

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


Re: [HACKERS] pg_upgrade and PGCLIENTENCODING

2011-03-30 Thread Bruce Momjian
Peter Eisentraut wrote:
 On ons, 2011-03-30 at 15:42 -0400, Bruce Momjian wrote:
  Peter Eisentraut wrote:
   pg_upgrade complains if any of the libpq connection variables are set.
   That's probably reasonable by default, but it seems to me that we ought
   to allow keeping PGCLIENTENCODING set, otherwise various values and
   error messages that are coming from the servers will not be in the
   encoding appropriate for the terminal.
  
  What is the error message?  I don't see pg_upgrade checking
  PGCLIENTENCODING.  I do check the cluster encodings.
 
 It's done via check_for_libpq_envvars().
 
 Actually, this is particular issue is new in 9.1, because the client
 encoding wasn't a connection option before.

Oh, we just blanket refuse any variable.  OK.  If you think it is safe,
feel free to apply a patch to skip it, or I can.

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

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

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


Re: [HACKERS] Problem with pg_upgrade?

2011-03-30 Thread Bruce Momjian
Bruce Momjian wrote:
  I wonder if the fact that these people never reported the bug means
  there were doing something odd with their servers.
 
 I just updated the C comment about what we are doing:
 
  * Using autovacuum=off disables cleanup vacuum and analyze, but
  * freeze vacuums can still happen, so we set
  * autovacuum_freeze_max_age very high.  We assume all datfrozenxid and
  * relfrozen values are less than a gap of 20 from the current
  * xid counter, so autovacuum will not touch them.

FYI, 20 is the maximum value for autovacuum_freeze_max_age, so a
user can't set it higher.

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

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

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


Re: [HACKERS] Process local hint bit cache

2011-03-30 Thread Merlin Moncure
On Wed, Mar 30, 2011 at 2:35 PM, Merlin Moncure mmonc...@gmail.com wrote:

 btw I haven't forgotten your idea to move TransactionIdInProgress
 Down. I think this is a good idea, and will experiment with it pre and
 post cache.

aside:
Moving TransactionIdInProgress below TransactionIdDidCommit can help
in once sense: TransactionIdDidCommit grabs the XidStatus but discards
the knowledge if the transaction is known aborted.   If it is in fact
aborted you can immediately set the hint bits and punt. This should
save an awful lot of calls to TransactionIdInProgress when scanning
unhinted dead tuples.

otoh, checking commit status first can burn you if you are repeatedly
tuples that are in transaction, especially your own.  Probably this
could be mitigated by splitting TransactionIdIsInProgress so that you
can do just the local checks and not shared memory, so that you could:
1. is this transaction mine? (if so, we are done)
2. get xid status if commit/abort done
3. do ProcArray portions of TransactionIdIsInProgress

merlin

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


Re: [HACKERS] Problem with pg_upgrade?

2011-03-30 Thread Robert Haas
On Wed, Mar 30, 2011 at 5:27 PM, Bruce Momjian br...@momjian.us wrote:
 First, I am not sure it is a problem, but based on the IRC reports I
 felt I should ask here for confirmation.  Here is a sample pg_dump
 output:

        CREATE TABLE sample (
            x integer
        );

        -- For binary upgrade, set relfrozenxid.
        UPDATE pg_catalog.pg_class
        SET relfrozenxid = '703'
        WHERE oid = 'sample'::pg_catalog.regclass;

 So, we set the cluster xid while we do this schema-only restore.  I
 belive it might be possible for autovacuum to run while the schema is
 restored, see an empty table, and set the relfrozenxid to be the current
 xid, when in fact we are about to put a heap file in place of the
 current empty file.  I thought the autovacuum_freeze_max_age=20
 would prevent this but now I am not sure.  I assumed that since the gap
 between the restored relfrozenxid and the current counter would
 certainly be  20 that autovacuum would not touch it.  It is
 possible these users had drastically modified autovacuum_freeze_max_age
 to cause 3-billion gaps --- again, I have no direct contact with the
 reporters, but I figured being paranoid is a good thing where pg_upgrade
 is involved.

It does seem possible that that could happen, but I'm not sure exactly
what would be causing autovacuum to fire in the first place.  It
wouldn't have to be triggered by the anti-wraparound machinery - if
the table appeared to be in need of vacuuming, then we'd vacuum it,
discover that is was empty, and update relfrozenxid.  Hmm... could it
fire just because the table has no stats?  But if that were the case
you'd think we'd be seeing this more often.

-- 
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 --binary-upgrade vs. ALTER TYPE ... DROP ATTRIBUTE

2011-03-30 Thread Noah Misch
On Wed, Mar 30, 2011 at 10:14:03AM -0400, Robert Haas wrote:
 On Tue, Mar 29, 2011 at 5:50 PM, Noah Misch n...@leadboat.com wrote:
  To reproduce that catalog state, the dump would need to create the type, 
  create
  all typed tables predating the DROP ATTRIBUTE, and finally create typed 
  tables
  postdating the DROP ATTRIBUTE. ?That implies an extra dump entry for the 
  DROP
  ATTRIBUTE with the appropriate dependencies to compel that order of events. 
  ?Is
  there a better way?
 
 I think so.  We have this same problem with regular table inheritance,
 and the way we fix it is to jigger the tuple descriptor for the child
 table so that it matches what we need, and THEN attach it to the
 parent:
snipped example
 I think we need to do something similar here -- use the same hack
 shown above to get the dropped column into the right state, and then
 jigger things so that the child is a typed table associated with the
 parent.

Good idea; I agree.

 Perhaps it would be reasonable to extend ALTER TABLE .. [NO]
 INHERIT to accept a type name as the final argument.  If used in this
 way, it converts a typed table into a regular table or visca versa.

Why extend ALTER TABLE ... INHERIT?  I would have guessed independent syntax.

 We could also do it with a direct catalog change, but there are some
 dependencies that would need to be frobbed, which makes me a bit
 reluctant to go that way.

Agreed; it's also an independently-useful capability to have.

-- 
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] Typed-tables patch broke pg_upgrade

2011-03-30 Thread Noah Misch
On Wed, Mar 30, 2011 at 07:50:12PM +0300, Peter Eisentraut wrote:
 On tor, 2011-02-10 at 06:31 +0200, Peter Eisentraut wrote:
   ERROR:  cannot drop column from typed table
   
   which probably is because test_type2 has a dropped column.
  
  It should call
  
  ALTER TYPE test_type2 DROP ATTRIBUTE xyz CASCADE;
  
  instead.  That will propagate to the table.
 
 Here is a patch that addresses this problem.

This only works when exactly one typed table uses each composite type having
dropped columns.  With zero users, the placeholder column never gets dropped.
Actually, it happens to work for 1 user, but only because ALTER TYPE mistakenly
only touches the first table-of-type:

create type t as (x int, y int);
create table is_a of t;
create table is_a2 of t;
alter type t drop attribute y cascade, add attribute z int cascade;
\d is_a
 Table public.is_a
 Column |  Type   | Modifiers
+-+---
 x  | integer |
 z  | integer |
Typed table of type: t
\d is_a2
 Table public.is_a2
 Column |  Type   | Modifiers
+-+---
 x  | integer |
 y  | integer |
Typed table of type: t

Might be a simple fix; looks like find_typed_table_dependencies() only grabs the
first match.  Incidentally, this led me to notice that you can hang a typed
table off a table row type.  ALTER TABLE never propagates to such typed tables,
allowing them to get out of sync:

create table t (x int, y int);
create table is_a of t;
create table is_a2 of t;
alter table t drop y, add z int;
\d is_a
 Table public.is_a
 Column |  Type   | Modifiers
+-+---
 x  | integer |
 y  | integer |
Typed table of type: t

Perhaps we should disallow the use of table row types in CREATE TABLE ... OF?

 It looks like Noah Misch might have found another problem in this area.
 We'll have to investigate that.

Your bits in dumpCompositeType() are most of what's needed to fix that, I think.

Thanks,
nm

-- 
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 --binary-upgrade vs. ALTER TYPE ... DROP ATTRIBUTE

2011-03-30 Thread Robert Haas
On Wed, Mar 30, 2011 at 9:30 PM, Noah Misch n...@leadboat.com wrote:
 Perhaps it would be reasonable to extend ALTER TABLE .. [NO]
 INHERIT to accept a type name as the final argument.  If used in this
 way, it converts a typed table into a regular table or visca versa.

 Why extend ALTER TABLE ... INHERIT?  I would have guessed independent syntax.

I just didn't feel the need to invent something new, but we could if
someone would rather.

 We could also do it with a direct catalog change, but there are some
 dependencies that would need to be frobbed, which makes me a bit
 reluctant to go that way.

 Agreed; it's also an independently-useful capability to have.

Yep.

-- 
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] Problem with pg_upgrade?

2011-03-30 Thread Bruce Momjian
Robert Haas wrote:
  So, we set the cluster xid while we do this schema-only restore. ?I
  belive it might be possible for autovacuum to run while the schema is
  restored, see an empty table, and set the relfrozenxid to be the current
  xid, when in fact we are about to put a heap file in place of the
  current empty file. ?I thought the autovacuum_freeze_max_age=20
  would prevent this but now I am not sure. ?I assumed that since the gap
  between the restored relfrozenxid and the current counter would
  certainly be  20 that autovacuum would not touch it. ?It is
  possible these users had drastically modified autovacuum_freeze_max_age
  to cause 3-billion gaps --- again, I have no direct contact with the
  reporters, but I figured being paranoid is a good thing where pg_upgrade
  is involved.
 
 It does seem possible that that could happen, but I'm not sure exactly
 what would be causing autovacuum to fire in the first place.  It
 wouldn't have to be triggered by the anti-wraparound machinery - if
 the table appeared to be in need of vacuuming, then we'd vacuum it,
 discover that is was empty, and update relfrozenxid.  Hmm... could it
 fire just because the table has no stats?  But if that were the case
 you'd think we'd be seeing this more often.

Well, autovacuum=off, so it should only run in freeze mode, and I can't
see how that could happen.  I am thinking I have to study autovacuum.c.

I wonder if datfrozenxid could be incremented because the database is
originally empty.  It would just need to scan pg_class, not actually
vacuum anything.  I wonder if we do that.  The bottom line is I am
hanging too much on autovacuum_freeze_max_age causing autovacuum to do
nothing.

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

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

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


Re: [HACKERS] Problem with pg_upgrade?

2011-03-30 Thread Bruce Momjian
Bruce Momjian wrote:
  It does seem possible that that could happen, but I'm not sure exactly
  what would be causing autovacuum to fire in the first place.  It
  wouldn't have to be triggered by the anti-wraparound machinery - if
  the table appeared to be in need of vacuuming, then we'd vacuum it,
  discover that is was empty, and update relfrozenxid.  Hmm... could it
  fire just because the table has no stats?  But if that were the case
  you'd think we'd be seeing this more often.
 
 Well, autovacuum=off, so it should only run in freeze mode, and I can't
 see how that could happen.  I am thinking I have to study autovacuum.c.
 
 I wonder if datfrozenxid could be incremented because the database is
 originally empty.  It would just need to scan pg_class, not actually
 vacuum anything.  I wonder if we do that.  The bottom line is I am
 hanging too much on autovacuum_freeze_max_age causing autovacuum to do
 nothing.

What if we allow autovacuum_max_workers to be set to zero;  the current
minimum is one.

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

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

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


Re: [HACKERS] Replication server timeout patch

2011-03-30 Thread Fujii Masao
On Wed, Mar 30, 2011 at 10:54 PM, Robert Haas robertmh...@gmail.com wrote:
 On Wed, Mar 30, 2011 at 4:08 AM, Fujii Masao masao.fu...@gmail.com wrote:
 On Wed, Mar 30, 2011 at 5:03 PM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 On 30.03.2011 10:58, Fujii Masao wrote:

 On Wed, Mar 30, 2011 at 4:24 PM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com  wrote:
 +        A value of zero means wait forever.  This parameter can only be
 set in

 The first sentence sounds misleading. Even if you set the parameter to
 zero,
 replication connections can be terminated because of keepalive or socket
 error.

 Hmm, should I change it back to A value of zero disables the timeout ? Any
 better suggestions?

 I like that. But I appreciate if anyone suggests the better.

 Maybe sticking the word mechanism in there would be a bit better.
 A value of zero disables the timeout mechanism?

I'm OK with that. Or, what about A value of zero turns this off which is
used in statement_timeout for the sake of consistency?

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] [GENERAL] Date conversion using day of week

2011-03-30 Thread Brendan Jurd
On 31 March 2011 03:15, Steve Crawford scrawf...@pinpointresearch.com wrote:
 On 03/29/2011 04:24 PM, Adrian Klaver wrote:
 ...
 Well the strange part is only fails for SUN:...
 test(5432)aklaver=select to_date('2011-13-SUN', 'IYYY-IW-DY');
   to_date
 
  2011-03-28
 ...

 You specified Sunday as the day but the date returned is a Monday. I would
 categorize that as a bug. (Hackers cc'd). Since Sunday is the last day of an
 ISO week, it should have returned 2011-04-03.

 My first inclination without consulting source or morning coffee is that
 PostgreSQL is seeing Sunday as day zero. Note that while:


The relevant paragraphs in the docs are:

--
An ISO week date (as distinct from a Gregorian date) can be specified
to to_timestamp and to_date in one of two ways:

* Year, week, and weekday: for example to_date('2006-42-4',
'IYYY-IW-ID') returns the date 2006-10-19. If you omit the weekday it
is assumed to be 1 (Monday).
* Year and day of year: for example to_date('2006-291',
'IYYY-IDDD') also returns 2006-10-19.

Attempting to construct a date using a mixture of ISO week and
Gregorian date fields is nonsensical, and will cause an error. In the
context of an ISO year, the concept of a month or day of month has
no meaning. In the context of a Gregorian year, the ISO week has no
meaning. Users should avoid mixing Gregorian and ISO date
specifications.
--

We *could* make the OP's query return the Sunday of ISO week 2011-13,
which would be properly written 2011-13-7, but I think the right move
here would be to throw the error for illegal mixture of format tokens.
 This is a trivial change -- just a matter of changing the from_date
type on the DAY, Day, day, DY, Dy, dy keys.

With the attached patch applied, this is what happens instead:

# select to_date('2011-13-SUN', 'IYYY-IW-DY');
ERROR:  invalid combination of date conventions
HINT:  Do not mix Gregorian and ISO week date conventions in a
formatting template.

If we wanted to make it work, then I think the thing to do would be
to add a new set of formatting tokens IDY, IDAY etc.  I don't like the
idea of interpreting DY and co. differently depending on whether the
other tokens happen to be ISO week or Gregorian.

Cheers,
BJ
diff --git a/src/backend/utils/adt/formatting.c 
b/src/backend/utils/adt/formatting.c
index 45e36f9..5ad6437 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -720,12 +720,12 @@ static const KeyWord DCH_keywords[] = {
{B.C., 4, DCH_B_C, FALSE, FROM_CHAR_DATE_NONE},   /* B */
{BC, 2, DCH_BC, FALSE, FROM_CHAR_DATE_NONE},
{CC, 2, DCH_CC, TRUE, FROM_CHAR_DATE_NONE},   /* C */
-   {DAY, 3, DCH_DAY, FALSE, FROM_CHAR_DATE_NONE},/* D */
+   {DAY, 3, DCH_DAY, FALSE, FROM_CHAR_DATE_GREGORIAN},/* D */
{DDD, 3, DCH_DDD, TRUE, FROM_CHAR_DATE_GREGORIAN},
{DD, 2, DCH_DD, TRUE, FROM_CHAR_DATE_GREGORIAN},
-   {DY, 2, DCH_DY, FALSE, FROM_CHAR_DATE_NONE},
-   {Day, 3, DCH_Day, FALSE, FROM_CHAR_DATE_NONE},
-   {Dy, 2, DCH_Dy, FALSE, FROM_CHAR_DATE_NONE},
+   {DY, 2, DCH_DY, FALSE, FROM_CHAR_DATE_GREGORIAN},
+   {Day, 3, DCH_Day, FALSE, FROM_CHAR_DATE_GREGORIAN},
+   {Dy, 2, DCH_Dy, FALSE, FROM_CHAR_DATE_GREGORIAN},
{D, 1, DCH_D, TRUE, FROM_CHAR_DATE_GREGORIAN},
{FX, 2, DCH_FX, FALSE, FROM_CHAR_DATE_NONE},  /* F */
{HH24, 4, DCH_HH24, TRUE, FROM_CHAR_DATE_NONE},   /* H */
@@ -768,10 +768,10 @@ static const KeyWord DCH_keywords[] = {
{b.c., 4, DCH_b_c, FALSE, FROM_CHAR_DATE_NONE},   /* b */
{bc, 2, DCH_bc, FALSE, FROM_CHAR_DATE_NONE},
{cc, 2, DCH_CC, TRUE, FROM_CHAR_DATE_NONE},   /* c */
-   {day, 3, DCH_day, FALSE, FROM_CHAR_DATE_NONE},/* d */
+   {day, 3, DCH_day, FALSE, FROM_CHAR_DATE_GREGORIAN},/* d */
{ddd, 3, DCH_DDD, TRUE, FROM_CHAR_DATE_GREGORIAN},
{dd, 2, DCH_DD, TRUE, FROM_CHAR_DATE_GREGORIAN},
-   {dy, 2, DCH_dy, FALSE, FROM_CHAR_DATE_NONE},
+   {dy, 2, DCH_dy, FALSE, FROM_CHAR_DATE_GREGORIAN},
{d, 1, DCH_D, TRUE, FROM_CHAR_DATE_GREGORIAN},
{fx, 2, DCH_FX, FALSE, FROM_CHAR_DATE_NONE},  /* f */
{hh24, 4, DCH_HH24, TRUE, FROM_CHAR_DATE_NONE},   /* h */

-- 
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: Fix plpgsql to release SPI plans when a function or DO block is

2011-03-30 Thread Heikki Linnakangas

On 30.03.2011 21:21, Jan Urbański wrote:

Valgrind showed me the way. PFA a trivial patch to avoid leaking a
PLyProcedure struct in inline blocks.


Hmm, any reason the PLyProcedure struct needs to be allocated in 
TopMemoryContext in the first place? Could you palloc0 it in a 
shorter-lived context, or even better, just allocate it in stack?


PS. I don't think the volatile qualifier in 'proc' is in necessary. The 
variable is not changed in PG_TRY-block.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index dd2b919..f3f5890 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -592,7 +592,7 @@ plpython_inline_handler(PG_FUNCTION_ARGS)
 	FunctionCallInfoData fake_fcinfo;
 	FmgrInfo	flinfo;
 	PLyProcedure *save_curr_proc;
-	PLyProcedure *volatile proc = NULL;
+	PLyProcedure proc;
 	ErrorContextCallback plerrcontext;
 
 	if (SPI_connect() != SPI_OK_CONNECT)
@@ -613,26 +613,26 @@ plpython_inline_handler(PG_FUNCTION_ARGS)
 	flinfo.fn_oid = InvalidOid;
 	flinfo.fn_mcxt = CurrentMemoryContext;
 
-	proc = PLy_malloc0(sizeof(PLyProcedure));
-	proc-pyname = PLy_strdup(__plpython_inline_block);
-	proc-result.out.d.typoid = VOIDOID;
+	MemSet(proc, 0, sizeof(PLyProcedure));
+	proc.pyname = PLy_strdup(__plpython_inline_block);
+	proc.result.out.d.typoid = VOIDOID;
 
 	PG_TRY();
 	{
-		PLy_procedure_compile(proc, codeblock-source_text);
-		PLy_curr_procedure = proc;
-		PLy_function_handler(fake_fcinfo, proc);
+		PLy_procedure_compile(proc, codeblock-source_text);
+		PLy_curr_procedure = proc;
+		PLy_function_handler(fake_fcinfo, proc);
 	}
 	PG_CATCH();
 	{
-		PLy_procedure_delete(proc);
+		PLy_procedure_delete(proc);
 		PLy_curr_procedure = save_curr_proc;
 		PyErr_Clear();
 		PG_RE_THROW();
 	}
 	PG_END_TRY();
 
-	PLy_procedure_delete(proc);
+	PLy_procedure_delete(proc);
 
 	/* Pop the error context stack */
 	error_context_stack = plerrcontext.previous;

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