Re: [HACKERS] REINDEX CONCURRENTLY 2.0

2014-10-01 Thread Michael Paquier
On Wed, Aug 27, 2014 at 3:53 PM, Michael Paquier michael.paqu...@gmail.com
wrote:

 On Wed, Aug 27, 2014 at 3:41 PM, Andres Freund and...@2ndquadrant.com
 wrote:
  Can you add it to the next CF? I'll try to look earlier, but can't
  promise anything.
 
  I very much would like this to get committed in some form or another.
 Added it here to keep track of it:
 https://commitfest.postgresql.org/action/patch_view?id=1563

Attached is a fairly-refreshed patch that should be used as a base for the
next commit fest. The following changes should be noticed:
- Use of AccessExclusiveLock when swapping relfilenodes of an index and its
concurrent entry instead of ShareUpdateExclusiveLock for safety. At the
limit of my understanding, that's the consensus reached until now.
- Cleanup of many comments and refresh of the documentation that was
somewhat wrongly-formulated or shaped at some places
- Addition of support for autocommit off in psql for REINDEX [ TABLE |
INDEX ] CONCURRENTLY
- Some more code cleanup..
I haven't been through the tab completion support for psql but looking at
tab-completion.c this seems a bit tricky with the stuff related to CREATE
INDEX CONCURRENTLY already present. Nothing huge though.
Regards,
-- 
Michael
From 5e9125da5cbeb2f6265b68ff14cc70e4cb10d502 Mon Sep 17 00:00:00 2001
From: Michael Paquier mich...@otacoo.com
Date: Wed, 27 Aug 2014 10:56:02 +0900
Subject: [PATCH] Support for REINDEX CONCURRENTLY

Fully supported patch, with regression and isolation tests, including
documentation and support for autocommit 'off' in psql. This version
uses an exclusive lock when swapping relations for safety. psql tab
completion has not been added yet in this version as this is rather
independent from the core version and actually psql makes things a
bit tricky with CREATE INDEX CONCURRENTLY using the same keywords.
---
 doc/src/sgml/mvcc.sgml |   5 +-
 doc/src/sgml/ref/reindex.sgml  | 160 +++-
 src/backend/catalog/index.c| 476 ++--
 src/backend/catalog/toasting.c |   2 +-
 src/backend/commands/indexcmds.c   | 821 ++---
 src/backend/commands/tablecmds.c   |  33 +-
 src/backend/executor/execUtils.c   |  14 +
 src/backend/nodes/copyfuncs.c  |   1 +
 src/backend/nodes/equalfuncs.c |   1 +
 src/backend/parser/gram.y  |  17 +-
 src/backend/tcop/utility.c |  12 +-
 src/bin/psql/common.c  |  17 +
 src/include/catalog/index.h|  19 +-
 src/include/commands/defrem.h  |   7 +-
 src/include/nodes/parsenodes.h |   1 +
 .../isolation/expected/reindex-concurrently.out|  78 ++
 src/test/isolation/isolation_schedule  |   1 +
 src/test/isolation/specs/reindex-concurrently.spec |  40 +
 src/test/regress/expected/create_index.out |  57 ++
 src/test/regress/sql/create_index.sql  |  42 ++
 20 files changed, 1615 insertions(+), 189 deletions(-)
 create mode 100644 src/test/isolation/expected/reindex-concurrently.out
 create mode 100644 src/test/isolation/specs/reindex-concurrently.spec

diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml
index cd55be8..653b120 100644
--- a/doc/src/sgml/mvcc.sgml
+++ b/doc/src/sgml/mvcc.sgml
@@ -864,7 +864,8 @@ ERROR:  could not serialize access due to read/write dependencies among transact
 
 para
  Acquired by commandVACUUM/command (without optionFULL/option),
- commandANALYZE/, commandCREATE INDEX CONCURRENTLY/, and
+ commandANALYZE/, commandCREATE INDEX CONCURRENTLY/,
+ commandREINDEX CONCURRENTLY/,
  commandALTER TABLE VALIDATE/command and other
  commandALTER TABLE/command variants (for full details see
  xref linkend=SQL-ALTERTABLE).
@@ -1143,7 +1144,7 @@ ERROR:  could not serialize access due to read/write dependencies among transact
 
sect2 id=locking-pages
 titlePage-level Locks/title
-  
+
 para
  In addition to table and row locks, page-level share/exclusive locks are
  used to control read/write access to table pages in the shared buffer
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index cabae19..0b7a93c 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  refsynopsisdiv
 synopsis
-REINDEX { INDEX | TABLE | DATABASE | SYSTEM } replaceable class=PARAMETERname/replaceable [ FORCE ]
+REINDEX { INDEX | TABLE | DATABASE | SYSTEM } [ CONCURRENTLY ] replaceable class=PARAMETERname/replaceable [ FORCE ]
 /synopsis
  /refsynopsisdiv
 
@@ -68,9 +68,22 @@ REINDEX { INDEX | TABLE | DATABASE | SYSTEM } replaceable class=PARAMETERnam
   An index build with the literalCONCURRENTLY/ option failed, leaving
   an quoteinvalid/ 

Re: [HACKERS] pgcrypto: PGP armor headers

2014-10-01 Thread Heikki Linnakangas

On 09/30/2014 06:39 PM, Marko Tiikkaja wrote:

On 9/30/14 5:17 PM, Heikki Linnakangas wrote:

I'm actually now leaning towards providing just a single function,
pgp_armor_headers(text, key OUT text, value OUT text), which returns all
the keys and values. That gives maximum flexibility, and leaves it up to
the user to decide what to do with duplicate keys. It's pretty easy to
use that to extract just a single header, too:

snip

What do you think? Attached patch implements that, but the docs and
regression tests now need adjustment.


(You forgot the patch, but I can imagine what it would have been.)


Oops.


I'm not exactly sure to be honest.  I would personally like to see a
simple function for extracting a single header value in a scalar context
without having to deal with all the pain of SRFs, multiple matches and
no matches.  Sure, that got a lot better in 9.3 with LATERAL but it's
still way inferior to pgp_armor_header().

I can also see why someone would argue that I should just create the
function myself and that it doesn't have to be shipped with postgres.
But on the other hand, this is already an extension one has to
explicitly go and CREATE, and that considered one more function probably
wouldn't hurt too much.


Yeah, building the function to extract a single value is pretty simple 
once you have the set-returning function:


create function pgp_armor_header(armor text, key text) returns text 
language sql as $$

 select string_agg(value, ' ') from pgp_armor_headers($1) where key = $2
$$;

I spent a little time cleaning up the regression tests and docs, and 
ended up with the attached. But then I realized that there's a problem 
with UTF-8 conversion in the armor() function. It returns the armored 
blob as text, but forcibly converts the keys and values to UTF-8. That's 
not cool, because you will get invalidly encoded strings into the 
database, if you use the function while connected to a database that 
uses some other encoding than UTF-8.


RFC4880 says that the headers are in UTF-8, but armor() cannot safely 
return UTF-8 encoded text unless the database's encoding is also UTF-8. 
It also rightly says that using anything else than plain ASCII, even 
though nominally it's UTF-8, is a bad idea, because the whole point of 
ASCII-armoring is to make the format safe from encoding conversions.


We have two options:

1. Throw an error if there are any non-ASCII characters in the key/value 
arrays.

2. Don't convert them to UTF-8, but use the current database encoding.

Both seem sane to me. If we use the current database encoding, then we 
have to also decide what to do with the input, in pgp_armor_headers(). 
If armor() uses the database encoding, but pgp_armor_headers() treats 
the input as UTF-8, then a round-trip with pgp_armor_headers(armor(?)) 
won't work.


- Heikki

diff --git a/contrib/pgcrypto/Makefile b/contrib/pgcrypto/Makefile
index b6c9484..d1d75ca 100644
--- a/contrib/pgcrypto/Makefile
+++ b/contrib/pgcrypto/Makefile
@@ -26,7 +26,7 @@ MODULE_big	= pgcrypto
 OBJS		= $(SRCS:.c=.o) $(WIN32RES)
 
 EXTENSION = pgcrypto
-DATA = pgcrypto--1.1.sql pgcrypto--1.0--1.1.sql pgcrypto--unpackaged--1.0.sql
+DATA = pgcrypto--1.2.sql pgcrypto--1.1--1.2.sql pgcrypto--1.0--1.1.sql pgcrypto--unpackaged--1.0.sql
 PGFILEDESC = pgcrypto - cryptographic functions
 
 REGRESS = init md5 sha1 hmac-md5 hmac-sha1 blowfish rijndael \
diff --git a/contrib/pgcrypto/expected/pgp-armor.out b/contrib/pgcrypto/expected/pgp-armor.out
index c955494..bcf9590 100644
--- a/contrib/pgcrypto/expected/pgp-armor.out
+++ b/contrib/pgcrypto/expected/pgp-armor.out
@@ -102,3 +102,245 @@ em9va2E=
 -END PGP MESSAGE-
 ');
 ERROR:  Corrupt ascii-armor
+-- corrupt (no space after the colon)
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-
+foo:
+
+em9va2E=
+=
+-END PGP MESSAGE-
+');
+ERROR:  Corrupt ascii-armor
+-- header with empty value
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-
+foo: 
+
+em9va2E=
+=
+-END PGP MESSAGE-
+');
+ key | value 
+-+---
+ foo | 
+(1 row)
+
+-- simple
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-
+fookey: foovalue
+barkey: barvalue
+
+em9va2E=
+=
+-END PGP MESSAGE-
+');
+  key   |  value   
++--
+ fookey | foovalue
+ barkey | barvalue
+(2 rows)
+
+-- insane keys, part 1
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-
+insane:key : 
+
+em9va2E=
+=
+-END PGP MESSAGE-
+');
+ key | value 
+-+---
+ insane:key  | 
+(1 row)
+
+-- insane keys, part 2
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-
+insane:key : text value here
+
+em9va2E=
+=
+-END PGP MESSAGE-
+');
+ key |  value  
+-+-
+ insane:key  | text value here
+(1 row)
+
+-- long value
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-
+long: this value is more than 76 characters long, but it should 

[HACKERS] Value locking Wiki page

2014-10-01 Thread Peter Geoghegan
The current approach to value locking remains a controversial aspect
of my INSERT ... ON CONFLICT UPDATE patch. I must admit that this is a
very complicated area, and it's difficult to keep things straight,
particularly with the relevant discussion scattered all over the
place.

In the hope of unblocking things, I have created this Wiki page, which
details the advantages and disadvantages of all 3 approaches that have
been discussed, as suggested by myself and others:

https://wiki.postgresql.org/wiki/Value_locking

I actually think that we're only in disagreement on the extent to
which the stated advantages and disadvantages are useful or harmful.
So I think that at the moment this description is a reasonably
balanced handling of the issues. Please add any points that I missed.

This is by no means complete yet. There is pretty limited handling of
what I call #3. Promise index tuples, because there was no
prototype version of that design, and there are many open questions
about how it would be implemented relative to the other 2 approaches.
Perhaps Andres or Simon would care to improve it. I think I've done a
better job of describing #2, Heikki's design - hardly surprising,
since there was a prototype that we discussed at length -  but I'd
appreciate it if other people would make sure that I have everything
right there. This is hopefully the beginning of working the issues
out. I have of course also described my own proposed design alongside
the others.

Please edit and add to my words as you see fit.
-- 
Peter Geoghegan


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


Re: [HACKERS] Value locking Wiki page

2014-10-01 Thread Andres Freund
On 2014-10-01 01:44:04 -0700, Peter Geoghegan wrote:
 In the hope of unblocking things, I have created this Wiki page, which
 details the advantages and disadvantages of all 3 approaches that have
 been discussed, as suggested by myself and others:
 
 https://wiki.postgresql.org/wiki/Value_locking

Good.

 I actually think that we're only in disagreement on the extent to
 which the stated advantages and disadvantages are useful or harmful.
 So I think that at the moment this description is a reasonably
 balanced handling of the issues. Please add any points that I missed.
 
 This is by no means complete yet. There is pretty limited handling of
 what I call #3. Promise index tuples, because there was no
 prototype version of that design, and there are many open questions
 about how it would be implemented relative to the other 2 approaches.
 Perhaps Andres or Simon would care to improve it. I think I've done a
 better job of describing #2, Heikki's design - hardly surprising,
 since there was a prototype that we discussed at length -  but I'd
 appreciate it if other people would make sure that I have everything
 right there. This is hopefully the beginning of working the issues
 out. I have of course also described my own proposed design alongside
 the others.

FWIW, Heikki's approach isn't what I'd have choosen, but it's something
I can live with.

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] pgcrypto: PGP armor headers

2014-10-01 Thread Marko Tiikkaja

On 10/1/14, 9:11 AM, Heikki Linnakangas wrote:

I spent a little time cleaning up the regression tests and docs, and
ended up with the attached. But then I realized that there's a problem
with UTF-8 conversion in the armor() function. It returns the armored
blob as text, but forcibly converts the keys and values to UTF-8. That's
not cool, because you will get invalidly encoded strings into the
database, if you use the function while connected to a database that
uses some other encoding than UTF-8.

RFC4880 says that the headers are in UTF-8, but armor() cannot safely
return UTF-8 encoded text unless the database's encoding is also UTF-8.
It also rightly says that using anything else than plain ASCII, even
though nominally it's UTF-8, is a bad idea, because the whole point of
ASCII-armoring is to make the format safe from encoding conversions.


Ugh.  Right.


We have two options:

1. Throw an error if there are any non-ASCII characters in the key/value
arrays.
2. Don't convert them to UTF-8, but use the current database encoding.

Both seem sane to me. If we use the current database encoding, then we
have to also decide what to do with the input, in pgp_armor_headers().
If armor() uses the database encoding, but pgp_armor_headers() treats
the input as UTF-8, then a round-trip with pgp_armor_headers(armor(?))
won't work.


