date:20130506


On 03.05.2013 18:17, Fujii Masao wrote:

Hi,

I got the following assertion failure when I promoted the standby.

2013-05-04 00:12:31 JST sby1 LOG:  received promote request
2013-05-04 00:12:31 JST sby1 FATAL:  terminating walreceiver process
due to administrator command
2013-05-04 00:12:31 JST sby1 LOG:  redo done at 0/6FFE038
2013-05-04 00:12:31 JST sby1 LOG:  last completed transaction was at
log time 2013-05-04 00:12:25.878909+09
2013-05-04 00:12:31 JST sby1 LOG:  selected new timeline ID: 2
2013-05-04 00:12:31 JST sby1 LOG:  archive recovery complete
2013-05-04 00:12:31 JST sby1 LOG:  checkpoint starting:
TRAP: FailedAssertion(!(sentPtr= sendTimeLineValidUpto), File:
walsender.c, Line: 1465)
2013-05-04 00:12:31 JST sby1 LOG:  autovacuum launcher started

The way to reproduce this is:

1. Create one master A, one standby B, and one cascade standby C.
2. Run pgbench -i -s 10
3. Promote the standby B before pgbench -i finishes


I was able to reproduce this. The assertion checks that if the system is 
promoted at WAL location X, we must not have already sent WAL at  X to 
the client. As the code stands, that assumption is wrong; the walsender 
will merrily stream WAL that hasn't been replayed yet, and the system 
can be promoted before replaying all the WAL that has been streamed to a 
cascading standby. The comment in GetStandbyFlushRecPtr(), which is the 
function that determined how far the WAL may be streamed to a cascading 
standby, says this:



/*
 * We can safely send what's already been replayed. Also, if walreceiver
 * is streaming WAL from the same timeline, we can send anything that
 * it has streamed, but hasn't been replayed yet.
 */


There seems to be two bugs here:

1. This used to work in 9.2, because the startup process would always 
replay all the WAL present in pg_xlog before promoting (the WAL present 
in pg_xlog was streamed from master). But the refactorings in xlog.c in 
9.3devel broke that, so that the startup process can promote earlier.


2. Even after fixing the logic in xlog.c, there is still a corner-case 
where the startup process can promote before all the WAL that has been 
received from walreceiver has been received. That happens if the WAL 
streaming is terminated at a page boundary, rather than at a record 
boundary. For example, we might have received WAL up to the page 
boundary at 0/5BFA000, but the last *complete* record that we have 
received ends at 0/5BF9BD8.


To fix the second issue, I think two things need to happen. First, we 
need to suppress the check in walsender. Second, we need to teach the 
WAL replay to back off when that happens. At the moment, the replay in 
the cascading standby gets stuck, trying to fetch the next page 
containing rest of the partial WAL record. Instead, it should throw away 
the partial record it has, and resync at the end of the last replayed 
record. I think 9.2 has the same bug, BTW. Without support for timeline 
switches over streaming replication, it was just more difficult to hit.


- 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] Assertion failure at standby promotion


On 05.05.2013 12:13, Amit Langote wrote:

Hello,

I  tried reproducing the scenario. Note that I did not archive xlogs
(that is, archive_command = '/bin/true' and corresponding
restore_command = '/bin/false'). I performed the steps you mentioned
and could find following:
[snip]

[Standby-2]FATAL:  could not receive data from WAL stream: ERROR:  server 
switched off timeline 1 at 0/3510B14, but walsender already streamed up to 
0/3512000



Is this related to the assertion failure that you have reported?


Yep, same issue. If you compile with assertions disabled, you get that 
error instead of the assertion. They both check for the same thing, it 
was an oversight to have both.


- 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] 9.3 release notes suggestions

On May 5, 2013, at 11:51 AM, Andrew Dunstan and...@dunslane.net wrote:
 I can't off the top of my head see any good reason for zero padding, so I'm 
 with Tom.

Same here.

...Robert

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

Re: [HACKERS] The missing pg_get_*def functions

Andres Freund and...@2ndquadrant.com writes:
 On 2013-04-30 05:14:15 +0100, Joel Jacobson wrote:
 It would be better to find a way to update sql-language functions in
 minor upgrades, instead of shutting that door entirely for all future
 implementation ideas involving sql-language functions in the
 pg_catalog.

 I'd be very careful with jumping on this task. I am pretty sure its a
 very good way to get very, very frustrated if you don't present a widely
 accepted design beforehand. Doing this correctly is *far far* from easy.

 Just a little collection of problems:
 * You need to connect to all databases, not just one. There's no
   infrastructure for this.

I wonder if it wouldn't be possible to have a per database catalog
version and do the work either at first database connection or first use
of the modified function, with some more smarts (catversion changed, and
a list of tableoid, oid changes in the catalogs somewhere).

But basically, that means that I agree that doing this correctly is very
far from being easy. And maybe improving the extension feature list is
an easier way forward.

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


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

Re: [HACKERS] The missing pg_get_*def functions

* Dimitri Fontaine (dimi...@2ndquadrant.fr) wrote:
 Andres Freund and...@2ndquadrant.com writes:
  Just a little collection of problems:
  * You need to connect to all databases, not just one. There's no
infrastructure for this.
 
 I wonder if it wouldn't be possible to have a per database catalog
 version and do the work either at first database connection or first use
 of the modified function, with some more smarts (catversion changed, and
 a list of tableoid, oid changes in the catalogs somewhere).

I like the idea of having a per-database catversion, but we'd have to
have a cluster-wide version for the shared catalogs too.  I don't think
it's a good idea to wait until the first connection to a DB to apply the
update either..  My thinking was that we'd just kick off a backend for
each database (perhaps using the background worker structure) which
would then apply the update and exit, while the postmaster would wait
until all of these workers have finished before allowing general
connections.

 But basically, that means that I agree that doing this correctly is very
 far from being easy. And maybe improving the extension feature list is
 an easier way forward.

It likely is, but it would really be nice to be able to do catalog
updates like these in a better fashion than sticking some update command
into the release notes and hoping that someone reads them and runs the
command..

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] The missing pg_get_*def functions

Stephen Frost sfr...@snowman.net writes:
 It likely is, but it would really be nice to be able to do catalog
 updates like these in a better fashion than sticking some update command
 into the release notes and hoping that someone reads them and runs the
 command..

Agreed.

Another advantage of using more the extension infrastructure is that
shipping bug fixes in the C or SQL parts of them would allow a hot fix
to be shipped without restart when limited to an extension.

In-core installed-by-default extensions, anyone?

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


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

Re: [HACKERS] The missing pg_get_*def functions

* Dimitri Fontaine (dimi...@2ndquadrant.fr) wrote:
 Another advantage of using more the extension infrastructure is that
 shipping bug fixes in the C or SQL parts of them would allow a hot fix
 to be shipped without restart when limited to an extension.

I'm actually not thrilled with the security update situation when it
comes to extensions.  It really is a disservice to our users to ask them
to independently manage each and every extension and upgrade each one of
them by hand.

 In-core installed-by-default extensions, anyone?

I'm not against this idea- but we *still* need to solve the problem of
how we update the catalog during a point release and, imv anyway, we
would need to be able to upgrade any in-core installed-by-default
extensions during a point release too, to address any security or other
issues from them.  Right now we get to slide by on upgrading extensions
by not having any in-core / installed-by-default ones and punting to the
user with well, you installed it, not us, therefore you have to manage
it by hand for eternity; that doesn't work when we're installing it for
them and they may not even know they've got it..

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] The missing pg_get_*def functions

On 2013-05-06 14:34:52 +0200, Dimitri Fontaine wrote:
 In-core installed-by-default extensions, anyone?

We already have that in plpgsql ...

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] 9.3 release notes suggestions

On Mon, May  6, 2013 at 06:54:06AM -0400, Robert Haas wrote:
 On May 5, 2013, at 11:51 AM, Andrew Dunstan and...@dunslane.net wrote:
  I can't off the top of my head see any good reason for zero padding, so I'm 
  with Tom.
 
 Same here.

Agreed, reverted.

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

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


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

Re: [HACKERS] The missing pg_get_*def functions

Stephen Frost sfr...@snowman.net writes:
 I'm not against this idea- but we *still* need to solve the problem of
 how we update the catalog during a point release and, imv anyway, we
 would need to be able to upgrade any in-core installed-by-default
 extensions during a point release too, to address any security or other

In case it wasn't clear, I agree with your view here and consider the
capability to auto-upgrade extensions a must have. What I say is that if
you ship the .so part of an extension in a live system, the next backend
that starts will use that code, without a restart.

That does not allow us not to provide a way to force-reload modules
currently used in live backends, and we still need to be able to upgrade
the system catalogs and extension catalogs too, either at startup in
live operations.

Separating away some code and SQL into in-core installed-by-default
extensions means we have new problems and abilities, not that some
problem are solved by themselves.

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


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

[HACKERS] about index inheritance

2013-05-06 Thread Vincenzo Melandri

Hi guys,

My first post here :)
I stumbled into the same problem as this guy
http://www.postgresql.org/message-id/4be2835a.5020...@cybertec.at
, so since I have some spare time recently, I've set-up the development
environment for postgresql and I think I may be able to contibute for the
feature of index inheritance, that is currently unsopported, but listed in
TODOs.

I've spent some time reading the docs and I took a look at the code. Is
anybody out there working on this already? I don't want to overlap someone
else effort, plus I'll gladly take any advice or join the community efforts
if any, 'cause this feature seems pretty huge to me at a first glance..

-- 
Vincenzo.
http://www.linkedin.com/pub/vincenzo-melandri/14/16/730

Re: [HACKERS] Commit subject line

2013-05-06 Thread Magnus Hagander

On Fri, May 3, 2013 at 9:07 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-05-03 14:54:23 -0400, Andrew Dunstan wrote:

 On 05/03/2013 02:43 PM, Tom Lane wrote:
 Heikki Linnakangas hlinnakan...@vmware.com writes:
 On 03.05.2013 20:56, Bruce Momjian wrote:
 On Fri, May  3, 2013 at 01:42:33PM -0400, Andrew Dunstan wrote:
 Yeah. The recommended style is to have the first line be 50 chars or
 less, which is a bit unfortunate - it can be a challenge to keep to
 that limit for a meaningful or comprehensive subject.
 Oh, that's tight. I didn't know about the 50 char recommendation. I've
 tried to keep mine  76 chars, so that when you do git log, it fits on
 a 80 char display with the 4 char indentation that git log does.
 Yeah, that's news to me too.  I've been using a 75-char line length for
 all my commit messages since we switched to git.  It's frequently tough
 enough to get a useful headline into 75 chars --- I can't see trying to
 do 50.

 man git-commit says:

Though not required, it’s a good idea to begin the commit message
with a single short (less than 50 character) line summarizing the
change, followed by a blank line and then a more thorough
description. Tools that turn commits into email, for example, use
the first line on the Subject: line and the rest of the commit in
the body.

 I'd be happy to use 75 or whatever if we could convince the email tools not
 to truncate the subject lines at 50.

 Its worth to notice that neither git nor the kernel adhere to that
 limit...

FWIW, the tool we use to generate the commit emails truncate it at 80
(minus the pgsql:  header). We can increase that, but it only fixes
the email one, and not the one that people look at on the web...

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


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

Re: [HACKERS] erroneous restore into pg_catalog schema

Dimitri Fontaine dimi...@2ndquadrant.fr writes:
 Tom Lane t...@sss.pgh.pa.us writes:
 it seems like the extensions code should test for and reject an attempt
 to set a relocatable extension's schema to pg_catalog.  Otherwise you'd
 be likely to get not-too-intelligible errors from the extension script.

 Reading the code now, it seems to me that we lack a more general test
 and error situation to match with the comments.

   else if (control-schema != NULL)
   {
   /*
* The extension is not relocatable and the author gave us a 
 schema
* for it.  We create the schema here if it does not 
 already exist.
*/

 We should probably error out when entering in that block of code if the
 extension is relocatable at all, right? That would fix the pg_catalog
 case as well as the general one.

Huh?  According to the comment, at least, we don't get here for a
relocatable extension.  I don't see anything wrong with auto-creating
the target schema for a non-relocatable extension.

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] 9.3 Beta1 status report

2013-05-06 Thread 'Bruce Momjian'

On Mon, May  6, 2013 at 12:43:55PM +0530, Amit Kapila wrote:
 On Sunday, April 21, 2013 10:32 AM Bruce Momjian wrote:
  I am not sure if Tom shared yet, but we are planning to package 9.3
  beta1 on April 29, with a release on May 2.  Those dates might change,
  but that is the current plan.  I have completed a draft 9.3 release
  notes, which you can view here:
  
  http://momjian.us/pgsql_docs/release-9-3.html
  
  I will be working on polishing them for the next ten days, so any
  feedback, patches, or commits are welcome.  I still need to add lots of
  SGML markup.
 
 1. 
 .Add wal_receiver_timeout parameter to control the WAL receiver timeout
 (Amit Kapila) 
 This allows more rapid detection of connection failure. No longer set
 wal_receiver_status_interval? 
 
 I don't think we need to mention anything about
 wal_receiver_status_interval.

OK, removed.

 2. I am not able to figure out which item of release notes cover the below
 feature commit
 Avoid inserting Result nodes that only compute identity projections.
 http://www.postgresql.org/message-id/e1ugcbh-0006p3...@gemulon.postgresql.org

I did not think that warranted a mention in the release notes.  Was I
wrong?

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

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


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

Re: [HACKERS] Commit subject line

2013-05-06 Thread Andrew Dunstan



On 05/06/2013 10:19 AM, Magnus Hagander wrote:

On Fri, May 3, 2013 at 9:07 PM, Andres Freund and...@2ndquadrant.com wrote:

On 2013-05-03 14:54:23 -0400, Andrew Dunstan wrote:

On 05/03/2013 02:43 PM, Tom Lane wrote:

Heikki Linnakangas hlinnakan...@vmware.com writes:

On 03.05.2013 20:56, Bruce Momjian wrote:

On Fri, May  3, 2013 at 01:42:33PM -0400, Andrew Dunstan wrote:

Yeah. The recommended style is to have the first line be 50 chars or
less, which is a bit unfortunate - it can be a challenge to keep to
that limit for a meaningful or comprehensive subject.

Oh, that's tight. I didn't know about the 50 char recommendation. I've
tried to keep mine  76 chars, so that when you do git log, it fits on
a 80 char display with the 4 char indentation that git log does.

Yeah, that's news to me too.  I've been using a 75-char line length for
all my commit messages since we switched to git.  It's frequently tough
enough to get a useful headline into 75 chars --- I can't see trying to
do 50.

man git-commit says:

Though not required, it’s a good idea to begin the commit message
with a single short (less than 50 character) line summarizing the
change, followed by a blank line and then a more thorough
description. Tools that turn commits into email, for example, use
the first line on the Subject: line and the rest of the commit in
the body.

I'd be happy to use 75 or whatever if we could convince the email tools not
to truncate the subject lines at 50.

Its worth to notice that neither git nor the kernel adhere to that
limit...

FWIW, the tool we use to generate the commit emails truncate it at 80
(minus the pgsql:  header). We can increase that, but it only fixes
the email one, and not the one that people look at on the web...


In practice, something else must be further truncating it, at about 64 
chars by the look of it - see for example 
http://www.postgresql.org/message-id/e1uvtfj-00079k...@gemulon.postgresql.org


Re your other point, github at least seems to elide at about 70 chars  - 
see 
https://github.com/postgres/postgres/commit/b42ea7981ce1e7484951a22662937541066d8647 
- where Joe used a very long first sentence rather than a show summary 
line. I don't know if gitweb could be induced to elide after a greater 
length - I bet it could fairly easily. There does seem to be lots of 
spare screen real estate on the commit summary and history pages, which 
I think is where this occurs.


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] Remaining beta blockers

Kevin Grittner kgri...@ymail.com writes:
 Tom Lane t...@sss.pgh.pa.us wrote:
 If you want to call the pg_class column relispopulated rather
 than relisscannable, I have no particular objection to that

 That column name and the wording of some comments are the main
 things, although I'm also wondering whether it is bad form to force
 users to test the pg_class.relispopulated column if they want to
 test whether they can currently scan a matview, by removing the
 pg_relation_is_scannable() function.  As I mentioned earlier when
 you asked why these two distinct properties weren't both exposed, I
 mentioned that I hadn't thought that the populated property was
 likely to be useful at the SQL level, but then questioned that,
 saying that I wasn't sure I picked the right property to pay
 attention to in pg_dump - and if pg_dump needed the populated
 property it had to be exposed.  I've come around to thinking that
 it is more proper to use populated, but I have the same question
 you asked earlier -- If it will be important or users to understand
 that these are distinct properties, why are we just exposing one of
 them?

That's fair.  So what say we call the pg_class column relispopulated
or something like that, and reinstate pg_relation_is_scannable()
as a function, for any client-side code that wants to test that
property as distinct from is-populated?

 The flip side of that is that it might be confusing to try
 to explain why users should care which test they use before they
 are capable of returning different results.

That's a good point too, though; if they are returning the same thing
right now, it's not very clear that users will pick the right test to
make anyway.  Especially not if pg_relation_is_scannable() is a couple
orders of magnitude more expensive, which it will be, cf my original
complaint about pg_dump slowdown.

 Also, rather than do the direct update to pg_class in pg_dump, how
 would you feel about an ALTER MATERIALIZED VIEW option to set the
 populated state?

It seems a bit late to be adding such a thing; moreover, how would
you inject any data without doing something like what pg_upgrade is
doing?  I see no point in an ALTER command until there's some other
SQL-level infrastructure for incremental matview updates.

In the context of pg_dump's binary upgrade option, I had thought of
adding a new pg_upgrade_support function, but I saw that we already use
direct pg_class updates for other nearby binary-upgrade hacking; so it
didn't seem unreasonable to do it that way here.

 I'm just reviewing the changes I made, and figured it might be good
 to show a diff between my form of the patch and yours, but I'm
 getting a lot spurious differences based on how we generate our
 context diff files (or maybe the versions of some software
 involved).  You you share how you generate your patch file?

I use git diff with the context-style-diff external helper that's
described in our wiki.  It could well be a version-discrepancy
problem... this machine has got git version 1.7.9.6 and diffutils
2.8.1, and I think the latter is pretty old.

regards, tom lane


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

[HACKERS] pg_dump --snapshot

Patch to allow pg_dump to use a snapshot exported with an explicit
pg_export_snapshot() for when precise timing of the snapshot is
important.

This overrides the internally generated snapshot in parallel pg_dump.

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


pg_dump_snapshot.v1.patch
Description: Binary data

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

Re: [HACKERS] 9.3 Beta1 status report

On Sun, May  5, 2013 at 02:16:59PM -0700, Jeff Janes wrote:
 On Thu, May 2, 2013 at 4:13 PM, Bruce Momjian br...@momjian.us wrote:
 
 On Thu, May  2, 2013 at 03:03:58PM -0700, Jeff Janes wrote:
  Some suggestions, perhaps just based on my preference for verbosity:
 
 
 para
  Add cache of local locks (Jeff Janes)
 /para
 
 para
  This speeds lock release at statement completion in transactions
  that hold many locks; it is particularly useful for pg_dump.
 /para
 
 
  I think this is equally important for restoration of dumps, if the
 restoration
  is run all in one transaction.  (Making the dump and restoring it have
 similar
  locking and unlocking patterns)
 
 Do you have proposed wording?  I can't say just dump/restore as it only
 helps with _logical_ dump and _logical_ restore, and we don't have a
 clear word for logical restore, as it could be pg_restore or piped into
 psql.  We could do:
 
 that hold many locks; it is particularly useful for pg_dump and
 restore.
 
 but restore seems very vague.
 
 
 
 Yeah, I wasn't sure about how to work that either.
 
 ...and the restore of such dumps.?

s/restore/restoring/

I like it.  Done.

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

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


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

Re: [HACKERS] 9.3 Beta1 status report

On Sun, May  5, 2013 at 06:59:28PM -0400, Andrew Dunstan wrote:
  I think this is equally important for restoration of dumps, if
 the restoration
  is run all in one transaction.  (Making the dump and restoring
 it have similar
  locking and unlocking patterns)
 
 Do you have proposed wording?  I can't say just dump/restore as it
 only
 helps with _logical_ dump and _logical_ restore, and we don't have a
 clear word for logical restore, as it could be pg_restore or piped
 into
 psql.  We could do:
 
 that hold many locks; it is particularly useful for
 pg_dump and restore.
 
 but restore seems very vague.
 
 
 
 Yeah, I wasn't sure about how to work that either.
 
 ...and the restore of such dumps.?
 
 
 s/restore/restoration/

I like that even better!  Done.

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

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


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

Re: [HACKERS] erroneous restore into pg_catalog schema

