[HACKERS] Possible memory leak with SQL function?

2013-09-12 Thread Yeb Havinga

Hello list,

Is the following known behaviour, or should I put some time in writing a 
self contained test case?


We have a function that takes a value and returns a ROW type. With the 
function implemented in language SQL, when executing this function in a 
large transaction, memory usage of the backend process increases. 
MemoryContextStats showed a lot of SQL function data. Debugging 
init_sql_fcache() showed that it was for the same function oid each 
time, and the oid was the function from value to ROW type.


When the function is implemented in PL/pgSQL, the memory usage was much 
less.


I'm sorry I cannot be more specific at the moment, such as what is 'much 
less' memory with a PL/pgSQl function, and are there as many SQL 
function data's as calls to the SQL function, because I would have to 
write a test case for this. I was just wondering, if this is known 
behavior of SQL functions vs PL/pgSQL functions, or could it be a bug?


--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data



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


Re: [HACKERS] Successor of MD5 authentication, let's use SCRAM

2013-09-12 Thread Heikki Linnakangas

(reviving an old thread)

On 23.10.2012 19:53, Peter Eisentraut wrote:

On 10/22/12 1:25 PM, Stephen Frost wrote:

* Peter Eisentraut (pete...@gmx.net) wrote:

On 10/12/12 3:44 PM, Stephen Frost wrote:

In general, I think it's good to build on existing implementations where
possible.  Perhaps we could even consider using something which already
exists for this?


Sounds like SASL to me.


aiui, that would allow us to support SCRAM and we could support
Kerberos/GSSAPI under SASL as well...  Not sure how comfortable folks
would be with moving to that though.


Considering all the design and implementation challenges that have been
brought up in this thread:

- not using MD5

- not using whatever we replace MD5 with when that gets broken

- content of pg_shadow can be used to log in

- questions about salt collisions

- making the hash more expensive

- negotiating how much more expensive, allowing changes in the future

- using HMAC to guard against length-extension attacks

- support for poolers/proxies

I think I would be less comfortable with a hand-crafted solution to each
of these issues, and would be more comfortable with using an existing
solution that, from the look of it, already does all of that, and which
is used by mail and LDAP servers everywhere.

That said, I don't have any experience programming SASL clients or
servers, only managing existing implementations.  But I'd say it's
definitely worth a look.


SASL seems like a good approach to me. The SASL specification leaves it 
up to the application protocol how the SASL messages are transported. 
Each application protocol that uses SASL defines a SASL profile, which 
specifies that. So for PostgreSQL, we would need to document how to do 
SASL authentication. That's pretty straightforward, the SASL messages 
can be carried in Authentication and PasswordMessage messages, just like 
we do for GSS.


SASL specifies several methods, like PLAIN and GSSAPI. It also has a 
couple of methods that use MD5, which probably could be used with the 
hashes we already store in pg_authid. I believe we could map all the 
existing authentication methods to SASL methods. In the distant future, 
we could deprecate and remove the existing built-in authentication 
handshakes, and always use SASL for authentication.


The SASL specification is quite simple, so I think we could easily 
implement it ourselves without relying on an external library, for the 
authentication methods we already support. That doesn't buy us much, but 
would be required if we want to always use SASL for authentication. On 
top of that, we could also provide a configure option to use an external 
SASL library, which could provide more exotic authentication methods.



Now, to a completely different approach:

I just found out that OpenSSL has added support for SRP in version 
1.0.1. We're already using OpenSSL, so all we need to do is to provide a 
couple of callbacks to OpenSSL, and store SRP verifiers in pg_authid 
instead of MD5 hashes, and we're golden.


Well, not quite. There's one little problem: Currently, we first 
initialize SSL, then read the startup packet which contains the username 
and database to connect to. After that, we initialize database access to 
the specified database, and only then we proceed with authentication. 
That's not a problem for certificate authentication, because certificate 
authentication doesn't require any database access, but if we are to 
store the SRP verifiers in pg_authid, we'll need to database access much 
earlier. Before we know which database to connect to.


But that's just an implementation detail - no protocol changes would be 
required.


- 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] Protocol forced to V2 in low-memory conditions?

2013-09-12 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 On 09/11/2013 02:30 PM, Robert Haas wrote:
 On Tue, Sep 10, 2013 at 9:53 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Note that I was proposing removing libpq's support for V2 connections.
 Not the backend's.

 I vote against this.  If we remove V2 support from libpq, then we'll
 have no easy way to test that the backend's support still works.

 How is it tested now, and who is doing the testing?

Exactly.  The current support in libpq is nigh useless for testing
purposes, because there's no way to activate that code path on command.
It only runs if libpq (thinks it) is connecting to a pre-7.4 backend.

regards, tom lane


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


Re: [HACKERS] Successor of MD5 authentication, let's use SCRAM

2013-09-12 Thread Heikki Linnakangas

On 12.09.2013 17:30, Andrew Dunstan wrote:


On 09/12/2013 09:10 AM, Heikki Linnakangas wrote:


I just found out that OpenSSL has added support for SRP in version
1.0.1. We're already using OpenSSL, so all we need to do is to provide
a couple of callbacks to OpenSSL, and store SRP verifiers in pg_authid
instead of MD5 hashes, and we're golden.

Well, not quite. There's one little problem: Currently, we first
initialize SSL, then read the startup packet which contains the
username and database to connect to. After that, we initialize
database access to the specified database, and only then we proceed
with authentication. That's not a problem for certificate
authentication, because certificate authentication doesn't require any
database access, but if we are to store the SRP verifiers in
pg_authid, we'll need to database access much earlier. Before we know
which database to connect to.


You forgot to mention that we'd actually like to get away from being
tied closely to OpenSSL because it has caused license grief in the past
(not to mention that it's fairly dirty to manage).


Yeah. I've been looking more closely at the SRP API in OpenSSL; it's 
completely undocumented. There are examples on the web and mailing lists 
on how to use it, but no documentation. Hopefully that gets fixed 
eventually.


GnuTLS also supports SRP. They even have documentation for it :-). The 
API is slightly different than OpenSSL's, but not radically so. If we 
are to start supporting multiple TLS libraries, we're going to need some 
kind of a shim layer to abstract away the differences. Writing such a 
shim for the SRP stuff wouldn't be much additional effort, once you have 
the shim for all the other stuff in place.


- 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] Successor of MD5 authentication, let's use SCRAM

2013-09-12 Thread Andrew Dunstan


On 09/12/2013 09:10 AM, Heikki Linnakangas wrote:



Now, to a completely different approach:

I just found out that OpenSSL has added support for SRP in version 
1.0.1. We're already using OpenSSL, so all we need to do is to provide 
a couple of callbacks to OpenSSL, and store SRP verifiers in pg_authid 
instead of MD5 hashes, and we're golden.