Yeah.  Both options seem fine to me.  Throwing an error perhaps slightly 
more so.



.marko


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


Re: [HACKERS] Value locking Wiki page

2014-10-01 Thread Heikki Linnakangas

On 10/01/2014 11:49 AM, Andres Freund wrote:

On 2014-10-01 01:44:04 -0700, Peter Geoghegan wrote:

In the hope of unblocking things, I have created this Wiki page, which
details the advantages and disadvantages of all 3 approaches that have
been discussed, as suggested by myself and others:

https://wiki.postgresql.org/wiki/Value_locking


Thank! That's a good summary.


This is by no means complete yet. There is pretty limited handling of
what I call #3. Promise index tuples, because there was no
prototype version of that design, and there are many open questions
about how it would be implemented relative to the other 2 approaches.
Perhaps Andres or Simon would care to improve it. I think I've done a
better job of describing #2, Heikki's design - hardly surprising,
since there was a prototype that we discussed at length -  but I'd
appreciate it if other people would make sure that I have everything
right there. This is hopefully the beginning of working the issues
out. I have of course also described my own proposed design alongside
the others.


FWIW, Heikki's approach isn't what I'd have choosen, but it's something
I can live with.


I didn't realize that promise index tuples were even seriously 
discussed. I guess that can be made to work, too, although I don't see 
the point. It wouldn't work with GiST indexes, for the same reasons as 
page-level locking won't work (a tuple can legally be inserted anywhere 
in a GiST index - it just degrades the index making searching more 
expensive). And lossy GiST opclasses are a problem too.


It's actually more similar to approach #1 in that it puts the 
responsibility of the locking in the indexam. You would probably need 
the same kind of API changes to the indexam, and you could well imagine 
one indexam to implement index promise tuples, while another one might 
use heavy-weight locks. I don't see the advantage of #3.


- 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] Value locking Wiki page

2014-10-01 Thread Andres Freund
On 2014-10-01 12:44:25 +0300, Heikki Linnakangas wrote:
 I didn't realize that promise index tuples were even seriously discussed.
 I guess that can be made to work, too, although I don't see the point. It
 wouldn't work with GiST indexes, for the same reasons as page-level locking
 won't work (a tuple can legally be inserted anywhere in a GiST index - it
 just degrades the index making searching more expensive). And lossy GiST
 opclasses are a problem too.

 It's actually more similar to approach #1 in that it puts the responsibility
 of the locking in the indexam. You would probably need the same kind of API
 changes to the indexam, and you could well imagine one indexam to implement
 index promise tuples, while another one might use heavy-weight locks. I
 don't see the advantage of #3.

I don't think all of that is of necessary consequence. What I was
thinking of would actually works similar to #2: Just that, instead of a
heap tuple, it inserts a index tuple that a) has a PROMISE_TUPLE flag
set and b) the itemid points towards a xid instead of a heap
tuple. That's sufficient for detecting uniqueness violations using
a similar approach like in your proposal. Which does *not* have to
happen inside the AM.
Yes, it'd require some indexam API changes, but I don't think they'd be
to severe. Most importantly the ability to insert, return promise tuples to
outside and API to repoint a promise tuple to a real entry.

I haven't thought about lossy indexes, but I actually do think they
could be made work with that strategy.

The difference to #2 primarily is that it avoids speculatively inserting
in both the heap and index and just inserts into the index, thereby
detecting conflicts a bit earlier, after less work.

But, as I said, I'm ok with pursuing #2 on the basis that it's quite
likely to be simpler

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] libpq-dev: pg_config_manual.h redefines CACHE_LINE_SIZE

2014-10-01 Thread Andres Freund
On 2014-09-30 21:57:56 +0200, Christoph Berg wrote:
 Hi,
 
 there's a #define clash between pg_config_manual.h and FreeBSD's
 /usr/include/machine-amd64/param.h which also defines CACHE_LINE_SIZE.
 
 It's probably not really a PostgreSQL bug, but it seems easy enough to
 rename that definition now as it's only present in 9.4+. It's only
 used in one file, xlog.c: 375d8526f2900d0c377f44532f6d09ee06531f67

Changed in 9.4/master.

Thanks.

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] Value locking Wiki page

2014-10-01 Thread Simon Riggs
On 1 October 2014 10:44, Heikki Linnakangas hlinnakan...@vmware.com wrote:

 I didn't realize that promise index tuples were even seriously discussed.
 I guess that can be made to work, too, although I don't see the point. It
 wouldn't work with GiST indexes, for the same reasons as page-level locking
 won't work (a tuple can legally be inserted anywhere in a GiST index - it
 just degrades the index making searching more expensive). And lossy GiST
 opclasses are a problem too.

GiST doesn't support unique indexes, so it is not in any way a problem.

-- 
 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] Value locking Wiki page

2014-10-01 Thread Heikki Linnakangas

On 10/01/2014 01:50 PM, Simon Riggs wrote:

On 1 October 2014 10:44, Heikki Linnakangas hlinnakan...@vmware.com wrote:


I didn't realize that promise index tuples were even seriously discussed.
I guess that can be made to work, too, although I don't see the point. It
wouldn't work with GiST indexes, for the same reasons as page-level locking
won't work (a tuple can legally be inserted anywhere in a GiST index - it
just degrades the index making searching more expensive). And lossy GiST
opclasses are a problem too.


GiST doesn't support unique indexes, so it is not in any way a problem.


GiST supports exclusion constraints. That is one of the main reasons I 
want to do promise tuples, instead of locking within the indexam: to 
support this feature with exclusion constraints.


- 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] test_shm_mq failing on anole (was: Sending out a request for more buildfarm animals?)

2014-10-01 Thread Andres Freund
On 2014-09-29 14:46:20 -0400, Robert Haas wrote:
 This happened again, and I investigated further.

Uh. Interestingly anole just succeeded twice:
http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=anolebr=REL9_4_STABLE

I plan to commit the mask/unmask patch regardless, but it's curious. The
first of the two builds could have been you 'unsticking' it by manually
mucking around. Did you also do that for the second build?

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] pgcrypto: PGP armor headers

2014-10-01 Thread Heikki Linnakangas

On 10/01/2014 11:58 AM, Marko Tiikkaja wrote:

On 10/1/14, 9:11 AM, Heikki Linnakangas wrote:

We have two options:

1. Throw an error if there are any non-ASCII characters in the key/value
arrays.
2. Don't convert them to UTF-8, but use the current database encoding.

Both seem sane to me. If we use the current database encoding, then we
have to also decide what to do with the input, in pgp_armor_headers().
If armor() uses the database encoding, but pgp_armor_headers() treats
the input as UTF-8, then a round-trip with pgp_armor_headers(armor(?))
won't work.


Yeah.  Both options seem fine to me.  Throwing an error perhaps slightly
more so.


I went with 1, throw an error. I also added checks that the key or value 
doesn't contain any embedded newlines, and that the key doesn't contain 
an embedded : . Those would cause the armor to be invalid.


I think this is now ready for commit, but since I've changed it quite 
significantly from what you originally submitted, please take a moment 
to review this.


- Heikki

diff --git a/.gitattributes b/.gitattributes
index ff96567..9466800 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -14,6 +14,7 @@ README.*	conflict-marker-size=32
 # Certain data files that contain special whitespace, and other special cases
 *.data		-whitespace
 contrib/tsearch2/sql/tsearch2.sql		whitespace=space-before-tab,blank-at-eof,-blank-at-eol
+contrib/pgcrypto/sql/pgp-armor.sql		whitespace=-blank-at-eol
 doc/bug.templatewhitespace=space-before-tab,-blank-at-eof,blank-at-eol
 src/backend/catalog/sql_features.txt		whitespace=space-before-tab,blank-at-eof,-blank-at-eol
 src/backend/tsearch/hunspell_sample.affix	whitespace=-blank-at-eof
diff --git a/contrib/pgcrypto/Makefile b/contrib/pgcrypto/Makefile
index b6c9484..d1d75ca 100644
--- a/contrib/pgcrypto/Makefile
+++ b/contrib/pgcrypto/Makefile
@@ -26,7 +26,7 @@ MODULE_big	= pgcrypto
 OBJS		= $(SRCS:.c=.o) $(WIN32RES)
 
 EXTENSION = pgcrypto
-DATA = pgcrypto--1.1.sql pgcrypto--1.0--1.1.sql pgcrypto--unpackaged--1.0.sql
+DATA = pgcrypto--1.2.sql pgcrypto--1.1--1.2.sql pgcrypto--1.0--1.1.sql pgcrypto--unpackaged--1.0.sql
 PGFILEDESC = pgcrypto - cryptographic functions
 
 REGRESS = init md5 sha1 hmac-md5 hmac-sha1 blowfish rijndael \
diff --git a/contrib/pgcrypto/expected/pgp-armor.out b/contrib/pgcrypto/expected/pgp-armor.out
index c955494..a8f73ac 100644
--- a/contrib/pgcrypto/expected/pgp-armor.out
+++ b/contrib/pgcrypto/expected/pgp-armor.out
@@ -102,3 +102,251 @@ em9va2E=
 -END PGP MESSAGE-
 ');
 ERROR:  Corrupt ascii-armor
+-- corrupt (no space after the colon)
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-
+foo:
+
+em9va2E=
+=
+-END PGP MESSAGE-
+');
+ERROR:  Corrupt ascii-armor
+-- header with empty value
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-
+foo: 
+
+em9va2E=
+=
+-END PGP MESSAGE-
+');
+ key | value 
+-+---
+ foo | 
+(1 row)
+
+-- simple
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-
+fookey: foovalue
+barkey: barvalue
+
+em9va2E=
+=
+-END PGP MESSAGE-
+');
+  key   |  value   
++--
+ fookey | foovalue
+ barkey | barvalue
+(2 rows)
+
+-- insane keys, part 1
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-
+insane:key : 
+
+em9va2E=
+=
+-END PGP MESSAGE-
+');
+ key | value 
+-+---
+ insane:key  | 
+(1 row)
+
+-- insane keys, part 2
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-
+insane:key : text value here
+
+em9va2E=
+=
+-END PGP MESSAGE-
+');
+ key |  value  
+-+-
+ insane:key  | text value here
+(1 row)
+
+-- long value
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-
+long: this value is more than 76 characters long, but it should still parse correctly as that''s permitted by RFC 4880
+
+em9va2E=
+=
+-END PGP MESSAGE-
+');
+ key  |  value  
+--+-
+ long | this value is more than 76 characters long, but it should still parse correctly as that's permitted by RFC 4880
+(1 row)
+
+-- long value, split up
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-
+long: this value is more than 76 characters long, but it should still 
+long: parse correctly as that''s permitted by RFC 4880
+
+em9va2E=
+=
+-END PGP MESSAGE-
+');
+ key  |  value   
+--+--
+ long | this value is more than 76 characters long, but it should still 
+ long | parse correctly as that's permitted by RFC 4880
+(2 rows)
+
+-- long value, split up, part 2
+select * from pgp_armor_headers('
+-BEGIN PGP MESSAGE-

Re: [HACKERS] Value locking Wiki page

2014-10-01 Thread Simon Riggs
On 1 October 2014 09:44, Peter Geoghegan p...@heroku.com wrote:

 In the hope of unblocking things, I have created this Wiki page, which
 details the advantages and disadvantages of all 3 approaches that have
 been discussed, as suggested by myself and others:

 https://wiki.postgresql.org/wiki/Value_locking

A better job is needed at genuine balance on those items. You keep
saying you don't care which approach you take and then every word you
write goes against that, making it difficult to discuss.

Quoting general research and other points about value locking are
reasonable in the general section, but not in the description for 1.

I'm glad you've called the first option Heavyweight Page Locking.
That name sums up what I see as the major objection to it, which is
that performance and scalability of the whole server will be damaged
signiciantly by using heavyweight locks for each row inserted. Please
add that concern as a negative; I'm sure testing has been done, but
not comparative testing against other techniques. I am motivated to
explain the promise index tuple approach further because of this,
which is an optimistic approach to locking.

The stated disadvantages for 3 are not accurate, to put it mildly. But
that's been useful because what was a small thought experiment is
beginning to look like a useful approach. I will post a summary of my
understanding.

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

2014-10-01 Thread Andres Freund
On 2014-09-26 16:19:33 -0700, Peter Geoghegan wrote:
 On Fri, Sep 26, 2014 at 3:25 PM, Peter Geoghegan p...@heroku.com wrote:
  On Fri, Sep 26, 2014 at 3:11 PM, Alvaro Herrera
  alvhe...@2ndquadrant.com wrote:
  FWIW there are 28 callers of HeapTupleHeaderGetXmin.
 
  Don't forget about direct callers to HeapTupleHeaderGetRawXmin(),
  though. There are plenty of those in tqual.c.
 
 Which reminds me: commit 37484ad2 added the opportunistic freezing
 stuff. To quote the commit message:
 
 
 Instead of changing the tuple xmin to FrozenTransactionId, the combination
 of HEAP_XMIN_COMMITTED and HEAP_XMIN_INVALID, which were previously never
 set together, is now defined as HEAP_XMIN_FROZEN.  A variety of previous
 proposals to freeze tuples opportunistically before vacuum_freeze_min_age
 is reached have foundered on the objection that replacing xmin by
 FrozenTransactionId might hinder debugging efforts when things in this
 area go awry; this patch is intended to solve that problem by keeping
 the XID around (but largely ignoring the value to which it is set).
 
 
 
 Why wouldn't the same objection (the objection that the earlier
 opportunistic freezing ideas stalled on) apply to directly setting
 tuple xmin to InvalidTransactionId?

Because it's pretty much unrelated? The FrozenTransactionId bit you
reference is about tuples that actually survive, which isn't the case
here.

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


[HACKERS] Promise index tuples for UPSERT

2014-10-01 Thread Simon Riggs
Summary of algorithm to use promise tuples for concurrency control
during UPSERT

