date:20150429

Heikki,

* Heikki Linnakangas (hlinn...@iki.fi) wrote:
 The problem seems to be that when the standby is promoted, it's a
 so-called fast promotion, where it writes an end-of-recovery
 record and starts accepting queries before creating a real
 checkpoint. pg_rewind looks at the TLI in the latest checkpoint, as
 it's in the control file, but that isn't updated until the
 checkpoint completes. I don't see it on my laptop normally, but I
 can reproduce it if I insert a sleep(5) in StartupXLog, just
 before it requests the checkpoint:

Ah, interesting.

 --- a/src/backend/access/transam/xlog.c
 +++ b/src/backend/access/transam/xlog.c
 @@ -7173,7 +7173,10 @@ StartupXLOG(void)
* than is appropriate now that we're not in standby mode anymore.
*/
   if (fast_promoted)
 + {
 + sleep(5);
   RequestCheckpoint(CHECKPOINT_FORCE);
 + }
  }
 
 The simplest fix would be to force a checkpoint in the regression
 test, before running pg_rewind. It's a bit of a cop out, since you'd
 still get the same issue when you tried to do the same thing in the
 real world. It should be rare in practice - you'd not normally run
 pg_rewind immediately after promoting the standby - but a better
 error message at least would be nice..

Forcing a checkpoint in the regression tests and then providing a better
error message sounds reasonable to me.  I agree that it's very unlikely
to happen in the real world, even when you're bouncing between systems
for upgrades, etc, you're unlikely to do it fast enough for this issue
to exhibit itself, and a better error message would help any users who
manage to run into this (perhaps during their own testing).

Another thought would be to provide an option to pg_rewind to have it do
an explicit checkpoint before it reads the control file..  I'm not
against having it simply always do it as I don't see pg_rewind being a
commonly run thing, but I know some environments have very heavy
checkpoints and that might not be ideal.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] FIX : teach expression walker about RestrictInfo


Hi,

On 04/29/15 05:55, Tom Lane wrote:

Tomas Vondra tomas.von...@2ndquadrant.com writes:

On 04/28/15 21:50, Tom Lane wrote:

RestrictInfo is not a general expression node and support for it has
been deliberately omitted from expression_tree_walker().  So I think
what you are proposing is a bad idea and probably a band-aid for some
other bad idea.



That's not what I said, though. I said that calling pull_varnos() causes
the issue - apparently that does not happen in master, but I ran into
that when hacking on a patch.


pull_varnos is not, and should not be, applied to a RestrictInfo; for one
thing, it'd be redundant with work that was already done when creating the
RestrictInfo (cf make_restrictinfo_internal).  You've not shown the
context of your problem very fully, but in general most code in the planner
should be well aware of whether it is working with a RestrictInfo (or list
of same) or a bare expression.  I don't believe that it's a very good idea
to obscure that distinction.


OK, let me explain the context a bit more. When working on the 
multivariate statistics patch, I need to choose which stats to use for 
estimating the clauses. I do that in clauselist_selectivity(), although 
there might be other places where to do that.


Selecting the stats that best match the clauses is based on how well 
they cover the vars in the clauses (and some other criteria, that is 
mostly irrelevant here). So at some point I do call functions like 
pull_varnos() and pull_varattnos() to get this information.


I do handle RestrictInfo nodes explicitly - for those nodes I can do get 
the info from the node itself. But this does not work for the condition 
I posted, because that contains nested RestrictInfo nodes. So I do call 
pull_varnos() on a BoolExpr node, but in reality the node tree 
representing the clauses


WHERE (a = 10 AND a = 20) OR (b = 20 AND b = 40)

looks like this:

BoolExpr [OR_EXPR]
BoolExpr [AND_EXPR]
RestrictInfo
OpExpr [Var = Const]
RestrictInfo
OpExpr [Var = Const]
BoolExpr [AND_EXPR]
RestrictInfo
OpExpr [Var = Const]
RestrictInfo
OpExpr [Var = Const]

so the pull_varnos() fails because it runs into RestrictInfo node while 
walking the tree.


So I see three possibilities:

1) pull_varnos/pull_varattnos (and such functions, using the
   expression walker internally) are not supposed to be called unless
   you are sure there are no RestrictInfo nodes in the tree, but this
   seems really awkward

2) expression walker is supposed to know about RestrictInfo nodes (as I
   did in my patch), but you say this is not the case

3) pull_varnos_walker / pull_varattnos_walker are supposed to handle
   RestrictInfo nodes explicitly (either by using the cached information
   or by using RestrictInfo-clause in the next step)


regards

--
Tomas Vondra   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] forward vs backward slashes in msvc build code

On Mon, Apr 27, 2015 at 10:59 AM, Andrew Dunstan and...@dunslane.net wrote:
 Yeah. I've pushed this with tiny fixes to avoid Leaning Toothpick Syndrome
 (man perlop for definition).

I had to Google that.  Then I had to think about it.  Then I laughed.

-- 
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] Selectivity estimation for intarray

Oleg Bartunov obartu...@gmail.com writes:
 Any chance to have this patch in 9.5 ? Many intarray users will be happy.

Considering how desperately behind we are on reviewing/committing patches
that were submitted by the deadline, I don't think it would be appropriate
or fair to add on major new patches that came in months late.  Please add
this to the first 9.6 commitfest, instead.

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] basebackups during ALTER DATABASE ... SET TABLESPACE ... not safe?

2015-04-29 Thread Andres Freund

On 2015-04-28 10:26:54 +0530, Abhijit Menon-Sen wrote:
 Hi Andres. Do you intend to commit these patches? (I just verified that
 they apply without trouble to HEAD.)

I intend to come back to them, yes. I'm not fully sure whether we should
really apply them to the back branches.

Greetings,

Andres Freund


-- 
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] Help needed for PL/Ruby

2015-04-29 Thread Devrim Gündüz


Hi,

Anyone? :)

Regards, Devrim

On Wed, 2015-03-18 at 15:19 +0200, Devrim Gündüz wrote:
 Hi,
 
 Background info first: PL/Ruby was originally maintained by Guy Decoux,
 who passed away in 2008: https://www.ruby-forum.com/topic/166658 . After
 his death, Akinori MUSHA forked the project and maintained it until
 2010: https://github.com/knu/postgresql-plruby . Last release was on Jan
 2010, and recent distros started throwing build errors.
 
 I was having similar build issues while trying to RPMify PL/Ruby, and
 finally stepped up the plate and tried to fix those build issues by
 forking the project:
 
 https://github.com/devrimgunduz/postgresql-plruby/
 
 I mainly applied patches from Fedora, and also did some basic cleanup.
 However, I don't know Ruby and have limited C skills, so I need some
 help on these:
 
 * Complete the extension support: I committed initial infrastructure for
 it, but I don't think it is ready yet.
 
 * Fix the FIXME:
 https://github.com/devrimgunduz/postgresql-plruby/blob/master/src/plpl.c#L844
 
 * Documentation review and update: The recent example is against
 PostgreSQL 8.0. A recent Ruby example would be good, and also we need
 updates for creating the language.
 
 Any contributions are welcome. I recently released 0.5.5 that at least
 is ready for testing.
 
 I want to remind that I am not a Ruby guy, so this is really a community
 stuff for me.
 
 Thanks by now.
 
 Regards,


-- 
Devrim GÜNDÜZ
Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com
PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR



signature.asc
Description: This is a digitally signed message part

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

2015-04-29 Thread David Steele

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, passed
Spec compliant:   tested, passed
Documentation:tested, passed

Looks good - ready for committer.

The new status of this patch is: Ready for Committer


-- 
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] Missing psql tab-completion for ALTER FOREIGN TABLE

On Mon, Apr 27, 2015 at 2:50 AM, Etsuro Fujita
fujita.ets...@lab.ntt.co.jp wrote:
 Here is a patch to add missing tab-completion for ALTER FOREIGN TABLE.
 I'll add this to the next CF.

Committed, thanks.

-- 
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] Can pg_dump make use of CURRENT/SESSION_USER

Fabrízio de Royes Mello wrote:
 
 Looking at the patch again I realize the code is very ugly, so I'll rework
 the patch.

Yes, I think get_object_address should figure out what to do with the
representation of CURRENT DATABASE directly, rather than having the
COMMENT code morph from that into a list containing the name of the
current database.  Please see about changing SECURITY LABEL at the same
time, if only to make sure that the representation is sane.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] FIX : teach expression walker about RestrictInfo

Tomas Vondra tomas.von...@2ndquadrant.com writes:
 On 04/29/15 05:55, Tom Lane wrote:
 pull_varnos is not, and should not be, applied to a RestrictInfo; for one
 thing, it'd be redundant with work that was already done when creating the
 RestrictInfo (cf make_restrictinfo_internal).  You've not shown the
 context of your problem very fully, but in general most code in the planner
 should be well aware of whether it is working with a RestrictInfo (or list
 of same) or a bare expression.  I don't believe that it's a very good idea
 to obscure that distinction.

 OK, let me explain the context a bit more. When working on the 
 multivariate statistics patch, I need to choose which stats to use for 
 estimating the clauses. I do that in clauselist_selectivity(), although 
 there might be other places where to do that.

Hm, well, clauselist_selectivity and clause_selectivity already contain
extensive special-casing for RestrictInfos, so I'm not sure that I see why
you're having difficulty working within that structure for this change.

But there are basic reasons why expression_tree_walker should not try to
deal with RestrictInfos; the most obvious one being that it's not clear
whether it should descend into both the basic and OR-clause subtrees.
Also, if a node has expression_tree_walker support then it should
logically have expression_tree_mutator support as well, but that's
got multiple issues.  RestrictInfos are not supposed to be copied
once created; and the mutator couldn't detect whether their derived
fields are still valid.

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] FIX : teach expression walker about RestrictInfo


On 04/29/15 18:26, Tom Lane wrote:

Tomas Vondra tomas.von...@2ndquadrant.com writes:

...

OK, let me explain the context a bit more. When working on the
multivariate statistics patch, I need to choose which stats to use for
estimating the clauses. I do that in clauselist_selectivity(), although
there might be other places where to do that.


Hm, well, clauselist_selectivity and clause_selectivity already contain
extensive special-casing for RestrictInfos, so I'm not sure that I see why
you're having difficulty working within that structure for this change.


So let's I'm in the clause_selectivity(), and have AND or OR clause to 
estimate. I need to get varnos/varattnos for the clause(s). What should 
I do? I can't call pull_varnos() or pull_varattnos() because there might 
be nested RestrictInfos, as demonstrated by the query I posted.



But there are basic reasons why expression_tree_walker should not try
to deal with RestrictInfos; the most obvious one being that it's not
clear whether it should descend into both the basic and OR-clause
subtrees. Also, if a node has expression_tree_walker support then it
should logically have expression_tree_mutator support as well, but
that's got multiple issues. RestrictInfos are not supposed to be
copied once created; and the mutator couldn't detect whether their
derived fields are still valid.


OK, I do understand that. So what about pull_varnos_walker and 
pull_varattnos_walker - what about teaching them about RestrictInfos?




--
Tomas Vondra   http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] Can pg_dump make use of CURRENT/SESSION_USER

2015-04-29 Thread Fabrízio de Royes Mello

On Wed, Apr 29, 2015 at 1:14 PM, Alvaro Herrera alvhe...@2ndquadrant.com
wrote:

 Fabrízio de Royes Mello wrote:
 
  Looking at the patch again I realize the code is very ugly, so I'll
rework
  the patch.

 Yes, I think get_object_address should figure out what to do with the
 representation of CURRENT DATABASE directly, rather than having the
 COMMENT code morph from that into a list containing the name of the
 current database.  Please see about changing SECURITY LABEL at the same
 time, if only to make sure that the representation is sane.


I have this idea:

1) Add an ObjectAddress field to CommentStmt struct an set it in gram.y

2) In the CommentObject check if CommentStmt-address is
InvalidObjectAddress then call get_object_address else use it

Thoughts?

--
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] Can pg_dump make use of CURRENT/SESSION_USER

Fabrízio de Royes Mello wrote:

 I have this idea:
 
 1) Add an ObjectAddress field to CommentStmt struct an set it in gram.y
 
 2) In the CommentObject check if CommentStmt-address is
 InvalidObjectAddress then call get_object_address else use it

For DDL deparsing purposes, it seems important that the affected object
address can be reproduced somehow.  I think pg_get_object_address()
should be considered, as well as the object_address test.

If we do as you propose, we would have to deparse
COMMENT ON CURRENT DATABASE IS 'foo'
as
  COMMENT ON DATABASE whatever_the_name_is IS 'foo'

which is not a fatal objection but doesn't seem all that great.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] Feedback on getting rid of VACUUM FULL

On Tue, Apr 28, 2015 at 2:44 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
  I think what we need here is something that does heap_update to tuples
  at the end of the table, moving them to earlier pages; then wait for old
  snapshots to die (the infrastructure for which we have now, thanks to
  CREATE INDEX CONCURRENTLY); then truncate the empty pages.  Of course,
  there are lots of details to resolve.  It doesn't really matter that
  this runs for long: a process doing this for hours might be better than
  AccessExclusiveLock on the table for a much shorter period.

 Why do you need to do anything other than update the tuples and let
 autovacuum clean up the mess?

 Sure, that's one option.  I think autovac's current approach is too
 heavyweight: it always has to scan the whole relation and all the
 indexes.  It might be more convenient to do something more
 fine-grained; for instance, maybe instead of scanning the whole
 relation, start from the end of the relation walking backwards and stop
 once the first page containing a live or recently-dead tuple is found.
 Perhaps, while scanning the indexes you know that all CTIDs with pages
 higher than some threshold value are gone; you can remove them without
 scanning the heap at all perhaps.

I agree that scanning all of the indexes is awfully heavy-weight, but
I don't see how we're going to get around that.  The problem with
index vac is not that it's expensive to decide which CTIDs need to get
killed, but that we have to search for them in every page of the
index.  Unfortunately, I have no idea how to get around that.  The
only alternative approach is to regenerate the index tuples we expect
to find based on the heap tuples we're killing and search the index
for them one at a time.  Tom's been opposed to that in the past, but
maybe it's worth reconsidering.

-- 
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] Missing importing option of postgres_fdw

On Mon, Apr 27, 2015 at 7:47 AM, Michael Paquier
michael.paqu...@gmail.com wrote:
 Authorizing ALTER FOREIGN TABLE as query string that a FDW can use
 with IMPORT FOREIGN SCHEMA is a different feature than what is
 proposed in this patch, aka an option for postgres_fdw and meritates a
 discussion on its own because it impacts all the FDWs and not only
 postgres_fdw. Now, related to this patch, we could live without
 authorizing ALTER FOREIGN TABLE because CREATE FOREIGN TABLE does
 authorize the definition of CHECK constraints.

I agree.  I don't think there's a huge problem with allowing IMPORT
FOREIGN SCHEMA to return ALTER FOREIGN TABLE statements, but it
doesn't really seem to be necessary.  I don't see why we can't just
declare the CHECK constraints in the CREATE FOREIGN TABLE statement
instead of adding more DDL.

-- 
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] Selectivity estimation for intarray

* Tom Lane (t...@sss.pgh.pa.us) wrote:
 Alexander Korotkov aekorot...@gmail.com writes:
  My proposal is to let ALTER OPERATOR change restrict and join selectivity
  functions of the operator. Also it would be useful to be able to change
  commutator and negator of operator: extension could add commutators and
  negators in further versions. Any thoughts?
 
 I'm pretty dubious about this, because we lack any mechanism for undoing
 parser/planner decisions based on operator properties.  And there's quite
 a lot of stuff that is based on the assumption that operator properties
 will never change.
 
 An example of the pitfalls here is that we can never allow ALTER OPERATOR
 RENAME, because for example if you rename '' to '~~' that will change
 its precedence, and we have no way to fix the parse trees embedded in
 stored views to reflect that.
 
 For the specific cases you mention, perhaps it would be all right if we
 taught plancache.c to blow away *all* cached plans upon seeing any change
 in pg_operator; but that seems like a brute-force solution.

Agreed that it is- but is that really a problem...?  I've not run into
many (any?) systems where pg_operator is getting changed often...  The
worst part would be adding new operators/extensions, but perhaps we
could exclude that specific case from triggering the cache invalidation?

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Reducing tuple overhead

On Mon, Apr 27, 2015 at 5:01 PM, Jim Nasby jim.na...@bluetreble.com wrote:
 The problem with just having the value is that if *anything* changes between
 how you evaluated the value when you created the index tuple and when you
 evaluate it a second time you'll corrupt your index. This is actually an
 incredibly easy problem to have; witness how we allowed indexing
 timestamptz::date until very recently. That was clearly broken, but because
 we never attempted to re-run the index expression to do vacuuming at least
 we never corrupted the index itself.

True.  But I guess what I don't understand is: how big a deal is this,
really?  The uncorrupted index can still return wrong answers to
queries.  The fact that you won't end up with index entries pointing
to completely unrelated tuples is nice, but if index scans are missing
tuples that they should see, aren't you still pretty hosed?

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


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

Re: [HACKERS] Replication identifiers, take 4

2015-04-29 Thread David Rowley

On 25 April 2015 at 00:32, Andres Freund and...@anarazel.de wrote:


 Attached is a patch that does this, and some more, renaming. That was
 more work than I'd imagined.  I've also made the internal naming in
 origin.c more consistent/simpler and did a bunch of other cleanup.


Hi,

It looks like bowerbird is not too happy with this, neither is my build
environment.

http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbirddt=2015-04-29%2019%3A31%3A06

The attached seems to fix it.
I put the include in the origin.h rather than in the origin.c as the
DoNotReplicateId macro uses UINT16_MAX and is also used in xact.c.

Regards

David Rowley


UINT16_MAX_fix.patch
Description: Binary data

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

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

On Mon, Apr 27, 2015 at 5:05 AM, Shigeru HANADA
shigeru.han...@gmail.com wrote:
 Currently INNER JOINs with unsafe join conditions are not pushed down, so 
 such test is not in the suit.  As you say, in theory, INNER JOINs can be 
 pushed down even they have push-down-unsafe join conditions, because such 
 conditions can be evaluated no local side against rows retrieved without 
 those conditions.

I suspect it's worth trying to do the pushdown if there is at least
one safe joinclause.  If there are none, fetching a Cartesian product
figures to be a loser.

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


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

Re: [HACKERS] Replication identifiers, take 4

2015-04-29 Thread Petr Jelinek


On 29/04/15 22:12, David Rowley wrote:

On 25 April 2015 at 00:32, Andres Freund and...@anarazel.de
mailto:and...@anarazel.de wrote:


Attached is a patch that does this, and some more, renaming. That was
more work than I'd imagined.  I've also made the internal naming in
origin.c more consistent/simpler and did a bunch of other cleanup.


Hi,

It looks like bowerbird is not too happy with this, neither is my build
environment.

http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbirddt=2015-04-29%2019%3A31%3A06

The attached seems to fix it.
I put the include in the origin.h rather than in the origin.c as the
DoNotReplicateId macro uses UINT16_MAX and is also used in xact.c.



I think correct fix is using PG_UINT16_MAX.


--
 Petr Jelinek  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] [RFC] sepgsql: prohibit users to relabel objects

2015-04-29 Thread Denis Kirjanov

Enforce access control on security labels defined by admin
and prohibit users to relabel the objects

Signed-off-by: Denis Kirjanov k...@itsirius.su
---
 contrib/sepgsql/label.c |5 +
 1 file changed, 5 insertions(+)

diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c
index ef7661c..470b90e 100644
--- a/contrib/sepgsql/label.c
+++ b/contrib/sepgsql/label.c
@@ -504,6 +504,11 @@ sepgsql_object_relabel(const ObjectAddress *object, const 
char *seclabel)
(errcode(ERRCODE_INVALID_NAME),
   errmsg(SELinux: invalid security label: \%s\, 
seclabel)));
 
+   if (!superuser())
+   ereport(ERROR,
+   (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg(SELinux: must be superuser to relabel objects)));
+
/*
 * Do actual permission checks for each object classes
 */
-- 
1.7.10.4



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

Re: [HACKERS] Replication identifiers, take 4

On Tue, Apr 28, 2015 at 10:00 PM, Peter Eisentraut pete...@gmx.net wrote:
 On 4/24/15 4:29 PM, Andres Freund wrote:
 Shouldn't this be backed up by pg_dump(all?)?

 Given it deals with LSNs and is, quite fundamentally due to concurrency, non 
 transactional, I doubt it's worth it. The other side's slots also aren't 
 going to be backed up as pg dump obviously can't know about then. So the 
 represented data won't make much sense.

 I agree it might not be the best match.  But we should consider that we
 used to say, a backup by pg_dumpall plus configuration files is a
 complete backup.  Now we have replication slots and possibly replication
 identifiers and maybe similar features in the future that are not
 covered by this backup method.

That's true.  But if you did backup the replication slots with
pg_dump, and then you restored them as part of restoring the dump,
your replication setup would be just as broken as if you had never
backed up those replication slots at all.

I think the problem here is that replication slots are part of
*cluster* configuration, not individual node configuration.  If you
back up a set of nodes that make up a cluster, and then restore them,
you might hope that you will end up with working slots established
between the same pairs of machines that had working slots between them
before.  But I don't see a way to make that happen when you look at it
from the point of view of backing up and restoring just one node.  As
we get better clustering facilities into core, we may develop more
instances of this problem; the best way of solving it is not clear to
me.

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


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

Re: [HACKERS] mogrify and indent features for jsonb

2015-04-29 Thread Kevin Grittner

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

 It's a matter of taste, but I find things a lot easier to
 understand when they are symmetrical. Thus I like all the
 branches of an if to be either in a block or not, and I like
 braces to line up either horizontally or vertically. Perhaps this
 reflects my history, where I wrote huge amounts of Ada and other
 non-C-like languages, well before I ever wrote lots of C or C-ish
 languages.

 Another case where I think putting a single statement in a block
 makes sense is where the condition of the if spreads across
 more than one line. This works particularly well with our BSD
 style brace placement.

My personal preferences are the same on all of that, especially
that the closing paren, brace, or bracket should be either in the
same line or the same column as its mate.  If we were going to open
a green-field discussion about what style to *choose* I would be
arguing for all of the above (plus a few other things which are not
current PostgreSQL style).

That said, I feel very strongly that it is important that everyone
use the *same* style.  It is far more important to me that we stick
to a single style than that the style match my personal
preferences.  The project style seems to me to be that a single
statement is not put into braces unless needed for correctness or
to prevent warnings about ambiguity from the compilers.

By the way, my preference for the above are not strong enough to
want to open up the style choices to re-evaluation.  PLEASE, no!

--
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] mogrify and indent features for jsonb

2015-04-29 Thread Jan de Visser

On April 29, 2015 03:09:51 PM Andrew Dunstan wrote:
 On 04/29/2015 01:19 PM, Robert Haas wrote:
  On Mon, Apr 27, 2015 at 6:41 PM, Andrew Dunstan and...@dunslane.net 
wrote:
  There's one exception I, at least, have to this rule, namely when there's
  a
  corresponding compound if or else. I personally find this unaesthetic to
  put 
  it mildly:
  if (condition)
  
  statement;
  
  else
  {
  
  block of statements;
  
  }
  
  Hmm, I don't dislike that style.  If somebody submitted a patch with
  braces around the lone statement, I would remove them before
  committing.
  
  ducks
 
 It's a matter of taste, but I find things a lot easier to understand
 when they are symmetrical. Thus I like all the branches of an if to be
 either in a block or not, and I like braces to line up either
 horizontally or vertically. Perhaps this reflects my history, where I
 wrote huge amounts of Ada and other non-C-like languages, well before I
 ever wrote lots of C or C-ish languages.
 
 
 Another case where I think putting a single statement in a block makes
 sense is where the condition of the if spreads across more than one
 line. This works particularly well with our BSD style brace placement.

I'm sure that many, many bits have been spilled over this, reaching way back 
into the stone age of computing, sometimes almost reaching emacs-vs-vi levels 
of intensity.

My position is the better-safe-than-sorry corner, which says to always add 
braces, even if there's only one statement. Because one day somebody will be 
in a rush, and will add a second statement without adding the braces, and 
things will explode horribly.

But that's just me.

jan



-- 
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] [RFC] sepgsql: prohibit users to relabel objects

2015-04-29 Thread Adam Brightwell


 Really?  Why?  I would think it's the policy's job to restrict relabel
 operations.


I agree.  This seems like an unnecessary change.

-Adam

-- 
Adam Brightwell - adam.brightw...@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com

Re: [HACKERS] [PATCH] Add transforms feature

On Tue, Apr 28, 2015 at 12:47 PM, Jeff Janes jeff.ja...@gmail.com wrote:
 This commit is causing a compiler warning for me in non-cassert builds:

 funcapi.c: In function 'get_func_trftypes':
 funcapi.c:890: warning: unused variable 'procStruct'

 Adding PG_USED_FOR_ASSERTS_ONLY seems to fix it.

I took a stab at fixing this via a slightly different method.  Let me
know whether that worked.

Thanks,

-- 
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] mogrify and indent features for jsonb

On Mon, Apr 27, 2015 at 6:41 PM, Andrew Dunstan and...@dunslane.net wrote:
 There's one exception I, at least, have to this rule, namely when there's a
 corresponding compound if or else. I personally find this unaesthetic to put
 it mildly:

if (condition)
statement;
else
{
block of statements;
}

Hmm, I don't dislike that style.  If somebody submitted a patch with
braces around the lone statement, I would remove them before
committing.

ducks

-- 
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] Feedback on getting rid of VACUUM FULL

2015-04-29 Thread Jeff Janes

On Tue, Apr 28, 2015 at 11:32 AM, Robert Haas robertmh...@gmail.com wrote:

 On Fri, Apr 24, 2015 at 3:04 PM, Alvaro Herrera
 alvhe...@2ndquadrant.com wrote:

  I think what we need here is something that does heap_update to tuples
  at the end of the table, moving them to earlier pages; then wait for old
  snapshots to die (the infrastructure for which we have now, thanks to
  CREATE INDEX CONCURRENTLY); then truncate the empty pages.  Of course,
  there are lots of details to resolve.  It doesn't really matter that
  this runs for long: a process doing this for hours might be better than
  AccessExclusiveLock on the table for a much shorter period.

 Why do you need to do anything other than update the tuples and let
 autovacuum clean up the mess?


It could take a long time before autovacuum kicked in and did so.  I think
a lot of time when people need this, the lack of space in the file system
is blocking some other action they want to do, so they want a definitive
answer as to when the deed is done rather than manually polling the file
system with df.  You could invoke vacuum manually rather than waiting for
autovacuum, but it would kind of suck to do that only to find out you
didn't wait long enough for all the snapshots to go away and so no space
was actually released--and I don't think we have good ways of finding out
how long is long enough.  Ways of squeezing tables in the background would
be nice, but so would a way of doing it in the foreground and getting a
message when it is complete.

Cheers,

Jeff

Re: [HACKERS] [RFC] sepgsql: prohibit users to relabel objects

On Wed, Apr 29, 2015 at 9:15 AM, Denis Kirjanov k...@linux-powerpc.org wrote:
 Enforce access control on security labels defined by admin
 and prohibit users to relabel the objects

Really?  Why?  I would think it's the policy's job to restrict relabel
operations.

-- 
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] cache invalidation for PL/pgsql functions

On Tue, Apr 28, 2015 at 3:59 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 rhaas=# create table foo (a int);
 CREATE TABLE
 rhaas=# create or replace function test (x foo) returns int as $$begin
 return x.b; end$$ language plpgsql;
 CREATE FUNCTION
 rhaas=# alter table foo add column b int;
 ALTER TABLE
 rhaas=# select test(null::foo);
 ERROR:  record x has no field b
 LINE 1: SELECT x.b
^
 QUERY:  SELECT x.b
 CONTEXT:  PL/pgSQL function test(foo) line 1 at RETURN

 I believe that this was one of the cases I had in mind when I previously
 proposed that we stop using PLPGSQL_DTYPE_ROW entirely for composite-type
 variables, and make them use PLPGSQL_DTYPE_REC (that is, the same code
 paths used for RECORD).

 As I recall, that proposal was shot down with no investigation whatsoever,
 on the grounds that it might possibly make some cases slower.

I don't know whether that would help or not.  I was thinking about
whether PLs should be using CacheRegisterSyscacheCallback() to notice
when types that they care about change.

-- 
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] mogrify and indent features for jsonb

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
 Robert Haas wrote:
  On Mon, Apr 27, 2015 at 6:41 PM, Andrew Dunstan and...@dunslane.net wrote:
   There's one exception I, at least, have to this rule, namely when there's 
   a
   corresponding compound if or else. I personally find this unaesthetic to 
   put
   it mildly:
  
  if (condition)
  statement;
  else
  {
  block of statements;
  }
  
  Hmm, I don't dislike that style.  If somebody submitted a patch with
  braces around the lone statement, I would remove them before
  committing.
 
 Same here.

Agreed.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] mogrify and indent features for jsonb

