Re: [HACKERS] star schema and the optimizer

2015-02-28 Thread Marc Cousin 
On 27/02/2015 20:01, Marc Cousin wrote:
 On 27/02/2015 19:45, Tom Lane wrote:
 I wrote:
 I had actually thought that we'd fixed this type of problem in recent
 versions, and that you should be able to get a plan that would look
 like

 Nestloop
- scan dim1
- Nestloop
 - scan dim2
 - indexscan fact table using dim1.a and dim2.b

 After closer study, I think this is an oversight in commit
 e2fa76d80ba571d4de8992de6386536867250474, which quoth

 +It can be useful for the parameter value to be passed down through
 +intermediate layers of joins, for example:
 +
 +NestLoop
 +- Seq Scan on A
 +Hash Join
 +Join Condition: B.Y = C.W
 +- Seq Scan on B
 +- Index Scan using C_Z_IDX on C
 +Index Condition: C.Z = A.X
 +
 +If all joins are plain inner joins then this is unnecessary, because
 +it's always possible to reorder the joins so that a parameter is used
 +immediately below the nestloop node that provides it.  But in the
 +presence of outer joins, join reordering may not be possible, and then
 +this option can be critical.  Before version 9.2, Postgres used ad-hoc

 This reasoning overlooked the fact that if we need parameters from
 more than one relation, and there's no way to join those relations
 to each other directly, then we have to allow passing the dim1 parameter
 down through the join to dim2.

 The attached patch seems to fix it (modulo the need for some updates
 in the README, and maybe a regression test).  Could you see if this
 produces satisfactory plans for you?
 
 
 From what I see, it's just perfect. I'll give it a more thorough look a
 bit later, but it seems to be exactly what I was waiting for.
 
 Thanks a lot.
 
 Regards

I gave it another look this morning. It works very well with the initial test 
schema. The situation is much improved for me.

I still have one issue: I've extended the test to more than 2 dimensions.

marc=# explain analyze SELECT * FROM dim1,dim2,dim3,dim4,facts WHERE 
facts.dim1=dim1.a and facts.dim2=dim2.a and facts.dim3=dim3.a and 
facts.dim4=dim4.a and dim1.b between 10 and 60 AND dim2.b between 30 and 50 and 
dim3.b=8526 and dim4.b between 70 and 90; 
  QUERY 
PLAN  
--
 Nested Loop  (cost=15.00..957710.99 rows=1 width=200) (actual 
time=793.247..793.247 rows=0 loops=1)
   -  Nested Loop  (cost=14.71..957704.75 rows=1 width=192) (actual 
time=793.245..793.245 rows=0 loops=1)
 Join Filter: (facts.dim3 = dim3.a)
 Rows Removed by Join Filter: 942
 -  Index Scan using idxdim32 on dim3  (cost=0.29..3300.29 rows=10 
width=8) (actual time=49.498..67.923 rows=1 loops=1)
   Filter: (b = 8526)
   Rows Removed by Filter: 9
 -  Materialize  (cost=14.41..954262.56 rows=962 width=184) (actual 
time=0.552..724.988 rows=942 loops=1)
   -  Nested Loop  (cost=14.41..954257.75 rows=962 width=184) 
(actual time=0.542..723.380 rows=942 loops=1)
 -  Index Scan using idxdim22 on dim2  (cost=0.29..3648.29 
rows=192 width=16) (actual time=0.074..47.445 rows=229 loops=1)
   Filter: ((b = 30) AND (b = 50))
   Rows Removed by Filter: 99771
 -  Nested Loop  (cost=14.12..4946.08 rows=501 width=168) 
(actual time=2.575..2.946 rows=4 loops=229)
   -  Bitmap Heap Scan on dim1  (cost=13.43..573.60 
rows=501 width=16) (actual time=0.170..0.647 rows=509 loops=229)
 Recheck Cond: ((b = 10) AND (b = 60))
 Heap Blocks: exact=78547
 -  Bitmap Index Scan on idxdim11  
(cost=0.00..13.30 rows=501 width=0) (actual time=0.112..0.112 rows=509 
loops=229)
   Index Cond: ((b = 10) AND (b = 60))
   -  Index Scan using idxfacts on facts  
(cost=0.69..8.72 rows=1 width=152) (actual time=0.004..0.004 rows=0 
loops=116561)
 Index Cond: ((dim1 = dim1.a) AND (dim2 = 
dim2.a))
   -  Index Scan using idxdim42 on dim4  (cost=0.29..6.24 rows=1 width=8) 