1. Perform btree search to location of key, if it exists.
a) If an unkilled index tuple exists, we decide this is an UPDATE and
drop straight thru to step 2
b) If it does not exist, insert a promise tuple into unique index(s)
- marked with the xid of the inserting transaction, but using the key.
This happens while the page is locked, so it is not possible to insert
a second promise tuple concurrently.
Record the btree blockid on the index scan and move to step 3
When later insert scans see the promise tuple they perform
XactLockTableWait() and when they get control they look again for the
key. If they find a promise tuple with an aborted xid they replace
that value with their own xid and continue as a). Otherwise b).

2. Find existing heap tuple
Find heap tuple.
Check it is actually valid. If not, go back to (1), kill the prior
tuple and follow 1b) path
If it is valid, perform heap_update as normal.

3. Insert new heap tuple
Perform heap_insert
Re-find index tuple using the btree blockid recorded at step 1; this
may require moving right until we find the actual value we are looking
for, so block splits don't negatively affect this approach.
Once re-found we change the index tuple from a promise tuple to a
normal index tuple, by setting tid and removing promise flag. Tuple
remains same length because the value was known when promise tuple
inserted, so this is an inplace update.
Insert other index values normally.

If a transaction that inserted a promise tuple dies, how is that cleaned up?
Any user that sees a dead promise tuple with a value they want will clean it up.
Dead promise tuples can be removed as needed, just as killed tuples
currently are.
VACUUM can remove dead transactions from index as it scans, no problems.

Index bloat is less of a problem than with normal inserts since there
are additional ways of removing promise tuples. Only one index tuple
at a time can have a specific value, so we would actually reduce heap
bloat caused by concurrent inserts.

It's possible we find existing rows that we can't see according to our
snapshot. That is handled in exactly the same as the way we treat
UPDATEs.

There is a potential loop here in that we might find an index tuple,
follow it, find out the tuple is actually dead then return to insert a
promise tuple, only to find that someone else just did that and we
have to wait/re-follow the link to update the new tuple. That is an
extremely unlikely race condition, though possible; there is no heavy
locking to prevent that in this approach. In principle deadlocks are
possible from that, but that is not any different from the current
case where people that insert same key at same time might cause
deadlocks. Its not a common application pattern and not something we
should be protecting against.

All of this is only needed for unique indexes.

It can handled by a new API call for acquire_value_lock() and
release_value_lock(), or similar.

Advantages
* We don't do anything weird in the heap - if this breaks, data is not corrupt
* There is no heap bloat or index bloat above existing levels
* The approach leverages existing mechanism for transaction waiting
* Optimistic approach to value locking will improve performance over
heavy weight locking

Disadvantages
* Not written yet - 1 month to code, still possible for 9.5, doesn't look hard

Other stated possible disadvantages
* Violates existing invariant that every index tuple has a
corresponding heap tuple, which goes back to the Berkeley days.
Currently, we always create heap tuples first, and physically delete
them last. [Why is that a problem? Could be, but why?]
(Deleting them last implies something is being touched in that
regard by this suggestion. I'm not sure what you are referring to)

* Relatedly, concern about breaking VACUUM. VACUUMing of btrees occurs
with a list of TIDs to kill in index, built from the heap. Current
bulk delete callback cannot be used for this. Super-exclusive lock is
needed to delete tuples in btree page (i.e. VACUUM). Normally skipping
of LockBufferForCleanup() (as added by bbb6e559) works okay in heap
because it tends to imply that list of tids won't be bulk deleted in
index, where we currently cannot skip for failure to quickly acquire
super exclusive lock. So this could make the problem addressed by
bbb6e559 return to some extent.
[Don't see any problems; just test the xid as we scan a promise tuple
and remove it if needed]

Index-only bloat becomes a possibility. Implications are not well understood.
[FUD, no reason to believe there is a problem.]

We have to re-find any items using an ordinary index scan, not a tid.
Must match our xid to that.
[Explained above, easy and efficient.]

Doesn't have a strategy for dealing with unprincipled deadlocks, at
least at the moment. Presumably some aspects of #2 could be adopted
here.
[FUD. Unprincipled deadlock still not properly explained as yet. No
way of 

Re: [HACKERS] pg_receivexlog and replication slots

2014-10-01 Thread Andres Freund
Hi,

I've split and edited patch 0001:
0001) Old WIP patch for pg_recvlogical tests. Useful for easier development.
0002) Copy editing that should be in 9.4
0003) Rebased patches of yours
0004) My changes to 0003 besides the rebase. This'll be squashed, but I
  thought it might be interesting for you.

I haven't tested my edits besides running 0001...

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
From 45582de61f670967f72835cd14d1c520c3d04fce Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Wed, 1 Oct 2014 12:57:15 +0200
Subject: [PATCH 1/4] WIP: pg_recvlogical tests

---
 src/bin/pg_basebackup/Makefile|  3 ++
 src/bin/pg_basebackup/t/030_pg_recvlogical.pl | 45 +++
 2 files changed, 48 insertions(+)
 create mode 100644 src/bin/pg_basebackup/t/030_pg_recvlogical.pl

diff --git a/src/bin/pg_basebackup/Makefile b/src/bin/pg_basebackup/Makefile
index 90d19e7..3c0a14e 100644
--- a/src/bin/pg_basebackup/Makefile
+++ b/src/bin/pg_basebackup/Makefile
@@ -51,7 +51,10 @@ clean distclean maintainer-clean:
 	rm -rf tmp_check
 
 check: all
+# Needs test_decoding to run the pg_recvlogical tests
+	$(MAKE) -C $(top_builddir)/contrib/test_decoding DESTDIR='$(CURDIR)'/tmp_check/install install '$(CURDIR)'/tmp_check/log/install.log 21
 	$(prove_check)
 
 installcheck:
+# XXX: We rely on test_decoding already being installed here
 	$(prove_installcheck)
diff --git a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
new file mode 100644
index 000..1741319
--- /dev/null
+++ b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
@@ -0,0 +1,45 @@
+use strict;
+use warnings;
+use TestLib;
+use Test::More tests = 9;
+
+program_help_ok('pg_recvlogical');
+program_version_ok('pg_recvlogical');
+program_options_handling_ok('pg_recvlogical');
+
+
+my $tempdir = tempdir;
+start_test_server $tempdir;
+
+open HBA, $tempdir/pgdata/pg_hba.conf;
+print HBA local replication all trust\n;
+close HBA;
+system_or_bail 'pg_ctl', '-s', '-D', $tempdir/pgdata, 'reload';
+
+open HBA, $tempdir/pgdata/pg_hba.conf;
+print HBA local replication all trust\n;
+close HBA;
+
+open CONF, $tempdir/pgdata/postgresql.conf;
+print CONF max_wal_senders = 1\n;
+print CONF max_replication_slots = 2\n;
+print CONF wal_level = logical\n;
+close CONF;
+
+restart_test_server;
+
+command_fails([ 'pg_recvlogical', '-d', 'postgres', '--slot', 'regress', '--plugin', 'barf', '--create'],
+	   'pg recvlogical cannot create slot with nonexistant plugin');
+command_ok([ 'pg_recvlogical', '-d', 'postgres', '--slot', 'regress', '--create'],
+	   'pg recvlogical can create a slot');
+command_fails([ 'pg_recvlogical', '-d', 'postgres', '--slot', 'regress', '--create'],
+	   'pg recvlogical cannot create slot with a already used name');
+command_fails([ 'pg_recvlogical', '-d', 'postgres', '--slot', 'regress', '--create'],
+	   'pg recvlogical cannot create slot when all slots are in use');
+command_ok([ 'pg_recvlogical', '-d', 'postgres', '--slot', 'regress', '--drop'],
+	   'pg recvlogical can drop slot');
+command_fails([ 'pg_recvlogical', '-d', 'postgres', '--slot', 'regress', '--start'],
+	   'pg recvlogical cannot stream from nonexistant slot');
+
+# cannot easily test streaming actual changes because that goes on
+# forever.
-- 
1.8.3.251.g1462b67

From b9f73cef1a9dc1aa96ad1807893e78c924f690f6 Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Mon, 29 Sep 2014 15:35:40 +0200
Subject: [PATCH 2/4] Message and comment policing of pg_recvlogical.c.

Several comments still referred to 'initiating', 'freeing', 'stopping'
replication slots. These were terms used during different phases of
the development of logical decoding, but are no long accurate.

Author: Michael Paquier

