Re: [HACKERS] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Mike Blackwell
​This would default to being available to superusers only, right?  Details
of the file system shouldn't be available to any random user.​

__
*Mike Blackwell | Technical Analyst, Distribution Services/Rollout
Management | RR Donnelley*
1750 Wallace Ave | St Charles, IL 60174-3401
Office: 630.313.7818
mike.blackw...@rrd.com
http://www.rrdonnelley.com


http://www.rrdonnelley.com/
* mike.blackw...@rrd.com*

On Fri, Jan 30, 2015 at 9:50 AM, Sawada Masahiko sawada.m...@gmail.com
wrote:

 On Sat, Jan 31, 2015 at 12:24 AM, David Fetter da...@fetter.org wrote:
  On Fri, Jan 30, 2015 at 09:38:10PM +0900, Sawada Masahiko wrote:
  On Tue, Jan 27, 2015 at 3:34 AM, Robert Haas robertmh...@gmail.com
 wrote:
   On Thu, Jan 22, 2015 at 5:19 PM, Tom Lane t...@sss.pgh.pa.us wrote:
   David Johnston david.g.johns...@gmail.com writes:
   On Thu, Jan 22, 2015 at 3:04 PM, Tom Lane t...@sss.pgh.pa.us
 wrote:
   Is that a requirement, and if so why?  Because this proposal
 doesn't
   guarantee any such knowledge AFAICS.
  
   The proposal provides for SQL access to all possible sources of
 variable
   value setting and, ideally, a means of ordering them in priority
 order, so
   that a search for TimeZone would return two records, one for
   postgresql.auto.conf and one for postgresql.conf - which are
 numbered 1 and
   2 respectively - so that in looking at that result if the
   postgresql.auto.conf entry were to be removed the user would know
 that what
   the value is in postgresql.conf that would become active.
 Furthermore, if
   postgresql.conf has a setting AND there is a mapping in an
 #included file
   that information would be accessible via SQL as well.
  
   I know what the proposal is.  What I am questioning is the use-case
 that
   justifies having us build and support all this extra mechanism.  How
 often
   does anyone need to know what the next down variable value would
 be?
  
   I believe I've run into situations where this would be useful.  I
   wouldn't be willing to put in the effort to do this myself, but I
   wouldn't be prepared to vote against a high-quality patch that someone
   else felt motivated to write.
  
 
  Attached patch adds new view pg_file_settings (WIP version).
  This view behaves like followings,
  [postgres][5432](1)=# select * from pg_file_settings ;
  name|  setting   |
sourcefile
 
 ++
   max_connections| 100|
  /home/masahiko/pgsql/master/3data/postgresql.conf
   shared_buffers | 128MB  |
  /home/masahiko/pgsql/master/3data/postgresql.conf
 
  This looks great!
 
  Is there a reason not to have the sourcefile as a column in
  pg_settings?
 

 pg_file_settings view also has source file column same as pg_settings.
 It might was hard to confirm that column in previous mail.
 I put example of pg_file_settings again as follows.

 [postgres][5432](1)=# select * from pg_file_settings where name =
 'work_mem';
 -[ RECORD 1 ]--
 name   | work_mem
 setting| 128MB
 sourcefile | /home/masahiko/pgsql/master/3data/hoge.conf
 -[ RECORD 2 ]--
 name   | work_mem
 setting| 64MB
 sourcefile | /home/masahiko/pgsql/master/3data/postgresql.auto.conf

 Regards,

 ---
 Sawada Masahiko


 --
 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] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Bruce Momjian
On Fri, Jan 30, 2015 at 11:12:43AM -0500, Robert Haas wrote:
 I think everyone who has read this mailing list for a while is
 probably already aware of this problem.  When you create a tablespace
 somewhere inside the data directory, weird things happen. If you
 pg_upgrade and then incautiously run the delete_old_cluster.sh script
 thus created, you will blow away large chunks of your data.[1]  If you

pg_upgrade doesn't create the deletion script in this case, and warns
the user:

Could not create a script to delete the old cluster's data
files because user-defined tablespaces exist in the old cluster
directory.  The old cluster's contents must be deleted manually.

 In the short term, I favor just adding a warning, so that people get
 some clue that they are doing something that might be a bad idea.  In
 the long term, we might want to do more.  Thoughts?

Yes, good idea.

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

  + Everyone has their own god. +


-- 
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] Providing catalog view to pg_hba.conf file - Patch submission

2015-01-30 Thread Robert Haas
On Thu, Jan 29, 2015 at 10:13 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 I think the big problem you are mentioning can be resolved in
 a similar way as we have done for ALTER SYSTEM which is
 to have a separate file (.auto.conf) for settings done via
 ALTER SYSTEM command, do you see any major problem
 with that approach.

Yes.  The contents of postgresql.conf are only mildly order-dependent.
If you put the same setting in more than once, it matters which one is
last.  Apart from that, though, it doesn't really matter:
wal_keep_segments=10 means the same thing if it occurs before
max_connections=401 that it means after that.  The same is not true of
pg_hba.conf, where the order matters a lot.  This makes merging two
files together much less feasible, and much more confusing.

You are also a lot more likely to lock yourself out of the database by
adjusting pg_hba.conf.  You can do that by modifying postgresql.conf,
say by putting an invalid combination of parameters in there or
getting it to request more semaphores or more RAM than the system can
accommodate or changing listen_addresses to 127.0.0.1, but there are
lots of things that you can do that carry no such risk.  This is much
less true with pg_hba.conf.  Even if I had a feature that would let me
modify pg_hba.conf remotely, I'm not sure I'd be brave enough to use
it.

Overall, this seems to me like a can of worms better left unopened.

-- 
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 with both --serializable-deferrable and -j

2015-01-30 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote:
 Kevin Grittner kgri...@ymail.com writes:

 I propose to apply the attached to master and back-patch to 9.3

 Objections?

 Only the nit-picky one that I quite dislike putting a comment 
 block inside an if-condition like that.

Comment moved above the if-condition, and pushed.

Thanks for the report, Alexander!

--
Kevin Grittner
EDB: 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] Exposing the stats snapshot timestamp to SQL

2015-01-30 Thread David Fetter
On Fri, Jan 30, 2015 at 12:07:22AM -0500, Tom Lane wrote:
 Matt Kelly mkell...@gmail.com writes:
  On Thu, Jan 29, 2015 at 8:42 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  ... or double.  Have you checked that the code behaves properly with
  --disable-integer-timestamps?
 
  I presume you meant --disable-integer-datetimes.  I just ran that test case
  now, all set.
 
 Right, my own sloppiness there.
 
  For my own edification, was there really any risk of something so trivial
  breaking due to that setting, or was it just for completeness? (which is a
  perfectly valid reason)
 
 I hadn't looked at your code at that point, and now that I have, I agree
 it's unlikely to have had a problem with this.  Still, it's a good thing
 to check, particularly considering the lack of buildfarm coverage of this
 option :-(.

Given that this has been deprecated for years, maybe it's time we took
it out in 9.5.  That the interested parties haven't bothered to put
buildfarm members in that use the option tells me that they're not all
that interested.

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

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] Buildfarm broken for 9.3 and up

2015-01-30 Thread Kevin Grittner
Kevin Grittner kgri...@ymail.com wrote:

 My push to branches 9.3 and up seems to have broken the buildfarm
 with this:

 error: object file 
 /home/pgfarm/buildroot/pgmirror.git/objects/93/d7706cbf2ce58e63ab8bbc9d16453b2c792ed4
  is empty
 error: unable to find 93d7706cbf2ce58e63ab8bbc9d16453b2c792ed4
 fatal: SHA1 COLLISION FOUND WITH 93d7706cbf2ce58e63ab8bbc9d16453b2c792ed4 !
 fatal: index-pack failed
 Your configuration specifies to merge with the ref 'master'
 from the remote, but no such ref was fetched.

 I don't see any problems on my machine

Since I posted the above some animals have built successfully, so 
it seems to be only affecting some animals.

--
Kevin Grittner
EDB: 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] Buildfarm broken for 9.3 and up

2015-01-30 Thread Andres Freund
On 2015-01-30 15:34:02 +, Kevin Grittner wrote:
 My push to branches 9.3 and up seems to have broken the buildfarm 
 with this:

I think this isn't anything related to your commit. Both racoon and
macaque have failed for a while. They just happened to run quickly after
your commit, giving the impression that two animals failed
consecutively.

Greetings,

Andres Freund

-- 
 Andres Freund 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] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Joshua D. Drake


On 01/30/2015 08:19 AM, Bruce Momjian wrote:


On Fri, Jan 30, 2015 at 11:12:43AM -0500, Robert Haas wrote:

I think everyone who has read this mailing list for a while is
probably already aware of this problem.  When you create a tablespace
somewhere inside the data directory, weird things happen. If you
pg_upgrade and then incautiously run the delete_old_cluster.sh script
thus created, you will blow away large chunks of your data.[1]  If you


pg_upgrade doesn't create the deletion script in this case, and warns
the user:

 Could not create a script to delete the old cluster's data
 files because user-defined tablespaces exist in the old cluster
 directory.  The old cluster's contents must be deleted manually.


In the short term, I favor just adding a warning, so that people get
some clue that they are doing something that might be a bad idea.  In
the long term, we might want to do more.  Thoughts?


Yes, good idea.


Uhm, wouldn't it be a rather simple patch to say:

if tablespace_create() in $PGDATA:
  ERROR!

?

I mean yes a warning is good but it is after the fact, the tablespace is 
already created. We know that tablespaces in $PGDATA are a bad idea, why 
not protect the user?


JD







--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Support, Training, Professional Services and Development
High Availability, Oracle Conversion, @cmdpromptinc
If we send our children to Caesar for their education, we should
 not be surprised when they come back as Romans.


--
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: knowing detail of config files via SQL

2015-01-30 Thread David Fetter
On Fri, Jan 30, 2015 at 09:38:10PM +0900, Sawada Masahiko wrote:
 On Tue, Jan 27, 2015 at 3:34 AM, Robert Haas robertmh...@gmail.com wrote:
  On Thu, Jan 22, 2015 at 5:19 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  David Johnston david.g.johns...@gmail.com writes:
  On Thu, Jan 22, 2015 at 3:04 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Is that a requirement, and if so why?  Because this proposal doesn't
  guarantee any such knowledge AFAICS.
 
  The proposal provides for SQL access to all possible sources of variable
  value setting and, ideally, a means of ordering them in priority order, so
  that a search for TimeZone would return two records, one for
  postgresql.auto.conf and one for postgresql.conf - which are numbered 1 
  and
  2 respectively - so that in looking at that result if the
  postgresql.auto.conf entry were to be removed the user would know that 
  what
  the value is in postgresql.conf that would become active.  Furthermore, if
  postgresql.conf has a setting AND there is a mapping in an #included file
  that information would be accessible via SQL as well.
 
  I know what the proposal is.  What I am questioning is the use-case that
  justifies having us build and support all this extra mechanism.  How often
  does anyone need to know what the next down variable value would be?
 
  I believe I've run into situations where this would be useful.  I
  wouldn't be willing to put in the effort to do this myself, but I
  wouldn't be prepared to vote against a high-quality patch that someone
  else felt motivated to write.
 
 
 Attached patch adds new view pg_file_settings (WIP version).
 This view behaves like followings,
 [postgres][5432](1)=# select * from pg_file_settings ;
 name|  setting   |
   sourcefile
 ++
  max_connections| 100|
 /home/masahiko/pgsql/master/3data/postgresql.conf
  shared_buffers | 128MB  |
 /home/masahiko/pgsql/master/3data/postgresql.conf