Robert Haas wrote:
 On Mon, Apr 27, 2015 at 6:41 PM, Andrew Dunstan and...@dunslane.net wrote:
  There's one exception I, at least, have to this rule, namely when there's a
  corresponding compound if or else. I personally find this unaesthetic to put
  it mildly:
 
 if (condition)
 statement;
 else
 {
 block of statements;
 }
 
 Hmm, I don't dislike that style.  If somebody submitted a patch with
 braces around the lone statement, I would remove them before
 committing.

Same here.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] Additional role attributes superuser review

On Wed, Apr 29, 2015 at 10:47 AM, Stephen Frost sfr...@snowman.net wrote:
 Here is the latest revision of this patch.

I think this patch is too big and does too many things.  It should be
broken up into small patches which can be discussed and validated
independently.  The fact that your commit message is incredibly long
is a sign that there's too much going on here, and that message
doesn't even cover all of it.

It seems to me that the core change here is really to pg_dump.  You're
adding the ability for pg_dump to dump and restore privileges on
objects in pg_dump.  That capability is a complete - and useful -
feature in its own right, with no other changes.

Then the next thing you've got here is a series of patches that change
the rights to execute various utility functions.  Some of those, like
the changes to pg_stop_backup(), are pretty much a slam-dunk, because
they don't really change user-visible behavior; they just give the DBA
more options.  Others, like the changes to replication permissions,
are likely to be more controversial.  You should split the stuff
that's a slam-dunk apart from the stuff that makes policy decisions,
and plan to commit the former changes first, and the latter changes
only if and when there is very clear agreement on the specific
policies to be changed.

Finally, you've got the idea of making pg_ a reserved prefix for
roles, adding some predefined roles, and giving them some predefined
privileges.  That should be yet another patch.

I think that if you commit this the way you have it today, everybody
will go, oh, look, Stephen committed something, but it looks
complicated, I won't pay attention.  And then, six months from now
when we're in beta, or maybe after final, people will start looking at
this and realizing that there are parts of it they hate, but it will
be hard to fix at that point.  Breaking it out will hopefully allow
more discussion on the individual features of in here, of which there
are probably at least four.  It will also make it easier to revert
part of it rather than all of it if that turns out to be needed.

-- 
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] alternative compression algorithms?

On Mon, Apr 20, 2015 at 9:03 AM, Tomas Vondra
tomas.von...@2ndquadrant.com wrote:
 Sure, it's not an ultimate solution, but it might help a bit. I do have
 other ideas how to optimize this, but in the planner every milisecond
 counts. Looking at 'perf top' and seeing pglz_decompress() in top 3.

I suggested years ago that we should not compress data in
pg_statistic.  Tom shot that down, but I don't understand why.  It
seems to me that when we know data is extremely frequently accessed,
storing it uncompressed makes sense.

-- 
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: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

On Sun, Apr 26, 2015 at 10:00 PM, Kouhei Kaigai kai...@ak.jp.nec.com wrote:
 The attached patch v13 is revised one according to the suggestion
 by Robert.

Thanks.

The last hunk in foreign.c is a useless whitespace change.

+   /* actually, not shift members */

Change to: shift of 0 is the same as copying

But actually, do we really need all of this?  I think you could reduce
the size of this function to three lines of code if you just did this:

x = -1;
while ((x = bms_next_member(inputset, x)) = 0)
outputset = bms_add_member(inputset, x + shift);

It might be very slightly slower, but I think it would be worth it to
reduce the amount of code needed.

+* 5. Consider paths added by FDW, in case when both of outer and
+* inner relations are managed by the same driver.

Change to: If both inner and outer relations are managed by the same
FDW, give it a chance to push down joins.

+* 6. At the last, consider paths added by extension, in addition to the
+* built-in paths.

Change to: Finally, give extensions a chance to manipulate the path list.

+* Fetch relation-id, if this foreign-scan node actuall scans on
+* a particular real relation. Elsewhere, InvalidOid shall be
+* informed to the FDW driver.

Change to: If we're scanning a base relation, look up the OID.  (We
can skip this if scanning a join relation.)

+* Sanity check. Pseudo scan tuple-descriptor shall be constructed
+* based on the fdw_ps_tlist, excluding resjunk=true, so we need to
+* ensure all valid TLEs have to locate prior to junk ones.

Is the goal here to make attribute numbers match up?  If so, between
where and where?  If not, please explain further.

+   if (splan-scan.scanrelid == 0)
+   {
...
+   }
splan-scan.scanrelid += rtoffset;

Does this need an else?  It seems surprising that you would offset
scanrelid even if it's starting out as zero.

(Note that there are two instances of this pattern.)

+ * 'found' : indicates whether RelOptInfo is actually constructed.
+ * true, if it was already built and on the cache.

Leftover hunk.  Revert this.

+typedef void (*GetForeignJoinPaths_function ) (PlannerInfo *root,

Whitespace is wrong, still.

+ * An optional fdw_ps_tlist is used to map a reference to an attribute of
+ * underlying relation(s) on a pair of INDEX_VAR and alternative varattno.

on - onto

+ * It looks like a scan on pseudo relation that is usually result of
+ * relations join on remote data source, and FDW driver is responsible to
+ * set expected target list for this.

Change to: When fdw_ps_tlist is used, this represents a remote join,
and the FDW driver is responsible for setting this field to an
appropriate value.

If FDW returns records as foreign-
+ * table definition, just put NIL here.

I think this is just referring to the non-join case; if so, just drop
it.  Otherwise, I'm confused and need a further explanation.

+ *  Note that since Plan trees can be copied, custom scan providers *must*

Extra space before Note

+   Bitmapset  *custom_relids;  /* set of relid (index of range-tables)
+*
represented by this node */

Maybe RTIs this node generates?

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


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

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

On Fri, Apr 24, 2015 at 2:40 PM, David Steele da...@pgmasters.net wrote:
   para
The view structnamepg_file_settings/structname provides access to
 run-time parameters
that are defined in configuration files via SQL.  In contrast to
structnamepg_settings/structname a row is provided for each
 occurrence
of the parameter in a configuration file.  This is helpful for
 discovering why one value
may have been used in preference to another when the parameters were
 loaded.
   /para

This seems to imply that this gives information about only a subset of
configuration files; specifically, those auto-generated based on SQL
commands - i.e. postgresql.conf.auto.  But I think it's supposed to
give information about all configuration files, regardless of how
generated.  Am I wrong?  If not, I'd suggest run-time parameters that
are defined in configuration files via SQL - run-time parameters
stored in configuration files.

-- 
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] patch for xidin

On Fri, Apr 17, 2015 at 10:27 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 The patch will correct it. I have justly copy some code of 'OID'. Whether we 
 need to extract the common code?

 This seems like an awful lot of code to solve a problem that will never
 occur in practice.

It does seem like an awful lot of code.  We should be able to come up
with something shorter.  But the bug report is legitimate.  It's not
too much to ask that data types sanity check their inputs.

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


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

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

2015-04-29 Thread David Steele

On 4/29/15 5:16 PM, Robert Haas wrote:
 On Fri, Apr 24, 2015 at 2:40 PM, David Steele da...@pgmasters.net wrote:
   para
The view structnamepg_file_settings/structname provides access to
 run-time parameters
that are defined in configuration files via SQL.  In contrast to
structnamepg_settings/structname a row is provided for each
 occurrence
of the parameter in a configuration file.  This is helpful for
 discovering why one value
may have been used in preference to another when the parameters were
 loaded.
   /para
 
 This seems to imply that this gives information about only a subset of
 configuration files; specifically, those auto-generated based on SQL
 commands - i.e. postgresql.conf.auto.  But I think it's supposed to
 give information about all configuration files, regardless of how
 generated.  Am I wrong?  If not, I'd suggest run-time parameters that
 are defined in configuration files via SQL - run-time parameters
 stored in configuration files.

The view does give information about all configuration files regardless
of how they were created.

That's what I intended the text to say but I think your phrasing is clearer.

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



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] [COMMITTERS] pgsql: Add transforms feature

2015-04-29 Thread Michael Paquier

On Thu, Apr 30, 2015 at 3:30 AM, Andrew Dunstan and...@dunslane.net wrote:

 On 04/28/2015 04:10 PM, Andrew Dunstan wrote:


 On 04/28/2015 12:03 PM, Andrew Dunstan wrote:


 [switching to -hackers]

 On 04/28/2015 11:51 AM, Andrew Dunstan wrote:


 On 04/28/2015 09:04 AM, Michael Paquier wrote:

 On Tue, Apr 28, 2015 at 10:01 AM, Michael Paquier
 michael.paqu...@gmail.com wrote:

 On Tue, Apr 28, 2015 at 9:55 AM, Andrew Dunstan wrote:

 w.r.t MSVC builds, it looks like we need entries in
 $contrib_extraincludes
 in src/tools/msvc/Mkvcbuild.pm at the very least.

 If our goal is to put back to green the Windows nodes as quick as
 possible, we could bypass their build this way , and we'll
 additionally need to update the install script and
 vcregress.pl/contribcheck to bypass those modules accordingly. Now I
 don't think that we should make the things produced inconsistent.

 OK, attached are two patches to put back the buildfarm nodes using MSVC
 to green
 - 0001 adds support for build and installation of the new transform
 modules: hstore_plperl, hstore_plpython and  ltree_plpython. Note that
 this patch is enough to put back the buildfarm to a green state for
 MSVC, but it disables the regression tests for those modules,
 something addressed in the next patch...
 - 0002 adds support for regression tests for the new modules. The
 thing is that we need to check the major version version of Python
 available in configuration and choose what are the extensions to
 preload before the tests run. It is a little bit hacky... But it has
 the merit to work, and I am not sure we could have a cleaner solution
 by looking at the Makefiles of the transform modules that use
 currently conditions based on $(python_majorversion).
 Regards,



 I have pushed the first of these with some tweaks.

 I'm looking at the second.




 Regarding this second patch - apart from the buggy python version test
 which I just fixed in the first patch, I see this:

+   if ($pyver eq 2)
+   {
+   push @opts, --load-extension=plpythonu;
+   push @opts, '--load-extension=' . $module . 'u';
+   }


 But what do we do for Python3?

 Do we actually have a Windows machine building with Python3?



 The answer seems to be probably not. When I tried enabling this with
 bowerbird I got a ton of errors like these:

plpy_cursorobject.obj : error LNK2001: unresolved external symbol
PyObject_SelfIter [C:\prog\bf\root\HEAD\pgsql\plpython3.vcxproj]
plpy_cursorobject.obj : error LNK2019: unresolved external symbol
__imp_PyType_Ready referenced in function PLy_cursor_init_type
[C:\prog\bf\root\HEAD\pgsql\plpython3.vcxproj]


 Something else to fix I guess.



 OK, I fixed this as Michael suggested by installing the 64 bit python3 (it's
 a pity that python.org didn't offer that to me as a download on their front
 page - that's a bit ugly).

 However, when it came to running these tests that was a miserable failure.
 And indeed, we don't run any regression tests for plpython3 on MSVC.

I arrived to the same conclusion. Perhaps we shall write a TODO in the
code or on the wiki to not forget about that..

 So I committed this with just python2 tests enabled. All the buildfarm MSVC
 hosts seem to be using python2 anyway.

OK, thanks.

 Meanwhile, we have some work to do on the mingw/gcc side.

 These changes help make some progress - they let compilation succeed for
 hstore_plperl but I still get linkage failures. I suspect we need some
 things marked for export that we haven't to be marked needed before.

 [...]

I'll try to have a look at that a bit if possible...
Regards,
-- 
Michael


-- 
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 syntax issues

2015-04-29 Thread Andres Freund

On 2015-04-29 15:31:59 -0400, Robert Haas wrote:
 On Wed, Apr 29, 2015 at 3:13 PM, Stephen Frost sfr...@snowman.net wrote:
  I still think that constraints should never be named in the syntax.
 
  I guess I don't see a particular problem with that..?  Perhaps I'm
  missing something, but if there's multiple ways for something to
  conflict, it might be nice to be able to differentiate between them?
  Then again, I'm not sure if that's what the intent here is.
 
 So, with unique indexes, people can create an index concurrently, then
 drop the old index concurrently, and nothing breaks.  I don't think we
 have a similar capacity for constraints at the moment, but we should.
 When somebody does that dance, the object names change, but all of the
 DML keeps working.  That's a property I'd like to preserve.

On the other hand it's way more convenient to specify a single
constraint name than several columns and a predicate. I'm pretty sure
there's situations where I a) rather live with a smaller chance of error
during a replacement of the constraint b) if we get concurrently
replaceable constraints the naming should be doable too.

I don't see your argument strong enough to argue against allowing this
*as an alternative*.

Greetings,

Andres Freund


-- 
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] Minor improvement to config.sgml

On Thu, Apr 16, 2015 at 3:30 AM, Etsuro Fujita
fujita.ets...@lab.ntt.co.jp wrote:
 Attached is a small patch to mark up on with literal in
 doc/src/sgml/config.sgml.

Committed.

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


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

[HACKERS] Incompatible trig error handling

2015-04-29 Thread John Gorman

Two of the trigonometry functions have differing error condition behavior
between Linux and OSX. The Linux behavior follows the standard set by the
other trig functions.

On Linux:

SELECT asin(2);
 ERROR:  input is out of range



SELECT acos(2);
 ERROR:  input is out of range


On OSX:

SELECT asin(2);
  asin
 --
   NaN
 (1 row)



SELECT asin(2);
  asin
 --
   NaN
 (1 row)


The attached patch brings OSX into line with the expected behaviour and the
additional regression tests verify this.

Is this worth fixing and if so what is the next step?

Best, John


trig-v1.patch
Description: Binary data

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

Re: [HACKERS] [PATCH] libpq: Allow specifying multiple host names to try to connect to

On Sun, Apr 19, 2015 at 11:18 AM, Mikko Tiihonen
mikko.tiiho...@nitorcreations.com wrote:
 I would like allow specifying multiple host names for libpq to try to
 connecting to. This is currently only supported if the host name resolves to
 multiple addresses. Having the support for it without complex dns setup
 would be much easier.

 Example:

 psql -h dbslave,dbmaster -p 5432 dbname

 psql 'postgresql://dbslave,dbmaster:5432/dbname'


 Here the idea is that without any added complexity of pgbouncer or similar
 tool I can get any libpq client to try connecting to multiple nodes until
 one answers. I have added the similar functionality to the jdbc driver few
 years ago.

I'm not sure if this exact idea is what we want to do, but I like the
concept, and I think a lot of users would find it handy.

-- 
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] Reducing tuple overhead

2015-04-29 Thread Jim Nasby


On 4/29/15 12:18 PM, Robert Haas wrote:

On Mon, Apr 27, 2015 at 5:01 PM, Jim Nasby jim.na...@bluetreble.com wrote:

The problem with just having the value is that if *anything* changes between
how you evaluated the value when you created the index tuple and when you
evaluate it a second time you'll corrupt your index. This is actually an
incredibly easy problem to have; witness how we allowed indexing
timestamptz::date until very recently. That was clearly broken, but because
we never attempted to re-run the index expression to do vacuuming at least
we never corrupted the index itself.


True.  But I guess what I don't understand is: how big a deal is this,
really?  The uncorrupted index can still return wrong answers to
queries.  The fact that you won't end up with index entries pointing
to completely unrelated tuples is nice, but if index scans are missing
tuples that they should see, aren't you still pretty hosed?


Maybe, maybe not. You could argue it's better to miss some rows than 
have completely unrelated ones.


My recollection is there's other scenarios where this causes problems, 
but that's from several years ago and I wasn't able to find anything on 
a quick search of archives. I've wondered the same in the past and Tom 
had reasons it was bad, but perhaps they're overstated.

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


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

Re: [HACKERS] alternative compression algorithms?

2015-04-29 Thread Petr Jelinek


On 30/04/15 00:44, Tom Lane wrote:

Robert Haas robertmh...@gmail.com writes:

On Mon, Apr 20, 2015 at 9:03 AM, Tomas Vondra
tomas.von...@2ndquadrant.com wrote:

Sure, it's not an ultimate solution, but it might help a bit. I do have
other ideas how to optimize this, but in the planner every milisecond
counts. Looking at 'perf top' and seeing pglz_decompress() in top 3.



I suggested years ago that we should not compress data in
pg_statistic.  Tom shot that down, but I don't understand why.  It
seems to me that when we know data is extremely frequently accessed,
storing it uncompressed makes sense.


I've not been following this thread, but I do not think your argument here
holds any water.  pg_statistic entries are generally fetched via the
syscaches, and we fixed things years ago so that toasted tuple entries
are detoasted before insertion in syscache.  So I don't believe that
preventing on-disk compression would make for any significant improvement,
at least not after the first reference within a session.


AFAICS the syscache tuples are detoasted but not decompressed before 
insertion to syscache (catcache calls toast_flatten_tuple which doesn't 
do decompression) so the pglz_decompress is still called every time.


--
 Petr Jelinek  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] alternative compression algorithms?

Robert Haas robertmh...@gmail.com writes:
 On Mon, Apr 20, 2015 at 9:03 AM, Tomas Vondra
 tomas.von...@2ndquadrant.com wrote:
 Sure, it's not an ultimate solution, but it might help a bit. I do have
 other ideas how to optimize this, but in the planner every milisecond
 counts. Looking at 'perf top' and seeing pglz_decompress() in top 3.

 I suggested years ago that we should not compress data in
 pg_statistic.  Tom shot that down, but I don't understand why.  It
 seems to me that when we know data is extremely frequently accessed,
 storing it uncompressed makes sense.

I've not been following this thread, but I do not think your argument here
holds any water.  pg_statistic entries are generally fetched via the
syscaches, and we fixed things years ago so that toasted tuple entries
are detoasted before insertion in syscache.  So I don't believe that
preventing on-disk compression would make for any significant improvement,
at least not after the first reference within a session.

Also, it's a very long way from some pg_statistic entries are frequently
accessed to all pg_statistic entries are frequently accessed.

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] alternative compression algorithms?


Hi,

On 04/29/15 23:54, Robert Haas wrote:

On Mon, Apr 20, 2015 at 9:03 AM, Tomas Vondra
tomas.von...@2ndquadrant.com wrote:

Sure, it's not an ultimate solution, but it might help a bit. I do have
other ideas how to optimize this, but in the planner every milisecond
counts. Looking at 'perf top' and seeing pglz_decompress() in top 3.


I suggested years ago that we should not compress data in
pg_statistic.  Tom shot that down, but I don't understand why.  It
seems to me that when we know data is extremely frequently accessed,
storing it uncompressed makes sense.


I'm not convinced not compressing the data is a good idea - it suspect 
it would only move the time to TOAST, increase memory pressure (in 
general and in shared buffers). But I think that using a more efficient 
compression algorithm would help a lot.


For example, when profiling the multivariate stats patch (with multiple 
quite large histograms), the pglz_decompress is #1 in the profile, 
occupying more than 30% of the time. After replacing it with the lz4, 
the data are bit larger, but it drops to ~0.25% in the profile and 
planning the drops proportionally.


It's not a silver bullet, but it would help a lot in those cases.


--
Tomas Vondra  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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 syntax issues

2015-04-29 Thread Peter Geoghegan

On Wed, Apr 29, 2015 at 4:09 PM, Simon Riggs si...@2ndquadrant.com wrote:
I dislike the way that ignoring objections for a period leads them to be
potentially discarded. I'd prefer to think that as a community we are able
to listen to people even when they aren't continually present to reinforce
the original objection(s).

What objection was ignored? Because, I looked just now, and the only
thing I could find of substance that you said on the MySQL syntax [1]
seemed pretty lukewarm. You mostly argued for MERGE.