Backpatch to 9.4 where pg_recvlogical was introduced.
---
 src/bin/pg_basebackup/pg_recvlogical.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index f3b0f34..4144688 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -868,7 +868,7 @@ main(int argc, char **argv)
 
 
 	/*
-	 * stop a replication slot
+	 * drop a replication slot
 	 */
 	if (do_drop_slot)
 	{
@@ -876,7 +876,7 @@ main(int argc, char **argv)
 
 		if (verbose)
 			fprintf(stderr,
-	_(%s: freeing replication slot \%s\\n),
+	_(%s: dropping replication slot \%s\\n),
 	progname, replication_slot);
 
 		snprintf(query, sizeof(query), DROP_REPLICATION_SLOT \%s\,
@@ -892,8 +892,8 @@ main(int argc, char **argv)
 		if (PQntuples(res) != 0 || PQnfields(res) != 0)
 		{
 			fprintf(stderr,
-	_(%s: could not stop logical replication: got %d rows and %d fields, expected %d rows and %d fields\n),
-	progname, PQntuples(res), PQnfields(res), 0, 0);
+	

Re: [HACKERS] CREATE IF NOT EXISTS INDEX

2014-10-01 Thread Fabrízio de Royes Mello
On Wed, Oct 1, 2014 at 7:57 AM, José Luis Tallón jltal...@adv-solutions.net
wrote:

 [snip]

 Please excuse my jumping in, but the EXPECTED syntax is:

 CREATE INDEX IF NOT EXISTS .

 whereas your current patch implements:

 CREATE [IF NOT EXISTS] INDEX 


 if I'm reading the grammar correctly.


I think it's not wrong. Look at other CINE that already implemented [1] [2].

But CINE for CREATE TABLE is like your proposal [3] :

CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF
NOT EXISTS ] table_name ...

So, what's the correct/best grammar?

CREATE [ IF NOT EXISTS ] [ UNIQUE ] INDEX index_name

or

CREATE [ UNIQUE ] INDEX [ IF NOT EXISTS ] index_name


 I guess it would be most interesting to implement this minor change for
the next version of the patch. Please do remember to update the
documentation accordingly.


I will...


 By the way, you also forgot to remove a previous patch implementing
namespace_nameDOTrelation_name for RLS messages. Maybe a rebase is
needed?


Sorry... my mistake. Fix attached.


Regards,

[1] http://www.postgresql.org/docs/devel/static/sql-createschema.html
[2] http://www.postgresql.org/docs/devel/static/sql-createsequence.html
[3] http://www.postgresql.org/docs/devel/static/sql-createtable.html

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
 Timbira: http://www.timbira.com.br
 Blog: http://fabriziomello.github.io
 Linkedin: http://br.linkedin.com/in/fabriziomello
 Twitter: http://twitter.com/fabriziomello
 Github: http://github.com/fabriziomello
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index e469b17..7886729 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  refsynopsisdiv
 synopsis
-CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ replaceable class=parametername/replaceable ] ON replaceable class=parametertable_name/replaceable [ USING replaceable class=parametermethod/replaceable ]
+CREATE [ IF NOT EXISTS ] [ UNIQUE ] INDEX [ CONCURRENTLY ] [ replaceable class=parametername/replaceable ] ON replaceable class=parametertable_name/replaceable [ USING replaceable class=parametermethod/replaceable ]
 ( { replaceable class=parametercolumn_name/replaceable | ( replaceable class=parameterexpression/replaceable ) } [ COLLATE replaceable class=parametercollation/replaceable ] [ replaceable class=parameteropclass/replaceable ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [, ...] )
 [ WITH ( replaceable class=PARAMETERstorage_parameter/replaceable = replaceable class=PARAMETERvalue/replaceable [, ... ] ) ]
 [ TABLESPACE replaceable class=parametertablespace_name/replaceable ]
@@ -99,6 +99,16 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ replaceable class=parametername/
 
 variablelist
  varlistentry
+  termliteralIF NOT EXISTS/literal/term
+  listitem
+   para
+Do nothing (except issuing a notice) if a index with the same name
+already exists.
+   /para
+  /listitem
+ /varlistentry
+
+ varlistentry
   termliteralUNIQUE/literal/term
   listitem
para
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index ee10594..8905e30 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -697,7 +697,8 @@ index_create(Relation heapRelation,
 			 bool allow_system_table_mods,
 			 bool skip_build,
 			 bool concurrent,
-			 bool is_internal)
+			 bool is_internal,
+			 bool if_not_exists)
 {
 	Oid			heapRelationId = RelationGetRelid(heapRelation);
 	Relation	pg_class;
@@ -773,10 +774,22 @@ index_create(Relation heapRelation,
 		elog(ERROR, shared relations must be placed in pg_global tablespace);
 
 	if (get_relname_relid(indexRelationName, namespaceId))
+	{
+		if (if_not_exists)
+		{
+			ereport(NOTICE,
+	(errcode(ERRCODE_DUPLICATE_TABLE),
+	 errmsg(relation \%s\ already exists, skipping,
+			indexRelationName)));
+			heap_close(pg_class, RowExclusiveLock);
+			return InvalidOid;
+		}
+
 		ereport(ERROR,
 (errcode(ERRCODE_DUPLICATE_TABLE),
  errmsg(relation \%s\ already exists,
 		indexRelationName)));
+	}
 
 	/*
 	 * construct tuple descriptor for index tuples
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 160f006..5ef6dcc 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -342,7 +342,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
  rel-rd_rel-reltablespace,
  collationObjectId, classObjectId, coloptions, (Datum) 0,
  true, false, false, false,
- true, false, false, true);
+ true, false, false, true, false);
 
 	heap_close(toast_rel, NoLock);
 
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8a1cb4b..a03773b 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -610,7 +610,14 @@ DefineIndex(Oid relationId,
 	

Re: [HACKERS] Value locking Wiki page

2014-10-01 Thread Simon Riggs
On 1 October 2014 11:58, Heikki Linnakangas hlinnakan...@vmware.com wrote:
 On 10/01/2014 01:50 PM, Simon Riggs wrote:

 On 1 October 2014 10:44, Heikki Linnakangas hlinnakan...@vmware.com
 wrote:

 I didn't realize that promise index tuples were even seriously
 discussed.
 I guess that can be made to work, too, although I don't see the point. It
 wouldn't work with GiST indexes, for the same reasons as page-level
 locking
 won't work (a tuple can legally be inserted anywhere in a GiST index - it
 just degrades the index making searching more expensive). And lossy GiST
 opclasses are a problem too.


 GiST doesn't support unique indexes, so it is not in any way a problem.


 GiST supports exclusion constraints. That is one of the main reasons I want
 to do promise tuples, instead of locking within the indexam: to support this
 feature with exclusion constraints.

That does sound interesting, but I am concerned the semantics may cause issues.

If I go to insert a row for 'UK' and find an existing row for
'Europe', do we really want to update the population of Europe to be
the population of the UK, simply because the UK and Europe have an
exclusion conflict?

Please give some concrete examples of a business request that might be
satisified by such a feature.

-- 
 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] pgcrypto: PGP armor headers

2014-10-01 Thread Marko Tiikkaja

On 10/1/14 1:01 PM, Heikki Linnakangas wrote:

On 10/01/2014 11:58 AM, Marko Tiikkaja wrote:

On 10/1/14, 9:11 AM, Heikki Linnakangas wrote:

We have two options:

1. Throw an error if there are any non-ASCII characters in the key/value
arrays.
2. Don't convert them to UTF-8, but use the current database encoding.

Both seem sane to me. If we use the current database encoding, then we
have to also decide what to do with the input, in pgp_armor_headers().
If armor() uses the database encoding, but pgp_armor_headers() treats
the input as UTF-8, then a round-trip with pgp_armor_headers(armor(?))
won't work.


Yeah.  Both options seem fine to me.  Throwing an error perhaps slightly
more so.


I went with 1, throw an error. I also added checks that the key or value
doesn't contain any embedded newlines, and that the key doesn't contain
an embedded : . Those would cause the armor to be invalid.


Great.


I think this is now ready for commit, but since I've changed it quite
significantly from what you originally submitted, please take a moment
to review this.


  1) I see this compiler warning:

pgp-pgsql.c: In function ‘pg_armor’:
pgp-pgsql.c:960:18: warning: ‘values’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]

pgp_armor_encode((uint8 *) VARDATA(data), data_len, buf,

 It's bogus, but worth silencing anyway.

  2) There's what looks like an extra whitespace change in 
pgp_armor_encode(), but maybe that's intentional?


  3) Also, I think the attached two corner cases deserve their own tests.

Other than the above, the patch looks good to me.  Huge thanks for your 
work on this one!



.marko
*** a/contrib/pgcrypto/expected/pgp-armor.out
--- b/contrib/pgcrypto/expected/pgp-armor.out
***
*** 112,117  em9va2E=
--- 112,137 
  -END PGP MESSAGE-
  ');
  ERROR:  Corrupt ascii-armor
+ -- corrupt (no empty line)
+ select * from pgp_armor_headers('
+ -BEGIN PGP MESSAGE-
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ');
+ ERROR:  Corrupt ascii-armor
+ -- no headers
+ select * from pgp_armor_headers('
+ -BEGIN PGP MESSAGE-
+ 
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ');
+  key | value 
+ -+---
+ (0 rows)
+ 
  -- header with empty value
  select * from pgp_armor_headers('
  -BEGIN PGP MESSAGE-
*** a/contrib/pgcrypto/sql/pgp-armor.sql
--- b/contrib/pgcrypto/sql/pgp-armor.sql
***
*** 67,72  em9va2E=
--- 67,89 
  -END PGP MESSAGE-
  ');
  
+ -- corrupt (no empty line)
+ select * from pgp_armor_headers('
+ -BEGIN PGP MESSAGE-
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ');
+ 
+ -- no headers
+ select * from pgp_armor_headers('
+ -BEGIN PGP MESSAGE-
+ 
+ em9va2E=
+ =
+ -END PGP MESSAGE-
+ ');
+ 
  -- header with empty value
  select * from pgp_armor_headers('
  -BEGIN PGP MESSAGE-

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


[HACKERS] CINE in CREATE TABLE AS ... and CREATE MATERIALIZED VIEW ...

2014-10-01 Thread Fabrízio de Royes Mello
Hi all,

We already have IF NOT EXISTS for CREATE TABLE. There are some reason to
don't have to CREATE TABLE AS and CREATE MATERIALIZED VIEW??

Regards,

-- 
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
 Timbira: http://www.timbira.com.br
 Blog: http://fabriziomello.github.io
 Linkedin: http://br.linkedin.com/in/fabriziomello
 Twitter: http://twitter.com/fabriziomello
 Github: http://github.com/fabriziomello


Re: [HACKERS] Value locking Wiki page

2014-10-01 Thread Heikki Linnakangas

On 10/01/2014 02:42 PM, Simon Riggs wrote:

On 1 October 2014 11:58, Heikki Linnakangas hlinnakan...@vmware.com wrote:

On 10/01/2014 01:50 PM, Simon Riggs wrote:


On 1 October 2014 10:44, Heikki Linnakangas hlinnakan...@vmware.com
wrote:


I didn't realize that promise index tuples were even seriously
discussed.
I guess that can be made to work, too, although I don't see the point. It
wouldn't work with GiST indexes, for the same reasons as page-level
locking
won't work (a tuple can legally be inserted anywhere in a GiST index - it
just degrades the index making searching more expensive). And lossy GiST
opclasses are a problem too.


GiST doesn't support unique indexes, so it is not in any way a problem.


GiST supports exclusion constraints. That is one of the main reasons I want
to do promise tuples, instead of locking within the indexam: to support this
feature with exclusion constraints.


That does sound interesting, but I am concerned the semantics may cause issues.

If I go to insert a row for 'UK' and find an existing row for
'Europe', do we really want to update the population of Europe to be
the population of the UK, simply because the UK and Europe have an
exclusion conflict?


Clearly not, but you might want to insert the tuple to another table 
instead, or skip it altogether. Or you might want to UPDATE Europe into 
Continental Europe, and then insert the row for UK.


- 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] pg_receivexlog and replication slots

2014-10-01 Thread Michael Paquier
Thanks!

On Wed, Oct 1, 2014 at 8:38 PM, Andres Freund and...@2ndquadrant.com
wrote:

 0001) Old WIP patch for pg_recvlogical tests. Useful for easier
 development.

From where is this patch? Is it some rest from the time when pg_recvlogical
was developed?


 0002) Copy editing that should be in 9.4

Definitely makes sense for a backpatch.


 0003) Rebased patches of yours

0004) My changes to 0003 besides the rebase. This'll be squashed, but I
   thought it might be interesting for you.

(thanks for caring)
-/* Drop a replication slot */
+/* Drop a replication slot. */
I don't recall that comments on a single line have a dot even if this
single line is a complete sentence.

-static void StreamLog();
+static void StreamLogicalLog();
Not related at all to those patches, but for correctness it is better to
add a declaration (void).
Except those small things the changes look good to me.

Regards,
-- 
Michael


Re: [HACKERS] pg_receivexlog and replication slots

2014-10-01 Thread Andres Freund
On 2014-10-01 21:54:56 +0900, Michael Paquier wrote:
 Thanks!
 
 On Wed, Oct 1, 2014 at 8:38 PM, Andres Freund and...@2ndquadrant.com
 wrote:
 
  0001) Old WIP patch for pg_recvlogical tests. Useful for easier
  development.
 
 From where is this patch? Is it some rest from the time when pg_recvlogical
 was developed?
 
 
  0002) Copy editing that should be in 9.4
 
 Definitely makes sense for a backpatch.
 
 
  0003) Rebased patches of yours
 
 0004) My changes to 0003 besides the rebase. This'll be squashed, but I
thought it might be interesting for you.
 
 (thanks for caring)
 -/* Drop a replication slot */
 +/* Drop a replication slot. */
 I don't recall that comments on a single line have a dot even if this
 single line is a complete sentence.

Then it shouldn't start with a uppercase - which you changed...

 -static void StreamLog();
 +static void StreamLogicalLog();
 Not related at all to those patches, but for correctness it is better to
 add a declaration (void).

Agreed.

 Except those small things the changes look good to me.

Cool. Will push and then, sometime this week, review the next patch.

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] pgcrypto: PGP armor headers

2014-10-01 Thread Heikki Linnakangas

On 10/01/2014 02:47 PM, Marko Tiikkaja wrote:

On 10/1/14 1:01 PM, Heikki Linnakangas wrote:

I think this is now ready for commit, but since I've changed it quite
significantly from what you originally submitted, please take a moment
to review this.


1) I see this compiler warning:

  pgp-pgsql.c: In function ‘pg_armor’:
  pgp-pgsql.c:960:18: warning: ‘values’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
  pgp_armor_encode((uint8 *) VARDATA(data), data_len, buf,

   It's bogus, but worth silencing anyway.

2) There's what looks like an extra whitespace change in
pgp_armor_encode(), but maybe that's intentional?

3) Also, I think the attached two corner cases deserve their own tests.

Other than the above, the patch looks good to me.  Huge thanks for your
work on this one!


Ok, committed with those fixes.

- 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] Value locking Wiki page

2014-10-01 Thread Simon Riggs
On 1 October 2014 13:43, Heikki Linnakangas hlinnakan...@vmware.com wrote:

 That does sound interesting, but I am concerned the semantics may cause
 issues.

 If I go to insert a row for 'UK' and find an existing row for
 'Europe', do we really want to update the population of Europe to be
 the population of the UK, simply because the UK and Europe have an
 exclusion conflict?

 Clearly not, but you might want to insert the tuple to another table
 instead, or skip it altogether. Or you might want to UPDATE Europe into
 Continental Europe, and then insert the row for UK.

Not trying to catch you out, just trying to make sure we don't make
technical decisions based upon unachievable ideas.

I can't see value in having upsert work against exclusion constraint
indexes; thus this only needs to work for btrees, or similar exact
indexes.

-- 
 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] Value locking Wiki page

2014-10-01 Thread Simon Riggs
On 1 October 2014 14:31, Simon Riggs si...@2ndquadrant.com wrote:
 On 1 October 2014 13:43, Heikki Linnakangas hlinnakan...@vmware.com wrote:

 That does sound interesting, but I am concerned the semantics may cause
 issues.

 If I go to insert a row for 'UK' and find an existing row for
 'Europe', do we really want to update the population of Europe to be
 the population of the UK, simply because the UK and Europe have an
 exclusion conflict?

 Clearly not, but you might want to insert the tuple to another table
 instead, or skip it altogether. Or you might want to UPDATE Europe into
 Continental Europe, and then insert the row for UK.

Sorry, let me explain more clearly - neither of those two use cases
matches the upsert case.

If you wish to skip an insert that fails on a uniqueness constraint,
that is already possible. Just wrap in a subtransaction and trap the
error. The only reason we need UPSERT is if we intend to update. If we
just intend to ignore, then we could add such a check to any index AM
that supports unique/exclusion constraints, but without pursuing the
full locking needed for upsert path.

I wasn't aware that you could do both an INSERT and an UPDATE at same
time using the proposed feature.

There is no feature option to refer to the conflicting row that is
already in the table. The only thing we have at present is the ability
to refer to the incoming data row using CONFLICTING(). Update triggers
allow you to refer to OLD and NEW, but with an exclusion constraint
the row values don't match, so using OLD and NEW would not be
appropriate - that isn't how an update trigger works now.

So AFAICS neither of those use cases matches.


 Not trying to catch you out, just trying to make sure we don't make
 technical decisions based upon unachievable ideas.

 I can't see value in having upsert work against exclusion constraint
 indexes; thus this only needs to work for btrees, or similar exact
 indexes.



-- 
 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] Value locking Wiki page

2014-10-01 Thread Heikki Linnakangas

On 10/01/2014 04:31 PM, Simon Riggs wrote:

On 1 October 2014 13:43, Heikki Linnakangas hlinnakan...@vmware.com wrote:


That does sound interesting, but I am concerned the semantics may cause
issues.

If I go to insert a row for 'UK' and find an existing row for
'Europe', do we really want to update the population of Europe to be
the population of the UK, simply because the UK and Europe have an
exclusion conflict?


Clearly not, but you might want to insert the tuple to another table
instead, or skip it altogether. Or you might want to UPDATE Europe into
Continental Europe, and then insert the row for UK.


Not trying to catch you out, just trying to make sure we don't make
technical decisions based upon unachievable ideas.

I can't see value in having upsert work against exclusion constraint
indexes; thus this only needs to work for btrees, or similar exact
indexes.


Well, if nothing else, it would be nice to fix the concurrency issue we 
have with exclusion constraints today, which is that if two backends 
insert the same value at the same time, they might both get an error, 
even though you'd only need to abort one of them. That's a special case 
of UPSERT if you squint a little.


- 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] Time measurement format - more human readable

2014-10-01 Thread Gregory Smith

On 9/29/14, 1:08 AM, Andres Freund wrote:

On 2014-09-28 20:32:30 -0400, Gregory Smith wrote:

There are already a wide range of human readable time interval output
formats available in the database; see the list at 
http://www.postgresql.org/docs/current/static/datatype-datetime.html#INTERVAL-STYLE-OUTPUT-TABLE

He's talking about psql's \timing...


I got that.  My point was that even though psql's timing report is kind 
of a quick thing hacked into there, if it were revised I'd expect two 
things will happen eventually:


-Asking if any of the the interval conversion code can be re-used for 
this purpose, rather than adding yet another custom to one code path 
standard.


-Asking if this should really just be treated like a full interval 
instead, and then overlapping with a significant amount of that baggage 
so that you have all the existing format choices.




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


Re: [HACKERS] Value locking Wiki page

2014-10-01 Thread Heikki Linnakangas

On 10/01/2014 04:46 PM, Simon Riggs wrote:

On 1 October 2014 14:31, Simon Riggs si...@2ndquadrant.com wrote:

On 1 October 2014 13:43, Heikki Linnakangas hlinnakan...@vmware.com wrote:


That does sound interesting, but I am concerned the semantics may cause
issues.

If I go to insert a row for 'UK' and find an existing row for
'Europe', do we really want to update the population of Europe to be
the population of the UK, simply because the UK and Europe have an
exclusion conflict?


Clearly not, but you might want to insert the tuple to another table
instead, or skip it altogether. Or you might want to UPDATE Europe into
Continental Europe, and then insert the row for UK.


Sorry, let me explain more clearly - neither of those two use cases
matches the upsert case.

If you wish to skip an insert that fails on a uniqueness constraint,
that is already possible. Just wrap in a subtransaction and trap the
error.


Uh, if that's an option, couldn't you just use a subtransaction always, 
and forget about this patch? However, a subtransaction is a lot more 
expensive; you'll consume an XID for every inserted or updated row, for 
starters.



The only reason we need UPSERT is if we intend to update. If we
just intend to ignore, then we could add such a check to any index AM
that supports unique/exclusion constraints, but without pursuing the
full locking needed for upsert path.

I wasn't aware that you could do both an INSERT and an UPDATE at same
time using the proposed feature.


I'm not actually sure if the proposed syntax would allow that, I haven't 
been paying much attention to that part.


- 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] pg_receivexlog and replication slots

2014-10-01 Thread Alvaro Herrera
Andres Freund wrote:

 From d667f7a63cd62733d88ec5b7228dfd5d7136b9d7 Mon Sep 17 00:00:00 2001
 From: Michael Paquier mich...@otacoo.com
 Date: Mon, 1 Sep 2014 20:48:43 +0900
 Subject: [PATCH 3/4] Refactoring of pg_basebackup utilities
 
 Code duplication is reduced with the introduction of new APIs for each
 individual replication command:
 - IDENTIFY_SYSTEM
 - CREATE_REPLICATION_SLOT
 - DROP_REPLICATION_SLOT
 A couple of variables used to identify a timeline ID are changed as well
 to be more consistent with core code.
 ---
  src/bin/pg_basebackup/pg_basebackup.c  |  21 +
  src/bin/pg_basebackup/pg_receivexlog.c |  38 ++--
  src/bin/pg_basebackup/pg_recvlogical.c | 116 ++--
  src/bin/pg_basebackup/streamutil.c | 159 
 +
  src/bin/pg_basebackup/streamutil.h |  10 +++
  5 files changed, 207 insertions(+), 137 deletions(-)

Not objecting to any of this, but isn't it a bit funny that a patch that
aims to reduce duplication ends up with more lines than there were
originally?

-- 
Álvaro Herrerahttp://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] open items for 9.4

2014-10-01 Thread Heikki Linnakangas

On 09/30/2014 09:10 PM, Gregory Smith wrote:

On 9/29/14, 2:30 PM, Andres Freund wrote:

Can we explain those reasons in the form of documentation?

Yes. Try and benchmark it. It'll be hardware and workload dependant.


I missed this whole thing, and eventually I have to circle back to it.
I could do it this week.


Ah, that would be great!


Could you (or someone else familiar with the
useful benchmarks) give me more detail on how to replicate one case
where things should improve?  I can do that, and I have a lab full of
hardware if it's easier to spot on one type of server. That exercise
will probably lead to a useful opinion on the feature in its final form,
any associated GUC, tunables, and necessary level of associated
documentation in a day or two.


To see the most improvement from the patch, try a workload that's 
otherwise bottlenecked on XLogInsert. For example, with pgbench:


psql postgres  -c create table foo (id int4)

pgbench postgres -n -f fooinsert.sql -c 4 -T10

and in the test script:

insert into foo select g from generate_series(1, 1) g;

- 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] pg_receivexlog and replication slots

2014-10-01 Thread Andres Freund
On 2014-10-01 11:21:12 -0300, Alvaro Herrera wrote:
 Andres Freund wrote:
 
  From d667f7a63cd62733d88ec5b7228dfd5d7136b9d7 Mon Sep 17 00:00:00 2001
  From: Michael Paquier mich...@otacoo.com
  Date: Mon, 1 Sep 2014 20:48:43 +0900
  Subject: [PATCH 3/4] Refactoring of pg_basebackup utilities
  
  Code duplication is reduced with the introduction of new APIs for each
  individual replication command:
  - IDENTIFY_SYSTEM
  - CREATE_REPLICATION_SLOT
  - DROP_REPLICATION_SLOT
  A couple of variables used to identify a timeline ID are changed as well
  to be more consistent with core code.
  ---
   src/bin/pg_basebackup/pg_basebackup.c  |  21 +
   src/bin/pg_basebackup/pg_receivexlog.c |  38 ++--
   src/bin/pg_basebackup/pg_recvlogical.c | 116 ++--
   src/bin/pg_basebackup/streamutil.c | 159 
  +
   src/bin/pg_basebackup/streamutil.h |  10 +++
   5 files changed, 207 insertions(+), 137 deletions(-)
 
 Not objecting to any of this, but isn't it a bit funny that a patch that
 aims to reduce duplication ends up with more lines than there were
 originally?

The reduction is there in combination with a later patch - which needs
most of the code moved to streamutil.c.

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] Full_page_write is off in backup mode

2014-10-01 Thread Robert Haas
On Mon, Sep 29, 2014 at 12:06 PM, searcher s just.search...@gmail.com wrote:
 Hi,
 I am using postgres 9.2.2.
 The postgresql documentation says, full_page_writes is forcibly on after
 executing the function pg_start_backup. But in my server full_page_writes is
 still off  (not changed).

The value of the GUC doesn't actually change.  But the system still
performs full page writes during the backup.  Search the source code
for forcePageWrites to see how it works internally.

-- 
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] test_shm_mq failing on anole (was: Sending out a request for more buildfarm animals?)

2014-10-01 Thread Robert Haas
On Wed, Oct 1, 2014 at 7:00 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-09-29 14:46:20 -0400, Robert Haas wrote:
 This happened again, and I investigated further.

 Uh. Interestingly anole just succeeded twice:
 http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=anolebr=REL9_4_STABLE

 I plan to commit the mask/unmask patch regardless, but it's curious. The
 first of the two builds could have been you 'unsticking' it by manually
 mucking around. Did you also do that for the second build?

No, but I think the failures have always been intermittent.

-- 
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] test_shm_mq failing on anole (was: Sending out a request for more buildfarm animals?)

2014-10-01 Thread Andres Freund
On 2014-10-01 10:45:13 -0400, Robert Haas wrote:
 On Wed, Oct 1, 2014 at 7:00 AM, Andres Freund and...@2ndquadrant.com wrote:
  On 2014-09-29 14:46:20 -0400, Robert Haas wrote:
  This happened again, and I investigated further.
 
  Uh. Interestingly anole just succeeded twice:
  http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=anolebr=REL9_4_STABLE
 
  I plan to commit the mask/unmask patch regardless, but it's curious. The
  first of the two builds could have been you 'unsticking' it by manually
  mucking around. Did you also do that for the second build?
 
 No, but I think the failures have always been intermittent.

There's no record of any relevantly failing builds on 9.4:
http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=anolebr=REL9_4_STABLE
and none from master either:
http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=anolebr=HEAD

Is it setup for master now? Because it has reported back for 9.4 twice,
but never for master.

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] test_shm_mq failing on anole (was: Sending out a request for more buildfarm animals?)

2014-10-01 Thread Robert Haas
On Wed, Oct 1, 2014 at 10:50 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-10-01 10:45:13 -0400, Robert Haas wrote:
 On Wed, Oct 1, 2014 at 7:00 AM, Andres Freund and...@2ndquadrant.com wrote:
  On 2014-09-29 14:46:20 -0400, Robert Haas wrote:
  This happened again, and I investigated further.
 
  Uh. Interestingly anole just succeeded twice:
  http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=anolebr=REL9_4_STABLE
 
  I plan to commit the mask/unmask patch regardless, but it's curious. The
  first of the two builds could have been you 'unsticking' it by manually
  mucking around. Did you also do that for the second build?

 No, but I think the failures have always been intermittent.

 There's no record of any relevantly failing builds on 9.4:
 http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=anolebr=REL9_4_STABLE
 and none from master either:
 http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=anolebr=HEAD

 Is it setup for master now? Because it has reported back for 9.4 twice,
 but never for master.

As far as I can tell, it's configured to run everything.  I just
checked, though, and found it wedged again.  I'm not sure whether it
was the same problem, though; I ended up just killing all of the
postgres processes to fix it.  We may be just at the beginning of an
exciting debugging journey.

-- 
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] Per table autovacuum vacuum cost limit behaviour strange

2014-10-01 Thread Robert Haas
On Tue, Sep 30, 2014 at 6:16 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Robert Haas wrote:
 I favor option (a).   There's something to be said for your proposal
 in terms of logical consistency with what we have now, but to be
 honest I'm not sure it's the behavior anyone wants (I would welcome
 more feedback on what people actually want).  I think we should view
 an attempt to set a limit for a particular table as a way to control
 the rate at which that table is vacuumed - period.

 After re-reading this whole thread one more time, I think I have come to
 agree with you and Amit here, because not only it is simpler to
 implement, but it is also simpler to document.  Per Greg Smith's opinion
 elsewhere in the thread, it seems that for end users it doesn't make
 sense to make the already complicated mechanism even more complicated.

 So in essence what we're going to do is that the balance mechanism
 considers only tables that don't have per-table configuration options;
 for those that do, we will use the values configured there without any
 changes.

 I'll see about implementing this and making sure it finds its way to
 9.4beta3.

Cool!

-- 
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] autovacuum scheduling starvation and frenzy

2014-10-01 Thread Robert Haas
On Tue, Sep 30, 2014 at 5:59 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Jeff Janes wrote:
  I think that instead of
  trying to get a single target database in that foreach loop, we could
  try to build a prioritized list (in-wraparound-danger first, then
  in-multixid-wraparound danger, then the one with the oldest autovac time
  of all the ones that remain); then recheck the wrap-around condition by
  seeing whether there are other workers in that database that started
  after the wraparound condition appeared.

 I think we would want to check for one worker that is still running, and at
 least one other worker that started and completed since the wraparound
 threshold was exceeded.  If there are multiple tables in the database that
 need full scanning, it would make sense to have multiple workers.  But if a
 worker already started and finished without increasing the frozenxid and,
 another attempt probably won't accomplish much either.  But I have no idea
 how to do that bookkeeping, or how much of an improvement it would be over
 something simpler.

 How about something like this:

 * if autovacuum is disabled, then don't check these conditions; the only
 reason we're in do_start_worker() in that case is that somebody
 signalled postmaster that some database needs a for-wraparound emergency
 vacuum.

 * if autovacuum is on, and the database was processed less than
 autovac_naptime/2 ago, and there are no workers running in that database
 now, then ignore the database.

 Otherwise, consider it for xid-wraparound vacuuming.  So if we launched
 a worker recently, but it already finished, we would start another one.
 (If the worker finished, the database should not be in need of a
 for-wraparound vacuum again, so this seems sensible).  Also, we give
 priority to a database in danger sooner than the full autovac_naptime
 period; not immediately after the previous worker started, which should
 give room for other databases to be processed.

 The attached patch implements that.  I only tested it on HEAD, but
 AFAICS it applies cleanly to 9.4 and 9.3; fairly sure it won't apply to
 9.2.  Given the lack of complaints, I'm unsure about backpatching
 further back than 9.3 anyway.

This kind of seems like throwing darts at the wall.  It could be
better if we are right to skip the database already being vacuumed for
wraparound, or worse if we're not.

I'm not sure that we should do this at all, or at least not without
testing it extensively first.  We could easily shoot ourselves in the
foot.

-- 
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] [v9.5] Custom Plan API

2014-10-01 Thread Merlin Moncure
On Tue, Jul 8, 2014 at 6:55 AM, Kouhei Kaigai kai...@ak.jp.nec.com wrote:
 * Syntax also reflects what the command does more. New syntax to
   define custom plan provider is:
 CREATE CUSTOM PLAN PROVIDER cpp_name
   FOR cpp_class HANDLER cpp_function;

-1 on 'cpp' prefix.  I don't see acronyms used in the syntax
documentation and cpp will make people reflexively think 'c++'.  How
about  provider_name and provider_function?

merlin


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


Re: [HACKERS] autovacuum scheduling starvation and frenzy

2014-10-01 Thread Alvaro Herrera
Robert Haas wrote:

 This kind of seems like throwing darts at the wall.  It could be
 better if we are right to skip the database already being vacuumed for
 wraparound, or worse if we're not.

Well, it only skips the DB for half the naptime interval, so that other
databases have a chance to be chosen before that.  If you set up a
nonsensical interval such as one day, this might be problematic.