This looks great!

Is there a reason not to have the sourcefile as a column in
pg_settings?

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

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] Buildfarm broken for 9.3 and up

2015-01-30 Thread Stephen Frost
Kevin,

* Kevin Grittner (kgri...@ymail.com) wrote:
 My push to branches 9.3 and up seems to have broken the buildfarm 
 with this:

I don't think it has anything to do with your push.  A number of members
have built with your latest and look fine at this point, and I'm not
seeing any issues here either.

I do see that macque is having issues, but it's been busted for the past
3 days it looks like.

We have backups from prior to your commit, just in case, but I don't
think there's anything wrong.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Buildfarm broken for 9.3 and up

2015-01-30 Thread Tom Lane
Stephen Frost sfr...@snowman.net writes:
 * Kevin Grittner (kgri...@ymail.com) wrote:
 My push to branches 9.3 and up seems to have broken the buildfarm 
 with this:

 I don't think it has anything to do with your push.  A number of members
 have built with your latest and look fine at this point, and I'm not
 seeing any issues here either.

 I do see that macque is having issues, but it's been busted for the past
 3 days it looks like.

Yeah.  Several of the critters have been having git issues for weeks
to months.  I wonder whether we couldn't teach the buildfarm script
to recover from this automatically ...

regards, tom lane


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


Re: [HACKERS] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Sawada Masahiko
On Sat, Jan 31, 2015 at 12:58 AM, Mike Blackwell mike.blackw...@rrd.com wrote:
 This would default to being available to superusers only, right?  Details of
 the file system shouldn't be available to any random user.


This WIP patch does not behave like that, but I agree.
This view would be effective combine with ALTER SYSTEM,  and ALTER
SYSTEM command is executable to superuser only also.

Regards,

---
Sawada Masahiko


-- 
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] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
 Given all this, it seems like a good idea to at least give a warning
 if somebody tries to create a tablespace instead the data directory.

A warning seems like a good idea.  I actually thought we *did* prevent
it..

 Arguably, we should prohibit it altogether, but there are obviously
 people that want to do it, and there could even be somewhat valid
 reasons for that, like wanting to set per-tablespace settings
 differently for different tablespaces.  Possibly we should prohibit it
 anyway, or maybe there should be an option to create a tablespace
 whose directory is a real directory, not a symlink.  So then:
 
 CREATE TABLESPACE foo LOCATION '/home/rhaas/pgdata/pg_tblspc/foo';
 
 ...would fail, but if you really want a separate tablespace inside the
 data directory, we could allow:
 
 CREATE TABLESPACE foo NO LOCATION;
 
 ...which would just create a bare directory where the symlink would normally 
 go.

I actually really like this 'NO LOCATION' idea.  Are there reasons why
that would be difficult or ill-advised to do?

I could see the NO LOCATION approach being useful for migrating between
systems, in particular, or a way to have pg_basebackup work that doesn't
involve having to actually map all the tablespaces...

Thanks!

Stephen


signature.asc
Description: Digital signature


[HACKERS] Re: Overhauling our interrupt handling (was Escaping from blocked send() reprised.)

2015-01-30 Thread Andres Freund
On 2015-01-30 09:29:59 -0500, Robert Haas wrote:
 On Wed, Jan 14, 2015 at 9:03 PM, Andres Freund and...@2ndquadrant.com wrote:
  0002: Use a nonblocking socket for FE/BE communication and block using
latches.
 
Has previously been reviewed by Heikki. I think Noah also had a
look, although I'm not sure how close that was.
 
I think this can be committed soon.
 
 Doesn't this significantly increase the number of system calls?  I
 worry there could be a performance issue here.

I've posted benchmarks upthread and I only could start to measure any
overhead in pretty absurd cases (i.e. several hundred connections on a
few core machine, all doing SELECT 1;statements). As we try
the read before the poll/select it's not that bad - there's no
superflous work done if we're actually busy.

  0003: Introduce and use infrastructure for interrupt processing during 
  client reads.
 
From here on ImmediateInterruptOK isn't set during client
communication. Normal interrupts and sinval/async interrupts are
processed outside of signal handlers. Especially the sinval/async
greatly simplify the respective code.
 
 ProcessNotifyInterrupt() seems like it could lead to a failure to
 respond to other interrupts if there is a sufficiently vigorous stream
 of notify interrupts.

That's nothing new though. It just used to be executed inside interrupts
directly, with looping. And we looped when enabling the notify
interrupts. Since I can't recall a report of this being problematic I'm
not that inclined to change even more than the patch already does. Given
that queuing notifies requires a lock I have a hard time seing this ever
fast enough to cause that problem.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Sawada Masahiko
On Sat, Jan 31, 2015 at 12:24 AM, David Fetter da...@fetter.org wrote:
 On Fri, Jan 30, 2015 at 09:38:10PM +0900, Sawada Masahiko wrote:
 On Tue, Jan 27, 2015 at 3:34 AM, Robert Haas robertmh...@gmail.com wrote:
  On Thu, Jan 22, 2015 at 5:19 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  David Johnston david.g.johns...@gmail.com writes:
  On Thu, Jan 22, 2015 at 3:04 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Is that a requirement, and if so why?  Because this proposal doesn't
  guarantee any such knowledge AFAICS.
 
  The proposal provides for SQL access to all possible sources of variable
  value setting and, ideally, a means of ordering them in priority order, 
  so
  that a search for TimeZone would return two records, one for
  postgresql.auto.conf and one for postgresql.conf - which are numbered 1 
  and
  2 respectively - so that in looking at that result if the
  postgresql.auto.conf entry were to be removed the user would know that 
  what
  the value is in postgresql.conf that would become active.  Furthermore, 
  if
  postgresql.conf has a setting AND there is a mapping in an #included file
  that information would be accessible via SQL as well.
 
  I know what the proposal is.  What I am questioning is the use-case that
  justifies having us build and support all this extra mechanism.  How often
  does anyone need to know what the next down variable value would be?
 
  I believe I've run into situations where this would be useful.  I
  wouldn't be willing to put in the effort to do this myself, but I
  wouldn't be prepared to vote against a high-quality patch that someone
  else felt motivated to write.
 

 Attached patch adds new view pg_file_settings (WIP version).
 This view behaves like followings,
 [postgres][5432](1)=# select * from pg_file_settings ;
 name|  setting   |
   sourcefile
 ++
  max_connections| 100|
 /home/masahiko/pgsql/master/3data/postgresql.conf
  shared_buffers | 128MB  |
 /home/masahiko/pgsql/master/3data/postgresql.conf

 This looks great!

 Is there a reason not to have the sourcefile as a column in
 pg_settings?


pg_file_settings view also has source file column same as pg_settings.
It might was hard to confirm that column in previous mail.
I put example of pg_file_settings again as follows.

[postgres][5432](1)=# select * from pg_file_settings where name = 'work_mem';
-[ RECORD 1 ]--
name   | work_mem
setting| 128MB
sourcefile | /home/masahiko/pgsql/master/3data/hoge.conf
-[ RECORD 2 ]--
name   | work_mem
setting| 64MB
sourcefile | /home/masahiko/pgsql/master/3data/postgresql.auto.conf

Regards,

---
Sawada Masahiko


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


[HACKERS] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Robert Haas
I think everyone who has read this mailing list for a while is
probably already aware of this problem.  When you create a tablespace
somewhere inside the data directory, weird things happen. If you
pg_upgrade and then incautiously run the delete_old_cluster.sh script
thus created, you will blow away large chunks of your data.[1]  If you
try to use pg_basebackup, it will back up your data twice and maybe
throw some warnings.[2]  You can also induce pg_database_size() to
give wrong results --- it'll count pg_tblspace/$TABLESPACE_OID as well
as pg_tblspace/some-stupid-tablespace-name, the former being a symlink
to the latter.

Given all this, it seems like a good idea to at least give a warning
if somebody tries to create a tablespace instead the data directory.
Arguably, we should prohibit it altogether, but there are obviously
people that want to do it, and there could even be somewhat valid
reasons for that, like wanting to set per-tablespace settings
differently for different tablespaces.  Possibly we should prohibit it
anyway, or maybe there should be an option to create a tablespace
whose directory is a real directory, not a symlink.  So then:

CREATE TABLESPACE foo LOCATION '/home/rhaas/pgdata/pg_tblspc/foo';

...would fail, but if you really want a separate tablespace inside the
data directory, we could allow:

CREATE TABLESPACE foo NO LOCATION;

...which would just create a bare directory where the symlink would normally go.

In the short term, I favor just adding a warning, so that people get
some clue that they are doing something that might be a bad idea.  In
the long term, we might want to do more.  Thoughts?

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

[1] 
http://www.postgresql.org/message-id/b6f6fd62f2624c4c9916ac0175d56d880ce46...@jenmbs01.ad.intershop.net
[2] 
http://www.postgresql.org/message-id/cabuevexkhe+kcqa+flueaizp5i5qvcnnjz2j0zzqcamjfhe...@mail.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] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Robert Haas
On Fri, Jan 30, 2015 at 11:43 AM, Joshua D. Drake j...@commandprompt.com 
wrote:
 I mean yes a warning is good but it is after the fact, the tablespace is
 already created. We know that tablespaces in $PGDATA are a bad idea, why not
 protect the user?

Please go back and read the discussion of that option in the OP.

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


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


[HACKERS] Buildfarm broken for 9.3 and up

2015-01-30 Thread Kevin Grittner
My push to branches 9.3 and up seems to have broken the buildfarm 
with this:

error: object file 
/home/pgfarm/buildroot/pgmirror.git/objects/93/d7706cbf2ce58e63ab8bbc9d16453b2c792ed4
 is empty
error: unable to find 93d7706cbf2ce58e63ab8bbc9d16453b2c792ed4
fatal: SHA1 COLLISION FOUND WITH 93d7706cbf2ce58e63ab8bbc9d16453b2c792ed4 !
fatal: index-pack failed
Your configuration specifies to merge with the ref 'master'
from the remote, but no such ref was fetched.