Well, not quite. There's one little problem: Currently, we first 
initialize SSL, then read the startup packet which contains the 
username and database to connect to. After that, we initialize 
database access to the specified database, and only then we proceed 
with authentication. That's not a problem for certificate 
authentication, because certificate authentication doesn't require any 
database access, but if we are to store the SRP verifiers in 
pg_authid, we'll need to database access much earlier. Before we know 
which database to connect to.






You forgot to mention that we'd actually like to get away from being 
tied closely to OpenSSL because it has caused license grief in the past 
(not to mention that it's fairly dirty to manage).


cheers

andrew



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


Re: [HACKERS] Successor of MD5 authentication, let's use SCRAM

2013-09-12 Thread Stephen Frost
* Andrew Dunstan (and...@dunslane.net) wrote:
 You forgot to mention that we'd actually like to get away from being
 tied closely to OpenSSL because it has caused license grief in the
 past (not to mention that it's fairly dirty to manage).

While I agree with this sentiment (and have complained bitterly about
OpenSSL's license in the past), I'd rather see us implement this
(perhaps with a shim layer, if that's possible/sensible) even if
only OpenSSL is supported than to not have the capability at all.  It
seems highly unlikely we'd ever be able to drop support for OpenSSL
completely; we've certainly not made any progress towards that and I
don't think forgoing adding new features would make such a change any
more or less likely to happen.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] review: pgbench progress report improvements

2013-09-12 Thread Fabien COELHO


Hello Pavel,

Thanks for your review.


* patched with minor warning
* compilable cleanly
* zero impact on PostgreSQL server functionality
* it does what was in proposal
** change 5sec progress as default (instead no progress)
** finalise a rate limit support - fixes a latency calculation


Just a point about the motivation: the rationale for having a continuous 
progress report is that benchmarking is subject to possibly long warmup 
times, and thus a test may have to run for hours so as to be significant. 
I find running a command for hours without any hint about what is going on 
quite annoying.



* code is clean
* documentation is included
* there is no voices against this patch and this patch increases a pgbench
usability/

I have only one question. When I tested this patch with throttling I got a
very similar values of lag.


Yep. That is just good!


What is sense, or what is semantic of this value?


The lag measures the stochastic processus health. Actually, it measures 
how far behind schedule the clients are when performing throttled 
transactions. If it was to increase, that would mean that something is 
amiss, possibly not enough client threads or other issues. If it is small, 
then all is well.



It is not detailed documented.


It is documented in the section about the --rate option, see
http://www.postgresql.org/docs/devel/static/pgbench.html


Should be printed this value in this form on every row? We can
print some warning when lag is higher than latency instead?


Hmmm... what is important is when the lag changes values.

Generally one would indeed expect that to be smaller than the latency, but 
that is not really possible when transaction are very fast, say under -S 
with read-only queries that hit the memory cache.


Also the problem with printing warnings is that it changes the output 
format, but it seems to me more useful to print the value, so that it can 
be processed automatically and simply.


Also, from a remote client perspective, say a web application, the overall 
latency is the lag plus the transaction latency: you first wait to get 
through the database (lag), and then you can perform your transaction 
(latency).



Or we can use this value, but it should be better documented, please.


Is the documentation pointed above enough?

--
Fabien.


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


Re: [HACKERS] Successor of MD5 authentication, let's use SCRAM

2013-09-12 Thread Magnus Hagander
On Thu, Sep 12, 2013 at 4:41 PM, Heikki Linnakangas
hlinnakan...@vmware.com wrote:
 On 12.09.2013 17:30, Andrew Dunstan wrote:


 On 09/12/2013 09:10 AM, Heikki Linnakangas wrote:


 I just found out that OpenSSL has added support for SRP in version
 1.0.1. We're already using OpenSSL, so all we need to do is to provide
 a couple of callbacks to OpenSSL, and store SRP verifiers in pg_authid
 instead of MD5 hashes, and we're golden.

 Well, not quite. There's one little problem: Currently, we first
 initialize SSL, then read the startup packet which contains the
 username and database to connect to. After that, we initialize
 database access to the specified database, and only then we proceed
 with authentication. That's not a problem for certificate
 authentication, because certificate authentication doesn't require any
 database access, but if we are to store the SRP verifiers in
 pg_authid, we'll need to database access much earlier. Before we know
 which database to connect to.


 You forgot to mention that we'd actually like to get away from being
 tied closely to OpenSSL because it has caused license grief in the past
 (not to mention that it's fairly dirty to manage).


 Yeah. I've been looking more closely at the SRP API in OpenSSL; it's
 completely undocumented. There are examples on the web and mailing lists on
 how to use it, but no documentation. Hopefully that gets fixed eventually.

Well, undocumented and OpenSSL tend to go hand in hand a lot. Or,
well, it might be documented, but not in a useful way. I wouldn't
count on it.


 GnuTLS also supports SRP. They even have documentation for it :-). The API
 is slightly different than OpenSSL's, but not radically so. If we are to
 start supporting multiple TLS libraries, we're going to need some kind of a
 shim layer to abstract away the differences. Writing such a shim for the SRP
 stuff wouldn't be much additional effort, once you have the shim for all the
 other stuff in place.

http://en.wikipedia.org/wiki/Secure_Remote_Password_protocol#Real_world_implementations

That does not paint a very good pictures. I'd say the most likely
library that we'd *want* instead of openssl is NSS, if we have to pick
one of the big ones. Or one of the newer implementations that are a
lot more focused and lean. And none of them support SRP.

I fear starting to use that is going to make it even harder to break
out from our openssl dependency, which people do complain about at
least semi-regularly.

I wonder how much work it would be to build something on top of lower
level primitives that are provided in them all. For example,
https://github.com/cocagne/csrp/ implements it on top of openssl, and
it's around 1000 LOC (bsd licensed). And that's generic - it might
well be shorter if we do something ourselves. And if it's BSD
licensed, we could import.. (And then extend to run on top of other
cyrpto libraries, of course)

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


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


Re: [HACKERS] review: pgbench progress report improvements

2013-09-12 Thread Fabien COELHO



* patched with minor warning



some minor issue:

patch warning

make[1]: Leaving directory `/home/pavel/src/postgresql/config'
[pavel@localhost postgresql]$ patch -p1  pgbench-measurements-v2.patch
(Stripping trailing CRs from patch; use --binary to disable.)
patching file contrib/pgbench/pgbench.c
(Stripping trailing CRs from patch; use --binary to disable.)
patching file doc/src/sgml/pgbench.sgml


I cannot reproduce these warnings:

  postgresql git branch test
  postgresql git checkout test
Switched to branch 'test'
  postgresql patch -p1  ../pgbench-measurements-v2.patch
patching file contrib/pgbench/pgbench.c
patching file doc/src/sgml/pgbench.sgml

Some details:

  postgresql patch --version
patch 2.6.1 [...]
  postgresql sha1sum ../pgbench-measurements-v2.patch
f095557ceae1409d2339f9d29d332cefa96e2153 [...]

--
Fabien.


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


Re: [HACKERS] Patch for fail-back without fresh backup

2013-09-12 Thread Peter Eisentraut
On 9/12/13 3:00 AM, Samrat Revagade wrote:
 
 We are improving the patch for Commit Fest 2 now.
 We will fix above compiler warnings as soon as possible and submit
 the patch
 
 
 Attached *synchronous_transfer_v5.patch* implements review comments from
 commit fest-1 and reduces the performance overhead of synchronous_transfer.

There is still this compiler warning:

syncrep.c: In function ‘SyncRepReleaseWaiters’:
syncrep.c:421:6: warning: variable ‘numdataflush’ set but not used
[-Wunused-but-set-variable]



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


Re: [HACKERS] Triggers on foreign tables

2013-09-12 Thread Peter Eisentraut
On 9/11/13 10:14 AM, Ronan Dunklau wrote:
 On Wednesday 11 September 2013 06:27:24 Michael Paquier wrote:
 As your patch is targeting the implementation of a new feature, please
 consider adding this patch to the next commit fest that is going to
 begin in a couple of days:
 https://commitfest.postgresql.org/action/commitfest_view?id=19
 
 I intended to do that in the next couple of days if I don't get any feedback. 
 Thank you for the reminder, its done.

The documentation build fails:

openjade:trigger.sgml:72:9:E: end tag for ACRONYM omitted, but OMITTAG
NO was specified
openjade:trigger.sgml:70:56: start tag was here




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


Re: [HACKERS] lcr v5 - primary/candidate key in relcache

2013-09-12 Thread Kevin Grittner
Andres Freund and...@2ndquadrant.com wrote:
 Robert Haas wrote:
 Andres Freund and...@2ndquadrant.com wrote:
 0007 wal_decoding: Add information about a tables primary key to
  struct RelationData
 * Could be used in the matview refresh code

 I think you and Kevin should discuss whether this is actually the
 right way to do this.  ISTM that if logical replication and
 materialized views end up selecting different approaches to this
 problem, everybody loses.

 The patch we're discussion here adds a new struct RelationData field
 called 'rd_primary' (should possibly be renamed) which contains
 information about the best candidate key available for a table.

 From the header comments:
 /*
 * The 'best' primary or candidate key that has been found, only set
 * correctly if RelationGetIndexList has been called/rd_indexvalid  0.
 *
 * Indexes are chosen in the following order:
 * * Primary Key
 * * oid index
 * * the first (OID order) unique, immediate, non-partial and
 *  non-expression index over one or more NOT NULL'ed columns
 */
 Oid rd_primary;

 I thought we could use that in matview.c:refresh_by_match_merge() to
 select a more efficient diff if rd_primary has a valid index. In that
 case you only'd need to compare that index's fields which should result
 in an more efficient plan.

 Maybe it's also useful in other cases for you?

 If it's relevant at all, would you like to have a different priority
 list than the one above?

My first thought was that it was necessary to use all unique,
immediate, non-partial, non-expression indexes to avoid getting
errors on the UPDATE phase of the concurrent refresh for transient
duplicates; but then I remembered that I had to give up on that and
do it all with DELETE followed by INSERT, which eliminates that
risk.  As things now stand the *existence* of any unique,
non-partial, non-expression index (note that immediate is not
needed) is sufficient for correctness.  We could now even drop that,
I think, if we added a duplicate check at the end in the absence of
such an index.

The reason I left it comparing columns from *all* such indexes is
that it gives the optimizer the chance to pick the one that looks
fastest.  With the upcoming patch that can add some extra
equality comparisons in addition to the identical comparisons
the patch uses, so the mechanism you propose might be a worthwhile
optimization for some cases as long as it does a good job of
picking *the fastest* such index.  The above method of choosing an
index doesn't seem to necessarily ensure that.

Also, if you need to include the immediate test, it could not be
used for RMVC without fallback code if this mechanism didn't find
an appropriate index.  Of course, that would satisfy those who
would like to relax the requirement for a unique index on the MV to
be able to use RMVC.

--
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] ENABLE/DISABLE CONSTRAINT NAME

2013-09-12 Thread Peter Eisentraut
On 9/11/13 1:09 AM, wangs...@highgo.com.cn wrote:
 Peter Eisentraut wrote:
 Note that other schema objects can depend on the existence of
 constraints.  For example, the validity of a view might depend on the
 existence of a primary key constraint.  What would you do with the view
 if the primary key constraint is temporarily disabled?

 
 Thanks for your reply.
  I could't  clearly understand your opinion,  could you give  me more
 information or example?

= create table test1 (a int constraint pk primary key, b text);
= create view test2 as select a, b from test1 group by a;
= alter table test1 drop constraint pk;
ERROR:  2BP01: cannot drop constraint pk on table test1 because other
objects depend on it
DETAIL:  view test2 depends on constraint pk on table test1
HINT:  Use DROP ... CASCADE to drop the dependent objects too.

(This has to do with whether ungrouped columns are allowed in the select
list when the presence of constraints ensures well-defined results.)

When trying to drop the constraint, the choice is to abort the drop or
to drop dependent objects.  When you are talking about
enabling/disabling the constraint, it's not clear what to do.



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


[HACKERS] record identical operator

2013-09-12 Thread Kevin Grittner
Attached is a patch for a bit of infrastructure I believe to be
necessary for correct behavior of REFRESH MATERIALIZED VIEW
CONCURRENTLY as well as incremental maintenance of matviews.

The idea is that after RMVC or incremental maintenance, the matview
should not be visibly different that it would have been at creation
or on a non-concurrent REFRESH.  The issue is easy to demonstrate
with citext, but anywhere that the = operator allows user-visible
differences between equal values it can be an issue.

test=# CREATE TABLE citext_table (
test(#   id serial primary key,
test(#   name citext
test(# );
CREATE TABLE
test=# INSERT INTO citext_table (name)
test-#   VALUES ('one'), ('two'), ('three'), (NULL), (NULL);
INSERT 0 5
test=# CREATE MATERIALIZED VIEW citext_matview AS
test-#   SELECT * FROM citext_table;
SELECT 5
test=# CREATE UNIQUE INDEX citext_matview_id
test-#   ON citext_matview (id);
CREATE INDEX
test=# UPDATE citext_table SET name = 'Two' WHERE name = 'TWO';
UPDATE 1

At this point, the table and the matview have visibly different
values, yet without the patch the query used to find differences
for RMVC would be essentially like this (slightly simplified for
readability):

test=# SELECT *
  FROM citext_matview m
  FULL JOIN citext_table t ON (t.id = m.id AND t = m)
  WHERE t IS NULL OR m IS NULL;
 id | name | id | name
+--++--
(0 rows)

No differences were found, so without this patch, the matview would
remain visibly different from the results generated by a run of its
defining query.

The patch adds an identical operator (===) for the record type:

test=# SELECT *
  FROM citext_matview m
  FULL JOIN citext_table t ON (t.id = m.id AND t === m)
  WHERE t IS NULL OR m IS NULL;
 id | name | id | name
+--++--
    |  |  2 | Two
  2 | two  |    |
(2 rows)

The difference is now found, so RMVC makes the appropriate change.

test=# REFRESH MATERIALIZED VIEW CONCURRENTLY citext_matview;
REFRESH MATERIALIZED VIEW
test=# SELECT * FROM citext_matview ORDER BY id;
 id | name  
+---
  1 | one
  2 | Two
  3 | three
  4 |
  5 |
(5 rows)

The patch adds all of the functions, operators, and catalog
information to support merge joins using the identical operator.

The new operator is logically similar to IS NOT DISTINCT FROM for a
record, although its implementation is very different.  For one
thing, it doesn't replace the operation with column level operators
in the parser.  For another thing, it doesn't look up operators for
each type, so the identical operator does not need to be
implemented for each type to use it as shown above.  It compares
values byte-for-byte, after detoasting.  The test for identical
records can avoid the detoasting altogether for any values with
different lengths, and it stops when it finds the first column with
a difference.

I toyed with the idea of supporting hashing of records using this
operator, but could not see how that would be a performance win.

The identical (===) and not identical (!==) operator names were
chosen because of a vague similarity to the exactly equals
concepts in JavaScript and PHP, which use that name.  The semantics
aren't quite the same, but it seemed close enough not to be too
surprising.  The additional operator names seemed natural to me
based on the first two, but I'm not really that attached to these
names for the operators if someone has a better idea.

Since the comparison of record values is not documented (only
comparisons involving row value constructors), it doesn't seem like
we should document this special case.  It is intended primarily for
support of matview refresh and maintenance, and it seems likely
that record comparison was not documented on the basis that it is
intended primarily for support of such things as indexing and merge
joins -- so leaving the new operators undocumented seems consistent
with existing policy.  I'm open to arguments that the policy should
change.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company*** a/contrib/citext/expected/citext.out
--- b/contrib/citext/expected/citext.out
***
*** 2276,2278  SELECT like_escape( name::text, ''::citext ) = like_escape( name::text, '' ) AS
--- 2276,2319 
   t
  (5 rows)
  
+ -- Ensure correct behavior for citext with materialized views.
+ CREATE TABLE citext_table (
+   id serial primary key,
+   name citext
+ );
+ INSERT INTO citext_table (name)
+   VALUES ('one'), ('two'), ('three'), (NULL), (NULL);
+ CREATE MATERIALIZED VIEW citext_matview AS
+   SELECT * FROM citext_table;
+ CREATE UNIQUE INDEX citext_matview_id
+   ON citext_matview (id);
+ SELECT *
+   FROM citext_matview m
+   FULL JOIN citext_table t ON (t.id = m.id AND t === m)
+   WHERE t.id IS NULL OR m.id IS NULL;
+  id | name | id | name 
+ +--++--
+ (0 rows)
+ 
+ UPDATE citext_table SET name = 'Two' WHERE name = 'TWO';
+ SELECT *
+   FROM citext_matview m
+   FULL JOIN citext_table t ON (t.id = m.id AND t 

[HACKERS] 9.4 HEAD: select() failed in postmaster

2013-09-12 Thread Jeff Janes
On Wednesday, September 11, 2013, Alvaro Herrera wrote:

 Noah Misch escribió:
  On Tue, Sep 10, 2013 at 05:18:21PM -0700, Jeff Janes wrote:

   I think the problem is here, where there should be a Max rather than a
 Min:
  
   commit 82233ce7ea42d6ba519aaec63008aff49da6c7af
   Author: Alvaro Herrera alvhe...@alvh.no-ip.org
   Date:   Fri Jun 28 17:20:53 2013 -0400
  
   Send SIGKILL to children if they don't die quickly in immediate
 shutdown
  
   ...
  
   +   /* remaining time, but at least 1 second */
   +   timeout-tv_sec = Min(SIGKILL_CHILDREN_AFTER_SECS -
   + (time(NULL) - AbortStartTime), 1);
 
  Agreed; good catch.

 Yeah, thanks.  Should be a Max().  The current coding presumably makes
 it use one second most of the time, instead of whatever the remaining
 time is ... until the abort time is past, in which case it causes the
 whole thing to break down as reported.

 It might very well be that I used Max() there initially and changed to
 Min() at the last minute before commit in a moment of brain fade.


I've implemented the Min to Max change and did some more testing.  Now I
have a different  but related problem (which I also saw before, but less
often than the select() one).  The 5 second clock doesn't get turned off.
 So after all processes end, and a new startup is launched, if that startup
doesn't report back to the postmaster soon enough, it gets SIGKILLED.

postmaster.c near line 1681


if ((Shutdown = ImmediateShutdown || (FatalError  !SendStop)) 
now - AbortStartTime = SIGKILL_CHILDREN_AFTER_SECS)

It seems like this needs to have an additional and-test of pmState, but
which states to test I don't really know.

I've added in  (pmStatePM_RUN) and have not had any more failures, so
I think that this is on the right path but testing an enum for inequality
feels wrong.

Alternatively perhaps FatalError can get cleared when startup is launched,
rather than when WAL replay begins.  But I assume it was done the way it is
for a reason, even though I don't know that reason.

Cheers,

Jeff





Re: [HACKERS] Enabling Checksums

2013-09-12 Thread Greg Smith

On 3/18/13 10:52 AM, Bruce Momjian wrote:

With a potential 10-20% overhead, I am unclear who would enable this at
initdb time.


If you survey people who are running PostgreSQL on cloud hardware, be 
it Amazon's EC2 or similar options from other vendors, you will find a 
high percentage of them would pay quite a bit of performance to make 
their storage more reliable.  To pick one common measurement for 
popularity, a Google search on ebs corruption returns 17 million hits. 
 To quote one of those, Baron Schwartz of Percona talking about MySQL 
on EC2:


BTW, I have seen data corruption on EBS volumes. It’s not clear whether 
it was InnoDB’s fault (extremely unlikely IMO), the operating system’s 
fault, EBS’s fault, or something else.


http://www.mysqlperformanceblog.com/2011/08/04/mysql-performance-on-ec2ebs-versus-rds/

*That* uncertainty is where a lot of the demand for this feature is 
coming from.  People deploy into the cloud, their data gets corrupted, 
and no one call tell them what/why/how it happened.  And that means they 
don't even know what to change to make it better.  The only people I see 
really doing something about this problem all seem years off, and I'm 
not sure they are going to help--especially since some of them are 
targeting enterprise storage rather than the cloud-style installations.



I assume a user would wait until they suspected corruption to turn it
on, and because it is only initdb-enabled, they would have to
dump/reload their cluster.  The open question is whether this is a
usable feature as written, or whether we should wait until 9.4.


The reliability issues of both physical and virtual hardware are so 
widely known that many people will deploy with this on as their default 
configuration.


If you don't trust your existing data, you can't retroactively check it. 
 A checksum of an already corrupt block is useless.  Accordingly, there 
is no use case for converting an installation with real or even 
suspected problems to a checksummed one.  If you wait until you suspect 
corruption to care about checksums, it's really too late.  There is only 
one available next step:  you must do a dump to figure out what's 
readable.  That is the spot that all of the incoming data recovery 
customers we see at 2ndQuadrant are already in when we're called.  The 
cluster is suspicious, sometimes they can get data out of it with a 
dump, and if we hack up their install we can usually recover a bit more 
than they could.


After the data from a partially corrupted database is dumped, someone 
who has just been through that pain might decide they should turn 
checksums on when they restore the dump.  When it's on, they can access 
future damage easily at the block level when it happens, and possibly 
repair it without doing a full dump/reload.  What's implemented in the 
feature we're talking about has a good enough UI to handle this entire 
cycle I see damaged installations go through.



In fact, this feature is going to need
pg_upgrade changes to detect from pg_controldata that the old/new
clusters have the same checksum setting.


I think that's done already, but it's certainly something to test out too.

Good questions, Bruce, I don't think the reasons behind this feature's 
demand have been highlighted very well before.  I try not to spook the 
world by talking regularly about how many corrupt PostgreSQL databases 
I've seen, but they do happen.  Most of my regular ranting on crappy 
SSDs that lie about writes comes from a TB scale PostgreSQL install that 
got corrupted due to the write-cache flaws of the early Intel 
SSDs--twice.  The would have happily lost even the worst-case 20% of 
regular performance to avoid going down for two days each time they saw 
corruption, where we had to dump/reload to get them going again.  If the 
install had checksums, I could have figured out which blocks were 
damaged and manually fixed them.  Without checksums, there's no way to 
even tell for sure what is broken.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com


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


Re: [HACKERS] Enabling Checksums

2013-09-12 Thread Greg Smith

On 3/18/13 10:52 AM, Bruce Momjian wrote:

With a potential 10-20% overhead, I am unclear who would enable this at
initdb time.


If you survey people who are running PostgreSQL on cloud hardware, be 
it Amazon's EC2 or similar options from other vendors, you will find a 
high percentage of them would pay quite a bit of performance to make 
their storage more reliable.  To pick one common measurement for 
popularity, a Google search on ebs corruption returns 17 million hits. 
 To quote one of those, Baron Schwartz of Percona talking about MySQL 
on EC2:


BTW, I have seen data corruption on EBS volumes. It’s not clear whether 
it was InnoDB’s fault (extremely unlikely IMO), the operating system’s 
fault, EBS’s fault, or something else.


http://www.mysqlperformanceblog.com/2011/08/04/mysql-performance-on-ec2ebs-versus-rds/

*That* uncertainty is where a lot of the demand for this feature is 
coming from.  People deploy into the cloud, their data gets corrupted, 
and no one call tell them what/why/how it happened.  And that means they 
don't even know what to change to make it better.  The only people I see 
really doing something about this problem all seem years off, and I'm 
not sure they are going to help--especially since some of them are 
targeting enterprise storage rather than the cloud-style installations.



I assume a user would wait until they suspected corruption to turn it
on, and because it is only initdb-enabled, they would have to
dump/reload their cluster.  The open question is whether this is a
usable feature as written, or whether we should wait until 9.4.


The reliability issues of both physical and virtual hardware are so 
widely known that many people will deploy with this on as their default 
configuration.


If you don't trust your existing data, you can't retroactively check it. 
 A checksum of an already corrupt block is useless.  Accordingly, there 
is no use case for converting an installation with real or even 
suspected problems to a checksummed one.  If you wait until you suspect 
corruption to care about checksums, it's really too late.  There is only 
one available next step:  you must do a dump to figure out what's 
readable.  That is the spot that all of the incoming data recovery 
customers we see at 2ndQuadrant are already in when we're called.  The 
cluster is suspicious, sometimes they can get data out of it with a 
dump, and if we hack up their install we can usually recover a bit more 
than they could.


After the data from a partially corrupted database is dumped, someone 
who has just been through that pain might decide they should turn 
checksums on when they restore the dump.  When it's on, they can access 
future damage easily at the block level when it happens, and possibly 
repair it without doing a full dump/reload.  What's implemented in the 
feature we're talking about has a good enough UI to handle this entire 
cycle I see damaged installations go through.


Good questions, Bruce, I don't think the reasons behind this feature's 
demand have been highlighted very well before.  I try not to spook the 
world by talking regularly about how many corrupt PostgreSQL databases 
I've seen, but they do happen.  Most of my regular ranting on crappy 
SSDs that lie about writes comes from a TB scale PostgreSQL install that 
got corrupted due to the write-cache flaws of the early Intel 
SSDs--twice.  The would have happily lost even the worst-case 20% of 
regular performance to avoid going down for two days each time they saw 
corruption, where we had to dump/reload to get them going again.  If the 
install had checksums, I could have figured out which blocks were 
damaged and manually fixed them.  Without checksums, there really was 
nowhere to go except dump/reload.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com


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


Re: [HACKERS] Patch for fail-back without fresh backup

2013-09-12 Thread Sawada Masahiko
On Fri, Sep 13, 2013 at 1:11 AM, Peter Eisentraut pete...@gmx.net wrote:
 On 9/12/13 3:00 AM, Samrat Revagade wrote:

 We are improving the patch for Commit Fest 2 now.
 We will fix above compiler warnings as soon as possible and submit
 the patch


 Attached *synchronous_transfer_v5.patch* implements review comments from
 commit fest-1 and reduces the performance overhead of synchronous_transfer.

 There is still this compiler warning:

 syncrep.c: In function ‘SyncRepReleaseWaiters’:
 syncrep.c:421:6: warning: variable ‘numdataflush’ set but not used
 [-Wunused-but-set-variable]


Sorry I forgot fix it.

I have attached the patch which I modified.

Regards,

---
Sawada Masahiko


synchronous_transfer_v6.patch
Description: Binary data

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


[HACKERS] background workers, round three

2013-09-12 Thread Robert Haas
Last week, I attempted to write some code to perform a trivial
operation in parallel by launching background workers.  Despite my
earlier convictions that I'd built enough infrastructure to make this
possible, the experiment turned out badly - so, patches!

It's been pretty obvious to me from the beginning that any sort of
parallel computation would need a way to make sure that if any worker
dies, everybody dies.  Conversely, if the parent aborts, all the
workers should die.  My thought was that this could be implemented
using PG_TRY()/PG_CATCH() blocks over the existing infrastructure, but
this turned out to be naive.  If the original backend encounters an
error before the child manages to start up, there's no good recovery
strategy.  The parent can't kill the child because it doesn't exist
yet, and the parent can't stop the postmaster from starting the child
later.  The parent could try waiting until the child starts up and
THEN killing it, but that's probably unreliable and surely
mind-numbingly lame.  The first attached patch,
terminate-worker-v1.patch, rectifies this problem by providing a
TerminateBackgroundWorker() function.  When this function is invoked,
the postmaster will become unwilling to restart the worker and will
send it a SIGTERM if it's already running.  (It's important that the
SIGTERM be sent by the postmaster, because if the original backend
tries to send it, there's a race condition: the process might not be
started at the time the original backend tries to send the signal, but
the postmaster might start it before it sees the terminate request.)

By itself, this is useful, but painful.  The pain comes from the fact
that all of the house-keeping is left to the programmer.  It is
possible but not elegant to use something like
PG_ENSURE_ERROR_CLEANUP() to ensure that all background workers are
terminated even on an error exit from the affected code.  The other
half of the problem is harder: how do we ensure not only that the
untimely demise of a worker process aborts the original backend's
transaction, but that it does so in a relatively timely fashion?  If
the original backend is executing only a very limited amount of code
while the parallel workers remain in existence, it would be possible
to add explicit checks for the demise of a worker at periodic
intervals through all of that code.  But this seems a very limiting
approach.  In the hope of making things better, the second patch
attached herewith, ephemeral-precious-v1.patch, adds two new flags,
BGWORKER_EPHEMERAL and BGWORKER_PRECIOUS.

Setting the BGWORKER_EPHEMERAL flag causes the background worker to be
killed when the registrant's (sub)transaction ends.  This eliminates
the need to catch errors and explicitly invoke
TerminateBackgroundWorker() in the error path.  You can simply
register an ephemeral worker, write code to do stuff with it, and then
terminate it.  If an error occurs part-way through, the worker will be
terminated as part of the abort path.

Setting the BGWORKER_PRECIOUS flag causes the unexpected death of the
worker to abort the registrant's current (sub)transaction.  This
eliminates the need to sprinkle the code with checks for a deceased
worker.  Instead, you can simply register a precious worker, and then
just remember to CHECK_FOR_INTERRUPTS().  There were a couple of
awkward cases here.  First, all the existing stuff that hooks into
ProcessInterrupts() makes provision for handling the
ImmediateInterruptOK case.  I felt that was superfluous here, so
instead simply prohibited leaving a precious background worker running
beyond the end of the statement.  The point is to enable parallel
computation, which will, I think, begin and end within the lifespan of
one query.  Second, odd things happen if the original backend launches
precious workers and then begins a subtransaction.  Throwing an error
in the subtransaction will not do the right thing; the subtransaction
may easily be something like a PL/pgsql exception block and an error
will be caught and result in unexpected behavior.  So we don't.  I
just added a comment saying that if you do decide to start a
subtransaction while you've got precious workers outstanding, you'd
better insert an explicit check for whether they're still alive after
unwinding the subtransaction (there's a function to make that easy).
We could probably build a mechanism to allow an error to be thrown
against a (sub)xact other than the innermost one, implicitly aborting
everything below that with extreme prejudice, but it seems like
overkill.  I can't but imagine that early versions of
parallel-anything will include starting subtransactions on the list
of activities which are prohibited in parallel mode.

Using the infrastructure provided by those patches, I was able to
write some test code, attached as pingpong-v1.patch.  You can make a
backend fire up a background worker, and the two will take turns
setting each others latches for a number of iterations you specify.
This could 

Re: [HACKERS] ENABLE/DISABLE CONSTRAINT NAME

2013-09-12 Thread wangshuo

On 09/13/2013 05:23, Peter Eisentraut wrote:

= create table test1 (a int constraint pk primary key, b text);
= create view test2 as select a, b from test1 group by a;
= alter table test1 drop constraint pk;
ERROR:  2BP01: cannot drop constraint pk on table test1 because other
objects depend on it
DETAIL:  view test2 depends on constraint pk on table test1
HINT:  Use DROP ... CASCADE to drop the dependent objects too.

(This has to do with whether ungrouped columns are allowed in the 
select

list when the presence of constraints ensures well-defined results.)

When trying to drop the constraint, the choice is to abort the drop 
or

to drop dependent objects.  When you are talking about
enabling/disabling the constraint, it's not clear what to do.


Thanks for your reply.
First, I had said that I I only made ​​a few modifications to the check 
and the

foreign key constraint, and did nothing with primary key constraint.

On 08/30/2013 02:03 PM, I wrote:

Due to the above reasons,I realized this command.

I add a field named 'conenabled' to pg_constraint, identifying whether 
a constraint is enable or not;
I enable or disable a foreign key constraint, by enable or disable the 
triggers of the foreign key;
Our database will depend on the value of 'conenabled' to use the check 
constrint or not;


In the alter_table.sgml, I wrote:

This form enables or disables a foreign key or check constraint.


Second, I tested the check and the foreign key constraint as your test 
above.

And no error found, as fellow:

postgres=# create table a1 (a1 int check(a14));
CREATE TABLE
postgres=# create view a11 as select * from a1;
CREATE VIEW
postgres=# alter table a1 disable constraint a1_a1_check;
ALTER TABLE
postgres=# insert into a1 values (3);
INSERT 0 1
postgres=# select * from a11;
 a1

  3
(1 row)

postgres=# alter table a1 drop constraint a1_a1_check;
ALTER TABLE

postgres=# create table bb(b1 int primary key);
CREATE TABLE
postgres=# create table cc(c1 int references bb(b1));
CREATE TABLE
postgres=# create view c11 as select * from cc;
CREATE VIEW
postgres=# alter table cc disable constraint cc_c1_fkey;
ALTER TABLE
postgres=# insert into  cc values (1);
INSERT 0 1
postgres=# select * from c11;
 c1

  1
(1 row)

postgres=# alter table cc drop constraint cc_c1_fkey;
ALTER TABLE

 Wang Shuo
 HighGo Software Co.,Ltd.
 September 13, 2013











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


[HACKERS] [PATCH] pg_upgrade: Split off pg_fatal() from pg_log()

2013-09-12 Thread Peter Eisentraut
The experiences with elog() and ereport() have shown that having one
function that can return or not depending on some log level parameter
isn't a good idea when you want to communicate well with the compiler.
In pg_upgrade, there is a similar case with the pg_log() function.
Since that isn't a public API, I'm proposing to change it and introduce
a separate function pg_fatal() for those cases where the calls don't
return.

From 12ae7ed2b4174f694f67f31ac18eb1fe22d30ef9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut pete...@gmx.net
Date: Wed, 11 Sep 2013 22:43:03 -0400
Subject: [PATCH] pg_upgrade: Split off pg_fatal() from pg_log()

This allows decorating pg_fatal() with noreturn compiler hints, leading
to better diagnostics.
---
 contrib/pg_upgrade/check.c   | 68 --
 contrib/pg_upgrade/controldata.c | 92 
 contrib/pg_upgrade/exec.c| 21 
 contrib/pg_upgrade/file.c|  3 +-
 contrib/pg_upgrade/function.c| 11 ++---
 contrib/pg_upgrade/info.c|  6 +--
 contrib/pg_upgrade/option.c  | 23 +
 contrib/pg_upgrade/page.c|  6 +--
 contrib/pg_upgrade/parallel.c| 12 ++---
 contrib/pg_upgrade/pg_upgrade.c  |  8 ++--
 contrib/pg_upgrade/pg_upgrade.h  |  3 ++
 contrib/pg_upgrade/relfilenode.c | 11 ++---
 contrib/pg_upgrade/server.c  | 11 ++---
 contrib/pg_upgrade/tablespace.c  |  3 +-
 contrib/pg_upgrade/util.c| 35 +++---
 contrib/pg_upgrade/version.c |  2 +-
 contrib/pg_upgrade/version_old_8_3.c | 23 -
 17 files changed, 165 insertions(+), 173 deletions(-)

diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
index 0376fcb..1a37b79 100644
--- a/contrib/pg_upgrade/check.c
+++ b/contrib/pg_upgrade/check.c
@@ -158,8 +158,7 @@ static void check_locale_and_encoding(ControlData *oldctrl,
 	 * matching install-user oids.
 	 */
 	if (old_cluster.install_role_oid != new_cluster.install_role_oid)
-		pg_log(PG_FATAL,
-			   Old and new cluster install users have different values for pg_authid.oid.\n);
+		pg_fatal(Old and new cluster install users have different values for pg_authid.oid.\n);
 
 	/*
 	 * We only allow the install user in the new cluster because other defined
@@ -167,7 +166,7 @@ static void check_locale_and_encoding(ControlData *oldctrl,
 	 * error during pg_dump restore.
 	 */
 	if (new_cluster.role_count != 1)
-		pg_log(PG_FATAL, Only the install user can be defined in the new cluster.\n);
+		pg_fatal(Only the install user can be defined in the new cluster.\n);
 
 	check_for_prepared_transactions(new_cluster);
 }
@@ -271,11 +270,11 @@ static void check_locale_and_encoding(ControlData *oldctrl,
 	 */
 
 	if (GET_MAJOR_VERSION(old_cluster.major_version)  803)
-		pg_log(PG_FATAL, This utility can only upgrade from PostgreSQL version 8.3 and later.\n);
+		pg_fatal(This utility can only upgrade from PostgreSQL version 8.3 and later.\n);
 
 	/* Only current PG version is supported as a target */
 	if (GET_MAJOR_VERSION(new_cluster.major_version) != GET_MAJOR_VERSION(PG_VERSION_NUM))
-		pg_log(PG_FATAL, This utility can only upgrade to PostgreSQL version %s.\n,
+		pg_fatal(This utility can only upgrade to PostgreSQL version %s.\n,
 			   PG_MAJORVERSION);
 
 	/*
@@ -284,7 +283,7 @@ static void check_locale_and_encoding(ControlData *oldctrl,
 	 * versions.
 	 */
 	if (old_cluster.major_version  new_cluster.major_version)
-		pg_log(PG_FATAL, This utility cannot be used to downgrade to older major PostgreSQL versions.\n);
+		pg_fatal(This utility cannot be used to downgrade to older major PostgreSQL versions.\n);
 
 	/* get old and new binary versions */
 	get_bin_version(old_cluster);
@@ -293,12 +292,10 @@ static void check_locale_and_encoding(ControlData *oldctrl,
 	/* Ensure binaries match the designated data directories */
 	if (GET_MAJOR_VERSION(old_cluster.major_version) !=
 		GET_MAJOR_VERSION(old_cluster.bin_version))
-		pg_log(PG_FATAL,
-			   Old cluster data and binary directories are from different major versions.\n);
+		pg_fatal(Old cluster data and binary directories are from different major versions.\n);
 	if (GET_MAJOR_VERSION(new_cluster.major_version) !=
 		GET_MAJOR_VERSION(new_cluster.bin_version))
-		pg_log(PG_FATAL,
-			   New cluster data and binary directories are from different major versions.\n);
+		pg_fatal(New cluster data and binary directories are from different major versions.\n);
 
 	check_ok();
 }
@@ -315,17 +312,17 @@ static void check_locale_and_encoding(ControlData *oldctrl,
 	/* Is it 9.0 but without tablespace directories? */
 	if (GET_MAJOR_VERSION(new_cluster.major_version) == 900 
 		new_cluster.controldata.cat_ver  TABLE_SPACE_SUBDIRS_CAT_VER)
-		pg_log(PG_FATAL, This utility can only upgrade to PostgreSQL version 9.0 after 2010-01-11\n
+		pg_fatal(This utility can only upgrade to PostgreSQL version 9.0 after 2010-01-11\n
 			   because of backend API 

Re: [HACKERS] getting rid of maintainer-check

2013-09-12 Thread Peter Eisentraut
On Tue, 2013-09-03 at 22:41 -0400, Peter Eisentraut wrote:
 The maintainer-check target never really caught on, I think.  Most
 people don't run it, and that in turn annoys those who do.  Also, it
 doesn't provide much functionality.
 
 I propose that we get rid of it and roll the functionality into the
 regular build. 

Here is a patch for that.  I also integrated Andrew's Perl version of
duplicate_oids.

The MSVC build needs to be updated separately, if they want to have that
same functionality.
From 1beea944acf339bc13a8759262a0c8c41fc1760c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut pete...@gmx.net
Date: Wed, 11 Sep 2013 14:47:44 -0400
Subject: [PATCH 1/2] Replace duplicate_oids with Perl implementation

It is more portable, more robust, and more readable.

From: Andrew Dunstan and...@dunslane.net

diff --git a/src/include/catalog/duplicate_oids b/src/include/catalog/duplicate_oids
index 82c12f3..f3d1136 100755
--- a/src/include/catalog/duplicate_oids
+++ b/src/include/catalog/duplicate_oids
@@ -1,30 +1,36 @@
-#!/bin/sh
-#
-# duplicate_oids
-#
-# src/include/catalog/duplicate_oids
-#
-# finds manually-assigned oids that are duplicated in the system tables.
-#
-# run this script in src/include/catalog.
-#
+#!/usr/bin/perl
 
-# note: we exclude BKI_BOOTSTRAP relations since they are expected to have
-# matching DATA lines in pg_class.h and pg_type.h
+use strict;
+use warnings;
 
-cat pg_*.h toasting.h indexing.h | \
-egrep -v -e '^CATALOG\(.*BKI_BOOTSTRAP' | \
-sed -n	-e 's/^DATA(insert *OID *= *\([0-9][0-9]*\).*$/\1/p' \
-	-e 's/^CATALOG([^,]*, *\([0-9][0-9]*\).*BKI_ROWTYPE_OID(\([0-9][0-9]*\)).*$/\1,\2/p' \
-	-e 's/^CATALOG([^,]*, *\([0-9][0-9]*\).*$/\1/p' \
-	-e 's/^DECLARE_INDEX([^,]*, *\([0-9][0-9]*\).*$/\1/p' \
-	-e 's/^DECLARE_UNIQUE_INDEX([^,]*, *\([0-9][0-9]*\).*$/\1/p' \
-	-e 's/^DECLARE_TOAST([^,]*, *\([0-9][0-9]*\), *\([0-9][0-9]*\).*$/\1,\2/p' | \
-tr ',' '\n' | \
-sort -n | \
-uniq -d | \
-grep '.'
+BEGIN
+{
+	@ARGV = (glob(pg_*.h), qw(indexing.h toasting.h));
+}
 
-# nonzero exit code if lines were produced
-[ $? -eq 1 ]
-exit
+my %oidcounts;
+
+while()
+{
+	next if /^CATALOG\(.*BKI_BOOTSTRAP/;
+	next unless
+		/^DATA\(insert *OID *= *(\d+)/ ||
+		/^CATALOG\([^,]*, *(\d+).*BKI_ROWTYPE_OID\((\d+)\)/ ||
+		/^CATALOG\([^,]*, *(\d+)/ ||
+		/^DECLARE_INDEX\([^,]*, *(\d+)/ ||
+		/^DECLARE_UNIQUE_INDEX\([^,]*, *(\d+)/ ||
+		/^DECLARE_TOAST\([^,]*, *(\d+), *(\d+)/;
+	$oidcounts{$1}++;
+	$oidcounts{$2}++ if $2;
+}
+
+my $found = 0;
+
+foreach my $oid (sort {$a = $b} keys %oidcounts)
+{
+	next unless $oidcounts{$oid}  1;
+	$found = 1;
+	print $oid\n;
+}
+
+exit $found;
-- 
1.8.4.rc3

From 7474945c3f2ae3e0f1ff24654f8557106fa6d526 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut pete...@gmx.net
Date: Wed, 11 Sep 2013 14:34:28 -0400
Subject: [PATCH 2/2] Remove maintainer-check target, fold into normal build

make maintainer-check was obscure and rarely called in practice, and
many breakages were missed.  Fold everything that make maintainer-check
used to do into the normal build.  Specifically:

- Call duplicate_oids when genbki.pl is called.

- Check for tabs in SGML files when the documentation is built.

- Run msgfmt with the -c option during the regular build.  Add an
  additional configure check to see whether we are using the GNU
  version.  (make maintainer-check probably used to fail with non-GNU
  msgfmt.)

Keep maintainer-check as around as phony target for the time being in
case anyone is calling it.  But it won't do anything anymore.

diff --git a/GNUmakefile.in b/GNUmakefile.in
index 17b1b3b..80116a1 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -70,8 +70,6 @@ $(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib,check)
 
 $(call recurse,installcheck-world,src/test src/pl src/interfaces/ecpg contrib,installcheck)
 
-$(call recurse,maintainer-check,doc src config contrib)
-
 GNUmakefile: GNUmakefile.in $(top_builddir)/config.status
 	./config.status $@
 
diff --git a/config/programs.m4 b/config/programs.m4
index c70a0c2..fd3a9a4 100644
--- a/config/programs.m4
+++ b/config/programs.m4
@@ -197,6 +197,11 @@ AC_DEFUN([PGAC_CHECK_GETTEXT],
   if test -z $MSGFMT; then
 AC_MSG_ERROR([msgfmt is required for NLS])
   fi
+  AC_CACHE_CHECK([for msgfmt flags], pgac_cv_msgfmt_flags,
+[if test x$MSGFMT != x  $MSGFMT --version 21 | grep GNU /dev/null; then
+pgac_cv_msgfmt_flags=-c
+fi])
+  AC_SUBST(MSGFMT_FLAGS, $pgac_cv_msgfmt_flags)
   AC_CHECK_PROGS(MSGMERGE, msgmerge)
   AC_CHECK_PROGS(XGETTEXT, xgettext)
 ])# PGAC_CHECK_GETTEXT
diff --git a/configure b/configure
index c685ca3..db8e006 100755
--- a/configure
+++ b/configure
@@ -659,6 +659,7 @@ TCL_CONFIG_SH
 TCLSH
 XGETTEXT
 MSGMERGE
+MSGFMT_FLAGS
 MSGFMT
 HAVE_POSIX_SIGNALS
 LDAP_LIBS_BE
@@ -29171,6 +29172,19 @@ done
 $as_echo $as_me: error: msgfmt is required for NLS 2;}
{ (exit 1); exit 1; }; }
   fi
+  { $as_echo $as_me:$LINENO: checking for msgfmt flags 5
+$as_echo_n checking for msgfmt flags...  6; }
+if 

Re: [HACKERS] review: pgbench progress report improvements

2013-09-12 Thread Pavel Stehule
Dne 12. 9. 2013 17:34 Fabien COELHO coe...@cri.ensmp.fr napsal(a):


 * patched with minor warning


 some minor issue:

 patch warning

 make[1]: Leaving directory `/home/pavel/src/postgresql/config'
 [pavel@localhost postgresql]$ patch -p1  pgbench-measurements-v2.patch
 (Stripping trailing CRs from patch; use --binary to disable.)
 patching file contrib/pgbench/pgbench.c
 (Stripping trailing CRs from patch; use --binary to disable.)
 patching file doc/src/sgml/pgbench.sgml


 I cannot reproduce these warnings:

   postgresql git branch test
   postgresql git checkout test
 Switched to branch 'test'
   postgresql patch -p1  ../pgbench-measurements-v2.patch
 patching file contrib/pgbench/pgbench.c
 patching file doc/src/sgml/pgbench.sgml


it can depends on o.s. I did tests on Fedora 14. and for patching without
warning I had to use dos2unix tool.

 Some details:

   postgresql patch --version
 patch 2.6.1 [...]
   postgresql sha1sum ../pgbench-measurements-v2.patch
 f095557ceae1409d2339f9d29d332cefa96e2153 [...]

 --
 Fabien.