(never executed)
 Index Cond: (a = facts.dim4)
 Filter: ((b = 70) AND (b = 90))
 Planning time: 8.092 ms
 Execution time: 793.621 ms
(25 lignes)

marc=# \d facts 
  Table « public.facts »
 Colonne |  Type  | Modificateurs 
-++---
 dim1| bigint | 
 dim2| bigint | 
 dim3| bigint | 
 dim4| bigint | 
 dim5| bigint | 
 dim6| bigint | 
 dim7| bigint | 
 dim8| bigint | 
 dim9| bigint | 
 dim10   | bigint | 
 data1   | text   | 
 data2   | text   | 
 data3   | text

Re: [HACKERS] Merge compact/non compact commits, make aborts dynamically sized

2015-02-28 Thread Robert Haas 
On Fri, Feb 27, 2015 at 7:10 AM, Michael Paquier
michael.paqu...@gmail.com wrote:
 No, no. I meant that it is good the way your patch does it in
 xactdesc.c, where both frontend and backend can reach it.

Agreed, that seems much better than duplicating it.

-- 
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] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-02-28 Thread Stephen Frost 
Michael,

* Michael Paquier (michael.paqu...@gmail.com) wrote:
 On Sat, Feb 28, 2015 at 2:45 PM, Stephen Frost sfr...@snowman.net wrote:
  So, basically, this feels like it's not really the right place
  for these checks and if there is an existing problem then it's probably
  with the grammar...  Does that make sense?
 
 As long as there is no more inconsistency between the parser, that
 sometimes does not set VACOPT_FREEZE, and those assertions, that do
 not use the freeze_* parameters of VacuumStmt, I think that it will be
 fine.

parsenodes.h points out that VACOPT_FREEZE is just an internal
convenience for the grammar- it doesn't need to be set for vacuum()'s
purposes and, even if it is, except for this Assert(), it isn't looked
at.  Now, I wouldn't be against changing the grammar to operate the same
way for all of these and therefore make it a bit easier to follow, eg:

if ($3)
n-options |= VACOPT_FREEZE;
if (n-options  VACOPT_FREEZE)
{
n-freeze_min_age = n-freeze_table_age = 0;
n-multixact_freeze_min_age = 0;
n-multixact_freeze_table_age = 0;
}
else
{
n-freeze_min_age = n-freeze_table_age = -1;
n-multixact_freeze_min_age = -1;
n-multixact_freeze_table_age = -1;
}

but I'm really not sure it's worth it.

 [nitpicking]We could improve things on both sides, aka change gram.y
 to set VACOPT_FREEZE correctly, and add some assertions with the
 params freeze_* at the beginning of vacuum().[/]

The other issue that I have with this is that most production operations
don't run with Asserts enabled, so if there is an actual issue here, we
shouldn't be using Asserts to check but regular conditionals and
throwing ereport()'s.

Another approach might be to change VACOPT_FREEZE to actually be used by
vacuum() instead of having to set 4 variables when it's not passed in.
Perhaps we would actually take those parameters out of VacuumStmt, add a
new argument to vacuum() for autovacuum to use which is a struct
containing those options, and remove all of nonsense of setting those
variables inside gram.y.  At least in my head, that'd be a lot cleaner-
have the grammar worry about options that are actually coming from the
grammar and give other callers a way to specify more options if they've
got them.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Strange assertion using VACOPT_FREEZE in vacuum.c

2015-02-28 Thread Michael Paquier 
On Sat, Feb 28, 2015 at 2:45 PM, Stephen Frost sfr...@snowman.net wrote:
 I'm trying to wrap my head around the reasoning for this also and not
 sure I'm following.  In general, I don't think we protect all that hard
 against functions being called with tokens that aren't allowed by the
 parse.

Check.

 So, basically, this feels like it's not really the right place
 for these checks and if there is an existing problem then it's probably
 with the grammar...  Does that make sense?

As long as there is no more inconsistency between the parser, that
sometimes does not set VACOPT_FREEZE, and those assertions, that do
not use the freeze_* parameters of VacuumStmt, I think that it will be
fine.

[nitpicking]We could improve things on both sides, aka change gram.y
to set VACOPT_FREEZE correctly, and add some assertions with the
params freeze_* at the beginning of vacuum().[/]
-- 
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] Bug in pg_dump

2015-02-28 Thread Stephen Frost 
Michael, all,

* Michael Paquier (michael.paqu...@gmail.com) wrote:
 On Wed, Feb 25, 2015 at 7:46 AM, Gilles Darold gilles.dar...@dalibo.com 
 wrote:
  This is a far better patch and the test to export/import of the
  postgis_topology extension works great for me.
 
  Thanks for the work.
 
 Attached is a patch that uses an even better approach by querying only
 once all the FK dependencies of tables in extensions whose data is
 registered as dumpable by getExtensionMembership(). Could you check
 that it works correctly? My test case passes but an extra check would
 be a good nice. The patch footprint is pretty low so we may be able to
 backport this patch easily.