I don't see any problems on my machine, although when I attempt to 
checkout the referenced SHA1 value on branch REL9_3_STABLE, I get 
this:

kgrittn@Kevin-Desktop:~/pg/REL9_3_STABLE$ git checkout 
93d7706cbf2ce58e63ab8bbc9d16453b2c792ed4
fatal: reference is not a tree: 93d7706cbf2ce58e63ab8bbc9d16453b2c792ed4

I can't find that SHA1 in my git log.

I have no clue what to do about this.

-- 
Kevin Grittner
EDB: 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] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread David Steele
On 1/30/15 11:43 AM, Joshua D. Drake wrote:
 On 01/30/2015 08:19 AM, Bruce Momjian wrote:

 On Fri, Jan 30, 2015 at 11:12:43AM -0500, Robert Haas wrote:
 I think everyone who has read this mailing list for a while is
 probably already aware of this problem.  When you create a tablespace
 somewhere inside the data directory, weird things happen. If you
 pg_upgrade and then incautiously run the delete_old_cluster.sh script
 thus created, you will blow away large chunks of your data.[1]  If you

 pg_upgrade doesn't create the deletion script in this case, and warns
 the user:

  Could not create a script to delete the old cluster's data
  files because user-defined tablespaces exist in the old cluster
  directory.  The old cluster's contents must be deleted
 manually.

 In the short term, I favor just adding a warning, so that people get
 some clue that they are doing something that might be a bad idea.  In
 the long term, we might want to do more.  Thoughts?

 Yes, good idea.

 Uhm, wouldn't it be a rather simple patch to say:

 if tablespace_create() in $PGDATA:
   ERROR!

 ?

 I mean yes a warning is good but it is after the fact, the tablespace
 is already created. We know that tablespaces in $PGDATA are a bad
 idea, why not protect the user?

I would be in favor of an error.  It would then be OK for basebackup,
pg_upgrade, and friends to error when a tablespace lives in $PGDATA,
rather than trying to deal with the situation in strange ways.

If the user really wants tablespaces in $PGDATA they can always change
the links manually in the filesystem and deal with any consequences on
their own. 

-- 
- David Steele
da...@pgmasters.net




signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] Hot Standby WAL reply uses heavyweight session locks, but doesn't have enough infrastructure set up

2015-01-30 Thread Andres Freund
On 2015-01-29 11:01:51 -0500, Robert Haas wrote:
 On Wed, Jan 28, 2015 at 2:41 AM, Michael Paquier
 michael.paqu...@gmail.com wrote:
  Andres Freund wrote:
  I think this isn't particularly pretty, but it seems to be working well
  enough, and changing it would be pretty invasive. So keeping in line
  with all that code seems to be easier.
  OK, I'm convinced with this part to remove the call of
  LockSharedObjectForSession that uses dontWait and replace it by a loop
  in ResolveRecoveryConflictWithDatabase.
 
 That seems right to me, too.

It's slightly more complicated than that. The lock conflict should
actually be resolved using ResolveRecoveryConflictWithLock()... That,
combined with the race of connecting a actually already deleted database
(see the XXXs I removed) seem to make the approach in here.

Attached are two patches:
1) Infrastructure for attaching more kinds of locks on the startup
   process.
2) Use that infrastructure for database locks during replay.

I'm not sure 2) alone would be sufficient justification for 1), but the
nearby thread about basebackups also require similar infrastructure...

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
From 8531ca4d200d24ee45265774f7ead613563adca4 Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Fri, 30 Jan 2015 09:28:17 +0100
Subject: [PATCH 1/4] Allow recovery lock infrastructure to not only hold
 relation locks.

This is a preparatory commit to fix several bugs, separated out for
easier review.

The startup process could so far only properly acquire access exlusive
locks on transactions. As it does not setup enough state to do lock
queuing - primarily to be able to issue recovery conflict interrupts,
and to release them when the primary releases locks - it has it's own
infrastructure to manage locks. That infrastructure so far assumed
that all locks are access exlusive locks on relations.

Unfortunately some code in the startup process has to acquire other
locks than what's supported by the aforementioned infrastructure in
standby.c. Namely dbase_redo() has to acquire locks on the database
objects.  Also further such locks will be added soon, to fix a another
bug.

So this patch shanges the infrastructure to be able to acquire locks
of different modes and locktags.

Additionally allow acquiring more heavyweight relation logs on the
standby than RowExclusive when acquired in session mode.

Discussion: 20150120152819.gc24...@alap3.anarazel.de

Backpatch all the way.
---
 src/backend/storage/ipc/standby.c | 120 ++
 src/backend/storage/lmgr/lock.c   |   7 +--
 src/include/storage/standby.h |   2 +-
 3 files changed, 61 insertions(+), 68 deletions(-)

diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 292bed5..0502aab 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -38,10 +38,16 @@ int			max_standby_archive_delay = 30 * 1000;
 int			max_standby_streaming_delay = 30 * 1000;
 
 static List *RecoveryLockList;
+typedef struct RecoveryLockListEntry
+{
+	TransactionId	xid;
+	LOCKMODE		lockmode;
+	LOCKTAG			locktag;
+} RecoveryLockListEntry;
 
 static void ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
 	   ProcSignalReason reason);
-static void ResolveRecoveryConflictWithLock(Oid dbOid, Oid relOid);
+static void ResolveRecoveryConflictWithLock(LOCKTAG *tag, LOCKMODE mode);
 static void SendRecoveryConflictWithBufferPin(ProcSignalReason reason);
 static XLogRecPtr LogCurrentRunningXacts(RunningTransactions CurrRunningXacts);
 static void LogAccessExclusiveLocks(int nlocks, xl_standby_lock *locks);
@@ -320,10 +326,10 @@ ResolveRecoveryConflictWithDatabase(Oid dbid)
 	 * us. This is rare enough that we do this as simply as possible: no wait,
 	 * just force them off immediately.
 	 *
-	 * No locking is required here because we already acquired
-	 * AccessExclusiveLock. Anybody trying to connect while we do this will
-	 * block during InitPostgres() and then disconnect when they see the
-	 * database has been removed.
+	 * No locking is required here because we already acquired a
+	 * AccessExclusiveLock on the database in dbase_redo(). Anybody trying to
+	 * connect while we do this will block during InitPostgres() and then
+	 * disconnect when they see the database has been removed.
 	 */
 	while (CountDBBackends(dbid)  0)
 	{
@@ -338,14 +344,11 @@ ResolveRecoveryConflictWithDatabase(Oid dbid)
 }
 
 static void