Tom Lane t...@sss.pgh.pa.us writes:
 Huh?  According to the comment, at least, we don't get here for a
 relocatable extension.  I don't see anything wrong with auto-creating
 the target schema for a non-relocatable extension.

I was not finding why I would trust the comment the other evening, hence
my proposal. I now see that parse_extension_control_file has this check:

if (control-relocatable  control-schema != NULL)
 ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
 errmsg(parameter \schema\ cannot be specified when 
\relocatable\ is true)));

So it's ok. I now wonder how do you install a relocatable extension with
schema = pg_catalog, which I assumed was possible when reading the code
the other day.

I feel like I'm missing something big for not reading the whole thread
in details. Will send the patch I just finished for some documentation
work, then have a more serious look. Sorry about sharing that much
confusion…

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


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

Re: [HACKERS] pg_dump --snapshot

2013-05-06 Thread Andrew Dunstan



On 05/06/2013 10:56 AM, Simon Riggs wrote:

Patch to allow pg_dump to use a snapshot exported with an explicit
pg_export_snapshot() for when precise timing of the snapshot is
important.

This overrides the internally generated snapshot in parallel pg_dump.






Could you be a bit more expansive about the use case, please?

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] Remaining beta blockers

Kevin Grittner kgri...@ymail.com writes:
 Kevin Grittner kgri...@ymail.com wrote:
 That column name and the wording of some comments are the main
 things

 Patch for that attached.  I left the part where you got rid of the
 SQL function to allow users to test whether a matview is currently
 scannable, and I did not add an AMV option to change the populated
 flag, since those haven't had any real discussion yet.

Per my other mail, I think adding an AMV option at this time is
inadvisable.  I could go either way on removing or keeping the
is_scannable function --- anybody else have an opinion on that point?

Which of us is going to commit this?  We're running low on time ...

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] matview niceties: pick any two of these three

Tom Lane t...@sss.pgh.pa.us writes:
 The long and the short of it is this: having unlogged matviews in 9.3
 is not worth taking that risk for.  IMO anyway.

FWIW, +1

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


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

Re: [HACKERS] event trigger API documentation?

Peter Eisentraut pete...@gmx.net writes:
 At this point, all that is appropriate is some documentation of the C
 API.  If the contrib example you have in mind is short enough, it might
 as well become part of the example in the documentation.

Please find attached a patch against the documentation, containing a
full code example of what I had in mind. The contrib would only be
useful to include if we want to ship something usable.

As you might want to tinker with the code in the docs patch and easily
check that it still runs, I include another patch with the new contrib
module. I don't expect that to get commited, of course, but I had to do
it to check the code so I'd better just share it, right?

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

*** /dev/null
--- b/contrib/noddl/Makefile
***
*** 0 
--- 1,17 
+ # contrib/lo/Makefile
+ 
+ MODULES = noddl
+ 
+ EXTENSION = noddl
+ DATA = noddl--1.0.sql
+ 
+ ifdef USE_PGXS
+ PG_CONFIG = pg_config
+ PGXS := $(shell $(PG_CONFIG) --pgxs)
+ include $(PGXS)
+ else
+ subdir = contrib/noddl
+ top_builddir = ../..
+ include $(top_builddir)/src/Makefile.global
+ include $(top_srcdir)/contrib/contrib-global.mk
+ endif
*** /dev/null
--- b/contrib/noddl/noddl--1.0.sql
***
*** 0 
--- 1,12 
+ /* contrib/lo/lo--1.0.sql */
+ 
+ -- complain if script is sourced in psql, rather than via CREATE EXTENSION
+ \echo Use CREATE EXTENSION lo to load this file. \quit
+ 
+ CREATE FUNCTION noddl()
+ RETURNS pg_catalog.event_trigger
+  AS 'noddl'
+  LANGUAGE C;
+ 
+ CREATE EVENT TRIGGER noddl on ddl_command_start
+execute procedure noddl();
*** /dev/null
--- b/contrib/noddl/noddl.c
***
*** 0 
--- 1,35 
+ /*
+  *	PostgreSQL definitions for noddl event trigger extension.
+  *
+  *	contrib/noddl/noddl.c
+  */
+ 
+ #include postgres.h
+ #include commands/event_trigger.h
+ 
+ 
+ PG_MODULE_MAGIC;
+ 
+ /* forward declarations */
+ Datum		noddl(PG_FUNCTION_ARGS);
+ 
+ 
+ /*
+  * This is the trigger that protects us from orphaned large objects
+  */
+ PG_FUNCTION_INFO_V1(noddl);
+ 
+ Datum
+ noddl(PG_FUNCTION_ARGS)
+ {
+ 	EventTriggerData *trigdata = (EventTriggerData *) fcinfo-context;
+ 
+ 	if (!CALLED_AS_EVENT_TRIGGER(fcinfo))		/* internal error */
+ 		elog(ERROR, not fired by event trigger manager);
+ 
+ 	ereport(ERROR,
+ 			(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ 			 errmsg(command %s denied, trigdata-tag)));
+ 
+ PG_RETURN_NULL();
+ }
*** /dev/null
--- b/contrib/noddl/noddl.control
***
*** 0 
--- 1,4 
+ # noddl extension
+ comment = 'Event Trigger to deny DDL operations'
+ default_version = '1.0'
+ relocatable = true
*** a/doc/src/sgml/event-trigger.sgml
--- b/doc/src/sgml/event-trigger.sgml
***
*** 607,610 
--- 607,861 
 /table
/sect1
  