I've started looking at this and it looks pretty simple and definitely
something to backpatch (and mention in the release notes that existing
pg_dump exports might be broken..).

One thing that might be missing is what Jim brought up though- that this
won't be able to deal with circular dependencies.  I'm not sure that we
need to care, but I *do* think we should document that in the extension
documentation as unsupported.  Perhaps in the future we can improve on
this situation by setting up to drop and recreate the constraints,
though another thought I had was to require extensions with circular
dependencies to use deferrable constraints and then make sure we set
constraints to deferred.  That isn't something we'd want to backpatch
though, so my plan is to push forward with this.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Bug in pg_dump

2015-02-28 Thread Stephen Frost 
Michael,

* Michael Paquier (michael.paqu...@gmail.com) wrote:
 + /*
 +  * Query all the foreign key dependencies for all the 
 extension
 +  * tables found previously. Only tables whose data need 
 to be
 +  * have to be tracked.
 +  */
 + appendPQExpBuffer(query2,
 +   SELECT conrelid, confrelid 
 +   FROM pg_catalog.pg_constraint 
 +   WHERE contype = 'f' AND conrelid IN 
 ();
 +
 + for (j = 0; j  nconfigitems; j++)
 + {

[...]

Instead of building up a (potentially) big list like this, couldn't we
simply join against pg_extension and check if conrelid = ANY (extconfig)
instead, based on the extension we're currently processing?

eg:

appendPQExpBuffer(query2,
  SELECT conrelid, confrelid 
  FROM pg_catalog.pg_constraint, pg_extension 
  WHERE contype = 'f' AND extname = '%s' AND 
conrelid = ANY (extconfig),
  fmtId(curext-dobj.name));

This seemed to work just fine for me, at least, and reduces the size of
the patch a bit further since we don't need the loop that builds up the
ids.

 Subject: [PATCH 2/3] Make prove_check install contents of current directory 
 as well

This is really an independent thing, no?  I don't see any particular
problem with it, for my part.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] CATUPDATE confusion?

2015-02-28 Thread Stephen Frost 
Peter,

* Peter Eisentraut (pete...@gmx.net) wrote:
 On 2/25/15 10:05 PM, Stephen Frost wrote:
  Agreed, but I'd also like to get rid of any reason, beyond emergency
  cases, for people to modify the catalog directly.  There's a few places
  which we aren't yet doing that, but I'd rather fix those cases than
  encourage people to give out rights to modify them and end up making
  things like:
  
  UPDATE pg_database SET datallowconn = false where datname = 'xyz';
  
  an accepted interface.
 
 I'm not sure those things are related.

Eh, I feel they are, but either way.

 Getting rid of the *reasons* for updating catalogs directly might be
 worthwhile, but I don't see why we need to install (or maintain) a
 special invisible permission trap for it.  We have a permission system,
 and it works pretty well.

We have this invisible permission trap known as checking if you're a
superuser all over the place.  I'm not against revisiting those cases
and considering using the GRANT permission system to manage access, but
that's certainly a larger project to work on.  What I'm referring to
here are all the functions that check if you're a superuser, instead of
just revoking EXECUTE from public and letting the user manage the
permission.

 The Unix kernels don't have special traps for someone to modify
 /etc/shadow or similar directly.  That file has appropriate permissions,
 and that's it.  I think that works pretty well.

This isn't really /etc/shadow though, this is more like direct access to
the filesystem through the device node- and you'll note that Linux
certainly has got an independent set of permissions for that called the
capabilities system.  That's because messing with those pieces can crash
the kernel.  You're not going to crash the kernel if you goof up
/etc/shadow.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Streaming replication and WAL archive interactions

2015-02-28 Thread Venkata Balaji N 


 Here's a first cut at this. It includes the changes from your
 standby_wal_archiving_v1.patch, so you get that behaviour if you set
 archive_mode='always', and the new behaviour I wanted with
 archive_mode='shared'. I wrote it on top of the other patch I posted
 recently to not archive bogus recycled WAL segments after promotion (
 http://www.postgresql.org/message-id/549489fa.4010...@vmware.com), but it
 seems to apply without it too.

 I suggest reading the documentation changes first, it hopefully explains
 pretty well how to use this. The code should work too, and comments on that
 are welcome too, but I haven't tested it much. I'll do more testing next
 week.


Patch did get applied successfully to the latest master. Can you please
rebase.

Regards,
Venkata Balaji N

Re: [HACKERS] CATUPDATE confusion?

2015-02-28 Thread Peter Eisentraut 
On 2/25/15 10:05 PM, Stephen Frost wrote:
 * Peter Eisentraut (pete...@gmx.net) wrote:
 On 2/25/15 3:39 PM, Stephen Frost wrote:
 I'd get rid of that whole check, not just replace rolcatupdate by rolsuper.

 Err, wouldn't this make it possible to grant normal users the ability to
 modify system catalogs?  I realize that they wouldn't have that
 initially, but I'm not sure we want the superuser to be able to grant
 that to non-superusers..

 Why not?  I thought we are trying to get rid of special superuser behavior.
 
 Agreed, but I'd also like to get rid of any reason, beyond emergency
 cases, for people to modify the catalog directly.  There's a few places
 which we aren't yet doing that, but I'd rather fix those cases than
 encourage people to give out rights to modify them and end up making
 things like:
 
 UPDATE pg_database SET datallowconn = false where datname = 'xyz';
 
 an accepted interface.

I'm not sure those things are related.

Getting rid of the *reasons* for updating catalogs directly might be
worthwhile, but I don't see why we need to install (or maintain) a
special invisible permission trap for it.  We have a permission system,
and it works pretty well.

The Unix kernels don't have special traps for someone to modify
/etc/shadow or similar directly.  That file has appropriate permissions,
and that's it.  I think that works pretty well.



-- 
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] add modulo (%) operator to pgbench

2015-02-28 Thread Stephen Frost 
* Fabien COELHO (coe...@cri.ensmp.fr) wrote:
 
 On Mon, Jan 5, 2015 at 10:37 AM, Fabien COELHO coe...@cri.ensmp.fr wrote:
 Anyway, I suggest to keep that for another round and keep the Robert's
 isofunctional patch as it is before extending.
 
 +1.  Let's please get the basic thing committed, and then people can
 write more patches to extend and improve it.  There is no reason to
 squash-merge every enhancement someone might want here into what I
 wrote.
 
 From my point of view the latest v6 of the patch is pretty ready
 for
 committers, but I'm not sure whom should decide that...
 
 Should I just update the state in the commitfest interface?

I took a look through the patch and the discussion and it certainly
seems ready to me.  I agree with Robert- let's go ahead and get this in
and then folks can build on top of it.  I'm guessing it was added as
Needs Review since that's the default for a new entry, but it's
clearly had review from multiple people, committers and non-committers
alike, so I went ahead and marked it as 'ready for committer' to make
that clear to folks looking at the CF app.

Robert, as this is mostly your code (and you're marked as author on the
CF app), do you want to do the actual commit, or are you impartial, or
would you prefer someone else handle it, or..?  I'm interested in this
also and would be happy to help in any way I can.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] star schema and the optimizer