-ResolveRecoveryConflictWithLock(Oid dbOid, Oid relOid)
+ResolveRecoveryConflictWithLock(LOCKTAG *locktag, LOCKMODE mode)
 {
 	VirtualTransactionId *backends;
 	bool		lock_acquired = false;
 	int			num_attempts = 0;
-	LOCKTAG		locktag;
-
-	SET_LOCKTAG_RELATION(locktag, dbOid, relOid);
 
 	/*
 	 * If blowing away everybody with conflicting locks doesn't work, after
@@ -358,7 +361,7 

Re: [HACKERS] Buildfarm broken for 9.3 and up

2015-01-30 Thread Andrew Dunstan


On 01/30/2015 11:00 AM, Tom Lane wrote:



I do see that macque is having issues, but it's been busted for the past
3 days it looks like.

Yeah.  Several of the critters have been having git issues for weeks
to months.  I wonder whether we couldn't teach the buildfarm script
to recover from this automatically ...






Could get very messy, especially when the error is in the mirror, as is 
apparently the case here. As I mentioned the other day on the buildfarm 
list, I'm playing with a mode that would largely obviate the need for a 
mirror. Failure cases are what's worrying me, though.


In this particular case, the owner should probably remove the mirror and 
each of the branch pgsql directories.


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] Possible typo in create_policy.sgml

2015-01-30 Thread Stephen Frost
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
 Don't forget the ALTER POLICY page. This and some of the other things
 being discussed on this thread ought to be copied there too.

Thanks, I've fixed this also.

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Josh Berkus
Robert, Stephen, etc.:

Apparently you can create a tablespace in the tablespace directory:

josh=# create tablespace tbl location '/home/josh/pg94/data/pg_tblspc/';
CREATE TABLESPACE
josh=# create table test_tbl ( test text ) tablespace tbl;
CREATE TABLE
josh=# \q
josh@Radegast:~/pg94/data/pg_tblspc$ ls
17656  PG_9.4_201409291
josh@Radegast:~/pg94/data/pg_tblspc$ ls -l
total 4
lrwxrwxrwx 1 josh josh   30 Jan 30 13:02 17656 -
/home/josh/pg94/data/pg_tblspc
drwx-- 3 josh josh 4096 Jan 30 13:02 PG_9.4_201409291
josh@Radegast:~/pg94/data/pg_tblspc$

In theory if I could guess the next OID, I could cause a failure there,
but that appears to be obscure enough to be not worth bothering about.

What is a real problem is that we don't block creating tablespaces
anywhere at all, including in obviously problematic places like the
transaction log directory:

josh=# create tablespace tbl2 location '/home/josh/pg94/data/pg_xlog/';
CREATE TABLESPACE

It really seems like we ought to block *THAT*.  Of course, if we block
tablespace creation in PGDATA generally, then that's covered.

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


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


Re: [HACKERS] Possible typo in create_policy.sgml

2015-01-30 Thread Stephen Frost
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
 On 30 January 2015 at 03:40, Stephen Frost sfr...@snowman.net wrote:
  * Robert Haas (robertmh...@gmail.com) wrote:
  On Thu, Jan 29, 2015 at 9:04 PM, Stephen Frost sfr...@snowman.net wrote:
   A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows
   which match the relevant policy expression. Existing table rows are
   checked against the expression specified via USING, while new rows
   that would be created via INSERT or UPDATE are checked against the
   expression specified via WITH CHECK.  When a USING expression returns
   false for a given row, that row is not visible to the user.  When a WITH
   CHECK expression returns false for a row which is to be added, an error
   occurs.
 
  Yeah, that's not bad.  I think it's an improvement, in fact.
 
 Yes I like that too. My main concern was that we should be describing
 policies in terms of permitting access to the table, not limiting
 access, because of the default-deny policy, and this new text clears
 that up.

Great, thanks, pushed.

 One additional quibble -- it's misleading to say expression returns
 false here (and later in the check_expression parameter description)
 because if the expression returns null, that's also a failure. So it
 ought to be false or null, but perhaps it could just be described in
 terms of rows matching the expression, with a separate note to say
 that a row only matches a policy expression if that expression returns
 true, not false or null.

Good point, I've made a few minor changes to address that also, please
let me know if you see any issus.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Parallel Seq Scan

2015-01-30 Thread Stephen Frost
Daniel,

* Daniel Bausch (bau...@dvs.tu-darmstadt.de) wrote:
 I have been researching this topic long time ago.  One notably fact is
 that active prefetching disables automatic readahead prefetching (by
 Linux kernel), which can occour in larger granularities than 8K.
 Automatic readahead prefetching occours when consecutive addresses are
 read, which may happen by a seqscan but also by accident through an
 indexscan in correlated cases.

That strikes me as a pretty good point to consider.

 My consequence was to NOT prefetch seqscans, because OS does good enough
 without advice.  Prefetching indexscan heap accesses is very valuable
 though, but you need to detect the accidential sequential accesses to
 not hurt your performance in correlated cases.

Seems like we might be able to do that, it's not that different from
what we do with the bitmap scan case, we'd just look at the bitmap and
see if there's long runs of 1's.

 In general I can give you the hint to not only focus on HDDs with their
 single spindle.  A single SATA SSD scales up to 32 (31 on Linux)
 requests in parallel (without RAID or anything else).  The difference in
 throughput is extreme for this type of storage device.  While single
 spinning HDDs can only gain up to ~20% by NCQ, SATA SSDs can easily gain
 up to 700%.

I definitely agree with the idea that we should be looking at SSD-based
systems but I don't know if anyone happens to have easy access to server
gear with SSDs.  I've got an SSD in my laptop, but that's not really the
same thing.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0

2015-01-30 Thread Peter Geoghegan
On Fri, Jan 30, 2015 at 6:59 AM, Geoff Winkless pgsqlad...@geoff.dj wrote:
 I appreciate the work you're doing and (as a user rather than a
 pg-hacker) don't want to butt in but if it would be possible to allow
 support for IGNORE for unique but not exclusion constraints that would
 be really helpful for my own use cases, where being able to insert
 from a dataset into a table containing unique constraints without
 having to first check the dataset for uniqueness (within both itself
 and the target table) is very useful.

 It's possible that I've misunderstood anyway and that you mean purely
 that exclusion constraint IGNORE should be shelved until 9.6, in which
 case I apologise.

Well, the issue is that we can't really add exclusion constraints to
the IGNORE case later. So the fact that we cannot do exclusion
constraints kind of implies that we can either shelve IGNORE and maybe
look at it later, or accept that we'll never support exclusion
constraints with IGNORE. We'd then include IGNORE without exclusion
constraint support now and forever. I tend to think that we'll end up
doing the latter anyway, but I really don't want to add additional
risk of this not getting into 9.5 by arguing about that now. It
doesn't matter that much.

 I suppose there's no reason why we couldn't use a no-op ON CONFLICT
 UPDATE anyway

Right. IGNORE isn't really all that compelling for that reason. Note
that this will still lock the unmodified row, though.

-- 
Peter Geoghegan


-- 
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] Providing catalog view to pg_hba.conf file - Patch submission

2015-01-30 Thread Jim Nasby

On 1/29/15 9:13 PM, Amit Kapila wrote:

  Aside from Tom's concern about sets not being a good way to handle
this (which I agree with), the idea of editing pg_hba.conf via SQL
raises all the problems that were brought up when ALTER SYSTEM was being
developed. One of the big problems is a question of how you can safely
modify a text file that's full of comments and what-not. You'd need to
address those issues if you hope to modify pg_hba.conf via SQL.
 

I think the big problem you are mentioning can be resolved in
a similar way as we have done for ALTER SYSTEM which is
to have a separate file (.auto.conf) for settings done via
ALTER SYSTEM command, do you see any major problem
with that approach.


Yes I do. pg_hba.conf is completely depending on ordering, so there's no 
way you can simply toss another file into the mix. It's bad enough that 
we do that with postgresql.auto.conf, but at least that's a simple 
over-ride. With HBA a single ALTER SYSTEM could activate (or deactivate) 
a huge swath of pg_hba.conf. That makes for a system that's fragile, and 
since it's security related, dangerous.


I could maybe see an interface where we allowed users to perform 
line-level operations on pg_hba.conf via SQL: UPDATE line X, INSERT 
BEFORE/AFTER line X, DELETE line X. At least that would preserve the 
critical nature of rules ordering.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.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] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread David G Johnston
Robert Haas wrote
 Arguably, we should prohibit it altogether, but there are obviously
 people that want to do it, and there could even be somewhat valid
 reasons for that, 

Lots of hand-waving here and it is just as likely they simply are not aware
of the downsides and the only reason they put it is $PGDATA is that
it seemed like a logical place to put a directory that is intended to hold
database data.


 like wanting to set per-tablespace settings differently for different
 tablespaces.

I do not follow where this has anything to do with the location of the
physical tablespace directory?


 Possibly we should prohibit it
 anyway, or maybe there should be an option to create a tablespace
 whose directory is a real directory, not a symlink.  So then:
 
 CREATE TABLESPACE foo LOCATION '/home/rhaas/pgdata/pg_tblspc/foo';
 
 ...would fail, but if you really want a separate tablespace inside the
 data directory, we could allow:
 
 CREATE TABLESPACE foo NO LOCATION;
 
 ...which would just create a bare directory where the symlink would
 normally go.

CREATE TABLE foo LOCATION INTERNAL

The creators of tablespaces seem to have envisioned their usage as a means
of pulling in
disparate file systems and not simply for namespaces within the main
filesystem
that $PGDATA exists on.

This seems arbitrary and while the internal location specification likely
doesn't buy one much in terms of real options it doesn't seem like it has
any serious downsides either.


 In the short term, I favor just adding a warning, so that people get
 some clue that they are doing something that might be a bad idea.  In
 the long term, we might want to do more.  Thoughts?

If this is intended to be back-patched then I'd go with just a warning.  If
this is strictly 9.5 material then I'd say that since our own tools behave
badly in the current situation we should simply outright disallow it.  In
either case we should consider what tools we can provide to detect the
now-illegal configuration and, during pg_upgrade, configure the new cluster
to adhere to the correct configuration or help the user migrate their
internalized tablespaces to a different part of their filesystem.

Writing this I ponder the situation that someone would mount a different
file system directly under $PGDATA so that they get both benefits - single
parent and the different properties of the filesystems they are using.  If
we force Internal to be in the same location as the default tablespace we
only accomplish half of their goals.

David J.



--
View this message in context: 
http://postgresql.nabble.com/tablespaces-inside-PGDATA-considered-harmful-tp5836161p5836180.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] basebackups during ALTER DATABASE ... SET TABLESPACE ... not safe?

2015-01-30 Thread Andres Freund
On 2015-01-27 20:16:43 +0100, Andres Freund wrote:
 Here's an alternative approach. I think it generally is superior and
 going in the right direction, but I'm not sure it's backpatchable.
 
 It basically consists out of:
 1) Make GetLockConflicts() actually work.

already commited as being a independent problem.

 2) Allow the startup process to actually acquire locks other than
AccessExclusiveLocks. There already is code acquiring other locks,
but it's currently broken because they're acquired in blocking mode
which isn't actually supported in the startup mode. Using this
infrastructure we can actually fix a couple small races around
database creation/drop.
 3) Allow session locks during recovery to be heavier than
RowExclusiveLock - since relation/object session locks aren't ever
taken out by the user (without a plain lock first) that's safe.

merged and submitted independently.

 5) Make walsender actually react to recovery conflict interrupts

submitted here. (0003)

 4) Perform streaming base backups from within a transaction command, to
provide error handling.
 6) Prevent access to the template database of a CREATE DATABASE during
WAL replay.
 7) Add an interlock to prevent base backups and movedb() to run
concurrently. What we actually came here for.

combined, submitted here. (0004)

I think this actually doesn't look that bad.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
From 8531ca4d200d24ee45265774f7ead613563adca4 Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Fri, 30 Jan 2015 09:28:17 +0100
Subject: [PATCH 1/4] Allow recovery lock infrastructure to not only hold
 relation locks.

This is a preparatory commit to fix several bugs, separated out for
easier review.

The startup process could so far only properly acquire access exlusive
locks on transactions. As it does not setup enough state to do lock
queuing - primarily to be able to issue recovery conflict interrupts,
and to release them when the primary releases locks - it has it's own
infrastructure to manage locks. That infrastructure so far assumed
that all locks are access exlusive locks on relations.

Unfortunately some code in the startup process has to acquire other
locks than what's supported by the aforementioned infrastructure in
standby.c. Namely dbase_redo() has to acquire locks on the database
objects.  Also further such locks will be added soon, to fix a another
bug.

So this patch shanges the infrastructure to be able to acquire locks
of different modes and locktags.

Additionally allow acquiring more heavyweight relation logs on the
standby than RowExclusive when acquired in session mode.

Discussion: 20150120152819.gc24...@alap3.anarazel.de

Backpatch all the way.
---
 src/backend/storage/ipc/standby.c | 120 ++
 src/backend/storage/lmgr/lock.c   |   7 +--
 src/include/storage/standby.h |   2 +-
 3 files changed, 61 insertions(+), 68 deletions(-)

diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 292bed5..0502aab 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -38,10 +38,16 @@ int			max_standby_archive_delay = 30 * 1000;
 int			max_standby_streaming_delay = 30 * 1000;
 
 static List *RecoveryLockList;
+typedef struct RecoveryLockListEntry
+{
+	TransactionId	xid;
+	LOCKMODE		lockmode;
+	LOCKTAG			locktag;
+} RecoveryLockListEntry;
 
 static void ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
 	   ProcSignalReason reason);
-static void ResolveRecoveryConflictWithLock(Oid dbOid, Oid relOid);
+static void ResolveRecoveryConflictWithLock(LOCKTAG *tag, LOCKMODE mode);
 static void SendRecoveryConflictWithBufferPin(ProcSignalReason reason);
 static XLogRecPtr LogCurrentRunningXacts(RunningTransactions CurrRunningXacts);
 static void LogAccessExclusiveLocks(int nlocks, xl_standby_lock *locks);
@@ -320,10 +326,10 @@ ResolveRecoveryConflictWithDatabase(Oid dbid)
 	 * us. This is rare enough that we do this as simply as possible: no wait,
 	 * just force them off immediately.
 	 *
-	 * No locking is required here because we already acquired
-	 * AccessExclusiveLock. Anybody trying to connect while we do this will
-	 * block during InitPostgres() and then disconnect when they see the
-	 * database has been removed.
+	 * No locking is required here because we already acquired a
+	 * AccessExclusiveLock on the database in dbase_redo(). Anybody trying to
+	 * connect while we do this will block during InitPostgres() and then
+	 * disconnect when they see the database has been removed.
 	 */
 	while (CountDBBackends(dbid)  0)
 	{
@@ -338,14 +344,11 @@ ResolveRecoveryConflictWithDatabase(Oid dbid)
 }
 
 static void
-ResolveRecoveryConflictWithLock(Oid dbOid, Oid relOid)
+ResolveRecoveryConflictWithLock(LOCKTAG *locktag, 

Re: [HACKERS] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Tom Lane
David Johnston david.g.johns...@gmail.com writes:
 On Fri, Jan 30, 2015 at 12:05 PM, David Fetter da...@fetter.org wrote:
 Why might the contents of pg_settings be different from what would be
 in pg_file_settings, apart from the existence of this column?

 ​​The contents of pg_settings uses the setting name as a primary key.
 The contents of pg_file_setting uses a compound key consisting of the
 setting name and the source file.

[ raised eyebrow... ]  Seems insufficient in the case that multiple
settings of the same value occur in the same source file.  Don't you
need a line number in the key as well?

regards, tom lane


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


Re: [HACKERS] Safe memory allocation functions

2015-01-30 Thread Robert Haas
On Fri, Jan 30, 2015 at 1:10 AM, Michael Paquier
michael.paqu...@gmail.com wrote:
 I wrote:
 Yes, this refactoring was good for testing actually...
 Oops, I have been too hasty when sending previous patch, there was a
 bug related to huge allocations. Patch correcting this bug is
 attached.

Committed.  I didn't think we really need to expose two separate flags
for the aligned and unaligned cases, so I ripped that out.  I also
removed the duplicate documentation of the new constants in the
function header; having two copies of the documentation, one far
removed from the constants themselves, is a recipe for them eventually
getting out of sync.  I also moved the function to what I thought was
a more logical place in the file, and rearranged the order of tests
slightly so that, in the common path, we test only whether ret == NULL
and not anything else.

-- 
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] Proposal: knowing detail of config files via SQL

2015-01-30 Thread David Johnston
On Fri, Jan 30, 2015 at 12:05 PM, David Fetter da...@fetter.org wrote:

 On Sat, Jan 31, 2015 at 12:50:20AM +0900, Sawada Masahiko wrote:
 
  [postgres][5432](1)=# select * from pg_file_settings where name =
 'work_mem';
  -[ RECORD 1 ]--
  name   | work_mem
  setting| 128MB
  sourcefile | /home/masahiko/pgsql/master/3data/hoge.conf
  -[ RECORD 2 ]--
  name   | work_mem
  setting| 64MB
  sourcefile | /home/masahiko/pgsql/master/3data/postgresql.auto.conf

 Masuhiko-san,

 I apologize for not communicating clearly.  My alternative proposal is
 to add a NULL-able sourcefile column to pg_settings.  This is as an
 alternative to creating a new pg_file_settings view.

 Why might the contents of pg_settings be different from what would be
 in pg_file_settings, apart from the existence of this column?


​
​​The contents of pg_settings uses the setting name as a primary key.

The contents of pg_file_setting uses a compound key consisting of the
setting name and the source file.

See work_mem in the provided example.

More specifically pg_settings only care about the currently active
setting while this cares about inactive settings as well.

David J.
​


[HACKERS] Re: Overhauling our interrupt handling (was Escaping from blocked send() reprised.)

2015-01-30 Thread Heikki Linnakangas

On 01/15/2015 03:03 AM, Andres Freund wrote:

0004: Process 'die' interrupts while reading/writing from the client socket.

   This is the reason Horiguchi-san started this thread.



+   ProcessClientWriteInterrupt(!port-noblock);

...

+/*
+ * ProcessClientWriteInterrupt() - Process interrupts specific to client writes
+ *
+ * This is called just after low-level writes. That might be after the read
+ * finished successfully, or it was interrupted via interrupt. 'blocked' tells
+ * us whether the
+ *
+ * Must preserve errno!
+ */
+void
+ProcessClientWriteInterrupt(bool blocked)


You're passing port-noblock as argument, but I thought the argument is 
supposed to mean whether the write would've blocked, i.e. if the write 
buffer was full. port-noblock doesn't mean that. But perhaps I 
misunderstood this - the comment on the 'blocked' argument above is a 
bit incomplete ;-).