(I'm not sure I understand the darts analogy.)

Maybe instead of some interval we could have a flag that alternates
between on and off: let one other database be chosen, then the one in
danger, then some other database again.  But if you have large numbers
of databases, this isn't a very solution; you only waste half the
workers rather than all of them .. meh.

Here's another idea: have a counter of the number of tables that are in
danger of xid/multixact wraparound; only let that many workers process
the database in a row.  Of course, the problem is how to determine how
many tables are in danger when we haven't even connected to the database
in the first place.  We could try to store a counter in pgstats, ugh.
Or have the first for-wraparound worker store a number in shared memory
which launcher can read.  Double ugh.


 I'm not sure that we should do this at all, or at least not without
 testing it extensively first.  We could easily shoot ourselves in the
 foot.

Well, we need to do *something*, because having workers directed towards
a database on which they can't do any good causes problems too -- other
databases accumulate bloat in the meantime.

-- 
Álvaro Herrerahttp://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_receivexlog and replication slots

2014-10-01 Thread Andres Freund
Hi,

I pushed the first part.

On 2014-10-01 21:54:56 +0900, Michael Paquier wrote:
 On Wed, Oct 1, 2014 at 8:38 PM, Andres Freund and...@2ndquadrant.com
 wrote:
  0001) Old WIP patch for pg_recvlogical tests. Useful for easier
  development.
 From where is this patch? Is it some rest from the time when pg_recvlogical
 was developed?

IIRC I wrote it when looking at Peter's prove patches. There's a bit of
a problem with it because it requires test_decoding, a contrib module,
to be installed. That's easily solvable for 'make check', but less
clearly so for 'make installcheck'.

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


[HACKERS] Dynamic LWLock tracing via pg_stat_lwlock (proof of concept)

2014-10-01 Thread Ilya Kosmodemiansky
Hi,

I have a patch which is actually not commitfest-ready now, but it
always better to start discussing proof of concept having some patch
instead of just an idea.

Since I'am a DBA rather than C programmer, I will appreciate any
suggestions/critics about the patch and code quality to make things
better.


What is all about.

From an Oracle DBA's point of view, currently we have a lack of
performance diagnostics tools. Saying that, principally they mean an
Oracle Wait Interface analogue. The Basic idea is to have counters or
sensors all around database kernel to measure what a particular
backend is currently waiting for and how long/how often it waits.
Obviously, implementing such a complex system is not an easy task.
However decomposing the task and implementing some small diagnostics
tools proved to be a good solution: things like pg_stat_bgwriter,
pg_stat_statements or pg_stat_archiver make life significantly easier.
Implementing such histogram for LWLock tracing was my goal.


Why LWLock tracing is important.

Suppose we have a PostgreSQL instance under heavy write workload, but
we do not know any details. We could pull from time to time
pg_stat_lwlock function which would say pid n1 currently in
WALWriteLock and pid n2 in WALInsertLock. That means we should think
about write ahead log tuning. Or pid n1 is in some clog-related
LWLock, which means we need move clog to ramdisk. This is a stupid
example, but it shows how useful LWLock tracing could be for DBAs.
Even better idea is to collect daily LWLock distribution, find most
frequent of them etc.


Problems.

As far as I know, there are two major problems implementing LWLock
tracing in Postgres: Performance and stability of the server.

The patch https://commitfest.postgresql.org/action/patch_view?id=885
(discussion starts here I hope -
http://www.postgresql.org/message-id/4fe8ca2c.3030...@uptime.jp)
demonstrates performance problems; LWLOCK_STAT,  LOCK_DEBUG and
DTrace-like approach are slow, unsafe for production use and a bit
clumsy for using by DBA.


An Idea.

An idea of this patch is to trace LWLocks with the lowest possible
performance impact. We put integer lwLockID into procarray, then
acquiring the LWLock we put its id to procarray and now we could pull
procarray using a function to see if particular pid holds LWLock. Not
perfect, but if we see sometimes somebody consumes a lot of particular
LWLocks, we could investigate this matter in a more precise way using
another tool. Something like that was implemented in the attached
patch:

issuing pgbench  -c 50 -t 1000 -j 50

we have something like that:

postgres=# select now(),* from pg_stat_lwlock ;
  now  | lwlockid | pid
---+--+--
 2014-10-01 15:11:43.848765+02 |   57 | 4257
(1 row)

postgres=# select now(),* from pg_stat_lwlock ;
  now  | lwlockid | pid
---+--+--
 2014-10-01 15:11:45.892428+02 |   67 | 4269
 2014-10-01 15:11:45.892428+02 |   67 | 4258
 2014-10-01 15:11:45.892428+02 |   57 | 4270
 2014-10-01 15:11:45.892428+02 |   67 | 4245
 2014-10-01 15:11:45.892428+02 |   67 | 4271
 2014-10-01 15:11:45.892428+02 |   57 | 4256
 2014-10-01 15:11:45.892428+02 |   54 | 4241
(7 rows)

postgres=# select now(),* from pg_stat_lwlock ;
  now  | lwlockid | pid
---+--+--
 2014-10-01 15:11:47.211024+02 |   58 | 4262
 2014-10-01 15:11:47.211024+02 |   69 | 4243
 2014-10-01 15:11:47.211024+02 |   69 | 4246
(3 rows)

postgres=# select now(),* from pg_stat_lwlock ;
 now | lwlockid | pid
-+--+-
(0 rows)

postgres=# select now(),* from pg_stat_lwlock ;
  now  | lwlockid | pid
---+--+--
 2014-10-01 15:11:49.897357+02 |   55 | 4240
 2014-10-01 15:11:49.897357+02 |   61 | 4264
 2014-10-01 15:11:49.897357+02 |   55 | 4258
 2014-10-01 15:11:49.897357+02 |   61 | 4260
 2014-10-01 15:11:49.897357+02 |   61 | 4283
 2014-10-01 15:11:49.897357+02 |   62 | 4242

We could collect it to some view or table on a periodic basis.

Questions.

1. I've decided to put pg_stat_lwlock into extension pg_stat_lwlock
(simply for test purposes). Is it OK, or better to implement it
somewhere inside pg_catalog or in another extension (for example
pg_stat_statements)?

2. Currently lwLockID is in main procarray (this is for PoC purpose
only). I know why procarray was split into two and I know why the main
one should be kept as small as possible. Anyway, which design approach
better: to keep it inside the main one (this is an important feature,
lwLockID is small and I use proc-lwLockId = T_ID(l) to get the id) or
to put it into another procarray (create new one or use existent)?

3. Which is the best way to retrieve the name of LWLock instead of
only its ID (for usability reasons, WALWriteLock looks 

Re: [HACKERS] Allow format 0000-0000-0000 in postgresql MAC parser

2014-10-01 Thread Herwin Weststrate
On 01-10-14 01:19, Michael Paquier wrote:
 Looking at your patch, you should update the documentation as well,
 the list of authorized outputs being clearly listed:
 http://www.postgresql.org/docs/devel/static/datatype-net-types.html#DATATYPE-MACADDR
 This consists in adding simply one line to doc/src/sgml/datatype.sgml.
 Regards,

It has been registered now
(https://commitfest.postgresql.org/action/patch_view?id=1585). I've got
an updated version of the patch with the documentation fix.

-- 
Herwin Weststrate
Quarantainenet BV
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index 3e83dbb..0d277fa 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -3628,6 +3628,7 @@ SELECT person.name, holidays.num_weeks FROM person, holidays
   memberliteral'08002b:010203'//member
   memberliteral'08002b-010203'//member
   memberliteral'0800.2b01.0203'//member
+  memberliteral'0800-2b01-0203'//member
   memberliteral'08002b010203'//member
  /simplelist
 
@@ -3649,7 +3650,7 @@ SELECT person.name, holidays.num_weeks FROM person, holidays
 /para
 
 para
- The remaining four input formats are not part of any standard.
+ The remaining five input formats are not part of any standard.
 /para
/sect2
 
diff --git a/src/backend/utils/adt/mac.c b/src/backend/utils/adt/mac.c
index aa9993f..509315a 100644
--- a/src/backend/utils/adt/mac.c
+++ b/src/backend/utils/adt/mac.c
@@ -57,6 +57,9 @@ macaddr_in(PG_FUNCTION_ARGS)
 		count = sscanf(str, %2x%2x.%2x%2x.%2x%2x%1s,
 	   a, b, c, d, e, f, junk);
 	if (count != 6)
+		count = sscanf(str, %2x%2x-%2x%2x-%2x%2x%1s,
+	   a, b, c, d, e, f, junk);
+	if (count != 6)
 		count = sscanf(str, %2x%2x%2x%2x%2x%2x%1s,
 	   a, b, c, d, e, f, junk);
 	if (count != 6)
diff --git a/src/test/regress/expected/macaddr.out b/src/test/regress/expected/macaddr.out
index 91edc5a..90e9b34 100644
--- a/src/test/regress/expected/macaddr.out
+++ b/src/test/regress/expected/macaddr.out
@@ -7,14 +7,15 @@ INSERT INTO macaddr_data VALUES (2, '08-00-2b-01-02-03');
 INSERT INTO macaddr_data VALUES (3, '08002b:010203');
 INSERT INTO macaddr_data VALUES (4, '08002b-010203');
 INSERT INTO macaddr_data VALUES (5, '0800.2b01.0203');
-INSERT INTO macaddr_data VALUES (6, '08002b010203');
-INSERT INTO macaddr_data VALUES (7, '0800:2b01:0203'); -- invalid
+INSERT INTO macaddr_data VALUES (6, '0800-2b01-0203');
+INSERT INTO macaddr_data VALUES (7, '08002b010203');
+INSERT INTO macaddr_data VALUES (8, '0800:2b01:0203'); -- invalid
 ERROR:  invalid input syntax for type macaddr: 0800:2b01:0203
-LINE 1: INSERT INTO macaddr_data VALUES (7, '0800:2b01:0203');
+LINE 1: INSERT INTO macaddr_data VALUES (8, '0800:2b01:0203');
 ^
-INSERT INTO macaddr_data VALUES (8, 'not even close'); -- invalid
+INSERT INTO macaddr_data VALUES (9, 'not even close'); -- invalid
 ERROR:  invalid input syntax for type macaddr: not even close
-LINE 1: INSERT INTO macaddr_data VALUES (8, 'not even close');
+LINE 1: INSERT INTO macaddr_data VALUES (9, 'not even close');
 ^
 INSERT INTO macaddr_data VALUES (10, '08:00:2b:01:02:04');
 INSERT INTO macaddr_data VALUES (11, '08:00:2b:01:02:02');
@@ -30,12 +31,13 @@ SELECT * FROM macaddr_data;
   4 | 08:00:2b:01:02:03
   5 | 08:00:2b:01:02:03
   6 | 08:00:2b:01:02:03
+  7 | 08:00:2b:01:02:03
  10 | 08:00:2b:01:02:04
  11 | 08:00:2b:01:02:02
  12 | 08:00:2a:01:02:03
  13 | 08:00:2c:01:02:03
  14 | 08:00:2a:01:02:04
-(11 rows)
+(12 rows)
 
 CREATE INDEX macaddr_data_btree ON macaddr_data USING btree (b);
 CREATE INDEX macaddr_data_hash ON macaddr_data USING hash (b);
@@ -52,9 +54,10 @@ SELECT a, b, trunc(b) FROM macaddr_data ORDER BY 2, 1;
   4 | 08:00:2b:01:02:03 | 08:00:2b:00:00:00
   5 | 08:00:2b:01:02:03 | 08:00:2b:00:00:00
   6 | 08:00:2b:01:02:03 | 08:00:2b:00:00:00
+  7 | 08:00:2b:01:02:03 | 08:00:2b:00:00:00
  10 | 08:00:2b:01:02:04 | 08:00:2b:00:00:00
  13 | 08:00:2c:01:02:03 | 08:00:2c:00:00:00
-(11 rows)
+(12 rows)
 
 SELECT b   '08:00:2b:01:02:04' FROM macaddr_data WHERE a = 1; -- true
  ?column? 
@@ -113,12 +116,13 @@ SELECT ~b   FROM macaddr_data;
  f7:ff:d4:fe:fd:fc
  f7:ff:d4:fe:fd:fc
  f7:ff:d4:fe:fd:fc
+ f7:ff:d4:fe:fd:fc
  f7:ff:d4:fe:fd:fb
  f7:ff:d4:fe:fd:fd
  f7:ff:d5:fe:fd:fc
  f7:ff:d3:fe:fd:fc
  f7:ff:d5:fe:fd:fb
-(11 rows)
+(12 rows)
 
 SELECT  b  '00:00:00:ff:ff:ff' FROM macaddr_data;
  ?column?  
@@ -129,12 +133,13 @@ SELECT  b  '00:00:00:ff:ff:ff' FROM macaddr_data;
  00:00:00:01:02:03
  00:00:00:01:02:03
  00:00:00:01:02:03
+ 00:00:00:01:02:03
  00:00:00:01:02:04
  00:00:00:01:02:02
  00:00:00:01:02:03
  00:00:00:01:02:03
  00:00:00:01:02:04
-(11 rows)
+(12 rows)
 
 SELECT  b | '01:02:03:04:05:06' FROM macaddr_data;
  ?column?  
@@ -145,11 +150,12 @@ SELECT  b | '01:02:03:04:05:06' FROM macaddr_data;
  09:02:2b:05:07:07
  09:02:2b:05:07:07
  09:02:2b:05:07:07
+ 

Re: [HACKERS] pg_receivexlog and replication slots

2014-10-01 Thread Jan Wieck

On 10/01/2014 08:59 AM, Andres Freund wrote:

On 2014-10-01 21:54:56 +0900, Michael Paquier wrote:

Thanks!

On Wed, Oct 1, 2014 at 8:38 PM, Andres Freund and...@2ndquadrant.com
wrote:

 0001) Old WIP patch for pg_recvlogical tests. Useful for easier
 development.

From where is this patch? Is it some rest from the time when pg_recvlogical
was developed?


 0002) Copy editing that should be in 9.4

Definitely makes sense for a backpatch.


 0003) Rebased patches of yours

0004) My changes to 0003 besides the rebase. This'll be squashed, but I
   thought it might be interesting for you.

(thanks for caring)
-/* Drop a replication slot */
+/* Drop a replication slot. */
I don't recall that comments on a single line have a dot even if this
single line is a complete sentence.


Then it shouldn't start with a uppercase - which you changed...


-static void StreamLog();
+static void StreamLogicalLog();
Not related at all to those patches, but for correctness it is better to
add a declaration (void).