2015-02-28 Thread Tom Lane 
Marc Cousin cousinm...@gmail.com writes:
 I gave it another look this morning. It works very well with the initial test 
 schema. The situation is much improved for me.

 I still have one issue: I've extended the test to more than 2 dimensions.

I tried your original test script with 3 dimension tables, and it gave me
a three-deep nestloop plan once I made the fact table big enough.  I think
your test case has simply not got statistics that encourage doing it that
way.  Notice that the planner thinks (correctly) that it's already down
to fetching only one row from the facts table with dim1 and dim2 as
inputs; so there's no cost advantage to stacking up more nestloops, and
it might as well just materialize the result from that and then join
against dim3 and dim4.  Another factor is that it predicts (less
correctly) that it will get 10 rows from dim3, which would make a straight
nestloop plan about 10x more expensive than what it did here.

You could experiment with turning off enable_material, but I think
the planner may actually be making the right choice here.

regards, tom lane


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

[HACKERS] Looking for Intro to Hacking teacher for pgCon

2015-02-28 Thread Josh Berkus 
Hackers:

The pgCon committee would like to have someone give some kind of Intro
to Hacking tutorial at pgCon this year, on the Advanced Tutorial
Saturday.  Since there will be lots of power users and other potential
contributors at the conference, it seems like an opportunity not to be
missed.

This could be either a general intro to hacking, or it could be more
specific, like a tutorial on writing your own data type, FDW, or
procedural language.  Whatever you think you can do a good job with.

Contact me offlist ASAP if you are interested.

P.S. Tutorial instructors get preferential treatment for financial aid.

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


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

Re: [HACKERS] Enforce creation of destination folders for source files in pg_regress (Was: pg_regress writes into source tree)