Not supporting MySQL syntax will seem like a bizarre choice to people
watching this from a distance. I accept that the patch implements useful
behaviour that MySQL does not implement and we thus provide enhanced syntax,
but the default should be match on PK using the MySQL syntax.

Does it?

We can't use the MERGE syntax, because this isn't MERGE. Everything
else UPSERT-like some new and distinct custom syntax, for various
reasons.

Note that the syntax is quite similar to the SQLite
syntax of the same feature, that has ON CONFLICT IGNORE (it also has
ON CONFLICT REPLACE, but not ON CONFLICT UPDATE).

Why are we not also supporting ON CONFLICT REPLACE and IGNORE then?

The point I was trying to make was that CONFLICT also appears as a
more general term than duplicate key or whatever.

If we are using syntax from other products then it should be identical
syntax, or the argument to use it doesn't stand.

I was making a narrow point about the keyword CONFLICT. Nothing more.

We must think about what SQL Standard people are likely to say and do. If we
act as independently, our thought may be ignored. If we act in support of
other previous implementations we may draw support to adopt that as the
standard. Whatever the standard says we will eventually support, so we
should be acting with an eye to that future.

UPSERT seems like exactly the kind of thing that the SQL standard does
not concern itself with. For example, I have a unique index inference
specification. The SQL standard does not have anything to say about
indexes. I would be extremely surprised if the SQL standard adopted
MySQL's UPSERT thing. They omit the SET on the UPDATE, probably to
fix some parser ambiguity issue. While there are some similarities to
what I have here, it's a bit shoddy.

I have requirements coming out of my ears for this patch, Simon. I
think it's odd that you're taking umbrage because I supposedly ignored
something you said 6 months ago.

[1]
http://www.postgresql.org/message-id/CA+U5nMK-efLg00FhCWk=asbet_77iss87egdsptq0ukzqdr...@mail.gmail.com
--
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] Additional role attributes superuser review

Robert Haas wrote:

 I think that if you commit this the way you have it today, everybody
 will go, oh, look, Stephen committed something, but it looks
 complicated, I won't pay attention.

Yeah, that sucks.

 Finally, you've got the idea of making pg_ a reserved prefix for
 roles, adding some predefined roles, and giving them some predefined
 privileges.  That should be yet another patch.

On this part I have a bit of a problem -- the prefix is not really
reserved, is it.  I mean, evidently it's still possible to create roles
with the pg_ prefix ... otherwise, how come the new lines to
system_views.sql that create the predefined roles work in the first
place?  I think if we're going to reserve role names, we should reserve
them for real: CREATE ROLE should flat out reject creation of such
roles, and the default ones should be created during bootstrap.

IMO anyway.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, 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] mogrify and indent features for jsonb

2015-04-29 Thread Petr Jelinek


On 27/04/15 18:46, Petr Jelinek wrote:

On 18/04/15 20:35, Dmitry Dolgov wrote:

Sorry for late reply. Here is a slightly improved version of the patch
with the new `h_atoi` function, I hope this implementation will be more
appropriate.



It's better, but a) I don't like the name of the function b) I don't see
why we need the function at all given that it's called from only one
place and is effectively couple lines of code.

In general I wonder if it wouldn't be better to split the replacePath
into 3 functions, one replacePath, one replacePathObject,
replacePathArray and call those Object/Array ones from the replacePath
and inlining the h_atoi code into the Array one. If this was done then
it would make also sense to change the main if/else in replacePath into
a switch.

Another thing I noticed now when reading the code again - you should not
be using braces when you only have one command in the code-block.



Hi,