- Heikki



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


[HACKERS] Fwd: [GENERAL] 4B row limit for CLOB tables

2015-01-30 Thread Roger Pack
 On 1/29/15, Roger Pack rogerdpa...@gmail.com wrote:
 Hello.  I see on this page a mention of basically a 4B row limit for
 tables that have BLOB's

 Oops I meant for BYTEA or TEXT columns, but it's possible the
 reasoning is the same...

 It only applies to large objects, not bytea or text.

OK I think I figured out possibly why the wiki says this.  I guess
BYTEA entries  2KB will be autostored via TOAST, which uses an OID in
its backend.  So BYTEA has a same limitation.  It appears that
disabling TOAST is not an option [1].
So I guess if the number of BYTEA entries (in the sum all tables?
partitioning doesn't help?) with size  2KB is  4 billion then there
is actually no option there?  If this occurred it might cause all
sorts of things to break? [2]
Thanks!
-roger-

[1] 
http://www.postgresql.org/message-id/20130405140348.gc4...@awork2.anarazel.de
[2] 
http://www.postgresql.org/message-id/CAL1QdWfb-p5kE9DT2pMqBxohaKG=vxmdremsbjc+7tkboek...@mail.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


[HACKERS]

2015-01-30 Thread jeff . janes
From nobody Fri Jan 30 18:20:23 2015
Content-Type: text/plain; charset=utf-8
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: Parallel Seq Scan
To: pgsql-hackers@postgresql.org
From: Jeff Janes jeff.ja...@gmail.com
Date: Fri, 30 Jan 2015 18:20:23 +
User-Agent: pgcommitfest
X-cfsender: jjanes
In-Reply-To: 
caa4ek1jf8bxt2bhl-o-xjqk0rjn75_yrs9noekalryymaj9...@mail.gmail.com
References: caa4ek1ktv73ud9_w5wr4himz_hgi8osewncxbc53xwzdp-8...@mail.gmail.com
 caa4ek1jf8bxt2bhl-o-xjqk0rjn75_yrs9noekalryymaj9...@mail.gmail.com
Message-ID: 20150130182023.2534.29459.p...@coridan.postgresql.org

This patch depends on https://commitfest.postgresql.org/3/22/ 


-- 
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] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Josh Berkus
On 01/30/2015 09:19 AM, Stephen Frost wrote:
 * Robert Haas (robertmh...@gmail.com) wrote:
 Given all this, it seems like a good idea to at least give a warning
 if somebody tries to create a tablespace instead the data directory.
 
 A warning seems like a good idea.  I actually thought we *did* prevent
 it..
 
 Arguably, we should prohibit it altogether, but there are obviously
 people that want to do it, and there could even be somewhat valid
 reasons for that, like wanting to set per-tablespace settings
 differently for different tablespaces.  Possibly we should prohibit it
 anyway, or maybe there should be an option to create a tablespace
 whose directory is a real directory, not a symlink.  So then:

 CREATE TABLESPACE foo LOCATION '/home/rhaas/pgdata/pg_tblspc/foo';

 ...would fail, but if you really want a separate tablespace inside the
 data directory, we could allow:

 CREATE TABLESPACE foo NO LOCATION;

 ...which would just create a bare directory where the symlink would normally 
 go.
 
 I actually really like this 'NO LOCATION' idea.  Are there reasons why
 that would be difficult or ill-advised to do?
 
 I could see the NO LOCATION approach being useful for migrating between
 systems, in particular, or a way to have pg_basebackup work that doesn't
 involve having to actually map all the tablespaces...

I like this idea too.  And it would make tablespaces more manageable for
people who are using them for reasons other than putting them on
different disks.

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


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


Re: [HACKERS] PATCH: Reducing lock strength of trigger and foreign key DDL

2015-01-30 Thread Andreas Karlsson

On 01/30/2015 07:48 AM, Michael Paquier wrote:

Ok, so the deal is to finally reduce the locks to
ShareRowExclusiveLock for the following commands :
- CREATE TRIGGER
- ALTER TABLE ENABLE/DISABLE
- ALTER TABLE ADD CONSTRAINT


Correct. I personally still find this useful enough to justify a patch.


Looking at the latest patch, it seems that in
AlterTableGetLockLevel@tablecmds.c we ought to put AT_ReAddConstraint,
AT_AddConstraintRecurse and AT_ProcessedConstraint under the same
banner as AT_AddConstraint. Thoughts?


Good point. I think moving those would be a good thing even though it is 
technically not necessary for AT_AddConstraintRecurse, since that one 
should only be related to check constraints.


--
Andreas



--
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: knowing detail of config files via SQL

2015-01-30 Thread David Fetter
On Sat, Jan 31, 2015 at 12:50:20AM +0900, Sawada Masahiko wrote:
 On Sat, Jan 31, 2015 at 12:24 AM, David Fetter da...@fetter.org wrote:
  On Fri, Jan 30, 2015 at 09:38:10PM +0900, Sawada Masahiko wrote:
  On Tue, Jan 27, 2015 at 3:34 AM, Robert Haas robertmh...@gmail.com wrote:
   On Thu, Jan 22, 2015 at 5:19 PM, Tom Lane t...@sss.pgh.pa.us wrote:
   David Johnston david.g.johns...@gmail.com writes:
   On Thu, Jan 22, 2015 at 3:04 PM, Tom Lane t...@sss.pgh.pa.us wrote:
   Is that a requirement, and if so why?  Because this proposal doesn't
   guarantee any such knowledge AFAICS.
  
   The proposal provides for SQL access to all possible sources of 
   variable
   value setting and, ideally, a means of ordering them in priority 
   order, so
   that a search for TimeZone would return two records, one for
   postgresql.auto.conf and one for postgresql.conf - which are numbered 
   1 and
   2 respectively - so that in looking at that result if the
   postgresql.auto.conf entry were to be removed the user would know that 
   what
   the value is in postgresql.conf that would become active.  
   Furthermore, if
   postgresql.conf has a setting AND there is a mapping in an #included 
   file
   that information would be accessible via SQL as well.
  
   I know what the proposal is.  What I am questioning is the use-case that
   justifies having us build and support all this extra mechanism.  How 
   often
   does anyone need to know what the next down variable value would be?
  
   I believe I've run into situations where this would be useful.  I
   wouldn't be willing to put in the effort to do this myself, but I
   wouldn't be prepared to vote against a high-quality patch that someone
   else felt motivated to write.
  
 
  Attached patch adds new view pg_file_settings (WIP version).
  This view behaves like followings,
  [postgres][5432](1)=# select * from pg_file_settings ;
  name|  setting   |