Agreed.


Except those small things the changes look good to me.


Cool. Will push and then, sometime this week, review the next patch.

Greetings,

Andres Freund




You might want to do that function renaming in pg_receivexlog.c as well.


Jan

--
Jan Wieck
Senior Software Engineer
http://slony.info


--
Sent 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_receivexlog and replication slots

2014-10-01 Thread Jan Wieck

On 10/01/2014 01:19 PM, Andres Freund wrote:

On 2014-10-01 13:17:17 -0400, Jan Wieck wrote:

-static void StreamLog();
+static void StreamLogicalLog();
Not related at all to those patches, but for correctness it is better to
add a declaration (void).

Agreed.

Except those small things the changes look good to me.

Cool. Will push and then, sometime this week, review the next patch.



You might want to do that function renaming in pg_receivexlog.c as well.


Why? You mean to StreamPhysicalLog()?

Greetings,

Andres Freund



Like that.

Plus, there is one more occurrence of StreamLog() in pg_recvlogical.c.


Jan

--
Jan Wieck
Senior Software Engineer
http://slony.info


--
Sent 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_receivexlog and replication slots

2014-10-01 Thread Andres Freund
On 2014-10-01 13:22:53 -0400, Jan Wieck wrote:
 On 10/01/2014 01:19 PM, Andres Freund wrote:
 On 2014-10-01 13:17:17 -0400, Jan Wieck wrote:
 -static void StreamLog();
 +static void StreamLogicalLog();
 Not related at all to those patches, but for correctness it is better to
 add a declaration (void).
 
 Agreed.
 
 Except those small things the changes look good to me.
 
 Cool. Will push and then, sometime this week, review the next patch.
 
 You might want to do that function renaming in pg_receivexlog.c as well.
 
 Why? You mean to StreamPhysicalLog()?
 
 Like that.

Don't see that much point - it just seemed wrong to have StreamLog do
two somewhat different things. And StreamLog() in receivexlog is much older...

 Plus, there is one more occurrence of StreamLog() in pg_recvlogical.c.

Gah. Wrongly split commit :(. Fixed in 9.4, it's already fixed by the
subsequent commit in master.

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_receivexlog and replication slots

2014-10-01 Thread Jan Wieck

On 10/01/2014 01:32 PM, Andres Freund wrote:

On 2014-10-01 13:22:53 -0400, Jan Wieck wrote:

On 10/01/2014 01:19 PM, Andres Freund wrote:
On 2014-10-01 13:17:17 -0400, Jan Wieck wrote:
-static void StreamLog();
+static void StreamLogicalLog();
Not related at all to those patches, but for correctness it is better to
add a declaration (void).

Agreed.

Except those small things the changes look good to me.

Cool. Will push and then, sometime this week, review the next patch.

You might want to do that function renaming in pg_receivexlog.c as well.

Why? You mean to StreamPhysicalLog()?

Like that.


Don't see that much point - it just seemed wrong to have StreamLog do
two somewhat different things. And StreamLog() in receivexlog is much older...


The point is that StreamLog is semantically a superset of StreamWhateverLog.


Jan





Plus, there is one more occurrence of StreamLog() in pg_recvlogical.c.


Gah. Wrongly split commit :(. Fixed in 9.4, it's already fixed by the
subsequent commit in master.

Greetings,

Andres Freund




--
Jan Wieck
Senior Software Engineer
http://slony.info


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


Re: [HACKERS] Value locking Wiki page

2014-10-01 Thread Peter Geoghegan
On Wed, Oct 1, 2014 at 4:23 AM, Simon Riggs si...@2ndquadrant.com wrote:
 Quoting general research and other points about value locking are
 reasonable in the general section, but not in the description for 1.

I also made a similar comparison of #3. I don't think that reflects a bias.

 I'm glad you've called the first option Heavyweight Page Locking.
 That name sums up what I see as the major objection to it, which is
 that performance and scalability of the whole server will be damaged
 signiciantly by using heavyweight locks for each row inserted. Please
 add that concern as a negative; I'm sure testing has been done, but
 not comparative testing against other techniques.

Comparative testing against both other techniques (#1, at a time when
#1 and #2 were otherwise comparable), and plain inserts and updates
has shown the performance to be very good. The evidence suggests that
using a heavyweight lock like this is fine. Don't promise tuples use
heavyweight locks? The coarser granularity did not appear to be
significant once we optimize retries to avoid hwlocking. #1 came out a
bit ahead of #2. So maybe the bloat of #2 is almost, but not quite,
cancelled out by the hwlocking coarseness. But then, I specifically
stated that it seemed that not having bloating was not much of an
advantage for #1.

We're going to have a benchmark between #1 and #2 when #2 is properly
integrated with the rest of the updated patch (just because we can).
#1 is the fastest of #1 and #2 last I checked, but there wasn't all
that much in it.

 I am motivated to
 explain the promise index tuple approach further because of this,
 which is an optimistic approach to locking.

 The stated disadvantages for 3 are not accurate, to put it mildly.

Honestly, how could I know what they are? The explanation I heard from
you and Andres has been very short on details. The points for #3 are
*my* concerns. I had to start somewhere. I'll consider your rebuttal
to those points that you make on a new thread separately.

 But that's been useful because what was a small thought experiment is
 beginning to look like a useful approach. I will post a summary of my
 understanding.

Thanks. Actually, this wiki page will probably pay for itself just by
allowing us to refer to #1, #2, and #3.

-- 
Peter Geoghegan


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


Re: [HACKERS] Scaling shared buffer eviction

2014-10-01 Thread Andres Freund
On 2014-10-01 20:54:39 +0200, Andres Freund wrote:
 Here we go.
 
 Postgres was configured with.
  -c shared_buffers=8GB \
  -c log_line_prefix=[%m %p]  \
  -c log_min_messages=debug1 \
  -p 5440 \
  -c checkpoint_segments=600
  -c max_connections=200

Robert reminded me that I missed to report the hardware aspect of
this...

4x E5-4620  @ 2.20GHz: 32 cores, 64 threads
256GB RAM

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] Value locking Wiki page

2014-10-01 Thread Peter Geoghegan
On Wed, Oct 1, 2014 at 6:49 AM, Heikki Linnakangas
hlinnakan...@vmware.com wrote:
 Well, if nothing else, it would be nice to fix the concurrency issue we have
 with exclusion constraints today, which is that if two backends insert the
 same value at the same time, they might both get an error, even though you'd
 only need to abort one of them. That's a special case of UPSERT if you
 squint a little.

In my view, it makes sense to fix that, and to make INSERT ... ON
CONFLICT IGNORE work with exclusion constraints. However, it does not
make sense to have INSERT ... ON CONFLICT UPDATE work with exclusion
constraints. The user-visible semantics are very tricky, and I don't
think it's useful.

-- 
Peter Geoghegan


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


Re: [HACKERS] autovacuum scheduling starvation and frenzy

2014-10-01 Thread Robert Haas
On Wed, Oct 1, 2014 at 11:44 AM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Robert Haas wrote:
 This kind of seems like throwing darts at the wall.  It could be
 better if we are right to skip the database already being vacuumed for
 wraparound, or worse if we're not.

 Well, it only skips the DB for half the naptime interval, so that other
 databases have a chance to be chosen before that.  If you set up a
 nonsensical interval such as one day, this might be problematic.

 (I'm not sure I understand the darts analogy.)

I guess I meant: this seems pretty hit-or-miss.  I don't see why we
should expect it to be better than what we have now.  Sure, maybe
there's a table in some other database that needs to be vacuumed for
bloat more urgently than a table in the wraparound database needs to
be vacuumed to prevent XID wraparound.  But the reverse could be true
also - in which case your patch could cause a cluster that would
merely have bloated to instead shut down.

The way to really know would be for the AV launcher to have knowledge
of how many tables there are in each database that are beyond the
wraparound theshold and not already been vacuumed.  Then we could skip
wraparound databases where that number is 0, and give priority to
those where it isn't.  I guess this is more or less what you said in
the portion of your email I'm not quoting here, but like you I'm not
quite sure how to implement that.  Still, I'm reluctant to just go
change the behavior; I think it's optimistic to think that any
algorithm for making decisions without real knowledge will be better
than any other.

-- 
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] Promise index tuples for UPSERT

2014-10-01 Thread Heikki Linnakangas

On 10/01/2014 02:34 PM, Simon Riggs wrote:

Summary of algorithm to use promise tuples for concurrency control
during UPSERT

1. Perform btree search to location of key, if it exists.
a) If an unkilled index tuple exists, we decide this is an UPDATE and
drop straight thru to step 2
b) If it does not exist, insert a promise tuple into unique index(s)
- marked with the xid of the inserting transaction, but using the key.
This happens while the page is locked, so it is not possible to insert
a second promise tuple concurrently.
Record the btree blockid on the index scan and move to step 3
When later insert scans see the promise tuple they perform
XactLockTableWait() and when they get control they look again for the
key. If they find a promise tuple with an aborted xid they replace
that value with their own xid and continue as a). Otherwise b).


XactLockTableWait() waits until the end of transaction, that's not you 
want here. If the backend that inserted the promise tuple decides to not 
proceed with the insertion, and removes the promise tuple, the backend 
waiting on it needs to be woken up more or less immediately, not when 
the transaction completes.


I had this exact same issue in my earlier patch versions, fixed it with 
a new kind of heavy-weight lock, and a new field in PGPROC 
(http://www.postgresql.org/message-id/52d00d2d.6030...@vmware.com). That 
wasn't very pretty, but it got the job done. Some other design might be 
better.


- 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] Promise index tuples for UPSERT

2014-10-01 Thread Peter Geoghegan
On Wed, Oct 1, 2014 at 4:34 AM, Simon Riggs si...@2ndquadrant.com wrote:
 Summary of algorithm to use promise tuples for concurrency control
 during UPSERT

 Index bloat is less of a problem than with normal inserts since there
 are additional ways of removing promise tuples. Only one index tuple
 at a time can have a specific value, so we would actually reduce heap
 bloat caused by concurrent inserts.

I am not all that concerned about bloat. I didn't think it was a major
advantage of #1 relative to #2 when I investigated the differences,
and looked at the numbers. And I'm speaking as the advocate of the
design with zero bloat.

 It's possible we find existing rows that we can't see according to our
 snapshot. That is handled in exactly the same as the way we treat
 UPDATEs.

This isn't a special case in the patch. It's more like REPEATABLE READ
and SERIALIZABLE have a special responsibility to make sure they're
not updating an invisible-to-MVCC-snapshot (not instantaneously
invisible, by which I mean invisible in the
HeapTupleSatisfiesUpdate() sense). Otherwise, we don't care if it's
MVCC-visible or not.