2015-02-28 Thread Stephen Frost 
* Michael Paquier (michael.paqu...@gmail.com) wrote:
 On Wed, Feb 25, 2015 at 4:27 AM, Peter Eisentraut pete...@gmx.net wrote:
  On 2/23/15 1:27 AM, Michael Paquier wrote:
  I would like to have an extension in tree that also does this, so we
   have a regression test of this functionality.
  Sure. Here is one in the patch attached added as a test module. The
  name of the module is regress_dynamic. Perhaps the name could be
  better..
 
  I was more thinking like we could use an existing module like file_fdw.
 
 Right. I forgot this one. A patch is attached to do so.

I'm always doing VPATH builds, but I believe there's still an
outstanding question on this from Peter about if we need to add entries
to .gitignore for, eg, sql/file_fdw.sql if these directories don't exist
because otherwise those would show up to someone doing a git status and
to Robert's point- if we need those anyway, it's probably better to have
them be in those directories rather than up a level and referring to
directories that don't exist in the tree.

Having the extra directories already exist doesn't seem like a bad thing
to me anyway.  If they are causing confusion or something then perhaps a
README could be added to let people know why they exist.

I'm going to mark this back to 'waiting on author' in case there's
something material that I've missed which you can clarify.  I had
started this review thinking to help move it along but after re-reading
the thread and thinking about it for a bit, I came around to the other
side and therefore think it should probably be rejected.  We certainly
appreciate the effort, of course.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] GSoC idea - Simulated annealing to search for query plans

2015-02-28 Thread Grzegorz Parka 
Thank you a lot for your feedback. I searched a lot about GEQO,
but I didn't find information about any earlier attempts.
I'm happy to know that this is important for Postgres.

I'm really interested in this project, so I just need to estimate if I can
handle it.
Now I will spend some time with SAIO and GEQO to find it out.

Best,
Grzegorz

2015-02-27 16:29 GMT+01:00 Jan Urbański wulc...@wulczer.org:


 Josh Berkus writes:

  On 02/26/2015 05:50 PM, Fabrízio de Royes Mello wrote:
 
  On Thu, Feb 26, 2015 at 10:27 PM, Andres Freund and...@2ndquadrant.com
  mailto:and...@2ndquadrant.com wrote:
 
  On 2015-02-26 20:23:33 -0500, Tom Lane wrote:
  
   I seem to recall somebody demo'ing a simulated-annealing GEQO
  replacement
   at PGCon a couple years back.  It never got to the point of being a
   submitted patch though.
 
  Yea, it was Jan Urbański (CCed).
 
 
  And the project link: https://github.com/wulczer/saio
 
  So what w'ere saying, Grzegorz, is that we would love to see someone
  pick this up and get it to the point of making it a feature as a GSOC
  project.  I think if you can start from where Jan left off, you could
  actually complete it.

 Sorry, late to the party.

 Yes, I wrote a GEQO replacement that used simulated annealing for my Master
 thesis. It got to a point where it was generating plans similar to what
 GEQO
 outputs for small queries and better plans for very large ones.

 The thesis itself is here: https://wulczer.org/saio.pdf and the linked
 GitHub
 repo contains source for the PGCon presentation, which gives a higher-level
 overview.

 The big problem turned out to be writing the step function that generates
 a new
 join order from a previous one. Look for the Simulated Annealing
 challenges
 and Moves generator chapters in my thesis, which are the only interesting
 ones :)

 If you'd like to pick up where I left, I'd be more than happy to help in
 any
 ways I can.

 Best,
 Jan

Re: [HACKERS] Additional role attributes superuser review

2015-02-28 Thread Stephen Frost 
Adam,

* Adam Brightwell (adam.brightw...@crunchydatasolutions.com) wrote:
 I have attached and updated patch for review.

Thanks!  I've gone over this and made quite a few documentation and
comment updates, but not too much else, so I'm pretty happy with how
this is coming along.  As mentioned elsewhere, this conflicts with the
GetUserId() to has_privs_of_role() cleanup, but as I anticipate handling
both this patch and that one, I'll find some way to manage. :)

Updated patch attached.  Barring objections, I'll be moving forward with
this soonish.  Would certainly appreciate any additional testing or
review that you (or anyone!) has time to provide.

Thanks again!

Stephen
From 58efbb5ebeadca15fb2f3ada4ca5c83b7ddd5d69 Mon Sep 17 00:00:00 2001
From: Stephen Frost sfr...@snowman.net
Date: Sat, 28 Feb 2015 20:23:38 -0500
Subject: [PATCH] Add role attributes for various operations