I worked this over a bit (I hope Dmitry won't mind) and I am now more or 
less happy with the patch. Here is list of changes I made:

- rebased to todays master (Oid conflicts, transforms patch conflicts)
- changed the main if/else if/else if/else to switch in replacePath
- split the replacePath into 3 functions (main one plus 2 helpers for 
Object and Array)

- removed the h_atoi and use the strtol directly
- renamed jsonb_indent to jsonb_pretty because we use pretty for 
similar use-case everywhere else

- fixed whitespace/brackets where needed
- added/reworded some comments and couple of lines in docs

I think it's ready for Andrew now.

--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
From b2da4958fc75e7c7009340e6b2e4ccb155632078 Mon Sep 17 00:00:00 2001
From: Petr Jelinek pjmodos@pjmodos.net
Date: Wed, 29 Apr 2015 22:54:01 +0200
Subject: [PATCH] jsonbxcore5

---
 doc/src/sgml/func.sgml|  62 +++
 src/backend/utils/adt/jsonb.c |  81 +++-
 src/backend/utils/adt/jsonfuncs.c | 709 ++
 src/include/catalog/pg_operator.h |   8 +
 src/include/catalog/pg_proc.h |   9 +-
 src/include/utils/jsonb.h |  19 +-
 src/test/regress/expected/jsonb.out   | 424 +++-
 src/test/regress/expected/jsonb_1.out | 424 +++-
 src/test/regress/sql/jsonb.sql|  85 +++-
 9 files changed, 1805 insertions(+), 16 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index dcade93..f0d8d96 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -10257,6 +10257,32 @@ table2-mapping
 entryDo all of these key/element emphasisstrings/emphasis exist?/entry
 entryliteral'[a, b]'::jsonb ?amp; array['a', 'b']/literal/entry
/row
+   row
+entryliteral||/literal/entry
+entrytypejsonb/type/entry
+entryConcatentate two jsonb values into a new jsonb value/entry
+entryliteral'[a, b]'::jsonb || '[c, d]'::jsonb/literal/entry
+   /row
+   row
+entryliteral-/literal/entry
+entrytypetext/type/entry
+entryDelete the field with a specified key, or element with this
+value/entry
+entryliteral'{a: b}'::jsonb - 'a' /literal/entry
+   /row
+   row
+entryliteral-/literal/entry
+entrytypeinteger/type/entry
+entryDelete the field or element with specified index (Negative
+integers count from the end)/entry
+entryliteral'[a, b]'::jsonb - 1 /literal/entry
+   /row
+   row
+entryliteral-/literal/entry
+entrytypetext[]/type/entry
+entryDelete the field or element with specified path/entry
+entryliteral'[a, {b:1}]'::jsonb - '{1,b}'::text[] /literal/entry
+   /row
   /tbody
  /tgroup
/table
@@ -10767,6 +10793,42 @@ table2-mapping
entryliteraljson_strip_nulls('[{f1:1,f2:null},2,null,3]')/literal/entry
entryliteral[{f1:1},2,null,3]/literal/entry
/row
+  row
+   entryparaliteraljsonb_replace(target jsonb, path text[], replacement jsonb)/literal
+ /para/entry
+   entryparatypejsonb/type/para/entry
+   entry
+ Returns replaceabletarget/replaceable
+ with the section designated by  replaceablepath/replaceable
+ replaced by replaceablereplacement/replaceable.
+   /entry
+   entryliteraljsonb_replace('[{f1:1,f2:null},2,null,3]', '{0,f1}','[2,3,4]')/literal/entry
+   entryliteral[{f1:[2,3,4],f2:null},2,null,3]/literal
+/entry
+   /row
+  row
+   entryparaliteraljsonb_pretty(from_json jsonb)/literal
+ /para/entry
+   entryparatypetext/type/para/entry
+   entry
+ Returns replaceablefrom_json/replaceable
+ as indented json text.
+   /entry
+   entryliteraljsonb_pretty('[{f1:1,f2:null},2,null,3]')/literal/entry
+   entry
+programlisting
+ [
+ {
+ f1: 1,
+

Re: [HACKERS] INSERT ... ON CONFLICT syntax issues

2015-04-29 Thread Simon Riggs

On 25 April 2015 at 14:05, Peter Geoghegan p...@heroku.com wrote:


  a) Why is is 'CONFLICT? We're talking about a uniquness violation. What
 if we, at some later point, also want to handle other kind of
 violations? Shouldn't it be ON UNIQUE CONFLICT/ERROR/VIOLATION ...

 I think that naming unique violations alone would be wrong (not to
 mention ludicrously verbose). Heikki and I both feel that the CONFLICT
 keyword captures the fact that this could be a dup violation, or an
 exclusion violation. The syntax has been like this for some time, and
 hasn't been a point of contention for a long time, so I thought this
 was settled.


I dislike the way that ignoring objections for a period leads them to be
potentially discarded. I'd prefer to think that as a community we are able
to listen to people even when they aren't continually present to reinforce
the original objection(s).

Not supporting MySQL syntax will seem like a bizarre choice to people
watching this from a distance. I accept that the patch implements useful
behaviour that MySQL does not implement and we thus provide enhanced
syntax, but the default should be match on PK using the MySQL syntax.


 Note that the syntax is quite similar to the SQLite
 syntax of the same feature, that has ON CONFLICT IGNORE (it also has
 ON CONFLICT REPLACE, but not ON CONFLICT UPDATE).


Why are we not also supporting ON CONFLICT REPLACE and IGNORE then?

If we are using syntax from other products then it should be identical
syntax, or the argument to use it doesn't stand.

We must think about what SQL Standard people are likely to say and do. If
we act as independently, our thought may be ignored. If we act in support
of other previous implementations we may draw support to adopt that as the
standard. Whatever the standard says we will eventually support, so we
should be acting with an eye to that future.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
http://www.2ndquadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services

Re: [HACKERS] Final Patch for GROUPING SETS

2015-04-29 Thread Andres Freund

Hi,

This is not a real review.  I'm just scanning through the patch, without
reading the thread, to understand if I see something worthy of
controversy. While scanning I might have a couple observations or
questions.


On 2015-03-13 15:46:15 +, Andrew Gierth wrote:

 + * A list of grouping sets which is structurally equivalent to a ROLLUP
 + * clause (e.g. (a,b,c), (a,b), (a)) can be processed in a single pass 
 over
 + * ordered data.  We do this by keeping a separate set of transition 
 values
 + * for each grouping set being concurrently processed; for each input 
 tuple
 + * we update them all, and on group boundaries we reset some initial 
 subset
 + * of the states (the list of grouping sets is ordered from most 
 specific to
 + * least specific).  One AGG_SORTED node thus handles any number of 
 grouping
 + * sets as long as they share a sort order.

Found initial subset not very clear, even if I probably guessed the
right meaning.

 + * To handle multiple grouping sets that _don't_ share a sort order, we 
 use
 + * a different strategy.  An AGG_CHAINED node receives rows in sorted 
 order
 + * and returns them unchanged, but computes transition values for its own
 + * list of grouping sets.  At group boundaries, rather than returning the
 + * aggregated row (which is incompatible with the input rows), it writes 
 it
 + * to a side-channel in the form of a tuplestore.  Thus, a number of
 + * AGG_CHAINED nodes are associated with a single AGG_SORTED node (the
 + * chain head), which creates the side channel and, when it has 
 returned
 + * all of its own data, returns the tuples from the tuplestore to its own
 + * caller.

This paragraph deserves to be expanded imo.

 + * In order to avoid excess memory consumption from a chain of 
 alternating
 + * Sort and AGG_CHAINED nodes, we reset each child Sort node 
 preemptively,
 + * allowing us to cap the memory usage for all the sorts in the chain at
 + * twice the usage for a single node.

What does reseting 'preemtively' mean?

 + * From the perspective of aggregate transition and final functions, the
 + * only issue regarding grouping sets is this: a single call site 
 (flinfo)
 + * of an aggregate function may be used for updating several different
 + * transition values in turn. So the function must not cache in the 
 flinfo
 + * anything which logically belongs as part of the transition value (most
 + * importantly, the memory context in which the transition value exists).
 + * The support API functions (AggCheckCallContext, AggRegisterCallback) 
 are
 + * sensitive to the grouping set for which the aggregate function is
 + * currently being called.

Hm. I've seen a bunch of aggreates do this.

 + * TODO: AGG_HASHED doesn't support multiple grouping sets yet.

Are you intending to resolve this before an eventual commit? Possibly
after the 'structural' issues are resolved? Or do you think this can
safely be put of for another release?

 @@ -534,11 +603,13 @@ static void
  advance_aggregates(AggState *aggstate, AggStatePerGroup pergroup)
  {
   int aggno;
 + int setno = 0;
 + int numGroupingSets = Max(aggstate-numsets, 1);
 + int numAggs = aggstate-numaggs;

 - for (aggno = 0; aggno  aggstate-numaggs; aggno++)
 + for (aggno = 0; aggno  numAggs; aggno++)
   {
   AggStatePerAgg peraggstate = aggstate-peragg[aggno];
 - AggStatePerGroup pergroupstate = pergroup[aggno];
   ExprState  *filter = peraggstate-aggrefstate-aggfilter;
   int numTransInputs = 
 peraggstate-numTransInputs;
   int i;
 @@ -582,13 +653,16 @@ advance_aggregates(AggState *aggstate, AggStatePerGroup 
 pergroup)
   continue;
   }

 - /* OK, put the tuple into the tuplesort object */
 - if (peraggstate-numInputs == 1)
 - tuplesort_putdatum(peraggstate-sortstate,
 -
 slot-tts_values[0],
 -
 slot-tts_isnull[0]);
 - else
 - tuplesort_puttupleslot(peraggstate-sortstate, 
 slot);
 + for (setno = 0; setno  numGroupingSets; setno++)
 + {
 + /* OK, put the tuple into the tuplesort object 
 */
 + if (peraggstate-numInputs == 1)
 + 
 tuplesort_putdatum(peraggstate-sortstates[setno],
 +
 slot-tts_values[0],
 +
 slot-tts_isnull[0]);
 +

Re: [HACKERS] Incompatible trig error handling

John Gorman johngorm...@gmail.com writes:
 Two of the trigonometry functions have differing error condition behavior
 between Linux and OSX. The Linux behavior follows the standard set by the
 other trig functions.

We have never considered it part of Postgres' charter to try to hide
platform-specific variations in floating-point behavior.  If we did,
we'd spend all our time doing that rather than more productive stuff.

In particular, it appears to me that both of these behaviors are allowed
per the POSIX standard, which makes it very questionable why we should
insist that one is correct and the other is not.

In addition, the proposed patch turns *all* cases that return NaN into
errors, which is wrong at least for the case where the input is NaN.

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] Additional role attributes superuser review

Robert, all,

* Stephen Frost (sfr...@snowman.net) wrote:
 * Stephen Frost (sfr...@snowman.net) wrote:
  * Robert Haas (robertmh...@gmail.com) wrote:
   The tricky part of this seems to me to be the pg_dump changes.  The
   new catalog flag seems a little sketchy to me; wouldn't it be better
   to make the dump flag into an enum, DUMP_EVERYTHING, DUMP_ACL,
   DUMP_NONE?
  
  I agree that the pg_dump changes are a very big part of this change.
  I'll look at using an enum and see if that would work.
 
 I've implemented this approach and there are things which I like about
 it and things which I don't.  I'd love to hear your thoughts.  As
 mentioned previously, this patch does not break the pg_stat_activity or
 pg_stat_replication view APIs.  Similairly, the functions which directly
 feed those views return the same results they did previously, there are
 just additional functions now which provide everything, and view on top
 of those, for users who are GRANT'd access to them.

Here is the latest revision of this patch.

The big change here is the addition of default roles.  This approach has
been brought up a few times and Magnus recently mentioned it to me
again.  Having the default roles greatly reduced the impact of this
change on the test_deparse regression test, which was quite nice.

Updates are included for pg_upgrade and pg_dumpall to handle roles which
start with pg_ specially as we are now claiming those as System
defined roles (similar to how we claim schemas starting with pg_ are
system defined, etc).  These new default roles are in line with the
previously discussed role attributes, but have the advantage that they
act just like normal roles and work inside of the normal permissions
system.  They are:

pg_backup - Start and stop backups, switch xlogs, create restore points.
pg_monitor - View privileged system info (user activity, replica lag)
pg_replay - Control XLOG replay on replica (pause, resume)
pg_replication - Create, destroy, work with replication slots

pg_admin - All of the above, plus rotate logfiles and signal backends

Feedback on all of this would be great.  One interesting idea is that,
with these defined default roles, we could rip out the majority of the
changes to pg_dump and declare that users have to use only the roles we
provide to manage access to those functions (or risk any changes they
make to the ACLs of system objects disappearing across upgrades or
pg_dump/restore's, which is what happens today anyway).  I'm a bit on
the fence about it myself; it'd certainly reduce the risk of this change
but it would also limit users to only being able to operate at the
pre-defined levels we've set, but then again, the same was going to be
true with the role attributes-based approach and I don't recall any
complaints during that discussion.

Thoughts?  Feedback on this would be most welcome; it's been a long time
incubating and I'd really like to get this capability in and close it
out of the current commitfest.  I'm certainly of the opinion that it
will be a welcome step forward for quite a few of our users as the
discussion about how to create non-superuser roles for certain
operations (a monitor role, in particular, but also backup and replay)
has come up quite a bit, both on the lists and directly from clients.

Thanks!

Stephen
From 488df9c567fdd0a56afa084a8f22f8b8a2412bd7 Mon Sep 17 00:00:00 2001
From: Stephen Frost sfr...@snowman.net
Date: Thu, 19 Mar 2015 14:49:26 -0400
Subject: [PATCH] Use GRANT for access to privileged functions

Historically, we have hard-coded the permissions for privileged
functions in C code (eg: if (superuser()) then allow X, else don't).
Unfortunately, that's pretty limiting and means that functions which are
useful for roles that should not be superusers (eg: monitoring, backup
management) require that the user calling them be a superuser.  This
leads to far more uses of superuser roles than is ideal.

Thankfully, we have a very handy and complex privilege system for
managing who has access to what already built into PG.  This is the
GRANT system which has existed since very near the beginning of PG.

This provides a set of system functions which are not able to be
executed by all users by default and allows administrators to grant
access to those functions for the users (eg: monitoring or other roles)
where they feel it is appropriate.

Further, create a set of pre-defined roles (which start with pg_)
for administrators to use to grant bulk access with.  This greatly
simplifies the granting of monitor, backup, and similar privileges.
pg_upgrade and pg_dumpall are updated to treat roles starting with
pg_ in much the same way as the default role is handled, and
pg_upgrade has been taught to complain if it finds any roles starting
with pg_ in a 9.4 or older system.  Having pre-defined roles also
allows 3rd-party utilities (eg: check_postgres.pl) to depend on these
roles and the access they provide in their documentation for

Re: [HACKERS] alternative compression algorithms?

On Wed, Apr 29, 2015 at 6:55 PM, Tomas Vondra
tomas.von...@2ndquadrant.com wrote:
 I'm not convinced not compressing the data is a good idea - it suspect it
 would only move the time to TOAST, increase memory pressure (in general and
 in shared buffers). But I think that using a more efficient compression
 algorithm would help a lot.

 For example, when profiling the multivariate stats patch (with multiple
 quite large histograms), the pglz_decompress is #1 in the profile, occupying
 more than 30% of the time. After replacing it with the lz4, the data are bit
 larger, but it drops to ~0.25% in the profile and planning the drops
 proportionally.

That seems to imply a 100x improvement in decompression speed.  Really???

-- 
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] Additional role attributes superuser review

2015-04-29 Thread Gavin Flower


On 30/04/15 12:20, Alvaro Herrera wrote:

Robert Haas wrote:


I think that if you commit this the way you have it today, everybody
will go, oh, look, Stephen committed something, but it looks
complicated, I won't pay attention.

Yeah, that sucks.


Finally, you've got the idea of making pg_ a reserved prefix for
roles, adding some predefined roles, and giving them some predefined
privileges.  That should be yet another patch.

On this part I have a bit of a problem -- the prefix is not really
reserved, is it.  I mean, evidently it's still possible to create roles
with the pg_ prefix ... otherwise, how come the new lines to
system_views.sql that create the predefined roles work in the first
place?  I think if we're going to reserve role names, we should reserve
them for real: CREATE ROLE should flat out reject creation of such
roles, and the default ones should be created during bootstrap.

IMO anyway.

What if I had a company with several subsidiaries using the same 
database, and want to prefix roles and other things with the 
subsidiary's initials? (I am not saying this would be a good 
architecture!!!)


For example if one subsidiary was called 'Perfect Gentleman', so I would 
want roles prefixed by 'pg_' and would be annoyed if I couldn't!



Cheers,
Gavin


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

Re: [HACKERS] Additional role attributes superuser review

On Wed, Apr 29, 2015 at 8:20 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Finally, you've got the idea of making pg_ a reserved prefix for
 roles, adding some predefined roles, and giving them some predefined
 privileges.  That should be yet another patch.

 On this part I have a bit of a problem -- the prefix is not really
 reserved, is it.  I mean, evidently it's still possible to create roles
 with the pg_ prefix ... otherwise, how come the new lines to
 system_views.sql that create the predefined roles work in the first
 place?  I think if we're going to reserve role names, we should reserve
 them for real: CREATE ROLE should flat out reject creation of such
 roles, and the default ones should be created during bootstrap.

 IMO anyway.

This is exactly what I mean about needing separate discussion for
separate parts of the patch.  There's so much different stuff in there
right now that objections like this won't necessarily come out until
it's far too late to change things around.

-- 
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] alternative compression algorithms?




On 04/30/15 02:42, Robert Haas wrote:

On Wed, Apr 29, 2015 at 6:55 PM, Tomas Vondra
tomas.von...@2ndquadrant.com wrote:

I'm not convinced not compressing the data is a good idea - it suspect it
would only move the time to TOAST, increase memory pressure (in general and
in shared buffers). But I think that using a more efficient compression
algorithm would help a lot.

For example, when profiling the multivariate stats patch (with multiple
quite large histograms), the pglz_decompress is #1 in the profile, occupying
more than 30% of the time. After replacing it with the lz4, the data are bit
larger, but it drops to ~0.25% in the profile and planning the drops
proportionally.


That seems to imply a 100x improvement in decompression speed.  Really???


Sorry, that was a bit misleading over-statement. The profiles (same 
dataset, same workload) look like this:



pglz_decompress
---
  44.51%  postgres  [.] pglz_decompress
  13.60%  postgres  [.] update_match_bitmap_histogram
   8.40%  postgres  [.] float8_cmp_internal
   7.43%  postgres  [.] float8lt
   6.49%  postgres  [.] deserialize_mv_histogram
   6.23%  postgres  [.] FunctionCall2Coll
   4.06%  postgres  [.] DatumGetFloat8
   3.48%  libc-2.18.so  [.] __isnan
   1.26%  postgres  [.] clauselist_mv_selectivity
   1.09%  libc-2.18.so  [.] __memcpy_sse2_unaligned

lz4
---
  18.05%  postgres  [.] update_match_bitmap_histogram
  11.67%  postgres  [.] float8_cmp_internal
  10.53%  postgres  [.] float8lt
   8.67%  postgres  [.] FunctionCall2Coll
   8.52%  postgres  [.] deserialize_mv_histogram
   5.52%  postgres  [.] DatumGetFloat8
   4.90%  libc-2.18.so  [.] __isnan
   3.92%  liblz4.so.1.6.0   [.] 0x2603
   2.08%  liblz4.so.1.6.0   [.] 0x2847
   1.81%  postgres  [.] clauselist_mv_selectivity
   1.47%  libc-2.18.so  [.] __memcpy_sse2_unaligned
   1.33%  liblz4.so.1.6.0   [.] 0x260f
   1.16%  liblz4.so.1.6.0   [.] 0x25e3
   (and then a long tail of other lz4 calls)

The difference used to more significant, but I've done a lot of 
improvements in the update_match_bitmap method (so the lz4 methods are 
more significant).


The whole script (doing a lot of estimates) takes 1:50 with pglz and 
only 1:25 with lz4. That's ~25-30% improvement.


The results are slightly unreliable because collected in a Xen VM, and 
the overhead is non-negligible (but the same in both cases). I wouldn't 
be surprised if the difference was more significant without the VM.


--
Tomas Vondra  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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_upgrade: quote directory names in delete_old_cluster script

2015-04-29 Thread Bruce Momjian

On Mon, Feb 16, 2015 at 05:03:45PM -0500, Bruce Momjian wrote:
  All of our makefiles use single quotes, so effectively the only
  character we don't support in directory paths is the single quote itself.
 
 This seems to say that Windows batch files don't have any special
 meaning for single quotes:
 
   
 http://stackoverflow.com/questions/24173825/what-does-single-quote-do-in-windows-batch-files
   
 http://stackoverflow.com/questions/10737283/single-quotes-and-double-quotes-how-to-have-the-same-behaviour-in-unix-and-wind
 
 Again, is it worth doing something platform-specific?  I can certainly
 define a platform-specific macro for  or ' as and use that.  Good idea?

I have developed the attached patch to use platform-specific quoting of
path names.

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

  + Everyone has their own god. +
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
new file mode 100644
index 6db223a..be66b24
*** a/src/bin/pg_upgrade/check.c
--- b/src/bin/pg_upgrade/check.c
*** create_script_for_old_cluster_deletion(c
*** 532,538 
  #endif
  
  	/* delete old cluster's default tablespace */
! 	fprintf(script, RMDIR_CMD  \%s\\n, fix_path_separator(old_cluster.pgdata));
  
  	/* delete old cluster's alternate tablespaces */
  	for (tblnum = 0; tblnum  os_info.num_old_tablespaces; tblnum++)
--- 532,539 
  #endif
  
  	/* delete old cluster's default tablespace */
! 	fprintf(script, RMDIR_CMD  %c%s%c\n, PATH_QUOTE,
! 			fix_path_separator(old_cluster.pgdata), PATH_QUOTE);
  
  	/* delete old cluster's alternate tablespaces */
  	for (tblnum = 0; tblnum  os_info.num_old_tablespaces; tblnum++)
*** create_script_for_old_cluster_deletion(c
*** 554,562 
  		PATH_SEPARATOR);
  
  			for (dbnum = 0; dbnum  old_cluster.dbarr.ndbs; dbnum++)
! fprintf(script, RMDIR_CMD  \%s%c%d\\n,
  		fix_path_separator(os_info.old_tablespaces[tblnum]),
! 		PATH_SEPARATOR, old_cluster.dbarr.dbs[dbnum].db_oid);
  		}
  		else
  		{
--- 555,564 
  		PATH_SEPARATOR);
  
  			for (dbnum = 0; dbnum  old_cluster.dbarr.ndbs; dbnum++)
! fprintf(script, RMDIR_CMD  %c%s%c%d%c\n, PATH_QUOTE,
  		fix_path_separator(os_info.old_tablespaces[tblnum]),
! 		PATH_SEPARATOR, old_cluster.dbarr.dbs[dbnum].db_oid,
! 		PATH_QUOTE);
  		}
  		else
  		{
*** create_script_for_old_cluster_deletion(c
*** 566,574 
  			 * Simply delete the tablespace directory, which might be .old
  			 * or a version-specific subdirectory.
  			 */
! 			fprintf(script, RMDIR_CMD  \%s%s\\n,
  	fix_path_separator(os_info.old_tablespaces[tblnum]),
! 	fix_path_separator(suffix_path));
  			pfree(suffix_path);
  		}
  	}
--- 568,576 
  			 * Simply delete the tablespace directory, which might be .old
  			 * or a version-specific subdirectory.
  			 */
! 			fprintf(script, RMDIR_CMD  %c%s%s%c\n, PATH_QUOTE,
  	fix_path_separator(os_info.old_tablespaces[tblnum]),
! 	fix_path_separator(suffix_path), PATH_QUOTE);
  			pfree(suffix_path);
  		}
  	}
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
new file mode 100644
index 4683c6f..bb035e1
*** a/src/bin/pg_upgrade/pg_upgrade.h
--- b/src/bin/pg_upgrade/pg_upgrade.h
*** extern char *output_files[];
*** 74,79 
--- 74,80 
  #define pg_mv_file			rename
  #define pg_link_file		link
  #define PATH_SEPARATOR		'/'