I imagine that by That is handled in exactly the same as the way we
treat UPDATEs, you mean that we do the EvalPlanQual() stuff. We only
do that in the event of a concurrent UPDATE/DELETE, though. Otherwise,
we trust the underlying relation scan to accurate represent that
tuples pulled up are visible. So I don't understand the comparison
(but tell me if I've misunderstood).

 There is a potential loop here in that we might find an index tuple,
 follow it, find out the tuple is actually dead then return to insert a
 promise tuple, only to find that someone else just did that and we
 have to wait/re-follow the link to update the new tuple. That is an
 extremely unlikely race condition, though possible; there is no heavy
 locking to prevent that in this approach. In principle deadlocks are
 possible from that, but that is not any different from the current
 case where people that insert same key at same time might cause
 deadlocks. Its not a common application pattern and not something we
 should be protecting against.

People are going to use this feature in cases where they could almost
use an UPDATE. It will happen if you let it happen. But even if you
don't, a guarantee is infinitely more useful than a non-guarantee to
app developers. I'll be up-front about this: you have very little
chance of getting me to budge on this point. I *really* hate the idea
of allowing this kind of thing. I don't think I'm alone here.

 All of this is only needed for unique indexes.

 It can handled by a new API call for acquire_value_lock() and
 release_value_lock(), or similar.

 Advantages
 * We don't do anything weird in the heap - if this breaks, data is not corrupt

Index corruption leads to wrong answers. I don't think this is a very
good argument, fwiw.

 * There is no heap bloat or index bloat above existing levels

Fair enough.

 * The approach leverages existing mechanism for transaction waiting

That's not really an advantage. That more or less applies to all designs.

 * Optimistic approach to value locking will improve performance over
 heavy weight locking

There is no evidence that promise index tuples will perform better. #2
didn't perform better than #1.

 Disadvantages
 * Not written yet - 1 month to code, still possible for 9.5, doesn't look 
 hard

Maybe, but that is beside the point, which is: If there were any
fundamental problems with either #1 or #2, I think I'd have figured
them out by now. They are less risky today - it might be that #3 turns
out to have the same properties, but I cannot be sure. That counts for
something. I feel perfectly entitled to hold that kind of uncertainty
against any design, tbh.

 Other stated possible disadvantages
 * Violates existing invariant that every index tuple has a
 corresponding heap tuple, which goes back to the Berkeley days.
 Currently, we always create heap tuples first, and physically delete
 them last. [Why is that a problem? Could be, but why?]

Unknown unknowns. Huge amounts of code must be audited to figure out
that it isn't an issue. So I don't know, but then I'm not the one
making the proposal.

 (Deleting them last implies something is being touched in that
 regard by this suggestion. I'm not sure what you are referring to)

The uncertain scope of the problem is a big part of the problem.

 * Relatedly, concern about breaking VACUUM. VACUUMing of btrees occurs
 with a list of TIDs to kill in index, built from the heap. Current
 bulk delete callback cannot be used for this. Super-exclusive lock is
 needed to delete tuples in btree page (i.e. VACUUM). Normally skipping
 of LockBufferForCleanup() (as added by bbb6e559) works okay in heap
 because it tends to imply that list of tids won't be bulk deleted in
 index, where we currently cannot skip for failure to quickly acquire
 super exclusive lock. So this could make the problem addressed by
 bbb6e559 

Re: [HACKERS] pg_background (and more parallelism infrastructure patches)

2014-10-01 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
 On Mon, Sep 29, 2014 at 12:05 PM, Stephen Frost sfr...@snowman.net wrote:
  Perhaps I'm just being a bit over the top, but all this per-character
  work feels a bit ridiculous..  When we're using MAXIMUM_ALIGNOF, I
  suppose it's not so bad, but is there no hope to increase that and make
  this whole process more efficient?  Just a thought.
 
 I'm not sure I understand what you're getting at here.

Was just thinking that we might be able to work out what needs to be
done without having to actually do it on a per-character basis.  That
said, I'm not sure it's really worth the effort given that we're talking
about at most 8 bytes currently.  I had originally been thinking that we
might increase the minimum size as it might make things more efficient,
but it's not clear that'd really end up being the case either and,
regardless, it's probably not worth worrying about at this point.

  After reading through the code for 0001, I decided to actually take it
  out for a spin- see attached.  I then passed a few megabytes of data
  through it and it seemed to work just fine.
 
 That's good.

Just to note this- the testing which I did used a random number of
segments and random amounts of data with each and hit specific edge
cases and all looked good.

  Lastly, I will say that I feel it'd be good to support bi-directional
  communication as I think it'll be needed eventually, but I'm not sure
  that has to happen now.
 
 I agree we need bidirectional communication; I just don't agree that
 the other direction should use the libpq format.  The data going from
 the worker to the process that launched it is stuff like errors and
 tuples, for which we already have a wire format.  The data going in
 the other direction is going to be things like plan trees to be
 executed, for which we don't.  But if we can defer the issue, so much
 the better.  Things will become clearer as we get closer to being
 done.

Sounds good to me.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Value locking Wiki page

2014-10-01 Thread Ants Aasma
On Wed, Oct 1, 2014 at 2:42 PM, Simon Riggs si...@2ndquadrant.com wrote:
 GiST supports exclusion constraints. That is one of the main reasons I want
 to do promise tuples, instead of locking within the indexam: to support this
 feature with exclusion constraints.

 That does sound interesting, but I am concerned the semantics may cause 
 issues.

 If I go to insert a row for 'UK' and find an existing row for
 'Europe', do we really want to update the population of Europe to be
 the population of the UK, simply because the UK and Europe have an
 exclusion conflict?

 Please give some concrete examples of a business request that might be
 satisified by such a feature.

The ON CONFLICT UPDATE semantics don't seem particularly useful for
exclusion constraints. However, a reasonable business request for
exclusion constraints would be to have a boss mode for the canonical
room reservation example - an INSERT that is guaranteed not to fail by
either deleting conflicting rows or updating them so the exclusion
constraints don't overlap (e.g. truncate the time intervals) or the
rows fail the index predicate (e.g. soft delete). AFAICS this is
currently not possible to implement correctly without a retry loop.

The hypothetical ON CONFLICT REPLACE and ON CONFLICT
UPDATE-AND-THEN-INSERT modes would also make sense in the unique index
case.

Not saying that I view this as necessary for the first cut of the
feature, just providing an example where it could be useful.

Regards,
Ants Aasma
-- 
Cybertec Schönig  Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de


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


[HACKERS] UPSERT wiki page, and SQL MERGE syntax

2014-10-01 Thread Peter Geoghegan
Having been surprisingly successful at advancing our understanding of
arguments for and against various approaches to value locking, I
decided to try the same thing out elsewhere. I have created a
general-purpose UPSERT wiki page.

The page is: https://wiki.postgresql.org/wiki/UPSERT

Right now, I would like to address the less contentious but still
unresolved question of whether or not we should use the SQL-standard
MERGE syntax instead of the non-standard INSERT ... ON CONFLICT UPDATE
syntax that my patch proposes.

Note that I'm trying to direct the conversation along certain lines:
Is MERGE the right syntax for this particular effort (UPSERT)? And,
in particular, not: Is SQL MERGE useful? I think that it is useful,
but is a largely unrelated feature.

Please correct the Wiki page if I have failed to credit SQL MERGE with
some advantage that someone stated at some point. I don't recall any
arguments for it, other than that it is in the SQL standard, but maybe
I missed one.

In general, add to this page, and edit it as you see fit. It'll be
useful to centralize the references, discussion and state of the patch
in one agreed upon place, as the patch continues to evolve.
-- 
Peter Geoghegan


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


Re: [HACKERS] bad estimation together with large work_mem generates terrible slow hash joins

2014-10-01 Thread Kevin Grittner
Tomas Vondra t...@fuzzy.cz wrote:
 On 12.9.2014 23:22, Robert Haas wrote:

 My first thought is to revert to NTUP_PER_BUCKET=1, but it's
 certainly arguable. Either method, though, figures to be better than
 doing nothing, so let's do something.

 OK, but can we commit the remaining part first? Because it'll certainly
 interact with this proposed part, and it's easier to tweak when the code
 is already there. Instead of rebasing over and over.

The patch applied and built without problem, and pass `make
check-world`.  The only thing that caught my eye was the addition
of debug code using printf() instead of logging at a DEBUG level.
Is there any reason for that?

I still need to work through the (rather voluminous) email threads
and the code to be totally comfortable with all the issues, but
if Robert and Heikki are comfortable with it, I'm not gonna argue.

Preliminary benchmarks look outstanding!  I tried to control
carefully to ensure consistent results (by dropping, recreating,
vacuuming, analyzing, and checkpointing before each test), but
there were surprising outliers in the unpatched version.  It turned
out that it was because slight differences in the random samples
caused different numbers of buckets for both unpatched and patched,
but the patched version dealt with the difference gracefully while
the unpatched version's performance fluctuated randomly.

My thinking is that we should get this much committed and then
discuss further optimizations.  I tried to throw something at it
that would be something of a worst case because with the new code
it would start with one batch and go to two.

Five runs per test, alternating between unpatched and patched.

First I tried with the default work_mem size of 4MB:

Planning time / Execution time (ms)

unpatched, work_mem = 4MB

0.694 / 10392.930
0.327 / 10388.787
0.412 / 10533.036
0.650 / 17186.719
0.338 / 10707.423

Fast: Buckets: 2048  Batches: 1  Memory Usage: 3516kB
Slow: Buckets: 1024  Batches: 1  Memory Usage: 3516kB

patched, work_mem = 4MB

0.764 / 5021.792
0.655 / 4991.887
0.436 / 5068.777
0.410 / 5057.902
0.444 / 5021.543

1st  5th:  Buckets: 131072 (originally 1024)  Batches: 2 (originally 1)  
Memory Usage: 3073kB
all others: Buckets: 131072 (originally 2048)  Batches: 2 (originally 1)  
Memory Usage: 3073kB

Then, just to see how both dealt with extra memory I did it again with 1GB:

unpatched, work_mem = 1GB

0.328 / 10407.836
0.319 / 10465.348
0.324 / 16606.948
0.569 / 10408.671
0.542 / 16996.209

Memory usage same as before.  Guess which runs started with 1024 buckets.  ;-)

0.556 / 3974.741
0.325 / 4012.613
0.334 / 3941.734
0.834 / 3894.391
0.603 / 3959.440

2nd  4th:  Buckets: 131072 (originally 2048)  Batches: 1 (originally 1)  
Memory Usage: 4540kB
all others: Buckets: 131072 (originally 1024)  Batches: 1 (originally 1)  
Memory Usage: 4540kB

My only concern from the benchmarks is that it seemed like there
was a statistically significant increase in planning time:

unpatched plan time average: 0.450 ms
patched plan time average:   0.536 ms

That *might* just be noise, but it seems likely to be real.  For
the improvement in run time, I'd put up with an extra 86us in
planning time per hash join; but if there's any way to shave some
of that off, all the better.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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


Re: [HACKERS] superuser() shortcuts

2014-10-01 Thread Stephen Frost
Adam,

* Brightwell, Adam (adam.brightw...@crunchydatasolutions.com) wrote:
   We, as a community, have gotten flak from time-to-time about the
superuser role.  We also tend to wish to avoid unnecessary
optimization as it complicates the code base and makes folks reviewing
the code wonder at the exceptions.
 
 I have attached a patch for consideration/discussion that addresses these
 items.  I have based it off of current master (0c013e0).  I have done some
 cursory testing including check-world with success.

Thanks!  Please add it to the next commitfest.

 With all the other pg_has_* functions being found in acl.c I agree that it
 would seem odd that this (or any other related function) would be found
 elsewhere.  Since aclchk.c also has a couple of functions that follow the
 pattern of has_priv_privilege, I moved this function there, for now, as
 well as modified it to include superuser checks as they were not previously
 there.  The only related function I found in acl.c was has_rolinherit
 which is a static function. :-/  There is also a static function
 has_rolcatupdate in aclchk.c.  I would agree that acl.c (or aclchk.c?)
 would be a more appropriate location for these functions, though in some of
 the above cases, it might require exposing them (I'm not sure that I see
 any harm in doing so).

I don't think has_rolinherit or has_rolcatupdate really need to move and
it seems unlikely that they'd be needed from elsewhere..  Is there a
reason you think they'd need to be exposed?  I've not looked at the
patch at all though, perhaps that makes it clear.

 Perhaps refactoring to consolidate all of these in either acl.c or aclchk.c
 with the following pattern might be a step in the right direction?
 
 * has_createrole_privilege
 * has_bypassrls_privilege

These are already in the right place, right?

 * has_inherit_privilege
 * has_catupdate_privilege

These probably don't need to move as they're only used in the .c files
that they're defined in (unless there's a reason that needs to change).

 * has_replication_privilege

This is what I was on about, so I agree that it should be created. ;)

 * has_???_privilege

Right, other things might be 'has_backup_privilege', for things like
pg_start/stop_backup and friends.

 In either case, I think following a convention on the naming of these
 functions (unless there is semantic reason not to) would also help to
 reduce any confusion or head scratching.

Agreed.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Dynamic LWLock tracing via pg_stat_lwlock (proof of concept)

2014-10-01 Thread Craig Ringer
On 10/02/2014 12:19 AM, Ilya Kosmodemiansky wrote:

 From an Oracle DBA's point of view, currently we have a lack of
 performance diagnostics tools. 

Agreed. Sometimes very frustratingly so.

 Even better idea is to collect daily LWLock distribution, find most
 frequent of them etc.

While we could add this within PostgreSQL, I wonder if it's worth
looking at whether it can be done non-intrusively with operating system
facilities like perf.

I've had really good results using perf to trace and graph xlog
generation and other metrics in the past.

 The patch https://commitfest.postgresql.org/action/patch_view?id=885
 (discussion starts here I hope -
 http://www.postgresql.org/message-id/4fe8ca2c.3030...@uptime.jp)
 demonstrates performance problems; LWLOCK_STAT,  LOCK_DEBUG and
 DTrace-like approach are slow, unsafe for production use and a bit
 clumsy for using by DBA.

It's not at all clear to me that a DTrace-like (or perf-based, rather)
approach is unsafe, slow, or unsuitable for production use.

Resolving lock IDs to names might be an issue, though.

With appropriate wrapper tools I think we could have quite a useful
library of perf-based diagnostics and tracing tools for PostgreSQL.

-- 
 Craig Ringer   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] Value locking Wiki page

2014-10-01 Thread Craig Ringer
On 10/02/2014 03:06 AM, Peter Geoghegan wrote:
 In my view, it makes sense to fix that, and to make INSERT ... ON
 CONFLICT IGNORE work with exclusion constraints. However, it does not
 make sense to have INSERT ... ON CONFLICT UPDATE work with exclusion
 constraints. The user-visible semantics are very tricky, and I don't
 think it's useful.

If you were doing a multi-valued INSERT ... ON CONFLICT IGNORE and
wanted to do this with exclusion constraints, what would you do if the
insert its self contains values that can't exist in the table together?

Right now the whole insert will fail. Would that still be the case?
Would you insert one tuple then ignore the other? If so, what guarantee
if any would be made about which tuple would get inserted?

I think insert...ignore for exclusion constraints could be interesting,
but I'm not sure it's really the same as the upsert case. I guess I also
just don't really understand the utility of insert ... on conflict
ignore for GiST exclusion constraints.

-- 
 Craig Ringer   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] UPSERT wiki page, and SQL MERGE syntax

2014-10-01 Thread Craig Ringer
On 10/02/2014 07:52 AM, Peter Geoghegan wrote:
 Having been surprisingly successful at advancing our understanding of
 arguments for and against various approaches to value locking, I
 decided to try the same thing out elsewhere. I have created a
 general-purpose UPSERT wiki page.
 
 The page is: https://wiki.postgresql.org/wiki/UPSERT

Thanks. That'll help keep things moving forward rather than around in
circles.

 In general, add to this page, and edit it as you see fit. It'll be
 useful to centralize the references, discussion and state of the patch
 in one agreed upon place, as the patch continues to evolve.

I added a summary of the status quo of upsert in Pg as it stands, and a
brief discussion of the state in other RDBMSes.

I'd love it if someone who knows MySQL better could add info on MySQL's
ON DUPLICATE KEY feature - advantages/problems, etc.


I've added a few points to the goals section:

- Any new upsert approach must be a usability improvement on the status
quo; we don't want to introduce subtle behaviour or unnecessary foot-guns.

- if possible, upsert of multiple values is desirable. We currently have
to loop on a per-value basis.


... and some miscellaneous edits/formatting changes.

I've also added sections for the other options:

* A custom syntax
https://wiki.postgresql.org/wiki/UPSERT#Custom_syntax

and

* Adopting an existing non-standard syntax
https://wiki.postgresql.org/wiki/UPSERT#Adopting_an_existing_non-standard_syntax

which I'd appreciate it if you could fill out based on your existing
research and notes. I don't think it makes sense for me to write those
when you've already done the required study/note taking and just need to
transfer it over.


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