Historically, many operations have been restricted to the superuser.
These additional role attributes allow an administrator to delegate the
right to run some of these operations out to other roles, reducing the
number of cases which strictly require superuser rights and therefore,
hopefully, reducing the number of users running as superuser in the
wild.

This patch introduces the following role attributes:

EXCLUSIVEBACKUP - allows the role to run pg_start_backup,
pg_stop_backup, pg_create_restore_point, and pg_switch_xlog.

XLOGREPLAY - allows the role to run pg_xlog_replay_pause and
pg_xlog_replay_resume.

LOGFILE - allows the role to run pg_rotate_logfile

MONITOR - allows the role to see the details of all running processes
(including both normal backends and replication backends).  The
documentation and code are also updated to use has_privs_of_role().

SIGNAL - allows the role to signal all other normal backends (except
those initiated by a superuser) with pg_cancel_backend and
pg_termiante_backend.  The documentation and code for the NOSIGNAL case
are also updated to use has_privs_of_role().

In passing, this also centralizes and standardizes the REPLICATION and
CREATEROLE support functions.

Lots of discussion, review, and commentary from Jim Nasby, Simon,
Magnus, Alvaro, and Robert.

Authored by Adam, bugs and various documentation updates from
me.
---
 doc/src/sgml/catalogs.sgml |  30 
 doc/src/sgml/func.sgml |  38 +++--
 doc/src/sgml/ref/create_role.sgml  |  81 +++
 doc/src/sgml/ref/create_user.sgml  |   6 +
 src/backend/access/transam/xlogfuncs.c |  27 ++--
 src/backend/catalog/aclchk.c   | 137 ++
 src/backend/commands/user.c| 183 ++---
 src/backend/parser/gram.y  |  20 +++
 src/backend/replication/logical/logicalfuncs.c |  15 +-
 src/backend/replication/slotfuncs.c|  25 ++--
 src/backend/replication/walsender.c|   8 +-
 src/backend/utils/adt/misc.c   |  19 ++-
 src/backend/utils/adt/pgstatfuncs.c|  54 ++--
 src/backend/utils/init/miscinit.c  |  19 ---
 src/backend/utils/init/postinit.c  |   2 +-
 src/include/catalog/pg_authid.h|  20 ++-
 src/include/miscadmin.h|   1 -
 src/include/utils/acl.h|   6 +
 18 files changed, 585 insertions(+), 106 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 515a40e..77b1090 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -1454,6 +1454,36 @@
  /row
 
  row
+  entrystructfieldrolexclbackup/structfield/entry
+  entrytypebool/type/entry
+  entryRole can perform on-line exclusive backup operations/entry
+ /row
+
+ row
+  entrystructfieldrolxlogreplay/structfield/entry
+  entrytypebool/type/entry
+  entryRole can control xlog recovery replay operations/entry
+ /row
+
+ row
+  entrystructfieldrollogfile/structfield/entry
+  entrytypebool/type/entry
+  entryRole can rotate log files/entry
+ /row
+
+ row
+  entrystructfieldrolmonitor/structfield/entry
+  entrytypebool/type/entry
+  entryRole can view pg_stat_* details/entry
+ /row
+
+ row
+  entrystructfieldrolsignal/structfield/entry
+  entrytypebool/type/entry
+  entryRole can signal normal backend processes/entry
+ /row
+
+ row
   entrystructfieldrolconnlimit/structfield/entry
   entrytypeint4/type/entry
   entry
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index da2ed67..7842cfc 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -16261,8 +16261,7 @@ SELECT set_config('log_statement_stats', 'off', false);
para
 The functions shown in xref
 linkend=functions-admin-signal-table send control signals to
-other server processes.  Use of these functions is usually

Re: [HACKERS] plpgsql versus domains