sourcefile
  ++
   max_connections| 100|
  /home/masahiko/pgsql/master/3data/postgresql.conf
   shared_buffers | 128MB  |
  /home/masahiko/pgsql/master/3data/postgresql.conf
 
  This looks great!
 
  Is there a reason not to have the sourcefile as a column in
  pg_settings?
 
 
 pg_file_settings view also has source file column same as pg_settings.
 It might was hard to confirm that column in previous mail.
 I put example of pg_file_settings again as follows.
 
 [postgres][5432](1)=# select * from pg_file_settings where name = 'work_mem';
 -[ RECORD 1 ]--
 name   | work_mem
 setting| 128MB
 sourcefile | /home/masahiko/pgsql/master/3data/hoge.conf
 -[ RECORD 2 ]--
 name   | work_mem
 setting| 64MB
 sourcefile | /home/masahiko/pgsql/master/3data/postgresql.auto.conf

Masuhiko-san,

I apologize for not communicating clearly.  My alternative proposal is
to add a NULL-able sourcefile column to pg_settings.  This is as an
alternative to creating a new pg_file_settings view.

Why might the contents of pg_settings be different from what would be
in pg_file_settings, apart from the existence of this column?

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

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] Fwd: [GENERAL] 4B row limit for CLOB tables

2015-01-30 Thread Jim Nasby

On 1/30/15 11:54 AM, Roger Pack wrote:

On 1/29/15, Roger Pack rogerdpa...@gmail.com wrote:

Hello.  I see on this page a mention of basically a 4B row limit for
tables that have BLOB's


Oops I meant for BYTEA or TEXT columns, but it's possible the
reasoning is the same...


It only applies to large objects, not bytea or text.


OK I think I figured out possibly why the wiki says this.  I guess
BYTEA entries  2KB will be autostored via TOAST, which uses an OID in
its backend.  So BYTEA has a same limitation.  It appears that
disabling TOAST is not an option [1].
So I guess if the number of BYTEA entries (in the sum all tables?
partitioning doesn't help?) with size  2KB is  4 billion then there
is actually no option there?  If this occurred it might cause all
sorts of things to break? [2]


It's a bit more complex than that. First, toast isn't limited to bytea; 
it holds for ALL varlena fields in a table that are allowed to store 
externally. Second, the limit is actually per-table: every table gets 
it's own toast table, and each toast table is limited to 4B unique OIDs. 
Third, the OID counter is actually global, but the code should handle 
conflicts by trying to get another OID. See toast_save_datum(), which 
calls GetNewOidWithIndex().


Now, the reality is that GetNewOidWithIndex() is going to keep 
incrementing the global OID counter until it finds an OID that isn't in 
the toast table. That means that if you actually get anywhere close to 
using 4B OIDs you're going to become extremely unhappy with the 
performance of toasting new data.


I don't think it would be horrifically hard to change the way toast OIDs 
are assigned (I'm thinking we'd basically switch to creating a sequence 
for every toast table), but I don't think anyone's ever tried to push 
toast hard enough to hit this kind of limit.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.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] parallel mode and parallel contexts

2015-01-30 Thread Jim Nasby

On 1/30/15 11:08 AM, Robert Haas wrote:

The final patch attached her (parallel-dummy-v2.patch) has been
updated slightly to incorporate some prefetching logic.  It's still
just demo code and is not intended for commit.  I'm not sure whether
the prefetching logic can actually be made to improve performance,
either; if anyone feels like playing with that and reporting results
back, that would be swell.


Wouldn't we want the prefetching to happen after ReadBuffer() instead of 
before? This way we risk pushing the buffer we actually want out, plus 
if it's not already available the OS probably won't try and read it 
until it's read all the prefetch blocks.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


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


[HACKERS] Re: Overhauling our interrupt handling (was Escaping from blocked send() reprised.)

2015-01-30 Thread Heikki Linnakangas

On 01/15/2015 03:03 AM, Andres Freund wrote:

0002: Use a nonblocking socket for FE/BE communication and block using
   latches.


s/suceeds/succeeds/


0004: Process 'die' interrupts while reading/writing from the client socket.



+* Check for interrupts here, in addition to 
secure_write(),
+* because a interrupted write in secure_raw_write() 
can possibly
+* only return to here, not to secure_write().
 */
+   ProcessClientWriteInterrupt(true);


Took me a while to parse that sentence. How about:

Check for interrupts here, in addition to secure_write(), because an 
interrupted write in secure_raw_write() will return here, and we cannot 
return to secure_write() until we've written something.



0005: Move deadlock and other interrupt handling in proc.c out of signal 
handlers.

   I'm surprised how comparatively simple this turned out to be. For
   robustness and understanding I think this is a great improvement.

   New and not reviewed at all. Needs significant review. But works
   in my simple testcases.



@@ -799,6 +791,7 @@ ProcKill(int code, Datum arg)
proc = MyProc;
MyProc = NULL;
DisownLatch(proc-procLatch);
+   SetLatch(MyLatch);

SpinLockAcquire(ProcStructLock);

@@ -868,6 +861,7 @@ AuxiliaryProcKill(int code, Datum arg)
proc = MyProc;
MyProc = NULL;
DisownLatch(proc-procLatch);
+   SetLatch(MyLatch);

SpinLockAcquire(ProcStructLock);


These SetLatch calls are unnecessary. SwitchBackToLocalLatch() already 
sets the latch. (According to the comment in SwitchToSharedLatch() even 
that should not be necessary.)



0006: Don't allow immediate interupts during authentication anymore.


In commit message: s/interupts/interrupts/.