+ #define PATH_QUOTE	'\''
  #define RM_CMDrm -f
  #define RMDIR_CMD			rm -rf
  #define SCRIPT_PREFIX		./
*** extern char *output_files[];
*** 85,90 
--- 86,92 
  #define pg_mv_file			pgrename
  #define pg_link_file		win32_pghardlink
  #define PATH_SEPARATOR		'\\'
+ #define PATH_QUOTE	''
  #define RM_CMDDEL /q
  #define RMDIR_CMD			RMDIR /s/q
  #define SCRIPT_PREFIX		

-- 
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] Selectivity estimation for intarray

Alexander Korotkov aekorot...@gmail.com writes:
 My proposal is to let ALTER OPERATOR change restrict and join selectivity
 functions of the operator. Also it would be useful to be able to change
 commutator and negator of operator: extension could add commutators and
 negators in further versions. Any thoughts?

I'm pretty dubious about this, because we lack any mechanism for undoing
parser/planner decisions based on operator properties.  And there's quite
a lot of stuff that is based on the assumption that operator properties
will never change.

An example of the pitfalls here is that we can never allow ALTER OPERATOR
RENAME, because for example if you rename '' to '~~' that will change
its precedence, and we have no way to fix the parse trees embedded in
stored views to reflect that.

For the specific cases you mention, perhaps it would be all right if we
taught plancache.c to blow away *all* cached plans upon seeing any change
in pg_operator; but that seems like a brute-force solution.

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] Selectivity estimation for intarray