+   sect1 id=event-trigger-interface
+titleWriting Event Trigger Functions in C/title
+ 
+indexterm zone=event-trigger-interface
+ primaryevent trigger/primary
+ secondaryin C/secondary
+/indexterm
+ 
+para
+ This section describes the low-level details of the interface to an
+ event trigger function. This information is only needed when writing
+ event trigger functions in C. If you are using a higher-level language
+ then these details are handled for you. In most cases you should
+ consider using a procedural language before writing your event triggers
+ in C. The documentation of each procedural language explains how to
+ write an event trigger in that language.
+/para
+ 
+para
+ Event trigger functions must use the quoteversion 1/ function
+ manager interface.
+/para
+ 
+para
+ When a function is called by the event trigger manager, it is not passed
+ any normal arguments, but it is passed a quotecontext/ pointer
+ pointing to a structnameEventTriggerData/ structure. C functions can
+ check whether they were called from the event trigger manager or not by
+ executing the macro:
+ programlisting
+ CALLED_AS_EVENT_TRIGGER(fcinfo)
+ /programlisting
+ which expands to:
+ programlisting
+ ((fcinfo)-gt;context != NULL amp;amp; IsA((fcinfo)-gt;context, EventTriggerData))
+ /programlisting
+ If this returns true, then it is safe to cast
+ literalfcinfo-gt;context/ to type literalEventTriggerData
+ */literal and make use of the pointed-to
+ structnameEventTriggerData/ structure.  The function must
+ emphasisnot/emphasis alter the structnameEventTriggerData/
+ structure or any of the data it points to.
+/para
+ 
+para
+ structnamestruct EventTriggerData/structname is defined in
+ filenamecommands/event_trigger.h/filename:
+ 
+ programlisting
+ typedef struct EventTriggerData
+ {
+ 	NodeTag		type;
+ 	const char *event;/* event name */
+ 	Node	   *parsetree;			/* parse tree */
+ 	const char *tag;/*

Re: [HACKERS] 9.3 release notes suggestions


On 05.05.2013 18:51, Andrew Dunstan wrote:


On 05/05/2013 01:35 AM, Tom Lane wrote:

Bruce Momjian br...@momjian.us writes:

OK, so we can either use 4 hex digits minimum and have a fixed with on
most platforms, extend it to 8 hex digits, or revert the entire
fixed-width idea.

I think we should lose the idea that it's fixed-width. 16-bit PIDs are
a limitation whose days are obviously numbered.



I'm late to this party - I must have been asleep at the wheel back in
October, but I guess I have an interest as the original author of
log_line_prefix ;-)

I can't off the top of my head see any good reason for zero padding, so
I'm with Tom.


Agreed. What might be useful, is a general padding feature in 
log_line_prefix, like in sprintf:


log_line_prefix = '%.5p '

- 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] Remaining beta blockers

2013-05-06 Thread Joshua D. Drake



On 05/06/2013 08:17 AM, Tom Lane wrote:


Per my other mail, I think adding an AMV option at this time is
inadvisable.  I could go either way on removing or keeping the
is_scannable function --- anybody else have an opinion on that point?

Which of us is going to commit this?  We're running low on time ...


As a my two cents, I have been watching this thread and the concern on 
timeline is bothering me. I fully understand our want to get into Beta 
and I know we don't want to slip schedule too much but quality is 
important. It is what makes our project what it is more than any other 
value we hold.


I also know we already slipped the beta once but we are not a 
corporation, we do not have shareholders and nobody can fire us. If we 
need to push it again for quality, shouldn't we?


Sincerely,

JD



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

Re: [HACKERS] Remaining beta blockers

2013-05-06 Thread Kevin Grittner

Tom Lane t...@sss.pgh.pa.us wrote:
 Kevin Grittner kgri...@ymail.com writes:

 The flip side of that is that it might be confusing to try
 to explain why users should care which test they use before they
 are capable of returning different results.

 That's a good point too, though; if they are returning the same
 thing right now, it's not very clear that users will pick the
 right test to make anyway.  Especially not if
 pg_relation_is_scannable() is a couple orders of magnitude more
 expensive, which it will be, cf my original complaint about
 pg_dump slowdown.

Since the patch we have floating around drops it, let's leave it
that way, in the interest of saving time getting to beta.  If it
was still there, I'd probably vote to leave it for the same reason.
It's pretty close to a toss-up at this point in terms of
cost/benefit, and that seems like the tie-breaker.

 Also, rather than do the direct update to pg_class in pg_dump,
 how would you feel about an ALTER MATERIALIZED VIEW option to
 set the populated state?

 It seems a bit late to be adding such a thing;

No kidding.  The same could be said for the rest of this.  It was
all talked to death months ago before I posted a patch which was
proposed for commit.  All this eleventh hour drama bothers me.
I've always maintained we should add an ALTER capabilities for such
things once they are in the catalog.  A few days ago they weren't.
Now they are.

 moreover, how would you inject any data without doing something
 like what pg_upgrade is doing?

I wouldn't.  I'm talking about taking that code out of pg_upgrade
and putting it in the server under an ALTER command.  If the point
of moving the info to the catalog was to avoid hacks, it would be
nice not to add a hack like that in the process.

 I see no point in an ALTER command until there's some other
 SQL-level infrastructure for incremental matview updates.

It's only important to avoid having client code directly update
system tables, which I generally view as a worthwhile goal.

 In the context of pg_dump's binary upgrade option, I had thought
 of adding a new pg_upgrade_support function, but I saw that we
 already use direct pg_class updates for other nearby
 binary-upgrade hacking; so it didn't seem unreasonable to do it
 that way here.

In that case, I guess we might as well follow suit and do it the
way you have it for 9.3.

I didn't see anything I thought needed changing in your first patch
(to disable unlogged matviews), and my suggested changes to your
second patch (to move tracking of populated status to pg_class) are
just names, aliases, and comments.  I suggest you review my
proposed tweak to your patch and apply both with any final
polishing you feel are appropriate.

-- 
Kevin Grittner
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] 9.3 release notes suggestions

On Mon, May  6, 2013 at 06:23:07PM +0300, Heikki Linnakangas wrote:
 On 05.05.2013 18:51, Andrew Dunstan wrote:
 
 On 05/05/2013 01:35 AM, Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
 OK, so we can either use 4 hex digits minimum and have a fixed with on
 most platforms, extend it to 8 hex digits, or revert the entire
 fixed-width idea.
 I think we should lose the idea that it's fixed-width. 16-bit PIDs are
 a limitation whose days are obviously numbered.
 
 
 I'm late to this party - I must have been asleep at the wheel back in
 October, but I guess I have an interest as the original author of
 log_line_prefix ;-)
 
 I can't off the top of my head see any good reason for zero padding, so
 I'm with Tom.
 
 Agreed. What might be useful, is a general padding feature in
 log_line_prefix, like in sprintf:
 
 log_line_prefix = '%.5p '

Yes, I can see that as useful, especially for things like username and
database name.  TODO item?

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

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


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

Re: [HACKERS] Remaining beta blockers

Kevin Grittner kgri...@ymail.com writes:
 Tom Lane t...@sss.pgh.pa.us wrote:
 It seems a bit late to be adding such a thing;

 No kidding.  The same could be said for the rest of this.  It was
 all talked to death months ago before I posted a patch which was
 proposed for commit.  All this eleventh hour drama bothers me.

Well, we've been going back and forth about it for weeks.  Without a
looming deadline, we'd probably still just be arguing inconclusively ...

 I didn't see anything I thought needed changing in your first patch
 (to disable unlogged matviews), and my suggested changes to your
 second patch (to move tracking of populated status to pg_class) are
 just names, aliases, and comments.  I suggest you review my
 proposed tweak to your patch and apply both with any final
 polishing you feel are appropriate.

OK, will do.

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] Remaining beta blockers

* Kevin Grittner (kgri...@ymail.com) wrote:
 Since the patch we have floating around drops it, let's leave it
 that way, in the interest of saving time getting to beta.  If it
 was still there, I'd probably vote to leave it for the same reason.

I'll vote for dropping it also, though for a slightly different reason-
people can't build things on something that isn't there.  Given that
we're still discussing it, that strikes me as the best idea.  What goes
into 9.4 could be quite different and it's a lot easier if we don't have
to deal with supporting what may end up being the 'old' approach.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] pg_dump --snapshot

On 6 May 2013 16:02, Andrew Dunstan and...@dunslane.net wrote:

 On 05/06/2013 10:56 AM, Simon Riggs wrote:

 Patch to allow pg_dump to use a snapshot exported with an explicit
 pg_export_snapshot() for when precise timing of the snapshot is
 important.

 This overrides the internally generated snapshot in parallel pg_dump.


 Could you be a bit more expansive about the use case, please?

Sure.

Exported snapshots allow you to coordinate a number of actions
together, so they all see a common view of the database. So this patch
allows a very general approach to this, much more so than pg_dump
allows currently since the exact timing of the snapshot is not
controlled by the user.

For example, you may wish to create a sequence of materialized views
all using the same snapshot, while at the same time dumping the
database, then follow that by dumping the mat views also. Or you may
wish to dump the database at the exact moment between two tasks,
rather than wait for pg_dump to startup before commencing the next
task.

The original pg_snapclone code from 2008 allowed an export of the
snapshot and then an import of the snapshot explicitly into pg_dump in
much the same way shown here. This patch would be the only aspect
missing from that original feature set: explicit control. (Thanks for
following up to get it all into production code).

Logical replication would be one of the users of this also, but it is
a standalone feature which complements the existing capabilities.

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


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

Re: [HACKERS] Commit subject line

2013-05-06 Thread Magnus Hagander

On Mon, May 6, 2013 at 4:47 PM, Andrew Dunstan and...@dunslane.net wrote:

 On 05/06/2013 10:19 AM, Magnus Hagander wrote:

 On Fri, May 3, 2013 at 9:07 PM, Andres Freund and...@2ndquadrant.com
 wrote:

 On 2013-05-03 14:54:23 -0400, Andrew Dunstan wrote:

 On 05/03/2013 02:43 PM, Tom Lane wrote:

 Heikki Linnakangas hlinnakan...@vmware.com writes:

 On 03.05.2013 20:56, Bruce Momjian wrote:

 On Fri, May  3, 2013 at 01:42:33PM -0400, Andrew Dunstan wrote:

 Yeah. The recommended style is to have the first line be 50 chars or
 less, which is a bit unfortunate - it can be a challenge to keep to
 that limit for a meaningful or comprehensive subject.

 Oh, that's tight. I didn't know about the 50 char recommendation. I've
 tried to keep mine  76 chars, so that when you do git log, it fits
 on
 a 80 char display with the 4 char indentation that git log does.

 Yeah, that's news to me too.  I've been using a 75-char line length for
 all my commit messages since we switched to git.  It's frequently tough
 enough to get a useful headline into 75 chars --- I can't see trying to
 do 50.

 man git-commit says:

 Though not required, it’s a good idea to begin the commit message
 with a single short (less than 50 character) line summarizing the
 change, followed by a blank line and then a more thorough
 description. Tools that turn commits into email, for example, use
 the first line on the Subject: line and the rest of the commit in
 the body.

 I'd be happy to use 75 or whatever if we could convince the email tools
 not
 to truncate the subject lines at 50.

 Its worth to notice that neither git nor the kernel adhere to that
 limit...

 FWIW, the tool we use to generate the commit emails truncate it at 80
 (minus the pgsql:  header). We can increase that, but it only fixes
 the email one, and not the one that people look at on the web...


 In practice, something else must be further truncating it, at about 64 chars
 by the look of it - see for example
 http://www.postgresql.org/message-id/e1uvtfj-00079k...@gemulon.postgresql.org

Ha. Good point. There's actually a bit of a bug in the code there :)
What it does is limit the length to 80-length(pgsql: $shortmsg),
which is 64. It is supposed to limit it to 80-length(pgsql: )..
(Since it substitutes the actual commit message where $shortmsg is
found).

That's fixable though :)


 Re your other point, github at least seems to elide at about 70 chars  - see
 https://github.com/postgres/postgres/commit/b42ea7981ce1e7484951a22662937541066d8647
 - where Joe used a very long first sentence rather than a show summary line.
 I don't know if gitweb could be induced to elide after a greater length - I
 bet it could fairly easily. There does seem to be lots of spare screen real
 estate on the commit summary and history pages, which I think is where this
 occurs.

Possibly. I can never find my way around that one though, and making
any modifications also has us ending up maintaining what's basically a
fork - unless there's always a config argument for it somewhere.


--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


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

Re: [HACKERS] pg_dump --snapshot

Simon Riggs si...@2ndquadrant.com writes:
 On 6 May 2013 16:02, Andrew Dunstan and...@dunslane.net wrote:
 On 05/06/2013 10:56 AM, Simon Riggs wrote:
 This overrides the internally generated snapshot in parallel pg_dump.

 Could you be a bit more expansive about the use case, please?

 Exported snapshots allow you to coordinate a number of actions
 together, so they all see a common view of the database. So this patch
 allows a very general approach to this, much more so than pg_dump
 allows currently since the exact timing of the snapshot is not
 controlled by the user.

I'm afraid that this is institutionalizing a design deficiency in
pg_dump; namely that it takes its snapshot before acquiring locks.
Ideally that would happen the other way around.  I don't have a good
idea how we could fix that --- but a feature that allows imposition
of an outside snapshot will permanently foreclose ever fixing it.

What's more, this would greatly widen the risk window between when
the snapshot is taken and when we have all the locks and can have
some confidence that the DB isn't changing under us.

Or in short: -1 for the very concept of letting the user control
pg_dump's snapshot.

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] Commit subject line

On Mon, May  6, 2013 at 06:41:53PM +0200, Magnus Hagander wrote:
  In practice, something else must be further truncating it, at about 64 chars
  by the look of it - see for example
  http://www.postgresql.org/message-id/e1uvtfj-00079k...@gemulon.postgresql.org
 
 Ha. Good point. There's actually a bit of a bug in the code there :)
 What it does is limit the length to 80-length(pgsql: $shortmsg),
 which is 64. It is supposed to limit it to 80-length(pgsql: )..
 (Since it substitutes the actual commit message where $shortmsg is
 found).
 
 That's fixable though :)
 
 
  Re your other point, github at least seems to elide at about 70 chars  - see
  https://github.com/postgres/postgres/commit/b42ea7981ce1e7484951a22662937541066d8647
  - where Joe used a very long first sentence rather than a show summary line.
  I don't know if gitweb could be induced to elide after a greater length - I
  bet it could fairly easily. There does seem to be lots of spare screen real
  estate on the commit summary and history pages, which I think is where this
  occurs.
 
 Possibly. I can never find my way around that one though, and making
 any modifications also has us ending up maintaining what's basically a
 fork - unless there's always a config argument for it somewhere.

So what should our goal length be?

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

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


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

Re: [HACKERS] pg_dump versus materialized views

2013-05-06 Thread Josh Berkus


 I've thought for some time that, given that it can't reproduce the MV
 states exactly, pg_dump shouldn't even try.  I think it would be more
 useful to have two operating modes selectable by command line switch:
 refresh all matviews, or refresh none of them.

This seems like a reasonable solution.  Also, I think refresh none
should be the default.

BTW, this does mean that REFRESH ALL MATERIALIZED VIEWS should be a
future feature ...


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

On 2013-05-06 13:07:17 -0400, Tom Lane wrote:
 Simon Riggs si...@2ndquadrant.com writes:
  On 6 May 2013 16:02, Andrew Dunstan and...@dunslane.net wrote:
  On 05/06/2013 10:56 AM, Simon Riggs wrote:
  This overrides the internally generated snapshot in parallel pg_dump.

  Could you be a bit more expansive about the use case, please?

  Exported snapshots allow you to coordinate a number of actions
  together, so they all see a common view of the database. So this patch
  allows a very general approach to this, much more so than pg_dump
  allows currently since the exact timing of the snapshot is not
  controlled by the user.

 I'm afraid that this is institutionalizing a design deficiency in
 pg_dump; namely that it takes its snapshot before acquiring locks.
 Ideally that would happen the other way around.  I don't have a good
 idea how we could fix that --- but a feature that allows imposition
 of an outside snapshot will permanently foreclose ever fixing it.

 What's more, this would greatly widen the risk window between when
 the snapshot is taken and when we have all the locks and can have
 some confidence that the DB isn't changing under us.

The initial transaction that exports the transaction would need to hold
locks until pg_dump started :/.

 Or in short: -1 for the very concept of letting the user control
 pg_dump's snapshot.

Its rather useful if you e.g. want to instantiate a new replica without
rebuilding pg_dump/pg_restore's capabilities wrt. ordering, parallelism,
separating initial data load from index creation and all that. Which
already has been incompletely reinvented by several solutions :(.

So besides the above and real problems you point out this seems
worthwile to me...

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

On 6 May 2013 18:07, Tom Lane t...@sss.pgh.pa.us wrote:

 Or in short: -1 for the very concept of letting the user control
 pg_dump's snapshot.

That API is already exposed, so not sure why you say this now? This
has been in PG since early in 9.2, about 2 years ago.

In any case, flashback database is one of the most requested
features I know of... the ability to dump the database as it appeared
in the past *after* that point has passed. I call it by its name as
used in Oracle, but many people have taken the trouble to describe
that in detail to me, even without knowing a similar feature existed
elsewhere.

So it will always be important to do SET TRANSACTION SNAPSHOT 'a point
in the past'
and if we can do that, why not pg_dump also?

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


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

Re: [HACKERS] pg_dump --snapshot

* Andres Freund (and...@2ndquadrant.com) wrote:
 Its rather useful if you e.g. want to instantiate a new replica without
 rebuilding pg_dump/pg_restore's capabilities wrt. ordering, parallelism,
 separating initial data load from index creation and all that. Which
 already has been incompletely reinvented by several solutions :(.

Perhaps it's just a wording thing, but I wouldn't use the term replica
when referring to something built with pg_dump/restore- that should
really be reserved for a slave system created through replication.

 So besides the above and real problems you point out this seems
 worthwile to me...

It certainly sounds interesting and I like the idea of it, but perhaps
we need a different mechanism than just passing in a raw snapshot, to
address the concerns that Tom raised.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] pg_dump --snapshot

On 2013-05-06 14:35:14 -0400, Stephen Frost wrote:
 * Andres Freund (and...@2ndquadrant.com) wrote:
  Its rather useful if you e.g. want to instantiate a new replica without
  rebuilding pg_dump/pg_restore's capabilities wrt. ordering, parallelism,
  separating initial data load from index creation and all that. Which
  already has been incompletely reinvented by several solutions :(.
 
 Perhaps it's just a wording thing, but I wouldn't use the term replica
 when referring to something built with pg_dump/restore- that should
 really be reserved for a slave system created through replication.

Well, part of the usecase *is* using it for the cloning in a replication
solution instead of open-coding it there. E.g. londiste and slony both
have home-grown hacks to do this.

  So besides the above and real problems you point out this seems
  worthwile to me...
 
 It certainly sounds interesting and I like the idea of it, but perhaps
 we need a different mechanism than just passing in a raw snapshot, to
 address the concerns that Tom raised.

If there is anything which isn't magnitudes more complex, I'd be
interested. But given we couldn't even find a sensible solution for
pg_dump internally I don't have all that high hopes...

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

2013-05-06 Thread Greg Stark

On Mon, May 6, 2013 at 6:58 PM, Simon Riggs si...@2ndquadrant.com wrote:
 In any case, flashback database is one of the most requested
 features I know of... the ability to dump the database as it appeared
 in the past *after* that point has passed.

Fwiw that's not what flashback database does. It rolls back the whole
database to that earlier point in time. it's equivalent to running
recovery but backwards in time.

Obviously if you had the ability to dump the database as of an earlier
point in time you could do a complete dump and then a complete restore
and effectively have accomplished the same thing. But that's like
calling pg_dump an incremental backup.





-- 
greg


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

Re: [HACKERS] Commit subject line


On 06.05.2013 20:12, Bruce Momjian wrote:

On Mon, May  6, 2013 at 06:41:53PM +0200, Magnus Hagander wrote:
So what should our goal length be?


Different tools seem to have different truncation points, so it sounds 
like the rule is something like: If you can keep it under 50 characters, 
great. If you can't, try to keep it below 70. But 75 at most. Or just 
keep it as short as possible, but still meaningful.


- 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] corrupt pages detected by enabling checksums

On Wed, May 1, 2013 at 3:04 PM, Jeff Davis pg...@j-davis.com wrote:
 Regardless, you have a reasonable claim that my patch had effects that
 were not necessary. I have attached a draft patch to remedy that. Only
 rudimentary testing was done.

This looks reasonable to me.

-- 
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] Recovery target 'immediate'

On Fri, May 3, 2013 at 11:13 AM, Cédric Villemain
ced...@2ndquadrant.com wrote:
 If we want to avoid adding a new option for this, how about a magic
 restore point called consistent or immediate:

 recovery_target_name='immediate'

 That would stop recovery right after reaching consistency, but there
 wouldn't be an actual restore point record in the WAL stream.

 Back to your first email then.
 +1 (as pointed by Simon, this is something we must document well: stopping at
 'immediate' is sure to reduce your chance of recovering all the possible data
 ... opposite to recovery_target_name=ultimate, the default ;)  )

Sounds good to me.

-- 
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] pgcrypto: Fix RSA password-protected keys

2013-05-06 Thread Marko Kreen

RSA secret key extraction code uses wrong variable so
that decryption is skipped and only secret keys without
password work for pgp_pub_decrypt().

Attached patch fixes it and also adds regtest.

Please apply to all branches.


Reported-by: Keith Fiske ke...@omniti.com

-- 
marko

diff --git a/contrib/pgcrypto/pgp-pubkey.c b/contrib/pgcrypto/pgp-pubkey.c
index 283e0ec..9651d5e 100644
--- a/contrib/pgcrypto/pgp-pubkey.c
+++ b/contrib/pgcrypto/pgp-pubkey.c
@@ -408,16 +408,16 @@ process_secret_key(PullFilter *pkt, PGP_PubKey **pk_p,
 		case PGP_PUB_RSA_SIGN:
 		case PGP_PUB_RSA_ENCRYPT:
 		case PGP_PUB_RSA_ENCRYPT_SIGN:
-			res = pgp_mpi_read(pkt, pk-sec.rsa.d);
+			res = pgp_mpi_read(pf_key, pk-sec.rsa.d);
 			if (res  0)
 break;
-			res = pgp_mpi_read(pkt, pk-sec.rsa.p);
+			res = pgp_mpi_read(pf_key, pk-sec.rsa.p);
 			if (res  0)
 break;
-			res = pgp_mpi_read(pkt, pk-sec.rsa.q);
+			res = pgp_mpi_read(pf_key, pk-sec.rsa.q);
 			if (res  0)
 break;
-			res = pgp_mpi_read(pkt, pk-sec.rsa.u);
+			res = pgp_mpi_read(pf_key, pk-sec.rsa.u);
 			if (res  0)
 break;
 			break;
diff --git a/contrib/pgcrypto/sql/pgp-pubkey-decrypt.sql b/contrib/pgcrypto/sql/pgp-pubkey-decrypt.sql
index cc82420..f8495d1 100644
--- a/contrib/pgcrypto/sql/pgp-pubkey-decrypt.sql
+++ b/contrib/pgcrypto/sql/pgp-pubkey-decrypt.sql
@@ -426,6 +426,71 @@ hbt6LhKhCLUNdz/udIt0JAC6c/HdPLSW3HnmM3+iNj+Kug==
 -END PGP PRIVATE KEY BLOCK-
 ');
 
+insert into keytbl (id, name, pubkey, seckey)
+values (7, 'rsaenc2048-psw', '
+same key with password
+', '
+-BEGIN PGP PRIVATE KEY BLOCK-
+Version: GnuPG v1.4.11 (GNU/Linux)
+
+lQPEBELr2m0BCADOrnknlnXI0EzRExf/TgoHvK7Xx/E0keWqV3KrOyC3/tY2KOrj
+UVxaAX5pkFX9wdQObGPIJm06u6D16CH6CildX/vxG7YgvvKzK8JGAbwrXAfk7OIW
+czO2zRaZGDynoK3mAxHRBReyTKtNv8rDQhuZs6AOozJNARdbyUO/yqUnqNNygWuT
+4htFDEuLPIJwAbMSD0BvFW6YQaPdxzaAZm3EWVNbwDzjgbBUdBiUUwRdZIFUhsjJ
+dirFdy5+uuZru6y6CNC1OERkJ7P8EyoFiZckAIE5gshVZzNuyLOZjc5DhWBvLbX4
+NZElAnfiv+4nA6y8wQLSIbmHA3nqJaBklj85AAYp/gcDCNnoEKwFo86JYCE1J92R
+HRQ7DoyAZpW1O0dTXL8Epk0sKsKDrCJOrIkDymsjfyBexADIeqOkioy/50wD2Mku
+CVHKWO2duAiJN5t/FoRgpR1/Q11K6QdfqOG0HxwfIXLcPv7eSIso8kWorj+I01BP
+Fn/atGEbIjdWaz/q2XHbu0Q3x6Et2gIsbLRVMhiYz1UG9uzGJ0TYCdBa2SFhs184
+52akMpD+XVdM0Sq9/Cx40Seo8hzERB96+GXnQ48q2OhlvcEXiFyD6M6wYCWbEV+6
+XQVMymbl22FPP/bD9ReQX2kjrkQlFAtmhr+0y8reMCbcxwLuQfA3173lSPo7jrbH
+oLrGhkRpqd2bYCelqdy/XMmRFso0+7uytHfTFrUNfDWfmHVrygoVrNnarCbxMMI0
+I8Q+tKHMThWgf0rIOSh0+w38kOXFCEqEWF8YkAqCrMZIlJIed78rOCFgG4aHajZR
+D8rpXdUOIr/WeUddK25Tu8IuNJb0kFf12IMgNh0nS+mzlqWiofS5kA0TeB8wBV6t
+RotaeyDNSsMoowfN8cf1yHMTxli+K1Tasg003WVUoWgUc+EsJ5+KTNwaX5uGv0Cs
+j6dg6/FVeVRL9UsyF+2kt7euX3mABuUtcVGx/ZKTq/MNGEh6/r3B5U37qt+FDRbw
+ppKPc2AP+yBUWsQskyrxFgv4eSpcLEg+lgdz/zLyG4qW4lrFUoO790Cm/J6C7/WQ
+Z+E8kcS8aINJkg1skahH31d59ZkbW9PVeJMFGzNb0Z2LowngNP/BMrJ0LT2CQyLs
+UxbT16S/gwAyUpJnbhWYr3nDdlwtC0rVopVTPD7khPRppcsq1f8D70rdIxI4Ouuw
+vbjNZ1EWRJ9f2Ywb++k/xgSXwJkGodUlrUr+3i8cv8mPx+fWvif9q7Y5Ex1wCRa8
+8FAj/o+hEbQlUlNBIDIwNDggRW5jIDxyc2EyMDQ4ZW5jQGV4YW1wbGUub3JnPokB
+NAQTAQIAHgUCQuvabQIbAwYLCQgHAwIDFQIDAxYCAQIeAQIXgAAKCRDImeqTRBlV
+WRzJCACbRhx2fYjPGKta69M5dS+kr5UD/CQmsR2t9cB9zyqhratjPnKW9q13+4AG
+P3aByT14IH1c5Mha8rJkNYD2wxmC8jrrcPiJIYoRG+W1sUATY/t8wBbNWF+r9h11
+m0lEpsmNVff/jU7SpNN6JQ3P7MHd5V85LlDoXIH6QYCLd0PjKU+jNvjiBe5VX0m9
+a1nacE3xoWc1vbM0DnqEuID78Qgkcrmm0ESeg1h+tRfHxSAyYNc/gPzm8eH6l+hj
+gOvUc4Gd6LpBQSF8TcFfT2TZwJh7WVWDvNIP6FWAW7rzmHnX3wwXkGq4REWeVtk5
+yBPp6mOtWDiwaqLJYsoHWU11C8zYnQPEBELr2roBCADrgiWXZMzkQOntZa/NS56+
+CczLFQRQPl/8iJAW1eql/wOJ1UiwGSjT189WCKzE7vtazCIstdCFmwOs4DE6cz4S
+UX4HjzjYHZwmMiuSrIefwuZ7cysMBsMXypQFyMSbqwh102xGvmLz3Z++rydx7Fzl
+1RC/ny2+FN5dzYPO2DNtNi4dR2tjHktsxBWXAKCmxagAIwyxGouuEqDhYdFtwrA9
+Qy+M5n6fmGa1Dx07WWnbIud4uCilv8LPVKx5aJamDYWM3v7kS8n51MfTzeK/xoRM
+2rsgzFdLJqPdbgd2nsD37fngqZnlp7tDxSVSuMckZoSKtq1QsNemtaQSYq7xjPst
+AAYp/gcDCNnoEKwFo86JYAsxoD+wQ0zBi5RBM5EphXTpM1qKxmigsKOvBSaMmr0y
+VjHtGY3poyV3t6VboOGCsFcaKm0tIdDL7vrxxwyYESETpF29b7QrYcoaLKMG7fsy
+t9SUI3UV2H9uUquHgqHtsqz0jYOgm9tYnpesgQ/kOAWI/tej1ZJXUIWEmZMH/W6d
+ATNvZ3ivwApfC0qF5G3oPgBSoIuQ/8I+pN/kmuyNAnJWNgagFhA/2VFBvh5XgztV
+NW7G//KpR1scsn140SO/wpGBM3Kr4m8ztl9w9U6a7NlQZ2ub3/pIUTpSzyLBxJZ/
+RfuZI7ROdgDMKmEgCYrN2kfp0LIxnYL6ZJu3FDcS4V098lyf5rHvB3PAEdL6Zyhd
+qYp3Sx68r0F4vzk5iAIWf6pG2YdfoP2Z48Pmq9xW8qD9iwFcoz9oAzDEMENn6dfq
+6MzfoaXEoYp8cR/o+aeEaGUtYBHiaxQcJYx35B9IhsXXA49yRORK8qdwhSHxB3NQ
+H3pUWkfw368f/A207hQVs9yYXlEvMZikxl58gldCd3BAPqHm/XzgknRRNQZBPPKJ
+BMZebZ22Dm0qDuIqW4GXLB4sLf0+UXydVINIUOlzg+S4jrwx7eZqb6UkRXTIWVo5
+psTsD14wzWBRdUQHZOZD33+M8ugmewvLY/0Uix+2RorkmB7/jqoZvx/MehDwmCZd
+VH8sb2wpZ55sj7gCXxvrfieQD/VeH54OwjjbtK56iYq56RVD0h1az8xDY2GZXeT7
+J0c3BGpuoca5xOFWr1SylAr/miEPxOBfnfk8oZQJvZrjSBGjsTbALep2vDJk8ROD
+sdQCJuU1RHDrwKHlbUL0NbGRO2juJGsatdWnuVKsFbaFW2pHHkezKuwOcaAJv7Xt
+8LRF17czAJ1uaLKwV8Paqx6UIv+089GbWZi7HIkBHwQYAQIACQUCQuvaugIbDAAK
+CRDImeqTRBlVWS7XCACDVstKM+SHD6V0bkfO6ampHzj4krKjN0lonN5+7b7WKpgT
+QHRYvPY8lUiIrjXGISQqEG9M5Bi5ea1aoBZem0P3U/lKheg0lYtA7dM3BqsA2EfG

Re: [HACKERS] [PATCH] add long options to pgbench (submission 1)

On Thu, May 2, 2013 at 1:59 PM, Fabien COELHO coe...@cri.ensmp.fr wrote:
 This is mostly for reference to the next commitfest.

 This very minor patch adds a corresponding long option to all short (one
 letter) options of pgbench. In particular for connection options there is
 now --host --username --port options similar to the psql client.

 While I was at developing some small extensions to pgbench, ISTM that I
 could do that without much effort.

I don't really have an opinion on whether this is worth doing, but
we'd probably want to update all of our client utilities, not just
pgbench, if we did.

 Note that I'm not so sure about whether to chose singular or plural long
 option names.

I've wondered that myself.

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

On 6 May 2013 19:35, Stephen Frost sfr...@snowman.net wrote:

 It certainly sounds interesting and I like the idea of it, but perhaps
 we need a different mechanism than just passing in a raw snapshot, to
 address the concerns that Tom raised.

It does *not* pass in a raw snapshot. All it does is to allow pg_dump
to use an API that is already exposed by the backend for this very
purpose, one that has been in Postgres since 9.2.
http://www.postgresql.org/docs/devel/static/functions-admin.html#FUNCTIONS-SNAPSHOT-SYNCHRONIZATION

Minor patch, no amazing new functionality, no drama.

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


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

Re: [HACKERS] pg_dump --snapshot

On 6 May 2013 19:48, Greg Stark st...@mit.edu wrote:
 On Mon, May 6, 2013 at 6:58 PM, Simon Riggs si...@2ndquadrant.com wrote:
 In any case, flashback database is one of the most requested
 features I know of... the ability to dump the database as it appeared
 in the past *after* that point has passed.

 Fwiw that's not what flashback database does. It rolls back the whole
 database to that earlier point in time. it's equivalent to running
 recovery but backwards in time.

 Obviously if you had the ability to dump the database as of an earlier
 point in time you could do a complete dump and then a complete restore
 and effectively have accomplished the same thing.

OK, so you know Oracle. So then you also know that there are actually
a number of related features all called flashback something, all
interrelated. What I meant by using their term was just a broad
reference to that capability, not an exact 1:1 match. Most people
requesting this have not asked for it by that name.

At the database level, it rolls back the whole kaboodle. Not what I
meant at all and I would expect people to start twitching at the
prospect.

The feature we have in PG9.2+ is the ability to set a transaction
snapshot to a snapshot that existed in the database at some point,
invoked by some external reference to it. The external reference is
the piece of information that must be specified by the user to allow
the database to look backwards. At the moment we can only specify a
snapshot from a currently running transaction, i.e. the recent past. I
foresee a feature that will allow us to look back further, possibly
with some restrictions, though certainly read only. There is a wide
stretch of water between current state and making that work, but the
existence of an ability to specify an external reference to a snapshot
is pivotal to that future capability and I would not wish to see that
capability removed.

This patch only allows pg_dump to use the existing API. As an example,
we would use it like this.

Session 1:
BEGIN; SELECT pg_export_snapshot(); --returns a textual reference to
the internally held snapshot
 pg_export_snapshot

 04F6-1
(1 row)

Session 2 -- some other user of the same snapshot
pg_dump --snapshot '04F6-1' database1

Session 3 -- some other user of the same snapshot
e.g.
pg_dump --snapshot '04F6-1' database2
some other programs etc..

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


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

Re: [HACKERS] corrupt pages detected by enabling checksums

2013-05-06 Thread Jeff Davis

On Mon, 2013-05-06 at 15:31 -0400, Robert Haas wrote:
 On Wed, May 1, 2013 at 3:04 PM, Jeff Davis pg...@j-davis.com wrote:
  Regardless, you have a reasonable claim that my patch had effects that
  were not necessary. I have attached a draft patch to remedy that. Only
  rudimentary testing was done.
 
 This looks reasonable to me.

Can you please explain the scenario that loses many VM bits at once
during a crash, and results in a bunch of already-all-visible heap pages
being dirtied for no reason?

Regards,
Jeff Davis




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

Re: [HACKERS] pg_dump --snapshot

Simon Riggs si...@2ndquadrant.com writes:
It does *not* pass in a raw snapshot. All it does is to allow pg_dump
to use an API that is already exposed by the backend for this very
purpose, one that has been in Postgres since 9.2.
http://www.postgresql.org/docs/devel/static/functions-admin.html#FUNCTIONS-SNAPSHOT-SYNCHRONIZATION

Minor patch, no amazing new functionality, no drama.

You're ignoring the objection ...

The snapshot-transfer facility is intended for supporting, in essence,
multi-threaded closed applications. In such a context we can expect
that the leader knows enough about the followers to predict which locks
need to be acquired before the to-be-shared snapshot is acquired.

Exposing that in pg_dump, without doing a lick of other work (which is
what I take to be your proposal), would leave us with a situation
wherein an application wishing to invoke pg_dump safely would need to
know what locks pg_dump will take --- something that's rather a moving
target. If it gets that wrong, it will be at risk of obtaining
inconsistent dumps without any warning.

I think a minimum requirement before we can accept this feature is that
there be a way to obtain all the same locks that pg_dump would get when
given the same command-line arguments. This would, perhaps, be a good
test criterion for the fabled library-ization of pg_dump.

regards, tom lane

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

[HACKERS] Re: [COMMITTERS] pgsql: Fix permission tests for views/tables proven empty by constraint

On Wed, May 1, 2013 at 6:27 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Fix permission tests for views/tables proven empty by constraint exclusion.

I believe that this commit is responsible for the fact that the
following test case now crashes the server:

rhaas=# create or replace view foo (x) AS (select 1 union all select 2);
CREATE VIEW
rhaas=# select * from foo where false;
The connection to the server was lost. Attempting reset: Failed.

(gdb) bt
#0  is_dummy_plan (plan=0x0) at planner.c:1850
#1  0x00010bd44c3e in create_append_plan [inlined] () at
/Users/rhaas/pgsql/src/backend/optimizer/plan/createplan.c:706
#2  0x00010bd44c3e in create_plan_recurse (root=0x7fff54080e60,
best_path=0x7f9d4b109270) at createplan.c:247
#3  0x00010bd3f4bd in create_plan (root=0x7f9d4b0389d0,
best_path=0x7f9d4b109270) at createplan.c:201
#4  0x00010bd4aa64 in grouping_planner (root=0x7f9d4b0389d0,
tuple_fraction=6.9532132623547611e-310) at planner.c:1294
#5  0x00010bd4c74e in subquery_planner (glob=0x7f9d4b0389d0,
parse=0x7fff54081290, parent_root=0x7f9d4b107838,
tuple_fraction=6.9532132623808478e-310, subroot=0x7fff54081290,
hasRecursion=0 '\0') at planner.c:558
#6  0x00010bd4ca0c in standard_planner (parse=0x7f9d4b038020,
cursorOptions=0, boundParams=0x7f9d4b038020) at planner.c:209
#7  0x00010bdc4ba3 in pg_plan_query (querytree=0x7f9d4b037ce8,
cursorOptions=1258519784, boundParams=0x7f9d4b109470) at
postgres.c:753
#8  0x00010bdc746c in pg_plan_queries [inlined] () at
/Users/rhaas/pgsql/src/backend/tcop/postgres.c:812
#9  0x00010bdc746c in exec_simple_query [inlined] () at
/Users/rhaas/pgsql/src/backend/tcop/postgres.c:977
#10 0x00010bdc746c in PostgresMain (dbname=0x7f9d4b01f028 rhaas,
argc=1, argv=0x10bf11824, username=0x7f9d4b037a58 ?\002) at
postgres.c:3985
#11 0x00010bd754b3 in PostmasterMain (argc=1409820832,
argv=0x7fff540828a0) at postmaster.c:3985
#12 0x00010bd09f18 in main (argc=1, argv=0x7f9d4ac04050) at main.c:196

-- 
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] [COMMITTERS] pgsql: Fix permission tests for views/tables proven empty by constraint

Robert Haas robertmh...@gmail.com writes:
 rhaas=# create or replace view foo (x) AS (select 1 union all select 2);
 CREATE VIEW
 rhaas=# select * from foo where false;
 The connection to the server was lost. Attempting reset: Failed.

Ugh.  I'm about to leave for the day, but I'll take a look later.

regards, tom lane


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

Re: [HACKERS] pg_dump --snapshot

Simon,

* Simon Riggs (si...@2ndquadrant.com) wrote:
 On 6 May 2013 19:35, Stephen Frost sfr...@snowman.net wrote:
  It certainly sounds interesting and I like the idea of it, but perhaps
  we need a different mechanism than just passing in a raw snapshot, to
  address the concerns that Tom raised.
 
 It does *not* pass in a raw snapshot. 

It wasn't my intent to impart anything more specific than what
pg_export_snapshot() returns when I said 'raw snapshot'.  What would you
call it?  Snapshot identifier?  All I was trying to say is that I agree
with Tom that pg_dump really needs more to happen than simply having the
results of pg_export_snapshot() passed to it- pg_dump wants all the
necessary locks taken immediately after the transaction opens and
pg_export_snapshot() simply doesn't do that.

 All it does is to allow pg_dump
 to use an API that is already exposed by the backend for this very
 purpose, one that has been in Postgres since 9.2.

In doing so it opens a much larger hole through which this approach can
break, namely that objects could disappear between the snapshot being
taken and appropriate locks being set up.  That issue already exists in
pg_dump, but at least it's a small window through one code path- and it
all happens before any parallelization or snapshot-sharing happens, as
best as I can tell.

If I understand correctly, right now we have:

connect to DB
start a transaction
run around and grab locks
get our snapshot ID
fork, connect in, glob onto the same snapshot

Assuming I'm following along correctly, this change would be:

someone in a far off land creates the snapshot
time passes..
then:
connect to DB
set the who-knows-how-old snapshot ID
run around and try to grab locks
fork, connect in, glob onto the same snapshot

One thing that, I believe, addresses this a bit is that we should at
least bomb out with an error while we're trying to acquire the locks,
should an object be dropped between transaction start and when we go to
lock it, right?  We'll still see the old version of pg_class from the
start of the transaction and therefore we'll try to lock everything from
the older viewpoint...?

For my part, I'm a bit less worried about error cases around this,
provided that they're made very clear and that they're quite obvious to
the end user, and very worried about us possibly missing some object
that we were expected to capture.

In any case, would a function along the lines of
pg_export_and_lock_snapshot(), which basically starts a transaction,
acquires locks on every object in the DB, and then returns the snapshot
ID, address this?  Perhaps even pg_dump could use that, on the belief
that it'd be faster for a backend function to acquire those locks than
for pg_dump to do it anyway?  I'm not sure that we want to support every
different combination of filtering options that pg_dump supports for
this, but we could start with the all-database option since that's, by
definition, the largest set which could be requested by pg_dump.

Or perhaps even a new lock type for this...  I'd love to see the
locking-all-objects portion of time disappear from my nightly
backups..

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] pg_dump --snapshot

2013-05-06 Thread Greg Stark

On Mon, May 6, 2013 at 10:02 PM, Simon Riggs si...@2ndquadrant.com wrote:
 At the database level, it rolls back the whole kaboodle. Not what I
 meant at all and I would expect people to start twitching at the
 prospect.

I think it would be pretty sweet but we don't have the infrastructure
for it. We would need to retain enough information in the WAL log (or
somewhere else) to reverse the records.

 The feature we have in PG9.2+ is the ability to set a transaction
 snapshot to a snapshot that existed in the database at some point,
 invoked by some external reference to it. The external reference is
 the piece of information that must be specified by the user to allow
 the database to look backwards. At the moment we can only specify a
 snapshot from a currently running transaction, i.e. the recent past. I
 foresee a feature that will allow us to look back further, possibly
 with some restrictions, though certainly read only.

This is similar to flashback query. And I think you're right that to
be comparable with Oracle's features we would need some option to
specify the snapshot based on time or wal position.  And fwiw I think
it could still be read-write in consistent-read or serializable mode.
If you tried to update any records that had been updated since you
would get a serialization failure.

So I just did some research. It seems Oracle's equivalent of pg_dump
expdp does use flashback internally to guarantee consistency in some
cases which is perhaps analogous to how pg_dump uses snapshots to
synchronize multiple sessions (though it sounds like Oracle uses it
for cases that just work in Postgres).

But more interestingly expdp does in fact have a user option to
specify a timestamp or scn (analogous to wal position) and use
flashback query to dump the data at that point in time. That's a
pretty clear a parallel to what you propose here.




-- 
greg


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

Re: [HACKERS] pg_dump --snapshot

* Greg Stark (st...@mit.edu) wrote:
 On Mon, May 6, 2013 at 10:02 PM, Simon Riggs si...@2ndquadrant.com wrote:
  At the database level, it rolls back the whole kaboodle. Not what I
  meant at all and I would expect people to start twitching at the
  prospect.
 
 I think it would be pretty sweet but we don't have the infrastructure
 for it. We would need to retain enough information in the WAL log (or
 somewhere else) to reverse the records.

Let me start off by saying that I do like the general idea.  We're
rather different from Oracle, which makes me wonder if we might be in a
slightly better position to handle this kind of an option from.

For example, I'm not sure that we need more information in the WAL..
What we need is a way to tell VACUUM to skip over 'recently modified'
records and not mark them as dead until some time has passed.  This is
essentially what we're already doing with the feedback mechanism in
replication, isn't it?  Would it be possible to make that a simple timer
instead of requiring a replication system which is feeding back that
information based on queries which are running on the replica?

 But more interestingly expdp does in fact have a user option to
 specify a timestamp or scn (analogous to wal position) and use
 flashback query to dump the data at that point in time. That's a
 pretty clear a parallel to what you propose here.

What happens to objects which have been dropped after the user-specified
flashback point?  My guess is that they simply aren't there for the user
to go look at, but perhaps I'm wrong.  Either way though, I'm not sure
that we'd accept such a poor solution to that problem.  For that matter,
what do we do today wrt replication feedback when/if that happens?

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] pg_dump --snapshot

2013-05-06 Thread Joe Conway

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 05/06/2013 03:00 PM, Stephen Frost wrote:
 For example, I'm not sure that we need more information in the
 WAL.. What we need is a way to tell VACUUM to skip over 'recently
 modified' records and not mark them as dead until some time has
 passed.  This is essentially what we're already doing with the
 feedback mechanism in replication, isn't it?  Would it be possible
 to make that a simple timer instead of requiring a replication
 system which is feeding back that information based on queries
 which are running on the replica?

This sounds a lot like having an idle in transaction session hanging
around. Could we use something akin to a prepared transaction to
preserve the ability to use the snapshot? Maybe with an (optional)
expiration timestamp?

Joe


- -- 
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting,  24x7 Support
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRiDCgAAoJEDfy90M199hlbroP/0vVnxo91Atc6hp9l7qFsYZA
YAsrLLHMcRGdP01o+XY50COhm0ScR2zJg88wSwJTwIve5PEKywu7waP6/7Ahty/s
7sHMHZJ90fNbRTqhb5L9/4hEMN0213biol4ANk/gVcNs1nF9t+BrQK3HMsGCe5P/
InMftpTHCuPdHOmAPLMgRi/rAzWgwEy/9A/B6sw+EmMvl7j7EX5Zjur/nHaZAE9s
mA5AxY8oZv7QRJNDmp3Bg0d6tR/6WzXQDv0eEkjpeInk8d/CSFZX/kOWwsGawIrz
9WpxuMRza/L08B0Faw+Bm1jRzjp9FW5SjYDzRLjEcheNreA6vLwHSKNneBfCofU3
SE6+kK/VRxrNyc4f2gq5gl6LmK/frDojoWSt9JUd5hhXSAcmuB5iEmryrnw6xRok
TyXO4PIT59zfLXbesONEJuVIekWVs6GHk5uC+h58Re1dt1cfdQzHrAlX39sodBb8
6uBp++DiPFCg/WklJ29qFL0p6IhXhywxmGuuHB8ca1p1rh8u13HsuJ70MjBAft62
r4T94A1N/vZ9veP6eE8XFYFLaXiNUiR+r1vHdKn6MXnFpqV9OMuJ2pm476j9xSb/
nMOHQFln4IM7W++tV2y9sKKG+C8RqtCAXVSdUe2fFX9FWfprmynecrsphyD17pCi
/ZQFv0jkmS/mBWF7gFjx
=3azd
-END PGP SIGNATURE-


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

On 6 May 2013 22:13, Tom Lane t...@sss.pgh.pa.us wrote:
Simon Riggs si...@2ndquadrant.com writes:
It does *not* pass in a raw snapshot. All it does is to allow pg_dump
to use an API that is already exposed by the backend for this very
purpose, one that has been in Postgres since 9.2.
http://www.postgresql.org/docs/devel/static/functions-admin.html#FUNCTIONS-SNAPSHOT-SYNCHRONIZATION

Minor patch, no amazing new functionality, no drama.

You're ignoring the objection ...

No, I just don't see a problem at all.

Locks and snapshots have got nothing to do with each other, in
Postgres. Taking a snapshot doesn't imply that database objects are
locked; whoever takes the snapshot should lock things first, if they
are worried by that.

If anybody really wanted to fix pg_dump, they could do. If that was so
important, why block this patch, but allow parallel pg_dump to be
committed without it?

There is no risk that is larger than the one already exposed by the
existing user API.

If you do see a risk in the existing API, please deprecate it and
remove it from the docs, or mark it not-for-use-by-users. I hope you
don't, but if you do, do it now - I'll be telling lots of people about
all the useful things you can do with it over the next few years,
hopefully in pg_dump as well.

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

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

Re: [HACKERS] pg_dump --snapshot

Simon,

* Simon Riggs (si...@2ndquadrant.com) wrote:
 If anybody really wanted to fix pg_dump, they could do. If that was so
 important, why block this patch, but allow parallel pg_dump to be
 committed without it?

Because parallel pg_dump didn't make the problem any *worse*..?  This
does.  The problem existed before parallel pg_dump.

 There is no risk that is larger than the one already exposed by the
 existing user API.

The API exposes it, yes, but *pg_dump* isn't any worse than it was
before.

 If you do see a risk in the existing API, please deprecate it and
 remove it from the docs, or mark it not-for-use-by-users. I hope you
 don't, but if you do, do it now - I'll be telling lots of people about
 all the useful things you can do with it over the next few years,
 hopefully in pg_dump as well.

pg_dump uses it already and uses it as best it can.  Users could use it
also, provided they understand the constraints around it.  However,
there really isn't a way for users to use this new option correctly-
they would need to intuit what pg_dump will want to lock, lock it
immediately after their transaction is created, and only *then* get the
snapshot ID and pass it to pg_dump, hoping against hope that pg_dump
will actually need the locks that they decided to acquire..

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] pg_dump --snapshot

On 2013-05-06 20:18:26 -0400, Stephen Frost wrote:
 Simon,
 
 * Simon Riggs (si...@2ndquadrant.com) wrote:
  If anybody really wanted to fix pg_dump, they could do. If that was so
  important, why block this patch, but allow parallel pg_dump to be
  committed without it?

 Because parallel pg_dump didn't make the problem any *worse*..?  This
 does.  The problem existed before parallel pg_dump.

Yes, it did.

  There is no risk that is larger than the one already exposed by the
  existing user API.

 The API exposes it, yes, but *pg_dump* isn't any worse than it was
 before.

No, but its still broken. pg_dump without the parameter being passed
isn't any worse off after the patch has been applied. With the parameter
the window gets a bit bigger sure...

  If you do see a risk in the existing API, please deprecate it and
  remove it from the docs, or mark it not-for-use-by-users. I hope you
  don't, but if you do, do it now - I'll be telling lots of people about
  all the useful things you can do with it over the next few years,
  hopefully in pg_dump as well.

 pg_dump uses it already and uses it as best it can.  Users could use it
 also, provided they understand the constraints around it.  However,
 there really isn't a way for users to use this new option correctly-
 they would need to intuit what pg_dump will want to lock, lock it
 immediately after their transaction is created, and only *then* get the
 snapshot ID and pass it to pg_dump, hoping against hope that pg_dump
 will actually need the locks that they decided to acquire..

Given that we don't have all that many types of objects we can lock,
that task isn't all that complicated. But I'd guess a very common usage
is to start the snapshot and immediately fork pg_dump. In that case the
window between snapshot acquiration and reading the object list is
probably smaller than the one between reading the object list and
locking.

This all reads like a textbook case of perfect is the enemy of good to
me.

A rather useful feature has to fix a bug in pg_dump which a) exists for
ages b) has yet to be reported to the lists c) is rather complicated to
fix and quite possibly requires proper snapshots for internals?

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

On 2013-05-07 02:53:16 +0200, Andres Freund wrote:
 A rather useful feature has to fix a bug in pg_dump which a) exists for
 ages b) has yet to be reported to the lists c) is rather complicated to
 fix and quite possibly requires proper snapshots for internals?

Just to clarify: I think this worth fixing, but it just seems like
something that needs to be fixed independently from this feature.

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

* Andres Freund (and...@2ndquadrant.com) wrote:
 On 2013-05-06 20:18:26 -0400, Stephen Frost wrote:
  Because parallel pg_dump didn't make the problem any *worse*..?  This
  does.  The problem existed before parallel pg_dump.
 
 Yes, it did.

That's not entirely clear- are you agreeing with my statements, or not?

  The API exposes it, yes, but *pg_dump* isn't any worse than it was
  before.
 
 No, but its still broken. pg_dump without the parameter being passed
 isn't any worse off after the patch has been applied. With the parameter
 the window gets a bit bigger sure...

I'm not entirely following the distinction you're making here.  What I
think you're saying is that pg_dump is still busted and pg_dump when
the parameter isn't passed is busted and pg_dump creates a bigger
window where it can break if the parameter is passed.  All of which I
think I agree with, but I don't agree with the conclusion that this
larger window is somehow acceptable because there's a very small window
(one which can't be made any smaller, today..) which exists today.

 Given that we don't have all that many types of objects we can lock,
 that task isn't all that complicated.

Alright, then let's provide a function which will do that and tell
people to use it instead of just using pg_export_snapshot(), which
clearly doesn't do that.

 But I'd guess a very common usage
 is to start the snapshot and immediately fork pg_dump. In that case the
 window between snapshot acquiration and reading the object list is
 probably smaller than the one between reading the object list and
 locking.

How would it be smaller..?  I agree that it may only be a few seconds
larger, but you're adding things to the front which the current code
doesn't run, yet running everything the current code runs, so it'd have
to be larger..

 This all reads like a textbook case of perfect is the enemy of good to
 me.

I believe the main argument here is really around you should think
about these issues before just throwing this in and not it must be
perfect before it goes in.  Perhaps it shouldn't make things *worse*
than they are now would also be apt..

 A rather useful feature has to fix a bug in pg_dump which a) exists for
 ages b) has yet to be reported to the lists c) is rather complicated to
 fix and quite possibly requires proper snapshots for internals?

I've not seen anyone calling for this to be fixed in pg_dump first,
though I did suggest how that might be done.  Rather, it shouldn't make
things *worse* than they are now, which apparently isn't difficult, per
your comments above...  so why not fix this to at least not make things
worse?

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] pg_dump --snapshot

2013-05-06 Thread Craig Ringer


-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 05/07/2013 06:37 AM, Joe Conway wrote:
 On 05/06/2013 03:00 PM, Stephen Frost wrote:
  For example, I'm not sure that we need more information in the
  WAL.. What we need is a way to tell VACUUM to skip over 'recently
  modified' records and not mark them as dead until some time has
  passed.  This is essentially what we're already doing with the
  feedback mechanism in replication, isn't it?  Would it be possible
  to make that a simple timer instead of requiring a replication
  system which is feeding back that information based on queries
  which are running on the replica?

 This sounds a lot like having an idle in transaction session hanging
 around. Could we use something akin to a prepared transaction to
 preserve the ability to use the snapshot? Maybe with an (optional)
 expiration timestamp?

I was thinking the same thing myself but assuming it was just another
one of my impractical ideas that're borne of lack of in-depth
understanding of the problem. A lock holder of some kind that's
independent of the session seems to be required here, either associated
directly with the snapshot or created after it and managed
independently. A prepared transaction created shortly after the commit
the snapshot refers to that holds all the required locks would seem to
serve the required purpose. A bit of a hack, but enough to make this
sane to use until/unless someone has the time/funding to do the major
rework needed to make this work right.

- -- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.13 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJRiFUOAAoJELBXNkqjr+S2TksH/2KqL+7eUxyE9aBg3Ci3gSc7
OP0y3jk34mpG5aXtzCcVD8jC81bDT0eRGDAEnjAliHW/UCWkaxdX6ziY3BRIfJ7B
vvpArYEA3I0CgewGypciT3/692iDVAvTsVXnd1Vx4jJLiyYt83MYr7EmOpsJwzG1
NJ7MFjAV+61SBW8uRwSopvqm2e6MZiYjCR3orvqBm7t3xKeuXAOv4zM5pM+m4hz5
gGB53XKPNsyr2m9pX8ScxprHvkAjflXB6QQBR07XBrkb1kWXifSKxw7bsscxP4hv
GQxcRzex2wWVJ654NH7v/QNt4Ynp2qUpl1tpTloIzv0aF+BTLXdlbGLpkjJvwRU=
=n/k9
-END PGP SIGNATURE-



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