@@ -80,9 +73,6 @@ md5_crypt_verify(const Port *port, const char *role, char 
*client_pass,
if (*shadow_pass == '\0')
return STATUS_ERROR;/* empty password */

-   /* Re-enable immediate response to SIGTERM/SIGINT/timeout interrupts */
-   ImmediateInterruptOK = true;
-   /* And don't forget to detect one that already arrived */
CHECK_FOR_INTERRUPTS();

/*


That lone CHECK_FOR_INTERRUPTS() that remains looks pretty randomly 
placed now that the ImmediateInterruptOK = true is gone. I would just 
remove it. Add one to the caller if it's needed, but it probably isn't.



0007: Remove the option to service interrupts during PGSemaphoreLock().


s/don't/doesn't/ in commit message.


Questions I have about the series right now:

1) Do others agree that getting rid of ImmediateInterruptOK is
worthwile. I personally think that we really should pursue that
aggressively as a goal.


Yes.


2) Does anybody see a significant problem with the reduction of cases
where we immediately react to authentication_timeout? Since we still
react immediately when we're waiting for the client (except maybe in
sspi/kerberos?) I think that's ok. The waiting cases are slow
ident/radius/kerberos/ldap/... servers. But we really shouldn't jump
out of the relevant code anyway.


Yeah, I think that's OK. Doing those things with 
ImmediateInterruptOK=true was quite scary, and we need to stop doing 
that. It would be nice to have a wholesale solution for those lookups 
but I don't see one. So all the ident/radius/kerberos/ldap lookups will 
need to be done in a non-blocking way to have them react to the timeout. 
That requires some work, but we can leave that to a followup patch, if 
someone wants to write it.


Overall, 1-8 look good to me, except for that one incomplete comment in 
ProcessClientWriteInterrupt() I mentioned in a separate email. I haven't 
reviewed patches 9 and 10 yet.


- Heikki



--
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] PageRepairFragmentation performance

2015-01-30 Thread Peter Geoghegan
On Tue, Nov 18, 2014 at 10:03 AM, Heikki Linnakangas
hlinnakan...@vmware.com wrote:
 So there's clearly some room for improvement here. A couple of ideas:

 1. Replace the qsort with something cheaper. The itemid arrays being sorted
 are small, a few hundred item at most, usually even smaller. In this pgbench
 test case I used, the typical size is about 60. With a small array a plain
 insertion sort is cheaper than the generic qsort(), because it can avoid the
 function overhead etc. involved with generic qsort. Or we could use
 something smarter, like a radix sort, knowing that we're sorting small
 integers. Or we could implement an inlineable version of qsort and use that.

 I spent some time hacking approach 1, and replaced the qsort() call with a
 bucket sort. I'm not sure if a bucket sort is optimal, or better than a
 specialized quicksort implementation, but it seemed simple.

Nice result.

Most quicksort implementations, including our own, switch to insertion
sort if the number of elements to be sorted is low enough (It occurs
when n less than 7, in our case). Also, specializing qsort() with
integers in isolation showed about a 3x improvement last I checked,
which was a few years ago now, so cache misses (reductions in which we
can gain through compile-time specialization) are likely by far the
dominant consideration here.

pgbench is probably a very conservative way of estimating how big the
array can get, given the structure of those tables. Even 60 seems high
if we're looking for some kind of rough idea of an average.

How did you arrive at this value? It seems rather high:

+#define N_SORT_BUCKETS 32

I think you're not sufficiently clear on why you're using bucket sort
rather than some other alternative. Maybe it's so obvious to you that
you forgot. I *think* it's because it's reasonable to suppose that for
this particular use-case, you know that in general there'll be a more
or less even distribution between the buckets. Right? That seems to
make sense to me. All the work that qsort() does to make sure it has a
good pivot at each level of recursion may be wasted here.

The refactoring patch certainly looks very reasonable.
-- 
Peter Geoghegan


-- 
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: knowing detail of config files via SQL

2015-01-30 Thread Sawada Masahiko
On Sat, Jan 31, 2015 at 4:56 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 David Johnston david.g.johns...@gmail.com writes:
 On Fri, Jan 30, 2015 at 12:05 PM, David Fetter da...@fetter.org wrote:
 Why might the contents of pg_settings be different from what would be
 in pg_file_settings, apart from the existence of this column?

 The contents of pg_settings uses the setting name as a primary key.
 The contents of pg_file_setting uses a compound key consisting of the
 setting name and the source file.

 [ raised eyebrow... ]  Seems insufficient in the case that multiple
 settings of the same value occur in the same source file.  Don't you
 need a line number in the key as well?


Yep, a line number is also needed.
The source file and line number would be primary key of pg_file_settings view.
I need to change like that.

Regards,

---
Sawada Masahiko


-- 
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] Providing catalog view to pg_hba.conf file - Patch submission

2015-01-30 Thread Amit Kapila
On Fri, Jan 30, 2015 at 10:58 PM, Robert Haas robertmh...@gmail.com wrote:

 On Thu, Jan 29, 2015 at 10:13 PM, Amit Kapila amit.kapil...@gmail.com
wrote:
  I think the big problem you are mentioning can be resolved in
  a similar way as we have done for ALTER SYSTEM which is
  to have a separate file (.auto.conf) for settings done via
  ALTER SYSTEM command, do you see any major problem
  with that approach.

 Yes.  The contents of postgresql.conf are only mildly order-dependent.
 If you put the same setting in more than once, it matters which one is
 last.  Apart from that, though, it doesn't really matter:
 wal_keep_segments=10 means the same thing if it occurs before
 max_connections=401 that it means after that.  The same is not true of
 pg_hba.conf, where the order matters a lot.

Do you mean to say that as authentication system uses just the
first record that matches to perform authentication, it could lead
to problems if an order is not maintained?  Won't the same
set of problems can occur if user tries to that manually and do
it without proper care of such rules.  Now the problem with
command is that user can't see the order in which entries are
being made, but it seems to me that we can provide a view or some
way to user so that the order of entries is visible and the same is
allowed to be manipulated via command.

 This makes merging two
 files together much less feasible, and much more confusing.

 You are also a lot more likely to lock yourself out of the database by
 adjusting pg_hba.conf.

I think that could be even possible via Alter User .. password, if the
password is changed then also kind of user can be locked out of
database, basically it is also part of authentication mechanism.

 Even if I had a feature that would let me
 modify pg_hba.conf remotely, I'm not sure I'd be brave enough to use
 it.


Okay, but how about via some server side utility or some other way with
which users don't need to manually edit the file?

It seems to be that some of the other databases like Oracle also provide
a way for users to operate of similar files via commands, although in a
different way [1].

 Overall, this seems to me like a can of worms better left unopened.

Sure, I can understand the dangers you want to highlight, however
OTOH it seems to me that providing some way to users with which
they can change things without manually editing file is a good move.


[1]
http://docs.oracle.com/cd/B28359_01/network.111/b28317/lsnrctl.htm#i553656


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Sawada Masahiko
On Sat, Jan 31, 2015 at 3:20 PM, Sawada Masahiko sawada.m...@gmail.com wrote:
 On Sat, Jan 31, 2015 at 2:00 PM, Sawada Masahiko sawada.m...@gmail.com 
 wrote:
 On Sat, Jan 31, 2015 at 4:56 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 David Johnston david.g.johns...@gmail.com writes:
 On Fri, Jan 30, 2015 at 12:05 PM, David Fetter da...@fetter.org wrote:
 Why might the contents of pg_settings be different from what would be
 in pg_file_settings, apart from the existence of this column?

 The contents of pg_settings uses the setting name as a primary key.
 The contents of pg_file_setting uses a compound key consisting of the
 setting name and the source file.

 [ raised eyebrow... ]  Seems insufficient in the case that multiple
 settings of the same value occur in the same source file.  Don't you
 need a line number in the key as well?


 Yep, a line number is also needed.
 The source file and line number would be primary key of pg_file_settings 
 view.
 I need to change like that.


 Attached patch is latest version including following changes.
 - This view is available to super use only
 - Add sourceline coulmn

 Also I registered CF app.


Sorry, I registered unnecessary (extra) threads to CF app.
How can I delete them?

Regards,

---
Sawada Masahiko


-- 
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: knowing detail of config files via SQL

2015-01-30 Thread Sawada Masahiko
On Sat, Jan 31, 2015 at 2:00 PM, Sawada Masahiko sawada.m...@gmail.com wrote:
 On Sat, Jan 31, 2015 at 4:56 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 David Johnston david.g.johns...@gmail.com writes:
 On Fri, Jan 30, 2015 at 12:05 PM, David Fetter da...@fetter.org wrote:
 Why might the contents of pg_settings be different from what would be
 in pg_file_settings, apart from the existence of this column?

 The contents of pg_settings uses the setting name as a primary key.
 The contents of pg_file_setting uses a compound key consisting of the
 setting name and the source file.

 [ raised eyebrow... ]  Seems insufficient in the case that multiple
 settings of the same value occur in the same source file.  Don't you
 need a line number in the key as well?


 Yep, a line number is also needed.
 The source file and line number would be primary key of pg_file_settings view.
 I need to change like that.


Attached patch is latest version including following changes.
- This view is available to super use only
- Add sourceline coulmn

Also I registered CF app.

Regards,

---
Sawada Masahiko


000_pg_file_settings_view_v2.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


[HACKERS] Re: Overhauling our interrupt handling (was Escaping from blocked send() reprised.)

2015-01-30 Thread Robert Haas
On Wed, Jan 14, 2015 at 9:03 PM, Andres Freund and...@2ndquadrant.com wrote:
 0002: Use a nonblocking socket for FE/BE communication and block using
   latches.

   Has previously been reviewed by Heikki. I think Noah also had a
   look, although I'm not sure how close that was.

   I think this can be committed soon.

Doesn't this significantly increase the number of system calls?  I
worry there could be a performance issue here.

 0003: Introduce and use infrastructure for interrupt processing during client 
 reads.

   From here on ImmediateInterruptOK isn't set during client
   communication. Normal interrupts and sinval/async interrupts are
   processed outside of signal handlers. Especially the sinval/async
   greatly simplify the respective code.

ProcessNotifyInterrupt() seems like it could lead to a failure to
respond to other interrupts if there is a sufficiently vigorous stream
of notify interrupts.  I think there ought to be one loop that goes
through and tries to handle each kind of interrupt in turn and then
loops until no interrupts remain.

-- 
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] INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0

2015-01-30 Thread Geoff Winkless
On Thu, Jan 29, 2015 at 11:38 PM, Peter Geoghegan pg(at)heroku(dot)com wrote:
 Simply removing IGNORE support until such time as we straighten
 that all out (9.6?) seems like the simplest solution. No need to block
 the progress of UPSERT, since exclusion constraint support was
 only ever going to be useful for the less compelling IGNORE variant.
 What do other people think? Do you agree with my view that we should
 shelve IGNORE support for now, Heikki?

I appreciate the work you're doing and (as a user rather than a
pg-hacker) don't want to butt in but if it would be possible to allow
support for IGNORE for unique but not exclusion constraints that would
be really helpful for my own use cases, where being able to insert
from a dataset into a table containing unique constraints without
having to first check the dataset for uniqueness (within both itself
and the target table) is very useful.

It's possible that I've misunderstood anyway and that you mean purely
that exclusion constraint IGNORE should be shelved until 9.6, in which
case I apologise.

Of course if there's no way to allow unique constraint IGNORE without
exclusion constraints then just ignore me; I (along I'm sure with all
the others who are following this conversation from afar) will be
incredibly grateful for the work you've done either way.

I suppose there's no reason why we couldn't use a no-op ON CONFLICT
UPDATE anyway, but that does seem rather messy and (I imagine) would
involve rather more work (unless the optimizer were able to optimize
away the update? I don't know enough to be able to say if it would).

Thanks

Geoff


-- 
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] Possible typo in create_policy.sgml

2015-01-30 Thread Dean Rasheed
On 30 January 2015 at 03:40, Stephen Frost sfr...@snowman.net wrote:
 * Robert Haas (robertmh...@gmail.com) wrote:
 On Thu, Jan 29, 2015 at 9:04 PM, Stephen Frost sfr...@snowman.net wrote:
  A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows
  which match the relevant policy expression. Existing table rows are
  checked against the expression specified via USING, while new rows
  that would be created via INSERT or UPDATE are checked against the
  expression specified via WITH CHECK.  When a USING expression returns
  false for a given row, that row is not visible to the user.  When a WITH
  CHECK expression returns false for a row which is to be added, an error
  occurs.

 Yeah, that's not bad.  I think it's an improvement, in fact.


Yes I like that too. My main concern was that we should be describing
policies in terms of permitting access to the table, not limiting
access, because of the default-deny policy, and this new text clears
that up.

One additional quibble -- it's misleading to say expression returns
false here (and later in the check_expression parameter description)
because if the expression returns null, that's also a failure. So it
ought to be false or null, but perhaps it could just be described in
terms of rows matching the expression, with a separate note to say
that a row only matches a policy expression if that expression returns
true, not false or null.

Regards,
Dean


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


[HACKERS] Odd behavior of updatable security barrier views on foreign tables

2015-01-30 Thread Etsuro Fujita
Hi,

I noticed that when updating security barrier views on foreign tables,
we fail to give FOR UPDATE to selection queries issued at ForeignScan.
Here is an example.

postgres=# create foreign table base_ftbl (person text, visibility text)
server loopback options (table_name 'base_tbl');
CREATE FOREIGN TABLE
postgres=# create view rw_view as select person from base_ftbl where
visibility = 'public';
CREATE VIEW
postgres=# explain verbose delete from rw_view;
  QUERY PLAN
---
 Delete on public.base_ftbl  (cost=100.00..144.40 rows=14 width=6)
   Remote SQL: DELETE FROM public.base_tbl WHERE ctid = $1
   -  Foreign Scan on public.base_ftbl  (cost=100.00..144.40 rows=14
width=6)
 Output: base_ftbl.ctid
 Remote SQL: SELECT ctid FROM public.base_tbl WHERE ((visibility
= 'public'::text)) FOR UPDATE
(5 rows)

postgres=# alter view rw_view set (security_barrier = true);
ALTER VIEW
postgres=# explain verbose delete from rw_view;
QUERY PLAN
--
 Delete on public.base_ftbl base_ftbl_1  (cost=100.00..144.54 rows=14
width=6)
   Remote SQL: DELETE FROM public.base_tbl WHERE ctid = $1
   -  Subquery Scan on base_ftbl  (cost=100.00..144.54 rows=14 width=6)
 Output: base_ftbl.ctid
 -  Foreign Scan on public.base_ftbl base_ftbl_2
(cost=100.00..144.40 rows=14 width=6)
   Output: base_ftbl_2.ctid
   Remote SQL: SELECT ctid FROM public.base_tbl WHERE
((visibility = 'public'::text))
(7 rows)

Correct me if I am wrong.

Best regards,
Etsuro Fujita


-- 
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] jsonb, unicode escapes and escaped backslashes

2015-01-30 Thread Peter Geoghegan
On Thu, Jan 29, 2015 at 11:28 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 The point of JSONB is that we take a position on certain aspects like
 this. We're bridging a pointedly loosey goosey interchange format,
 JSON, with native PostgreSQL types. For example, we take a firm
 position on encoding. The JSON type is a bit more permissive, to about
 the extent that that's possible. The whole point is that we're
 interpreting JSON data in a way that's consistent with *Postgres*
 conventions. You'd have to interpret the data according to *some*
 convention in order to do something non-trivial with it in any case,
 and users usually want that.

 I quite agree with you, actually, in terms of that perspective.

Sure, but I wasn't sure that that was evident to others.

To emphasize: I think it's appropriate that the JSON spec takes
somewhat of a back seat approach to things like encoding and the
precision of numbers. I also think it's appropriate that JSONB does
not, up to and including where JSONB forbids things that the JSON spec
supposes could be useful. We haven't failed users by (say) not
accepting NULs, even though the spec suggests that that might be
useful - we have provided them with a reasonable, concrete
interpretation of that JSON data, with lots of useful operators, that
they may take or leave. It really isn't historical that we have both a
JSON and JSONB type. For other examples of this, see every document
database in existence.

Depart from this perspective, as an interchange standard author, and
you end up with something like XML, which while easy to reason about
isn't all that useful, or BSON, the binary interchange format, which
is an oxymoron.

 But my point remains: \u is not invalid JSON syntax, and neither is
 \u1234.  If we choose to throw an error because we can't interpret or
 process that according to our conventions, fine, but we should call it
 something other than invalid syntax.

 ERRCODE_UNTRANSLATABLE_CHARACTER or ERRCODE_CHARACTER_NOT_IN_REPERTOIRE
 seem more apropos from here.

I see. I'd go with ERRCODE_UNTRANSLATABLE_CHARACTER, then.
-- 
Peter Geoghegan


-- 
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] Redesigning checkpoint_segments

2015-01-30 Thread Heikki Linnakangas

On 01/30/2015 04:48 AM, Venkata Balaji N wrote:

I performed series of tests for this patch and would like to share the
results. My comments are in-line.


Thanks for the testing!


*Test 1 :*

In this test, i see removed+recycled segments = 3 (except for the first 3
checkpoint cycles) and has been steady through out until the INSERT
operation completed.

Actual calculation of CheckPointSegments = 3.2 ( is getting rounded up to 3
)

pg_xlog size is 128M and has increased to 160M max during the INSERT
operation.

shared_buffers = 128M
checkpoint_wal_size = 128M
min_recycle_wal_size = 80M
checkpoint_timeout = 5min


Hmm, did I understand correctly that pg_xlog peaked at 160MB, but most 
of the stayed at 128 MB? That sounds like it's working as designed; 
checkpoint_wal_size is not a hard limit after all.



b) Are the two GUCs, checkpoint_wal_size, and min_recycle_wal_size,
intuitive to set?


During my tests, I did not observe the significance of min_recycle_wal_size
parameter yet. Ofcourse, i had sufficient disk space for pg_xlog.

I would like to understand more about min_recycle_wal_size parameter. In
theory, i only understand from the note in the patch that if the disk space
usage falls below certain threshold, min_recycle_wal_size number of WALs
will be removed to accommodate future pg_xlog segments. I will try to test
this out. Please let me know if there is any specific test to understand
min_recycle_wal_size behaviour.


min_recycle_wal_size comes into play when you have only light load, so 
that checkpoints are triggered by checkpoint_timeout rather than 
checkpoint_wal_size. In that scenario, the WAL usage will shrink down to 
min_recycle_wal_size, but not below that. Did that explanation help? Can 
you suggest changes to the docs to make it more clear?


- Heikki



--
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] Performance improvement for joins where outer side is unique

2015-01-30 Thread David Rowley
On 1 January 2015 at 02:47, David Rowley dgrowle...@gmail.com wrote:

 Hi,

 I've been hacking a bit at the join code again... This time I've been
 putting some effort into  optimising the case where the inner side of the
 join is known to be unique.
 For example, given the tables:

 create table t1 (id int primary key);
 create table t2 (id int primary key);

 And query such as:

 select * from t1 left outer join t2 on t1.id=t2.id;

 It is possible to deduce at planning time that for each row in the outer
 relation, only 0 or 1 rows can exist in the inner relation, (inner being
 t2)


I've been hacking at this unique join idea again and I've now got it
working for all join types -- Patch attached.

Here's how the performance is looking:

postgres=# create table t1 (id int primary key);
CREATE TABLE
postgres=# create table t2 (id int primary key);
CREATE TABLE
postgres=# insert into t1 select x.x from generate_series(1,100) x(x);
INSERT 0 100
postgres=# insert into t2 select x.x from generate_series(1,100) x(x);
INSERT 0 100
postgres=# vacuum analyze;
VACUUM
postgres=# \q

Query: select count(t1.id) from t1 inner join t2 on t1.id=t2.id;

With Patch on master as of 32bf6ee

D:\Postgres\install\binpgbench -f d:\unijoin3.sql -T 60 -n postgres
transaction type: Custom query
scaling factor: 1
query mode: simple
number of clients: 1
number of threads: 1
duration: 60 s
number of transactions actually processed: 78
latency average: 769.231 ms
tps = 1.288260 (including connections establishing)
tps = 1.288635 (excluding connections establishing)

Master as of 32bf6ee

D:\Postgres\install\binpgbench -f d:\unijoin3.sql -T 60 -n postgres
transaction type: Custom query
scaling factor: 1
query mode: simple
number of clients: 1
number of threads: 1
duration: 60 s
number of transactions actually processed: 70
latency average: 857.143 ms
tps = 1.158905 (including connections establishing)
tps = 1.159264 (excluding connections establishing)

That's a 10% performance increase.

I still need to perform more thorough benchmarking with different data
types.

One weird thing that I noticed before is that in an earlier revision of the
patch in the executor's join Initialise node code, I had set the
unique_inner to true for semi joins and replaced the SEMI_JOIN check for a
unique_join check in the execute node for each join method. With this the
performance results barely changed from standard... I've yet to find out
why.

The patch also has added a property to the EXPLAIN (VERBOSE) output which
states if the join was found to be unique or not.

The patch also still requires a final pass of comment fix-ups. I've just
plain run out of time for now.

I'll pick this up in 2 weeks time.

Regards

David Rowley


unijoin_2015-01-31_9af9cfa.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] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Sawada Masahiko
On Tue, Jan 27, 2015 at 3:34 AM, Robert Haas robertmh...@gmail.com wrote:
 On Thu, Jan 22, 2015 at 5:19 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 David Johnston david.g.johns...@gmail.com writes:
 On Thu, Jan 22, 2015 at 3:04 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Is that a requirement, and if so why?  Because this proposal doesn't
 guarantee any such knowledge AFAICS.

 The proposal provides for SQL access to all possible sources of variable
 value setting and, ideally, a means of ordering them in priority order, so
 that a search for TimeZone would return two records, one for
 postgresql.auto.conf and one for postgresql.conf - which are numbered 1 and
 2 respectively - so that in looking at that result if the
 postgresql.auto.conf entry were to be removed the user would know that what
 the value is in postgresql.conf that would become active.  Furthermore, if
 postgresql.conf has a setting AND there is a mapping in an #included file
 that information would be accessible via SQL as well.

 I know what the proposal is.  What I am questioning is the use-case that
 justifies having us build and support all this extra mechanism.  How often
 does anyone need to know what the next down variable value would be?

 I believe I've run into situations where this would be useful.  I
 wouldn't be willing to put in the effort to do this myself, but I
 wouldn't be prepared to vote against a high-quality patch that someone
 else felt motivated to write.


Attached patch adds new view pg_file_settings (WIP version).
This view behaves like followings,
[postgres][5432](1)=# select * from pg_file_settings ;
name|  setting   |
  sourcefile
++
 max_connections| 100|
/home/masahiko/pgsql/master/3data/postgresql.conf
 shared_buffers | 128MB  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 dynamic_shared_memory_type | posix  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 log_timezone   | Japan  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 datestyle  | iso, mdy   |
/home/masahiko/pgsql/master/3data/postgresql.conf
 timezone   | Japan  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 lc_messages| C  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 lc_monetary| C  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 lc_numeric | C  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 lc_time| C  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 default_text_search_config | pg_catalog.english |
/home/masahiko/pgsql/master/3data/postgresql.conf
 work_mem   | 128MB  |
/home/masahiko/pgsql/master/3data/hoge.conf
 checkpoint_segments| 300|
/home/masahiko/pgsql/master/3data/hoge.conf
 enable_indexscan   | off|
/home/masahiko/pgsql/master/3data/postgresql.auto.conf
 work_mem   | 64MB   |
/home/masahiko/pgsql/master/3data/postgresql.auto.conf

[postgres][5432](1)=# select name, setting from pg_settings where name
= 'work_mem';
   name   | setting
--+-
 work_mem | 65536
(1 row)

Above query result shows that both hoge.conf and postgresql.auto.conf
have same config parameter work_mem, and work_mem in auto.conf is
effecitve.
We can confirm these parameter to decide if the postgresql.auto.conf
is useful or not.

Regards,

---
Sawada Masahiko


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


[HACKERS] NULL checks of deferenced pointers in picksplit method of intarray

2015-01-30 Thread Michael Paquier
Hi all,

Coverity is pointing out that _int_split.c has unnecessary checks for
deferenced pointers in 5 places.

-   if (inter_d != (ArrayType *) NULL)
-   pfree(inter_d);
+   pfree(inter_d);
In this case inter_d is generated by inner_int_inter, a routine that
always generates an ArrayType with at least new_intArrayType.

In two places there is as well this pattern:
-   if (datum_l)
-   pfree(datum_l);
-   if (union_dr)
-   pfree(union_dr);
+   pfree(datum_l);
+   pfree(union_dr);
And that one:
-   if (datum_r)
-   pfree(datum_r);
-   if (union_dl)
-   pfree(union_dl);
+   pfree(datum_r);
+   pfree(union_dl);
union_dr and union_dl are generated by inner_int_union which never
returns NULL. Similarly, datum_r and datum_l are created with
copy_intArrayType the first time, which never returns NULL, and their
values are changed at each loop step. Also, as far as I understood
from this code, no elements manipulated are NULL, perhaps this is
worth an assertion?

Attached is a patch to adjust those things.
Regards,
-- 
Michael
diff --git a/contrib/intarray/_int_gist.c b/contrib/intarray/_int_gist.c
index 53abcc4..876a7b9 100644
--- a/contrib/intarray/_int_gist.c
+++ b/contrib/intarray/_int_gist.c
@@ -416,9 +416,7 @@ g_int_picksplit(PG_FUNCTION_ARGS)
 			size_waste = size_union - size_inter;
 
 			pfree(union_d);
-
-			if (inter_d != (ArrayType *) NULL)
-pfree(inter_d);
+			pfree(inter_d);
 
 			/*
 			 * are these a more promising split that what we've already seen?
@@ -517,10 +515,8 @@ g_int_picksplit(PG_FUNCTION_ARGS)
 		/* pick which page to add it to */
 		if (size_alpha - size_l  size_beta - size_r + WISH_F(v-spl_nleft, v-spl_nright, 0.01))
 		{
-			if (datum_l)
-pfree(datum_l);
-			if (union_dr)
-pfree(union_dr);
+			pfree(datum_l);
+			pfree(union_dr);
 			datum_l = union_dl;
 			size_l = size_alpha;
 			*left++ = i;
@@ -528,10 +524,8 @@ g_int_picksplit(PG_FUNCTION_ARGS)
 		}
 		else
 		{
-			if (datum_r)
-pfree(datum_r);
-			if (union_dl)
-pfree(union_dl);
+			pfree(datum_r);
+			pfree(union_dl);
 			datum_r = union_dr;
 			size_r = size_beta;
 			*right++ = i;

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