2015-02-28 Thread Tom Lane 
Andres Freund and...@2ndquadrant.com writes:
 On 2015-02-26 13:53:18 -0500, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
 You're probably going to kill me because of the increased complexity,
 but how about making typecache.c smarter, more in the vein of
 relcache.c, and store the relevant info in there?

 I thought that's what I was proposing in point #1.

 Sure, but my second point was to avoid duplicating that information into
 -fcinfo and such and instead reference the typecache entry from
 there. So that if the typecache entry is being rebuilt (a new mechanism
 I'm afraid), whatever uses -fcinfo gets the updated
 functions/definition.

Here's a draft patch for that.  This fixes the delayed-domain-constraint
problem pretty thoroughly, in that any attempt to check a domain's
constraints will use fully up-to-date info.

This is the first attempt at weaponizing the memory context reset/delete
feature, and I'm fairly happy with it, except for one thing: I had to
#include utils/memnodes.h into typcache.h in order to preserve the
intended property that the callback structs could be included directly
into structs using them.  Now, that's not awful in itself, because
typcache.h isn't used everywhere; but if this feature gets popular we'll
find memnodes.h being included pretty much everywhere, which is not so
great.  I'm thinking about moving struct MemoryContextCallback and the
extern for MemoryContextRegisterResetCallback into palloc.h to avoid this.
Maybe that's too far in the other direction.  Thoughts?  Compromise ideas?

Also, instrumenting the code showed that TypeCacheConstrCallback gets
called quite a lot during the standard regression tests, which is why
I went out of my way to make it quick.  Almost all of those cache flushes
are from non-domain-related updates on pg_type or pg_constraint, so
they're not really necessary from a logical perspective, and they're
surely going to hurt performance for heavy users of domains.  I think to
fix this we'd have to split pg_constraint into two catalogs, one for table
constraints and one for domain constraints; which would be a good thing
anyway from a normal-form-theory perspective.  And we'd have to get rid of
pg_type.typnotnull and instead store domain NOT NULL constraints in this
hypothetical domain constraint catalog.  I don't plan to do anything about
that myself right now, because I'm not sure that production databases
would have the kind of traffic on pg_type and pg_constraint that the
regression tests exhibit.  But at some point we might have to fix it.

regards, tom lane

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 07ab4b4..7455020 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*** ATExecAddColumn(List **wqueue, AlteredTa
*** 4824,4830 
  	{
  		defval = (Expr *) build_column_default(rel, attribute.attnum);
  
! 		if (!defval  GetDomainConstraints(typeOid) != NIL)
  		{
  			Oid			baseTypeId;
  			int32		baseTypeMod;
--- 4824,4830 
  	{
  		defval = (Expr *) build_column_default(rel, attribute.attnum);
  
! 		if (!defval  DomainHasConstraints(typeOid))
  		{
  			Oid			baseTypeId;
  			int32		baseTypeMod;
*** ATColumnChangeRequiresRewrite(Node *expr
*** 7778,7784 
  		{
  			CoerceToDomain *d = (CoerceToDomain *) expr;
  
! 			if (GetDomainConstraints(d-resulttype) != NIL)
  return true;
  			expr = (Node *) d-arg;
  		}
--- 7778,7784 
  		{
  			CoerceToDomain *d = (CoerceToDomain *) expr;
  
! 			if (DomainHasConstraints(d-resulttype))
  return true;
  			expr = (Node *) d-arg;
  		}
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index b77e1b4..d800033 100644
*** a/src/backend/commands/typecmds.c
--- b/src/backend/commands/typecmds.c
***
*** 59,65 
  #include executor/executor.h
  #include miscadmin.h
  #include nodes/makefuncs.h
- #include optimizer/planner.h
  #include optimizer/var.h
  #include parser/parse_coerce.h
  #include parser/parse_collate.h
--- 59,64 
*** domainAddConstraint(Oid domainOid, Oid d
*** 3081,3206 
  	return ccbin;
  }
  
- /*
-  * GetDomainConstraints - get a list of the current constraints of domain
-  *
-  * Returns a possibly-empty list of DomainConstraintState nodes.
-  *
-  * This is called by the executor during plan startup for a CoerceToDomain
-  * expression node.  The given constraints will be checked for each value
-  * passed through the node.
-  *
-  * We allow this to be called for non-domain types, in which case the result
-  * is always NIL.
-  */
- List *
- GetDomainConstraints(Oid typeOid)
- {
- 	List	   *result = NIL;
- 	bool		notNull = false;
- 	Relation	conRel;
- 
- 	conRel = heap_open(ConstraintRelationId, AccessShareLock);
- 
- 	for (;;)
- 	{
- 		HeapTuple	tup;
- 		HeapTuple	conTup;
- 		Form_pg_type typTup;
- 		ScanKeyData key[1];
- 		SysScanDesc scan;
- 
- 		tup =

[HACKERS] rm static libraries before rebuilding

2015-02-28 Thread Noah Misch 
We build static libraries with ar crs or ar cr.  If the static library
already exists in the build directory, those commands will add new members and
replace existing members.  They will not remove members present in the
existing library but not named on the ar command line.  This matters when,
for example, you rerun ./configure in a way that removes a file from
$(LIBOBJS).  libpgport carries the obsolete member until make clean.  Let's
fix that by issuing rm -f before running $(AR).  I would back-patch this.
diff --git a/src/Makefile.shlib b/src/Makefile.shlib
index 739033f..f96c709 100644
--- a/src/Makefile.shlib
+++ b/src/Makefile.shlib
@@ -296,6 +296,7 @@ all-shared-lib: $(shlib)
 
 ifndef haslibarule
 $(stlib): $(OBJS) | $(SHLIB_PREREQS)
+   rm -f $@
$(LINK.static) $@ $^
$(RANLIB) $@
 endif #haslibarule
@@ -337,6 +338,7 @@ else # PORTNAME == aix
 
 # AIX case
 $(shlib) $(stlib): $(OBJS) | $(SHLIB_PREREQS)
+   rm -f $(stlib)
$(LINK.static) $(stlib) $^
$(RANLIB) $(stlib)
$(MKLDEXPORT) $(stlib) $(exports_file)
@@ -356,6 +358,7 @@ $(shlib): $(OBJS) | $(SHLIB_PREREQS)
$(CC) $(CFLAGS)  -shared -o $@  $(OBJS) $(LDFLAGS) $(LDFLAGS_SL) 
$(SHLIB_LINK) $(LIBS) $(LDAP_LIBS_BE)
 
 $(stlib): $(OBJS) | $(SHLIB_PREREQS)
+   rm -f $@
$(LINK.static) $@ $^
$(RANLIB) $@
 
diff --git a/src/common/Makefile b/src/common/Makefile
index 372a21b..c71415e 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -43,6 +43,7 @@ uninstall:
rm -f '$(DESTDIR)$(libdir)/libpgcommon.a'
 
 libpgcommon.a: $(OBJS_FRONTEND)
+   rm -f $@
$(AR) $(AROPT) $@ $^
 
 #
@@ -50,6 +51,7 @@ libpgcommon.a: $(OBJS_FRONTEND)
 #
 
 libpgcommon_srv.a: $(OBJS_SRV)
+   rm -f $@
$(AR) $(AROPT) $@ $^
 
 # Because this uses its own compilation rule, it doesn't use the
diff --git a/src/port/Makefile b/src/port/Makefile
index a0908bf..a862d51 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -51,6 +51,7 @@ uninstall:
rm -f '$(DESTDIR)$(libdir)/libpgport.a'
 
 libpgport.a: $(OBJS)
+   rm -f $@
$(AR) $(AROPT) $@ $^
 
 # thread.o needs PTHREAD_CFLAGS (but thread_srv.o does not)
@@ -61,6 +62,7 @@ thread.o: CFLAGS+=$(PTHREAD_CFLAGS)
 #
 
 libpgport_srv.a: $(OBJS_SRV)
+   rm -f $@
$(AR) $(AROPT) $@ $^
 
 # Because this uses its own compilation rule, it doesn't use the

-- 
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] rm static libraries before rebuilding

2015-02-28 Thread Tom Lane 
Noah Misch n...@leadboat.com writes:
 We build static libraries with ar crs or ar cr.  If the static library
 already exists in the build directory, those commands will add new members and
 replace existing members.  They will not remove members present in the
 existing library but not named on the ar command line.  This matters when,
 for example, you rerun ./configure in a way that removes a file from
 $(LIBOBJS).  libpgport carries the obsolete member until make clean.  Let's
 fix that by issuing rm -f before running $(AR).  I would back-patch this.

+1 ... I doubt anybody's thought about this.

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] plpgsql versus domains

2015-02-28 Thread Andrew Gierth 
 Tom == Tom Lane t...@sss.pgh.pa.us writes:

 Tom This is the first attempt at weaponizing the memory context
 Tom reset/delete feature, and I'm fairly happy with it, except for one
 Tom thing: I had to #include utils/memnodes.h into typcache.h in order
 Tom to preserve the intended property that the callback structs could
 Tom be included directly into structs using them.  Now, that's not
 Tom awful in itself, because typcache.h isn't used everywhere; but if
 Tom this feature gets popular we'll find memnodes.h being included
 Tom pretty much everywhere, which is not so great.  I'm thinking about
 Tom moving struct MemoryContextCallback and the extern for
 Tom MemoryContextRegisterResetCallback into palloc.h to avoid this.
 Tom Maybe that's too far in the other direction.  Thoughts?
 Tom Compromise ideas?

This was pretty much my first thought on looking at the callback
patch.  It may seem logical to put the reset callback stuff in
memutils/memnodes alongside context creation and reset and so on, but in
fact the places that are going to want to use callbacks are primarily
_not_ the places that are doing their own context management - since
those could do their own cleanup - but rather places that are allocating
things in contexts supplied by others. So palloc.h is the place for it.

-- 
Andrew (irc:RhodiumToad)


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