date:20141223

Amit,

* Amit Kapila (amit.kapil...@gmail.com) wrote:
 There is a new column added in pg_authid (rolbypassrls)
 and the updation for same is missed in below places:
 
 a. System catalog page for pg_authid
 http://www.postgresql.org/docs/devel/static/catalog-pg-authid.html

Yup, thanks, will fix.

 b. Do we want to add this new column in pg_shadow view?

This was intentionally not done as I had really viewed pg_user, pg_group
and pg_shadow as deprecated and only for backwards-compatibility.
That's certainly why those views were added originally.  On the other
hand, I do see that 'userepl' was added, so we've been keeping it
updated.

I am inclined to suggest that we actually mark pg_user, pg_group, and
pg_shadow as deprecated and planned for removal.  We can't simply remove
them as we haven't actually said that but I don't think we should
continue to carry these pre-8.1 views around forever.  Still, until we
do that, we should keep them updated.

I'll make these changes in the next couple of days (after the role
attribute bitmask patch, as that touches a lot of the same places).

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Initdb-cs_CZ.WIN-1250 buildfarm failures

2014-12-23 Thread Tomas Vondra

On 23.12.2014 09:19, Noah Misch wrote:
 On Sat, Dec 20, 2014 at 07:28:33PM +0100, Tomas Vondra wrote:
 On 20.12.2014 19:05, Tom Lane wrote:
 Locale cs_CZ.WIN-1250 is evidently marked with a codeset property of
 ANSI_X3.4-1968 (which means old-school US-ASCII).  That's certainly
 wrong.  I believe the correct thing would be CP1250.

 Yes. I fixed the locales and added the locales back to the client
 configuration.
 
 Thanks.  These animals are now OK except on REL9_0_STABLE.  The log message of
 commit 2dfa15d explains their ongoing 9.0 failures.  I recommend adding
 --skip-steps=pl-install-check to their 9.0 invocations.  Dropping the affected
 locales is another option, but we benefit from the rare encoding coverage more
 than we benefit from the src/bin/pl coverage.

OK, can do. Something like this in build-farm.conf should work, right?

if ($branch eq 'REL9_0_STABLE')
{
push(@{$conf{config_opts}},--skip-steps=pl-install-check);
}


regards
Tomas


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

Re: [HACKERS] 9.5: Better memory accounting, towards memory-bounded HashAgg

2014-12-23 Thread Tomas Vondra

On 23.12.2014 10:16, Jeff Davis wrote:
 It seems that these two patches are being reviewed together. Should
 I just combine them into one? My understanding was that some wanted
 to review the memory accounting patch separately.

I think we should keep the patches separate. Applying two patches is
trivial, splitting them not so much.

 On Sun, 2014-12-21 at 20:19 +0100, Tomas Vondra wrote:
 That's the only conflict, and after fixing it it compiles OK.
 However, I got a segfault on the very first query I tried :-(
 
 If lookup_hash_entry doesn't find the group, and there's not enough 
 memory to create it, then it returns NULL; but the caller wasn't 
 checking for NULL. My apologies for such a trivial mistake, I was
 doing most of my testing using DISTINCT. My fix here was done
 quickly, so I'll take a closer look later to make sure I didn't miss
 something else.
 
 New patch attached (rebased, as well).
 
 I also see your other message about adding regression testing. I'm 
 hesitant to slow down the tests for everyone to run through this
 code path though. Should I add regression tests, and then remove them
 later after we're more comfortable that it works?

I think when done right, the additional time will be negligible. By
setting the work_mem low, we don't need that much data.

regards
Tomas


-- 
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] compress method for spgist - 2

2014-12-23 Thread Teodor Sigaev


Now that the input data type and leaf data type can be different, which one is
attType? It's the leaf data type, as the patch stands. I renamed that to
attLeafType, and went fixing all the references to it. In most places it's just
a matter of search  replace, but what about the reconstructed datum? In
freeScanStackEntry, we assume that att[Leaf]Type is the datatype for
reconstructedValue, but I believe assume elsewhere that reconstructedValue is of
the same data type as the input. At least if the opclass supports index-only 
scans.



Agree with rename. I doubt that there is a real-world example of datatype which 
can be a) effectivly compressed and b) restored to original form. If so, why 
don't store it in compressed state in database? In GiST all compress methods 
uses one-way compress. In PostGIS example, polygons are compressed into 
bounding box, and, obviously, they cannot be restored.




I think we'll need a separate SpGistTypeDesc for the input type. Or perhaps a
separate SpGistTypeDesc for the reconstructed value and an optional decompress
method to turn the reconstructedValue back into an actual reconstructed input
datum. Or something like that.


I suppose that compress and reconstruct are mutual exclusive options.

--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/


--
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] Role Attribute Bitmask Catalog Representation

Adam Brightwell wrote:
 Alvaro and Stephen,
 
 I propose this patch on top of Adam's v5.  Also included is a full patch
  against master.
 
 
 I have attached an updated patch for review
 (role-attribute-bitmask-v7.patch).
 
 This patch incorporates the 'v5a' patch proposed by Alvaro, input
 validation (Assert) check in 'check_role_attribute' and the documentation
 updates requested by Stephen.

Pushed with a couple of small changes (Catalog.pm complained about the
lack of a CATALOG() line in the new acldefs.h file; you had
pg_role_all_attributes as returning bool in the table, but it returns
text[]; and I added index entries for the new functions.)

-- 
Á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] Proposal VACUUM SCHEMA

On Mon, Dec 22, 2014 at 5:00 PM, Jim Nasby jim.na...@bluetreble.com wrote:
 I would MUCH rather that we find a way to special-case executing
 non-transactional commands dynamically, because VACUUM isn't the only one
 that suffers from this problem.

Is pg_background a solution to this problem?

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

On Mon, Dec 22, 2014 at 5:04 PM, Peter Geoghegan p...@heroku.com wrote:
 If you're dead set on having an escape hatch, maybe we should just get
 over it and add a way of specifying a unique index by name. As I said,
 these under-served use cases are either exceedingly rare or entirely
 theoretical.

I'm decidedly unenthusiastic about that.  People don't expect CREATE
INDEX CONCURRENTLY + DROP INDEX CONCURRENTLY to break their DML.  I
think the solution in this case would be a gateway to problems larger
than the one we're trying to solve.

-- 
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] Final Patch for GROUPING SETS

On Mon, Dec 22, 2014 at 6:57 PM, Andrew Gierth
and...@tao11.riddles.org.uk wrote:
 In the case of cube(a,b,c,d), our code currently gives:

 b,d,a,c:  (b,d,a,c),(b,d)
 a,b,d:(a,b,d),(a,b)
 d,a,c:(d,a,c),(d,a),(d)
 c,d:  (c,d),(c)
 b,c,d:(b,c,d),(b,c),(b)
 a,c,b:(a,c,b),(a,c),(a),()

That's pretty cool.

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


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

Re: [HACKERS] Proposal VACUUM SCHEMA

On Mon, Dec 22, 2014 at 11:51 AM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Multi-table CLUSTER uses multiple transactions, so this should not be an
 issue.  That said, I don't think there's much point in CLUSTER SCHEMA,
 much less TRUNCATE SCHEMA.  Do you normally organize your schemas so
 that there are some that contain only tables that need to be truncated
 together?  That would be a strange use case.

 Overall, this whole line of development seems like bloating the parse
 tables for little gain.

We added REINDEX SCHEMA less than three weeks ago; if we accept that
that was a good change, but think this is a bad one, it's not clear to
me that there is any guiding principle here beyond who happened to
weigh in on which threads.

-- 
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] [bug fix or improvement?] Correctly place DLLs for ECPG apps in bin folder

2014-12-23 Thread Michael Paquier

On Wed, Jul 9, 2014 at 9:53 PM, MauMau maumau...@gmail.com wrote:
 But the source tree contains as many as 206 Makefiles.  I'm worried that it
 will waste the install time.  Should all these Makefiles be examined, or 16
 Makefiles in src/interfaces/?
Not really. IMO the best solution is to extract the path of the
Makefile in the vcxproj file of the project in Install.pm by looking
at ModuleDefinitionFile or ResourceFile, and then to scan it for
SO_MAJOR_VERSION. This way it is possible to determine in
CopySolutionOutput where a library needs to be copied. I'd also rather
go with the def file, and not the rc file for the path identification.
-- 
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] Initdb-cs_CZ.WIN-1250 buildfarm failures



On 12/23/2014 07:14 AM, Tomas Vondra wrote:

On 23.12.2014 09:19, Noah Misch wrote:

On Sat, Dec 20, 2014 at 07:28:33PM +0100, Tomas Vondra wrote:

On 20.12.2014 19:05, Tom Lane wrote:

Locale cs_CZ.WIN-1250 is evidently marked with a codeset property of
ANSI_X3.4-1968 (which means old-school US-ASCII).  That's certainly
wrong.  I believe the correct thing would be CP1250.

Yes. I fixed the locales and added the locales back to the client
configuration.

Thanks.  These animals are now OK except on REL9_0_STABLE.  The log message of
commit 2dfa15d explains their ongoing 9.0 failures.  I recommend adding
--skip-steps=pl-install-check to their 9.0 invocations.  Dropping the affected
locales is another option, but we benefit from the rare encoding coverage more
than we benefit from the src/bin/pl coverage.

OK, can do. Something like this in build-farm.conf should work, right?

if ($branch eq 'REL9_0_STABLE')
{
 push(@{$conf{config_opts}},--skip-steps=pl-install-check);
}




No, config_opts is what's passed to configure. Try something like:

if ($branch eq 'REL9_0_STABLE')
{
$skip_steps{'pl-install-check'} = 1;
}

cheers

andrew


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

Re: [HACKERS] mysql with postgres



On 12/23/2014 04:36 AM, Ravi Kiran wrote:

hi all,


Is postgres source code compatible with mysql database?? If it is, 
could someone could give me some links so that I can do that.


I want to hack into the postgres source code, but as I am comfortable 
with mysql, I want to use the mysql database not postgres.


any references would be fine.





No. That's like asking if Unix is compatible with Windows, because you 
want to hack on Unix but you're comfortable with windows and want to use it.


If you want to use mysql you should hack on it. If you want to use 
postgres hack on it. But you can't mix those two.


cheers

andrew


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

Re: [HACKERS] Initdb-cs_CZ.WIN-1250 buildfarm failures

2014-12-23 Thread Tomas Vondra

On 23.12.2014 15:21, Andrew Dunstan wrote:
 
 No, config_opts is what's passed to configure. Try something like:
 
 if ($branch eq 'REL9_0_STABLE')
 {
 $skip_steps{'pl-install-check'} = 1;
 }

Applied to all three animals.

Tomas


-- 
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] pgbench -f and vacuum

On Mon, Dec 22, 2014 at 6:55 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Here's a completely different idea.  How about we add an option that
 means vacuum this table before running the test (can be given several
 times); by default the set of vacuumed tables is the current pgbench_*
 list, but if -f is specified then the default set is cleared.  So if you
 have a -f script and want to vacuum the default tables, you're forced to
 give a few --vacuum-table=foo options.  But this gives the option to
 vacuum some other table before the test, not just the pgbench default
 ones.

Well, really, you might want arbitrary initialization steps, not just
vacuums.  We could have --init-script=FILENAME.  Although that might
be taking this thread rather far off-topic.

-- 
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 VACUUM SCHEMA

2014-12-23 Thread Fabrízio de Royes Mello

On Mon, Dec 22, 2014 at 8:02 PM, Jim Nasby jim.na...@bluetreble.com wrote:

 On 12/21/14, 8:55 PM, Fabrízio de Royes Mello wrote:

And why that, but not
say schema-wide ANALYZE, CLUSTER, TRUNCATE, ...
   

 +1. I can write patches for each of this maintenance statement
too.


 If we're going to go that route, then perhaps it would make more
sense to create a command that allows you to apply a second command to
every object in a schema. We would have to be careful about
PreventTransactionChain commands.


   Sorry but I don't understand what you meant. Can you explain more
about your idea?


 There's a very large number of commands that could be useful to execute
on every object in a schema. (RE)INDEX, CLUSTER, ALTER come to mind besides
VACUUM.


ANALYZE too...



 Right now a lot of people just work around this with things like DO
blocks, but as mentioned elsewhere in the thread that fails for commands
that can't be in a transaction.


I use dblink to solve it. :-)


Regards,

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

Re: [HACKERS] Role Attribute Bitmask Catalog Representation

2014-12-23 Thread Adam Brightwell

Alvarao,


 Pushed with a couple of small changes (Catalog.pm complained about the
 lack of a CATALOG() line in the new acldefs.h file; you had
 pg_role_all_attributes as returning bool in the table, but it returns
 text[]; and I added index entries for the new functions.)


That's fantastic! Thanks, I appreciate it!

-Adam


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

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

Alvaro Herrera alvhe...@alvh.no-ip.org writes:
 Use a bitmask to represent role attributes
 The previous representation using a boolean column for each attribute
 would not scale as well as we want to add further attributes.

 Extra auxilliary functions are added to go along with this change, to
 make up for the lost convenience of access of the old representation.

I have to apologize for not having paid more attention, but ... is this
*really* such a great idea?  You've just broken any client-side code
that looks directly at pg_authid.  Moreover, I don't particularly buy
the idea that this somehow insulates us from the compatibility costs of
adding new role properties: you're still going to have to add columns to
the pg_roles view, and adjust clients that look at that, every time.
Replacing bool-column accesses with bitmask manipulation doesn't seem
like it's a win on a micro-optimization level either, certainly not for
SQL-level coding where you've probably made it two orders of magnitude
more expensive.  And lastly, what happens when you run out of bits in
that bigint column?

Again, I suppose I should have objected earlier, but I really seriously
doubt that this is a good idea.

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] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

Tom Lane wrote:

 Again, I suppose I should have objected earlier, but I really seriously
 doubt that this is a good idea.

Ugh.  I thought we had a consensus that this was the accepted way
forward; that's my reading of the old thread,
http://www.postgresql.org/message-id/flat/20141016133218.gw28...@tamriel.snowman.net#20141016133218.gw28...@tamriel.snowman.net

Breaking clients was considered acceptable, which is why some of these
functions were introduced.  There were some differing opinions; Simon
for instance suggested the use of an array rather than a bitmask, but
that would have broken clients all the same.

If there's strong opposition to this whole line of development, I can
revert.  Anyone else wants to give an opinion?

-- 
Á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] Proposal VACUUM SCHEMA

Robert Haas wrote:
 On Mon, Dec 22, 2014 at 11:51 AM, Alvaro Herrera
 alvhe...@2ndquadrant.com wrote:
  Multi-table CLUSTER uses multiple transactions, so this should not be an
  issue.  That said, I don't think there's much point in CLUSTER SCHEMA,
  much less TRUNCATE SCHEMA.  Do you normally organize your schemas so
  that there are some that contain only tables that need to be truncated
  together?  That would be a strange use case.
 
  Overall, this whole line of development seems like bloating the parse
  tables for little gain.
 
 We added REINDEX SCHEMA less than three weeks ago; if we accept that
 that was a good change, but think this is a bad one, it's not clear to
 me that there is any guiding principle here beyond who happened to
 weigh in on which threads.

I didn't think much of REINDEX SCHEMA, TBH.

-- 
Á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] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

* Tom Lane (t...@sss.pgh.pa.us) wrote:
 Alvaro Herrera alvhe...@alvh.no-ip.org writes:
  Use a bitmask to represent role attributes
  The previous representation using a boolean column for each attribute
  would not scale as well as we want to add further attributes.
 
  Extra auxilliary functions are added to go along with this change, to
  make up for the lost convenience of access of the old representation.
 
 I have to apologize for not having paid more attention, but ... is this
 *really* such a great idea?  You've just broken any client-side code
 that looks directly at pg_authid.

Anything which looks at pg_authid for the relevant columns needs to be
updated for the changes which were made for rolreplication previously,
and now rolbypassrls and any other role attributes added.  While I agree
that it's something to consider (and even mentioned it earlier in the
thread..), there wasn't any argument about it from those who were
following the discussion.

Perhaps we could have gone out of our way to ask the pgAdmin authors and
other client-side utilities which look at these columns if this will
more issues than new columns do.

 Moreover, I don't particularly buy
 the idea that this somehow insulates us from the compatibility costs of
 adding new role properties: you're still going to have to add columns to
 the pg_roles view, and adjust clients that look at that, every time.

That's correct, however, this change wasn't intended to insulate anyone
from those compatibility changes but rather to make better use of the
bytes in each pg_authid record.  We were already up to 8 individual bool
columns, wasting space for each.

 Replacing bool-column accesses with bitmask manipulation doesn't seem
 like it's a win on a micro-optimization level either, certainly not for
 SQL-level coding where you've probably made it two orders of magnitude
 more expensive.

I agree that it's more expensive for SQL users, but it's not our first
use of a bitmask in the catalog.  Perhaps this is more user-visible than
other use-cases but we also provide views to address common usages in
this case already, where we don't in other situations.

 And lastly, what happens when you run out of bits in
 that bigint column?

We can add another, though this seems unlikely to happen given the
previous discussion and review of what additional role attributes would
be nice to add.  There are some scenarios I've considered which would
lead to using perhaps another 15-20, but 64 sure seems far off.

 Again, I suppose I should have objected earlier, but I really seriously
 doubt that this is a good idea.

I tried to solicit discussion about these concerns earlier but we're all
busy, no problem.

For my 2c, I do think this is the right direction to go in as adding
another 15 boolean columns to pg_authid definitely strikes me as a bad
idea, but we can certainly discuss the trade-offs.  Another proposal
(from Simon, as I recall) was to use an array.  That runs into nearly
all the same problems and the on-disk representation ends up being even
larger, which is why I didn't see that being a good idea.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

Alvaro Herrera alvhe...@2ndquadrant.com writes:
 Tom Lane wrote:
 Again, I suppose I should have objected earlier, but I really seriously
 doubt that this is a good idea.

 Ugh.  I thought we had a consensus that this was the accepted way
 forward; that's my reading of the old thread,
 http://www.postgresql.org/message-id/flat/20141016133218.gw28...@tamriel.snowman.net#20141016133218.gw28...@tamriel.snowman.net

I was aware that we were thinking of introducing a bunch more role
attributes, but I'm wondering what's the rationale for assuming that
(a) they'll all be booleans, and (b) there will never, ever, be more
than 64 of them.  The argument that lots of boolean columns won't
scale nicely doesn't seem to lead to the conclusion that a fixed-size
bitmap is better.

I'd have gone with just adding more bool columns as needed.

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] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

On Tue, Dec 23, 2014 at 10:26 AM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Tom Lane wrote:
 Again, I suppose I should have objected earlier, but I really seriously
 doubt that this is a good idea.

 Ugh.  I thought we had a consensus that this was the accepted way
 forward; that's my reading of the old thread,
 http://www.postgresql.org/message-id/flat/20141016133218.gw28...@tamriel.snowman.net#20141016133218.gw28...@tamriel.snowman.net

 Breaking clients was considered acceptable, which is why some of these
 functions were introduced.  There were some differing opinions; Simon
 for instance suggested the use of an array rather than a bitmask, but
 that would have broken clients all the same.

 If there's strong opposition to this whole line of development, I can
 revert.  Anyone else wants to give an opinion?

I would have preferred (and I believe argued for) keeping the existing
catalog representation for existing attributes and using a bitmask for
new ones, to avoid breaking client code.  But I am not sure if that's
actually the best decision.  I find Tom's concern about needing more
than 64 attributes to be ill-founded; I can't really see that
happening on any timescale that matters.

-- 
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] pgbench -f and vacuum

Robert Haas wrote:
 On Mon, Dec 22, 2014 at 6:55 PM, Alvaro Herrera
 alvhe...@2ndquadrant.com wrote:
  Here's a completely different idea.  How about we add an option that
  means vacuum this table before running the test (can be given several
  times); by default the set of vacuumed tables is the current pgbench_*
  list, but if -f is specified then the default set is cleared.  So if you
  have a -f script and want to vacuum the default tables, you're forced to
  give a few --vacuum-table=foo options.  But this gives the option to
  vacuum some other table before the test, not just the pgbench default
  ones.
 
 Well, really, you might want arbitrary initialization steps, not just
 vacuums.  We could have --init-script=FILENAME.

Init (pgbench -i) is the step that creates the tables and populates
them, so I think this would need a different name, maybe startup, but
otherwise yeah.

 Although that might be taking this thread rather far off-topic.

Not really sure about that, because the only outstanding objection to
this discussion is what happens in the startup stage if you specify -f.
Right now vacuum is attempted on the standard tables, which is probably
not the right thing in the vast majority of cases.  But if we turn that
off, how do we reinstate it for the rare cases that want it?  Personally
I would just leave it turned off and be done with it, but if we want to
provide some way to re-enable it, this --startup-script=FILE gadget
sounds like a pretty decent idea.

-- 
Á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] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

On 2014-12-23 10:40:15 -0500, Robert Haas wrote:
On Tue, Dec 23, 2014 at 10:26 AM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
Tom Lane wrote:
Again, I suppose I should have objected earlier, but I really seriously
doubt that this is a good idea.

Ugh. I thought we had a consensus that this was the accepted way
forward; that's my reading of the old thread,
http://www.postgresql.org/message-id/flat/20141016133218.gw28...@tamriel.snowman.net#20141016133218.gw28...@tamriel.snowman.net

Breaking clients was considered acceptable, which is why some of these
functions were introduced. There were some differing opinions; Simon
for instance suggested the use of an array rather than a bitmask, but
that would have broken clients all the same.

If there's strong opposition to this whole line of development, I can
revert. Anyone else wants to give an opinion?

I would have preferred (and I believe argued for) keeping the existing
catalog representation for existing attributes and using a bitmask for
new ones, to avoid breaking client code. But I am not sure if that's
actually the best decision.

I personally think in this case the clear break is slightly better than
having different styles of representation around for a long while.

I find Tom's concern about needing more
than 64 attributes to be ill-founded; I can't really see that
happening on any timescale that matters.

I personally would prefer a 'custom' type to represent the
permissions. Internally that could very well be current bitmask, but the
external representation could be more complex (i.e. some textual
representation). That'd make it easy to make the representation wider/more
complex if needed.

Greetings,

Andres Freund

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

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

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

Stephen Frost sfr...@snowman.net writes:
 * Tom Lane (t...@sss.pgh.pa.us) wrote:
 I have to apologize for not having paid more attention, but ... is this
 *really* such a great idea?  You've just broken any client-side code
 that looks directly at pg_authid.

 Anything which looks at pg_authid for the relevant columns needs to be
 updated for the changes which were made for rolreplication previously,
 and now rolbypassrls and any other role attributes added.

Right.  95% of the compatibility costs of adding a property are going to
be sunk in any case because clients will need to know what flags exist,
how to spell them in CREATE/ALTER USER commands, etc.  (Some of this could
be alleviated perhaps if we invented a server-side function that produced
a text string representing all of a user's properties, but that's quite
orthogonal to this patch.)

 That's correct, however, this change wasn't intended to insulate anyone
 from those compatibility changes but rather to make better use of the
 bytes in each pg_authid record.  We were already up to 8 individual bool
 columns, wasting space for each.

You're really seriously concerned about a couple of dozen bytes per role?
That is micro-optimization of the very worst sort.  We are routinely
wasting multiples of that on things like using name rather than a
variable-length text representation for name columns, and I don't think
anyone is especially concerned about that anymore.  Maybe back in the
nineties it'd have been worth bit-shaving that way.

To me, a bitmask might make sense if the properties could usefully be
manipulated as a unit, but I'm not seeing any such advantage here.

 For my 2c, I do think this is the right direction to go in as adding
 another 15 boolean columns to pg_authid definitely strikes me as a bad
 idea, but we can certainly discuss the trade-offs.

Meh.  To the extent that users look at pg_roles rather than pg_authid,
it's going to look like another 15 boolean columns to them anyway ...
except that now, those columns are suddenly a lot more expensive to read.

15-20 more bool columns sounds entirely fine to me.  If we were talking
a couple of hundred, I'd start to worry; but this approach would not
handle that very well either.

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] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

Robert Haas robertmh...@gmail.com writes:
 I would have preferred (and I believe argued for) keeping the existing
 catalog representation for existing attributes and using a bitmask for
 new ones, to avoid breaking client code.  But I am not sure if that's
 actually the best decision.  I find Tom's concern about needing more
 than 64 attributes to be ill-founded; I can't really see that
 happening on any timescale that matters.

I tend to agree, which is why I'm questioning the decision to not just
keep adding bool columns.  I don't see how that's not both more convenient
and less surprising.

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] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

* Tom Lane (t...@sss.pgh.pa.us) wrote:
Alvaro Herrera alvhe...@2ndquadrant.com writes:
Tom Lane wrote:
Again, I suppose I should have objected earlier, but I really seriously
doubt that this is a good idea.

I was aware that we were thinking of introducing a bunch more role
attributes, but I'm wondering what's the rationale for assuming that
(a) they'll all be booleans,

I attempted to do a comprehensive review of the case by looking at all
of the existing superuser checks and considering where it made sense to
make things more flexible. The result was specifically that they were
all boolean cases except for the previously-discussed 'create directory'
idea. Had there been other cases which weren't boolean, I would have
been looking for another representation.

That said, this does not wall-off additional columns going into
pg_authid later, if there are any non-boolean cases which would make
sense. Those cases, I suspect, would lead to new catalog tables in
their own right anyway though, rather than additional columns in
pg_authid.

The 'create directory' idea certainly made more sense to me with a new
catalog table. Having an array of directories in pg_authid was never
considered, though I did consider adding a catalog table which was
essentially role+key/value where 'value' was an arbitrary string or
perhaps bytea blob, but all the boolean cases would have gone into
pg_authid and that new catalog would have only been used for the
'directory' case (or at least, I couldn't find any other use-cases for
it in my review).

and (b) there will never, ever, be more
than 64 of them.

I do not think this approach walls off adding more than 64. On the
other hand, I don't like the idea of doubling the size of pg_authid if
we do get to 64 because we really want to use boolean columns instead of
a bitmap.

The argument that lots of boolean columns won't
scale nicely doesn't seem to lead to the conclusion that a fixed-size
bitmap is better.

Perhaps it's because I just recently came out of an environment where we
were constantly fighting with size issues for boolean values, but
boolean columns definitely don't scale nicely. I admit that we still
have larger issues when it comes to running databases with lots of roles
(which means people tend to not do it), such as being unable to
partition catalog tables, but I'm still hopefull we'll one day get to a
point where more users can exist as real database users (and therefore
have the database enforcing the permission system, rather than each
application having to write its own) and I'd rather not have pg_authid
bloated without cause.

I'd have gone with just adding more bool columns as needed.

I don't think I was the only one concerned that adding a bunch of new
columns would bloat the size of pg_authid and the C structure behind it,
but I'm not remembering offhand who else considered it.

Thanks,

Stephen

signature.asc
Description: Digital signature

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes


On 12/23/2014 04:46 PM, Andres Freund wrote:

[snip]
I find Tom's concern about needing more
than 64 attributes to be ill-founded; I can't really see that
happening on any timescale that matters.


Hmm... most probably, not (or so I hope)... Unless we begin to add many 
differerent capabilities, like it was recently suggested.
I, for one, have at least two of them to propose, but I guess not that 
many more should be needed.



I personally would prefer a 'custom' type to represent the
permissions. Internally that could very well be current bitmask, but the
external representation could be more complex (i.e. some textual
representation). That'd make it easy to make the representation wider/more
complex if needed.


Indeed, though this would imply adding a new bitstring? type to core 
Postgres.
Do you have any further input on what this type would look like ? Any 
operators that might be useful? ISTM that this would actually be the 
greatest strength of a type proper (vs. hardcoded bit-wise operations 
in core)


In any case, having the type's input/output perform the conversion 
from/to text is quite equivalent to the current implementation. 
Considering that this custom type would need to be in core, the 
differences should be minimal.

Or am I missing something obvious?

Thanks,

/ J.L.



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

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

On Tue, Dec 23, 2014 at 10:51 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Meh.  To the extent that users look at pg_roles rather than pg_authid,
 it's going to look like another 15 boolean columns to them anyway ...
 except that now, those columns are suddenly a lot more expensive to read.

Ugh.  I think that's actually a really good point.  I guess I'll +1
reverting this, then.

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


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

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

Stephen Frost sfr...@snowman.net writes:
 * Tom Lane (t...@sss.pgh.pa.us) wrote:
 I'd have gone with just adding more bool columns as needed.

 I don't think I was the only one concerned that adding a bunch of new
 columns would bloat the size of pg_authid and the C structure behind it,
 but I'm not remembering offhand who else considered it.

Lessee, as of 9.4 pg_authid required 76 bytes per row, plus row header
overhead that'd have probably pushed it to 104 bytes per row (more if
you had non-null rolpassword or rolvaliduntil).  If we add as many as 20
more booleans we'd be at 124 bytes per row, whereas with this approach
we'd have, well, 104 bytes per row.  I'm not seeing much benefit to
justify such a drastic change of approach.

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] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

* Andres Freund (and...@2ndquadrant.com) wrote:
 On 2014-12-23 10:40:15 -0500, Robert Haas wrote:
  I would have preferred (and I believe argued for) keeping the existing
  catalog representation for existing attributes and using a bitmask for
  new ones, to avoid breaking client code.  But I am not sure if that's
  actually the best decision.
 
 I personally think in this case the clear break is slightly better than
 having different styles of representation around for a long while.

Yes, I'm completely with Andres on this point.  Having a mixed case
where there are some boolean columns and then a bitmask strikes as the
worst approach- it doesn't save anyone from the client-side code changes
but rather makes them have to consider both ways and keep an internal
list somewhere of which ones are in boolean columns and which are in the
bitmask, yuck.

  I find Tom's concern about needing more
  than 64 attributes to be ill-founded; I can't really see that
  happening on any timescale that matters.
 
 I personally would prefer a 'custom' type to represent the
 permissions. Internally that could very well be current bitmask, but the
 external representation could be more complex (i.e. some textual
 representation). That'd make it easy to make the representation wider/more
 complex if needed.

In some ways, I feel like this is what we actually have now..  If you
consider pg_authid to be 'internal' and pg_roles to be 'external'.  That
said, I'm not against what you're proposing, but at the same time I'm
not quite sure what that would end up looking like or how difficult it
would be to support a complex type in the catalog and I don't think
there's any way it would address the on-disk size concern, and if we
have to start having a different C-level representation for pg_authid
than the on-disk representation, well, that strikes me as a lot more
complicated too..

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

José Luis Tallón wrote:
 On 12/23/2014 04:46 PM, Andres Freund wrote:

 I personally would prefer a 'custom' type to represent the
 permissions. Internally that could very well be current bitmask, but the
 external representation could be more complex (i.e. some textual
 representation). That'd make it easy to make the representation wider/more
 complex if needed.
 
 Indeed, though this would imply adding a new bitstring? type to core
 Postgres.

We already have varlena bitstrings, in the guise of types bit and
varbit.

 Do you have any further input on what this type would look like ? Any
 operators that might be useful? ISTM that this would actually be the
 greatest strength of a type proper (vs. hardcoded bit-wise operations in
 core)

I imagine something like the reg* types (regclass, regtype etc): on
input you can pass them an OID, or an possibly-qualified object name;
internally they store the OID.  You can cast them to OID to obtain the
numerical value, or just print them out to get the possibly-qualified
name.

In the case at hand, on output you would get the equivalent of the
text[] you get from pg_role_all_attributes(), and you can input it in
the same way or you can input the bitmask; and the underlying storage is
the bitmask.

This doesn't solve the client compatibility break, or the issue that
querying pg_roles is expensive.

-- 
Á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] bin checks taking too long.



On 12/22/2014 10:41 PM, Peter Eisentraut wrote:

On 12/22/14 7:56 PM, Andrew Dunstan wrote:

Currently the buildfarm animal crake (my development instance) is
running the bin check, but not any other animal. These tests still take
for too long, not least because each set of tests requires a separate
install.

You can avoid that by using make installcheck.



Not working too well:

   make -C pg_ctl installcheck
   make[1]: Entering directory
   `/home/bf/bfr/root/HEAD/pgsql.build/src/bin/pg_ctl'
   cd /home/bf/bfr/root/HEAD/pgsql.build/../pgsql/src/bin/pg_ctl 
   TESTDIR='/home/bf/bfr/root/HEAD/pgsql.build/src/bin/pg_ctl'
   PATH=/home/bf/bfr/root/HEAD/inst/bin:$PATH PGPORT='65648' prove -I
   /home/bf/bfr/root/HEAD/pgsql.build/../pgsql/src/test/perl/ --verbose
   t/*.pl
   Use of uninitialized value $ENV{top_builddir} in concatenation (.)
   or string at t/001_start_stop.pl line 17.
   file not found: /src/test/regress/pg_regress at
   /home/bf/bfr/root/HEAD/pgsql.build/../pgsql/src/test/perl/TestLib.pm
   line 142.

This was invoked like this:

   cd $pgsql/src/bin  make NO_LOCALE=1 installcheck

Note that it's a vpath build.


cheers

andrew




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

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

* Robert Haas (robertmh...@gmail.com) wrote:
 On Tue, Dec 23, 2014 at 10:51 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  Meh.  To the extent that users look at pg_roles rather than pg_authid,
  it's going to look like another 15 boolean columns to them anyway ...
  except that now, those columns are suddenly a lot more expensive to read.
 
 Ugh.  I think that's actually a really good point.  I guess I'll +1
 reverting this, then.

If that's the only consideration for this, well, that's certainly quite
straight-forward to change in the other direction too.  The new function
suggested by Andres actually makes it really easy to get a textual list
of all the role attributes which a role has from the bitmask too.  I was
more concerned with the on-disk and C-level structure and size than
about the time required to get at the value of each bit at the
SQL-level.

Thanks,

Stephen


signature.asc
Description: Digital signature

[HACKERS] Proposal: two new role attributes and/or capabilities?


Hello,

I've found myself needing two role capabilities? as of lately, when 
thinking about restricting some roles to the barely minimum allowed 
permissions needed to perform their duties ... as opposed to having a 
superuser role devoted to these task.


The capabilities would be:
* MAINTENANCE --- Ability to run
VACUUM [ANALYZE | FREEZE] (but not VACUUM FULL),
ANALYZE (including SET LOCAL statistics_target TO 1),
REINDEX CONCURRENTLY  (but not the blocking, regular, one)
REFRESH MATERIALIZED VIEW CONCURRENTLY (but not the blocking one)
COPY ???

Rationale: delegate the routine maintenance tasks to a low 
privilege role, which can't do harm (apart from some performance 
degradation) --- hence the no exclusive locking operations requirement.


* IMPERSONATE --- Ability to do SET AUTHORIZATION TO some_role; and 
RESET AUTHORIZATION
This might be further refined to provide a way to say This role is 
authorized to impersonate role1 but no other


Rationale: for use by connection poolers (esp. pgBouncer), where 
the role used for connection would only have the LOGIN and IMPERSONATE 
privileges. The remaining operations would be authorized against the 
supplanted role (i.e. ability to create tables/indexes or views, perform 
DML and/or DDL, etc)

AFAIK, a superuser role is needed for this purpose currently.


The relevant code is quite simple and looks like it could be very 
useful. Any suggestions / input on this?
I can certainly prepare a patch for this (bear with me, It'll be my 
first here), and I'm willing to include more features if deemed useful.




Regards,

/ J.L.



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

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

* Tom Lane (t...@sss.pgh.pa.us) wrote:
 Stephen Frost sfr...@snowman.net writes:
  * Tom Lane (t...@sss.pgh.pa.us) wrote:
  I'd have gone with just adding more bool columns as needed.
 
  I don't think I was the only one concerned that adding a bunch of new
  columns would bloat the size of pg_authid and the C structure behind it,
  but I'm not remembering offhand who else considered it.
 
 Lessee, as of 9.4 pg_authid required 76 bytes per row, plus row header
 overhead that'd have probably pushed it to 104 bytes per row (more if
 you had non-null rolpassword or rolvaliduntil).  If we add as many as 20
 more booleans we'd be at 124 bytes per row, whereas with this approach
 we'd have, well, 104 bytes per row.  I'm not seeing much benefit to
 justify such a drastic change of approach.

I suppose.  I didn't consider it to be a terribly drastic change but
rather simply using a better representation for a mostly-internal bit of
data.  It also lended itself pretty nicely to maniuplation (at least,
imv, the code is a lot cleaner with the bitmask, but it's not a huge
deal).  Guess I had been expecting concerns to be raised around adding
many more bytes where there wouldn't have been.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Proposal: two new role attributes and/or capabilities?

* José Luis Tallón (jltal...@adv-solutions.net) wrote:
 I've found myself needing two role capabilities? as of lately,
 when thinking about restricting some roles to the barely minimum
 allowed permissions needed to perform their duties ... as opposed to
 having a superuser role devoted to these task.

Excellent.  We've been looking at the same considerations.

 The capabilities would be:
 * MAINTENANCE --- Ability to run
 VACUUM [ANALYZE | FREEZE] (but not VACUUM FULL),
 ANALYZE (including SET LOCAL statistics_target TO 1),

There's likely to be discussion about these from the perspective that
you really shouldn't need to run them all that much.  Why isn't
autovacuum able to handle this?

 REINDEX CONCURRENTLY  (but not the blocking, regular, one)
 REFRESH MATERIALIZED VIEW CONCURRENTLY (but not the blocking one)

These are interesting, but would these make sense at the role level?
Both of these commands explicitly take specific relations to operate
against, after all.

 COPY ???

The question around this one goes back to the CREATE DIRECTORY
discussion that happened this fall.  I'm still hopeful that we can do
*something* in this area, but I'm not sure what that's going to end up
looking like.  The problem with COPY is that it's either trivial to use
it to become a superuser, or insanely difficult to secure sufficiently.

 Rationale: delegate the routine maintenance tasks to a low
 privilege role, which can't do harm (apart from some performance
 degradation) --- hence the no exclusive locking operations
 requirement.

This makes sense for the reindex/refresh cases, though no harm might
be over-stating it.

 * IMPERSONATE --- Ability to do SET AUTHORIZATION TO some_role;
 and RESET AUTHORIZATION
 This might be further refined to provide a way to say This role
 is authorized to impersonate role1 but no other
 Rationale: for use by connection poolers (esp. pgBouncer), where
 the role used for connection would only have the LOGIN and
 IMPERSONATE privileges. The remaining operations would be authorized
 against the supplanted role (i.e. ability to create tables/indexes
 or views, perform DML and/or DDL, etc)
 AFAIK, a superuser role is needed for this purpose currently.

No..  You can have 'no-inherit' roles which you can use for exactly this
purpose.  The initial login role can have no rights on the database,
except to SET ROLE to other roles which have been granted to it.

You should never have your pgBouncer or other pooling connection logging
in as a superuser.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

Stephen Frost sfr...@snowman.net writes:
 If that's the only consideration for this, well, that's certainly quite
 straight-forward to change in the other direction too.  The new function
 suggested by Andres actually makes it really easy to get a textual list
 of all the role attributes which a role has from the bitmask too.

We could have that regardless of the representation, if the function is
defined along the lines of given a user OID, give me a text string
representing the user's attributes.  However, that only helps for
pg_dumpall and any other clients whose requirement is exactly satisfied
by a string that fits into CREATE/ALTER USER.  The current formatting
of psql's \du, for example, absolutely requires adding more client-side
code every time we add a property; whether the catalog representation is
bools or a bitmask really isn't going to change the pain level much there.

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] [PATCH] explain sortorder

2014-12-23 Thread Mike Blackwell

Looking forward to the new patch.  I'll give it a more complete testing
when you post it.

Mike

__
*Mike Blackwell | Technical Analyst, Distribution Services/Rollout
Management | RR Donnelley*
1750 Wallace Ave | St Charles, IL 60174-3401
Office: 630.313.7818
mike.blackw...@rrd.com
http://www.rrdonnelley.com


http://www.rrdonnelley.com/
* mike.blackw...@rrd.com*

On Tue, Dec 23, 2014 at 5:11 AM, Arne Scheffer 
arne.schef...@uni-muenster.de wrote:

 Heikki Linnakangas hlinnakangas(at)vmware(dot)com writes:
  I would suggest just adding the information to the Sort Key line. As
  long as you don't print the modifiers when they are defaults (ASC and
  NULLS LAST), we could print the information even in non-VERBOSE mode.

 +1.  I had assumed without looking that that was what it did already,
 else I'd have complained too.

regards, tom lane

 We will change the patch according to Heikkis suggestions.

 A nice Christmas  all the best in the New Year

 Arne Scheffer

 http://www.uni-muenster.de/ZIV/Mitarbeiter/ArneScheffer.shtml

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

On Tue, Dec 23, 2014 at 11:34:09AM -0500, Tom Lane wrote:
 Stephen Frost sfr...@snowman.net writes:
  If that's the only consideration for this, well, that's certainly quite
  straight-forward to change in the other direction too.  The new function
  suggested by Andres actually makes it really easy to get a textual list
  of all the role attributes which a role has from the bitmask too.
 
 We could have that regardless of the representation, if the function is
 defined along the lines of given a user OID, give me a text string
 representing the user's attributes.  However, that only helps for
 pg_dumpall and any other clients whose requirement is exactly satisfied
 by a string that fits into CREATE/ALTER USER.  The current formatting
 of psql's \du, for example, absolutely requires adding more client-side
 code every time we add a property; whether the catalog representation is
 bools or a bitmask really isn't going to change the pain level much there.

I am with Tom on this --- there is more wasted space in the 'name'
column pg_authid.rolname than by shoving 40 boolean values into a
bitmap.  Adding the complexity of a bitmap doesn't make sense here.  I
also apologize for the late feedback.

Offtopic, what I would really _love_ to see improved is our display of
object permissions:

 Access privileges
 Schema |  Name  | Type  | Access privileges | Column 
privileges | Policies

++---+---+---+--
 public | crypto | table | postgres=arwdDxt/postgres+|  
 |
||   | =r/postgres   |  
 |

That is nasty user display ---  it looks like line noise.

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

  + Everyone has their own god. +


-- 
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: two new role attributes and/or capabilities?


On 12/23/2014 05:29 PM, Stephen Frost wrote:

* José Luis Tallón (jltal...@adv-solutions.net) wrote:

 I've found myself needing two role capabilities? as of lately,
when thinking about restricting some roles to the barely minimum
allowed permissions needed to perform their duties ... as opposed to
having a superuser role devoted to these tasks.

Excellent.  We've been looking at the same considerations.


 The capabilities would be:
* MAINTENANCE --- Ability to run
 VACUUM [ANALYZE | FREEZE] (but not VACUUM FULL),
 ANALYZE (including SET LOCAL statistics_target TO 1),

There's likely to be discussion about these from the perspective that
you really shouldn't need to run them all that much.  Why isn't
autovacuum able to handle this?


For some (arguably, ill-devised) use cases of INSERT - SELECT aggregate 
- DELETE (third party, closed-source app, massive insert rate) at the 
very least, autovacuum can't possibly cope with the change rate in some 
tables, given that there are quite many other interactive queries running.


Manually performing VACUUM / VACUUM ANALYZE on the (few) affected tables 
every 12h or so fixes the performance problem for the particular queries 
without impacting the other users too much --- the tables and indexes in 
question have been moved to a separate tablespace/disk volume of their own.



In short, this addresses situations where some tables have a much higher 
update rate than the rest of the database so that performance degrades 
with time --- the application became unusable after about 6 days' worth 
of updates until the manual vacuums were setup



 REINDEX CONCURRENTLY  (but not the blocking, regular, one)
 REFRESH MATERIALIZED VIEW CONCURRENTLY (but not the blocking one)

These are interesting, but would these make sense at the role level?
Both of these commands explicitly take specific relations to operate
against, after all.


Yup. Let's imagine a cron job invoking psql in order to perform 
maintenance routine.
The particular command(s) can be generated on-the-fly by querying the 
catalog and then send them in one go to be run sequentially by the one 
backend as a crude form of rate limiting/quality-of-service of sorts 
(renice -p or even ionice -p seems quite inadequate).


This automation becomes impossible to do if the object owners differ 
(only the owner or a superuser can perform these operations AFAICS -- 
there is no mention of it in the current documentation) unless the DBA 
makes the maintenance role a member of every other role ... which 
quickly becomes a problem.



 COPY ???

The question around this one goes back to the CREATE DIRECTORY
discussion that happened this fall.  I'm still hopeful that we can do
*something* in this area, but I'm not sure what that's going to end up
looking like.  The problem with COPY is that it's either trivial to use
it to become a superuser, or insanely difficult to secure sufficiently.


Yes. That's the reason for the question marks  :-\
Some dump to csv then load somewhere else kind of jobs might benefit 
from this feature, but I'm not sure the convenience is worth the risk.



 Rationale: delegate the routine maintenance tasks to a low
privilege role, which can't do harm (apart from some performance
degradation) --- hence the no exclusive locking operations
requirement.

This makes sense for the reindex/refresh cases, though no harm might
be over-stating it.


Well it's performance degradation vs DoS due to massive (exclusive) 
locking  :S
At least restricting it to one backend (connection_limit=1) allows quite 
some rate limit.



* IMPERSONATE --- Ability to do SET AUTHORIZATION TO some_role;
and RESET AUTHORIZATION
 This might be further refined to provide a way to say This role
is authorized to impersonate role1 but no other
 Rationale: for use by connection poolers (esp. pgBouncer), where
the role used for connection would only have the LOGIN and
IMPERSONATE privileges. The remaining operations would be authorized
against the supplanted role (i.e. ability to create tables/indexes
or views, perform DML and/or DDL, etc)
 AFAIK, a superuser role is needed for this purpose currently.

No..  You can have 'no-inherit' roles which you can use for exactly this
purpose.  The initial login role can have no rights on the database,
except to SET ROLE to other roles which have been granted to it.


Hmm the current documentation states that: The specified role_name 
must be a role that the current session user is a member of.
I can see use cases where making the login role a member of every other 
used role quickly becomes a burden, and that's the main driver for this 
feature (I'm thinking about multiple app servers running several 
applications each, minimum two roles per application)



You should never have your pgBouncer or other pooling connection logging
in as a superuser.


At least the default pgBouncer config explicitly says (albeit for 8.2)

Re: Suppressing elog.c context messages (was Re: [HACKERS] Wait free LW_SHARED acquisition)

On 2014-12-22 10:35:35 +0530, Amit Kapila wrote:
 On Fri, Dec 19, 2014 at 9:36 PM, Andres Freund and...@2ndquadrant.com
 wrote:
 
  Hi,
 
  When debugging lwlock issues I found PRINT_LWDEBUG/LOG_LWDEBUG rather
  painful to use because of the amount of elog contexts/statements
  emitted. Given the number of lwlock acquirations that's just not doable.
 
  To solve that during development I've solved that by basically
  replacing:
  if (Trace_lwlocks)
 elog(LOG, %s(%s %d): %s, where, name, index, msg);
 
  with something like
 
  if (Trace_lwlocks)
  {
  ErrorContextCallback *old_error_context_stack;
  ...
  old_error_context_stack = error_context_stack;
  error_context_stack = NULL;
  ereport(LOG,
 (errhidestmt(true),
  errmsg(%s(%s %d): %s, where, T_NAME(lock),
  T_ID(lock), msg)));
 
  I think it'd generally be useful to have something like errhidecontext()
  akin to errhidestatement() to avoid things like the above.
 
 
 Under this proposal, do you want to suppress the context/statement
 unconditionally or via some hook/variable, because it might be useful to
 print the contexts/statements in certain cases where there is complex
 application and we don't know which part of application code causes
 problem.

I'm proposing to do model it after errhidestatement(). I.e. add
errhidecontext().

I've attached what I was tinkering with when I wrote this message.

  The usecases wher eI see this as being useful is high volume debug
  logging, not normal messages...
 
 
 I think usecase is valid, it is really difficult to dig such a log
 especially when statement size is big.

Right, that was what triggered to look me into it. I'd cases where the
same context was printed thousands of times.

 Also I think even with above, the number
 of logs generated are high for any statement which could still make
 debugging difficult, do you think it would be helpful if PRINT_LWDEBUG
 and LOG_LWDEBUG are used with separate defines (LOCK_DEBUG and
 LOCK_BLOCK_DEBUG) as in certain cases we might want to print info
 about locks which are acquired after waiting or in other words that gets
 blocked.

Hm, that seems like a separate thing. Personally I don't find it
interesting enough to write a patch for it, although I could see it
being interesting for somebody.

If you're looking at that level it's easy enough to just add a early
return in either routine...

Greetings,

Andres Freund
From 5e6af912377a1b43ea0168951238cb6a5e0b233e Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Sun, 21 Dec 2014 15:45:55 +0100
Subject: [PATCH 1/4] Add capability to suppress CONTEXT: messages to elog
 machinery.

Hiding context messages usually is not a good idea - except for rather
verbose debugging/development utensils like LOG_DEBUG. There the
amount of repeated context messages just bloat the log without adding
information.
---
 src/backend/utils/error/elog.c | 24 ++--
 src/include/utils/elog.h   |  2 ++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 2316464..8f04b19 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -1081,6 +1081,25 @@ errhidestmt(bool hide_stmt)
 	return 0;	/* return value does not matter */
 }
 
+/*
+ * errhidestmt --- optionally suppress CONTEXT: field of log entry
+ *
+ * This should only be used for verbose debugging messages where the repeated
+ * inclusion of CONTEXT: bloats the log volume too much.
+ */
+int
+errhidecontext(bool hide_ctx)
+{
+	ErrorData  *edata = errordata[errordata_stack_depth];
+
+	/* we don't bother incrementing recursion_depth */
+	CHECK_STACK_DEPTH();
+
+	edata-hide_ctx = hide_ctx;
+
+	return 0;	/* return value does not matter */
+}
+
 
 /*
  * errfunction --- add reporting function name to the current error
@@ -2724,7 +2743,8 @@ write_csvlog(ErrorData *edata)
 	appendStringInfoChar(buf, ',');
 
 	/* errcontext */
-	appendCSVLiteral(buf, edata-context);
+	if (!edata-hide_ctx)
+		appendCSVLiteral(buf, edata-context);
 	appendStringInfoChar(buf, ',');
 
 	/* user query --- only reported if not disabled by the caller */
@@ -2856,7 +2876,7 @@ send_message_to_server_log(ErrorData *edata)
 			append_with_tabs(buf, edata-internalquery);
 			appendStringInfoChar(buf, '\n');
 		}
-		if (edata-context)
+		if (edata-context  !edata-hide_ctx)
 		{
 			log_line_prefix(buf, edata);
 			appendStringInfoString(buf, _(CONTEXT:  ));
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 87438b8..ec7ed22 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -221,6 +221,7 @@ errcontext_msg(const char *fmt,...)
 __attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2)));
 
 extern int	errhidestmt(bool hide_stmt);
+extern int	errhidecontext(bool hide_ctx);
 
 extern int	errfunction(const char *funcname);
 extern int	errposition(int

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes


On 12/23/2014 06:06 PM, Bruce Momjian wrote:

On Tue, Dec 23, 2014 at 11:34:09AM -0500, Tom Lane wrote:

Stephen Frost sfr...@snowman.net writes:

If that's the only consideration for this, well, that's certainly quite
straight-forward to change in the other direction too.  The new function
suggested by Andres actually makes it really easy to get a textual list
of all the role attributes which a role has from the bitmask too.

We could have that regardless of the representation, if the function is
defined along the lines of given a user OID, give me a text string
representing the user's attributes.  However, that only helps for
pg_dumpall and any other clients whose requirement is exactly satisfied
by a string that fits into CREATE/ALTER USER.  The current formatting
of psql's \du, for example, absolutely requires adding more client-side
code every time we add a property; whether the catalog representation is
bools or a bitmask really isn't going to change the pain level much there.

I am with Tom on this --- there is more wasted space in the 'name'
column pg_authid.rolname than by shoving 40 boolean values into a
bitmap.  Adding the complexity of a bitmap doesn't make sense here.
Well, the code simplification alone might be worth the effort... and it 
does make adding additional attributes easier.



I also apologize for the late feedback.

Offtopic, what I would really _love_ to see improved is our display of
object permissions:

 Access privileges
 Schema |  Name  | Type  | Access privileges | Column 
privileges | Policies

++---+---+---+--
 public | crypto | table | postgres=arwdDxt/postgres+|  
 |
||   | =r/postgres   |  
 |

That is nasty user display ---  it looks like line noise.


Hmm...  http://www.postgresql.org/docs/9.4/static/sql-grant.html does 
describe the mapping from letters to permissions, but I agree that it 
could be easier for beginners.
Any idea on how this display can be made more human friendly? (just 
for the sake of discussion --- I don't think I have time to do much 
about that, unfortunately)



Cheers,

/ J.L.



--
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: two new role attributes and/or capabilities?

2014-12-23 Thread David G Johnston

José Luis Tallón-2 wrote
On 12/23/2014 05:29 PM, Stephen Frost wrote:
* José Luis Tallón (

jltallon@

) wrote:
* IMPERSONATE --- Ability to do SET AUTHORIZATION TO some_role;
and RESET AUTHORIZATION
This might be further refined to provide a way to say This role
is authorized to impersonate role1 but no other
Rationale: for use by connection poolers (esp. pgBouncer), where
the role used for connection would only have the LOGIN and
IMPERSONATE privileges. The remaining operations would be authorized
against the supplanted role (i.e. ability to create tables/indexes
or views, perform DML and/or DDL, etc)
AFAIK, a superuser role is needed for this purpose currently.
No.. You can have 'no-inherit' roles which you can use for exactly this
purpose. The initial login role can have no rights on the database,
except to SET ROLE to other roles which have been granted to it.

Hmm the current documentation states that: The specified role_name
must be a role that the current session user is a member of.
I can see use cases where making the login role a member of every other
used role quickly becomes a burden, and that's the main driver for this
feature (I'm thinking about multiple app servers running several
applications each, minimum two roles per application)

So you want to say:

GRANT IMPERSONATE TO bouncer; --covers the ALL requirement

instead of

GRANT victim1 TO bouncer;
GRANT victim2 TO bouncer;
etc...

-- these would still be used to cover the limited users requirement
?

Seems contrary to the principle of least privilege goal...

I'd rather there be better, more user friendly, SQL-based APIs to the
permissions system that would facilitate performing and reviewing grants.

If something like IMPERSONATE was added I would strongly suggest a
corresponding [NO]IMPERSONATE for CREATE USER so that the admin can make
specific roles unimpersonable - and also make SUPERUSER roles unimpersonable
by rule.

David J.

--
View this message in context:
http://postgresql.nabble.com/Proposal-two-new-role-attributes-and-or-capabilities-tp5831859p5831868.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

2014-12-23 Thread David G Johnston

José Luis Tallón-2 wrote
On 12/23/2014 06:06 PM, Bruce Momjian wrote:
On Tue, Dec 23, 2014 at 11:34:09AM -0500, Tom Lane wrote:
Stephen Frost lt;

sfrost@

gt; writes:
If that's the only consideration for this, well, that's certainly quite
straight-forward to change in the other direction too. The new
function
suggested by Andres actually makes it really easy to get a textual list
of all the role attributes which a role has from the bitmask too.
We could have that regardless of the representation, if the function is
defined along the lines of given a user OID, give me a text string
representing the user's attributes. However, that only helps for
pg_dumpall and any other clients whose requirement is exactly satisfied
by a string that fits into CREATE/ALTER USER. The current formatting
of psql's \du, for example, absolutely requires adding more client-side
code every time we add a property; whether the catalog representation is
bools or a bitmask really isn't going to change the pain level much
there.
I am with Tom on this --- there is more wasted space in the 'name'
column pg_authid.rolname than by shoving 40 boolean values into a
bitmap. Adding the complexity of a bitmap doesn't make sense here.
Well, the code simplification alone might be worth the effort... and it
does make adding additional attributes easier.

I also apologize for the late feedback.

Offtopic, what I would really _love_ to see improved is our display of
object permissions:

++---+---+---+--
public | crypto | table | postgres=arwdDxt/postgres+|
|
|| | =r/postgres |
|

That is nasty user display --- it looks like line noise.

Hmm... http://www.postgresql.org/docs/9.4/static/sql-grant.html does
describe the mapping from letters to permissions, but I agree that it
could be easier for beginners.
Any idea on how this display can be made more human friendly? (just
for the sake of discussion --- I don't think I have time to do much
about that, unfortunately)

I'm not exploring this at the moment but creating an ASCII table that looks
similar to what pgAdmin outputs would be the best solution. While pgAdmin
would allow for interaction on the table the presentation there is likely
more user-friendly than this (not a high bar to clear...)

Ideally the column headers would go vertical for narrow columns; but even
using header abbreviations with a key would work but the number of columns
should be constant and true indicators used to denote permission.

Spatial factors (position, space/fill, size) provide stronger cues compared
to trying to read a sequence of characters.

David J.

--
View this message in context:
http://postgresql.nabble.com/Re-COMMITTERS-pgsql-Use-a-bitmask-to-represent-role-attributes-tp5831838p5831869.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

Bruce Momjian wrote:

 I am with Tom on this --- there is more wasted space in the 'name'
 column pg_authid.rolname than by shoving 40 boolean values into a
 bitmap.  Adding the complexity of a bitmap doesn't make sense here.  I
 also apologize for the late feedback.

Okay, it seems we have a majority that does not want this patch -- at
least Tom, Robert and Bruce plus-one'd the reversion.  I am going to
revert it, mainly because I don't want to be on the hook for fixing it
later on.  I'm also going to mark it Rejected in commitfest.

Adam and Stephen can rework as they see fit, according to whatever
acceptable design is found.

-- 
Á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] replicating DROP commands across servers

I have pushed patches 0001 through 0004, with some revisions.  Only the
final part is missing.

-- 
Á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] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

* Bruce Momjian (br...@momjian.us) wrote:
 I am with Tom on this --- there is more wasted space in the 'name'
 column pg_authid.rolname than by shoving 40 boolean values into a
 bitmap.  

I agree that we waste a lot of space in 'name' but I don't think there's
an easy solution to that problem.  This, on the other hand, seemed like
a relatively minor change to an internal catalog definition.

 Adding the complexity of a bitmap doesn't make sense here.  I
 also apologize for the late feedback.

The complexity of the bitmap on the SQL side actually makes the code
side cleaner. :/

It would be great to figure out a way to get feedback like this earlier
on in the development.  This patch has been floating around for quite a
while, with intentional breaks for feedback taken prior to incremental
improvements and documentation additions.  Clearly that gets back to the
discussion around the commitfest situation.

 Offtopic, what I would really _love_ to see improved is our display of
 object permissions:

That's a whole different discussion and really belongs on a new thread.
I'm certainly curious what ideas you have regarding how to fix this;
it's not like Unix permission display is particularly elegant. Is there
something you've seen that looks nicer while still conveying the
necessary information in a relatively small amount of space?

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Proposal: two new role attributes and/or capabilities?


On 12/23/2014 07:01 PM, David G Johnston wrote:
Hmm the current documentation states that: The specified 
role_name must be a role that the current session user is a member 
of. I can see use cases where making the login role a member of every 
other used role quickly becomes a burden, and that's the main driver 
for this feature (I'm thinking about multiple app servers running 
several applications each, minimum two roles per application)

So you want to say:

GRANT IMPERSONATE TO bouncer; --covers the ALL requirement


Yes, and exclusively for this purpose.


instead of

GRANT victim1 TO bouncer;
GRANT victim2 TO bouncer;
etc...

-- these would still be used to cover the limited users requirement
?


Yup.


Seems contrary to the principle of least privilege goal...


We still wouldn't grant any CREATE DATABASE, CREATE TABLESPACE, 
CREATE/LOAD EXTENSION, CREATE LANGUAGE, etc
 (and the ability to create/use/manipulate data within the database 
will still be constrained by the impersonated login)



I'd rather there be better, more user friendly, SQL-based APIs to the
permissions system that would facilitate performing and reviewing grants.

+1. All suggestions welcome.

If something like IMPERSONATE was added I would strongly suggest a
corresponding [NO]IMPERSONATE for CREATE USER so that the admin can make
specific roles unimpersonable

Indeed, I had thought about this too.


- and also make SUPERUSER roles unimpersonable by rule.


Yes, of course. Otherwise, the distinction would not have any sense.


Thanks,

J.L.



--
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: two new role attributes and/or capabilities?

* José Luis Tallón (jltal...@adv-solutions.net) wrote:
 On 12/23/2014 05:29 PM, Stephen Frost wrote:
  The capabilities would be:
 * MAINTENANCE --- Ability to run
  VACUUM [ANALYZE | FREEZE] (but not VACUUM FULL),
  ANALYZE (including SET LOCAL statistics_target TO 1),
 There's likely to be discussion about these from the perspective that
 you really shouldn't need to run them all that much.  Why isn't
 autovacuum able to handle this?
 
 For some (arguably, ill-devised) use cases of INSERT - SELECT
 aggregate - DELETE (third party, closed-source app, massive insert
 rate) at the very least, autovacuum can't possibly cope with the
 change rate in some tables, given that there are quite many other
 interactive queries running.
 
 Manually performing VACUUM / VACUUM ANALYZE on the (few) affected
 tables every 12h or so fixes the performance problem for the
 particular queries without impacting the other users too much ---
 the tables and indexes in question have been moved to a separate
 tablespace/disk volume of their own.

Autovacuum can certainly run vacuum/analyze on a few tables every 12
hours, so I'm not really following where you see autovacuum being unable
to cope.  I agree that there *are* such cases, but getting more
information about those cases and exactly what solution *does* work
would really help us improve autovacuum to address those use-cases.

 In short, this addresses situations where some tables have a much
 higher update rate than the rest of the database so that performance
 degrades with time --- the application became unusable after about 6
 days' worth of updates until the manual vacuums were setup

This really looks like a configuration issue with autovacuum..  Perhaps
you need to make it more aggressive than the default and have it run
more threads?  Have you turned the autovacuum logging up all the way?
Is autovacuum giving up due to locking?

  REINDEX CONCURRENTLY  (but not the blocking, regular, one)
  REFRESH MATERIALIZED VIEW CONCURRENTLY (but not the blocking one)
 These are interesting, but would these make sense at the role level?
 Both of these commands explicitly take specific relations to operate
 against, after all.
 
 Yup. Let's imagine a cron job invoking psql in order to perform
 maintenance routine.

If they make sense at a relation level then they should be
relation-level GRANT'd permissions, not role-level attributes.

 The particular command(s) can be generated on-the-fly by querying
 the catalog and then send them in one go to be run sequentially by
 the one backend as a crude form of rate
 limiting/quality-of-service of sorts (renice -p or even ionice
 -p seems quite inadequate).

This sounds like it's something that we might want an autovacuum-like
background process to handle..  Some kind of auto-reindex-concurrently.
There are already plans to deal with updating of materialized views, as
I understand it.

 This automation becomes impossible to do if the object owners differ
 (only the owner or a superuser can perform these operations AFAICS
 -- there is no mention of it in the current documentation) unless
 the DBA makes the maintenance role a member of every other role ...
 which quickly becomes a problem.

I agree that having a maintenance role which is a member of every other
role isn't a very good solution.

  COPY ???
 The question around this one goes back to the CREATE DIRECTORY
 discussion that happened this fall.  I'm still hopeful that we can do
 *something* in this area, but I'm not sure what that's going to end up
 looking like.  The problem with COPY is that it's either trivial to use
 it to become a superuser, or insanely difficult to secure sufficiently.
 
 Yes. That's the reason for the question marks  :-\
 Some dump to csv then load somewhere else kind of jobs might
 benefit from this feature, but I'm not sure the convenience is worth
 the risk.

I've run into quite a few processes which would really benefit from
this, and would even be safe to use (the processes running the COPY
commands don't have any rights on the directories except through PG),
but it's not clear if that use-case is sufficiently broad for the
feature to be worthwhile..  At least, some feel it isn't.  Can you
describe your use-case more and perhaps the needle will move on that
point?

 * IMPERSONATE --- Ability to do SET AUTHORIZATION TO some_role;
 and RESET AUTHORIZATION
  This might be further refined to provide a way to say This role
 is authorized to impersonate role1 but no other
  Rationale: for use by connection poolers (esp. pgBouncer), where
 the role used for connection would only have the LOGIN and
 IMPERSONATE privileges. The remaining operations would be authorized
 against the supplanted role (i.e. ability to create tables/indexes
 or views, perform DML and/or DDL, etc)
  AFAIK, a superuser role is needed for this purpose currently.
 No..  You can have 'no-inherit' roles which you can use for exactly this

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

On Tue, Dec 23, 2014 at 01:43:47PM -0500, Stephen Frost wrote:
  Offtopic, what I would really _love_ to see improved is our display of
  object permissions:
 
 That's a whole different discussion and really belongs on a new thread.
 I'm certainly curious what ideas you have regarding how to fix this;
 it's not like Unix permission display is particularly elegant. Is there
 something you've seen that looks nicer while still conveying the
 necessary information in a relatively small amount of space?

No, just hoping for something better.


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

  + Everyone has their own god. +


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

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
 Bruce Momjian wrote:
 
  I am with Tom on this --- there is more wasted space in the 'name'
  column pg_authid.rolname than by shoving 40 boolean values into a
  bitmap.  Adding the complexity of a bitmap doesn't make sense here.  I
  also apologize for the late feedback.
 
 Okay, it seems we have a majority that does not want this patch -- at
 least Tom, Robert and Bruce plus-one'd the reversion.  I am going to
 revert it, mainly because I don't want to be on the hook for fixing it
 later on.  I'm also going to mark it Rejected in commitfest.

Well, I'd be happy for fixing it, but that's not what is at issue here.
If it was only about getting someone to maintain it, I don't think
there'd be a discussion.

 Adam and Stephen can rework as they see fit, according to whatever
 acceptable design is found.

In the end, this is simpler, and the patch to add the other role
attributes which we were looking to add is probably closer to being done
with this outcome anyway, since it doesn't need to be rebased on top of
the bitmap work.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] bin checks taking too long.



On 12/23/2014 11:11 AM, Andrew Dunstan wrote:


On 12/22/2014 10:41 PM, Peter Eisentraut wrote:

On 12/22/14 7:56 PM, Andrew Dunstan wrote:

Currently the buildfarm animal crake (my development instance) is
running the bin check, but not any other animal. These tests still take
for too long, not least because each set of tests requires a separate
install.

You can avoid that by using make installcheck.



Not working too well:

   make -C pg_ctl installcheck
   make[1]: Entering directory
   `/home/bf/bfr/root/HEAD/pgsql.build/src/bin/pg_ctl'
   cd /home/bf/bfr/root/HEAD/pgsql.build/../pgsql/src/bin/pg_ctl 
   TESTDIR='/home/bf/bfr/root/HEAD/pgsql.build/src/bin/pg_ctl'
   PATH=/home/bf/bfr/root/HEAD/inst/bin:$PATH PGPORT='65648' prove -I
   /home/bf/bfr/root/HEAD/pgsql.build/../pgsql/src/test/perl/ --verbose
   t/*.pl
   Use of uninitialized value $ENV{top_builddir} in concatenation (.)
   or string at t/001_start_stop.pl line 17.
   file not found: /src/test/regress/pg_regress at
/home/bf/bfr/root/HEAD/pgsql.build/../pgsql/src/test/perl/TestLib.pm
   line 142.

This was invoked like this:

   cd $pgsql/src/bin  make NO_LOCALE=1 installcheck

Note that it's a vpath build.




The attached patch seems to fix this. However, running installcheck also 
makes very little difference to the time. These checks still take around 
100 seconds on my machine, including around 60 for the script tests. 
That seems ridiculously long, when the whole core regression suite takes 
23 seconds.


cheers

andrew



diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index a77b5f1..7c39d82 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -317,7 +317,7 @@ endef
 ifeq ($(enable_tap_tests),yes)
 
 define prove_installcheck
-cd $(srcdir)  TESTDIR='$(CURDIR)' PATH=$(bindir):$$PATH PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl
+cd $(srcdir)  TESTDIR='$(CURDIR)' PATH=$(bindir):$$PATH PGPORT='6$(DEF_PGPORT)' top_builddir='$(CURDIR)/$(top_builddir)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl
 endef
 
 define prove_check

-- 
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 lock strength of adding foreign keys

2014-12-23 Thread Andreas Karlsson


On 12/17/2014 03:41 PM, Simon Riggs wrote:

All of this is just a replay of the earlier conversations about
reducing lock levels.

My original patch did reduce lock levels for CREATE TRIGGER, but we
stepped back from doing that in 9.4 until we had feedback as to
whether the whole idea of reducing lock levels was safe, which Robert,
Tom and others were slightly uncertain about.

Is there anything different here from work in my original patch series?


As far as I know, no.

--
Andreas Karlsson


--
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: two new role attributes and/or capabilities?


On 12/23/2014 07:01 PM, David G Johnston wrote:

[snip]
So you want to say:

GRANT IMPERSONATE TO bouncer; --covers the ALL requirement

instead of

GRANT victim1 TO bouncer;
GRANT victim2 TO bouncer;
etc...

-- these would still be used to cover the limited users requirement
?


|GRANT IMPERSONATE ON actual_role TO login_role|

would actually get us closer to how some other databases do, now 
that I think of it. This could be just some syntactic sugar.

Might definitively ease migrations, if nothing else.


I appreciate the feedback. Thanks!


/ J.L.

Re: [HACKERS] Proposal: two new role attributes and/or capabilities?

* David G Johnston (david.g.johns...@gmail.com) wrote:
 I'd rather there be better, more user friendly, SQL-based APIs to the
 permissions system that would facilitate performing and reviewing grants.

This would be *really* nice, I agree.  I've heard tale of people writing
functions that go through the catalog based on a given user and spit
back everything that they have permissions to.  Would be really nice if
we had those kinds of functions built-in.

 If something like IMPERSONATE was added I would strongly suggest a
 corresponding [NO]IMPERSONATE for CREATE USER so that the admin can make
 specific roles unimpersonable - and also make SUPERUSER roles unimpersonable
 by rule.

I agree that this would be necessary..  but strikes me as less of a
complete solution than what the existing pg_auth_members approach grants
you.

Perhaps a better idea would be to simply make the bouncer unnecessary by
having a in-PG connection pooler type of system.  That's been discussed
previously and shot down but it's still one of those things that's on my
wish-list for PG.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Proposal: two new role attributes and/or capabilities?

* José Luis Tallón (jltal...@adv-solutions.net) wrote:
 On 12/23/2014 07:01 PM, David G Johnston wrote:
 [snip]
 So you want to say:
 
 GRANT IMPERSONATE TO bouncer; --covers the ALL requirement
 
 instead of
 
 GRANT victim1 TO bouncer;
 GRANT victim2 TO bouncer;
 etc...
 
 -- these would still be used to cover the limited users requirement
 ?
 
 |GRANT IMPERSONATE ON actual_role TO login_role|
 
 would actually get us closer to how some other databases do, now
 that I think of it. This could be just some syntactic sugar.
 Might definitively ease migrations, if nothing else.

Uh, how is this different from GRANT actual_role TO login_role, with use
of noinherit..?

THanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Wait free LW_SHARED acquisition - v0.10

Hi,

Attached is a new version of the patchset which I intend to commit soon.

Stuff changed since 0.9:

* Greatly simplified locking logic - the whole concept that a lock could
  be spuriously acquired is gone. That cost a small bit of performance
  (0.5%, I thought it'd be much bigger) on x86, but is a noticeable
  performance *benefit* on PPC.

* releaseOK (and other internal flags) are rolled into the former
  'lockcount' variable which is now named state. By having it inside the
  same atomic reasoning about the state gets easier as there's no skew
  between observing the lockcount and other variables.

* The number of queued waiters isn't required anymore, it's only a
  debugging aid (#ifdef LOCK_DEBUG) at this point.

Patches:
0001: errhidecontext() patch
0002: dlist()ify lwWaitLink
0003: LW_SHARED scalability

I've done a fair amount of benchmarking and on bigger system the new
code seems to be a win pretty much generally.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
From 5e6af912377a1b43ea0168951238cb6a5e0b233e Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Sun, 21 Dec 2014 15:45:55 +0100
Subject: [PATCH 1/4] Add capability to suppress CONTEXT: messages to elog
 machinery.

Hiding context messages usually is not a good idea - except for rather
verbose debugging/development utensils like LOG_DEBUG. There the
amount of repeated context messages just bloat the log without adding
information.
---
 src/backend/utils/error/elog.c | 24 ++--
 src/include/utils/elog.h   |  2 ++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 2316464..8f04b19 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -1081,6 +1081,25 @@ errhidestmt(bool hide_stmt)
 	return 0;	/* return value does not matter */
 }
 
+/*
+ * errhidestmt --- optionally suppress CONTEXT: field of log entry
+ *
+ * This should only be used for verbose debugging messages where the repeated
+ * inclusion of CONTEXT: bloats the log volume too much.
+ */
+int
+errhidecontext(bool hide_ctx)
+{
+	ErrorData  *edata = errordata[errordata_stack_depth];
+
+	/* we don't bother incrementing recursion_depth */
+	CHECK_STACK_DEPTH();
+
+	edata-hide_ctx = hide_ctx;
+
+	return 0;	/* return value does not matter */
+}
+
 
 /*
  * errfunction --- add reporting function name to the current error
@@ -2724,7 +2743,8 @@ write_csvlog(ErrorData *edata)
 	appendStringInfoChar(buf, ',');
 
 	/* errcontext */
-	appendCSVLiteral(buf, edata-context);
+	if (!edata-hide_ctx)
+		appendCSVLiteral(buf, edata-context);
 	appendStringInfoChar(buf, ',');
 
 	/* user query --- only reported if not disabled by the caller */
@@ -2856,7 +2876,7 @@ send_message_to_server_log(ErrorData *edata)
 			append_with_tabs(buf, edata-internalquery);
 			appendStringInfoChar(buf, '\n');
 		}
-		if (edata-context)
+		if (edata-context  !edata-hide_ctx)
 		{
 			log_line_prefix(buf, edata);
 			appendStringInfoString(buf, _(CONTEXT:  ));
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 87438b8..ec7ed22 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -221,6 +221,7 @@ errcontext_msg(const char *fmt,...)
 __attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2)));
 
 extern int	errhidestmt(bool hide_stmt);
+extern int	errhidecontext(bool hide_ctx);
 
 extern int	errfunction(const char *funcname);
 extern int	errposition(int cursorpos);
@@ -385,6 +386,7 @@ typedef struct ErrorData
 	bool		output_to_client;		/* will report to client? */
 	bool		show_funcname;	/* true to force funcname inclusion */
 	bool		hide_stmt;		/* true to prevent STATEMENT: inclusion */
+	bool		hide_ctx;		/* true to prevent CONTEXT: inclusion */
 	const char *filename;		/* __FILE__ of ereport() call */
 	int			lineno;			/* __LINE__ of ereport() call */
 	const char *funcname;		/* __func__ of ereport() call */
-- 
2.2.0.rc0.18.ga1ad247

From 0ef51c826faa53620fc7d8d39d5df6206be729f3 Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Tue, 7 Oct 2014 15:32:34 +0200
Subject: [PATCH 2/4] Convert the PGPROC-lwWaitLink list into a dlist instead
 of open coding it.

Besides being shorter and much easier to read it changes the logic in
LWLockRelease() to release all shared lockers when waking up any. This
can yield some significant performance improvements - and the fairness
isn't really much worse than before, as we always allowed new shared
lockers to jump the queue.
---
 src/backend/access/transam/twophase.c |   1 -
 src/backend/storage/lmgr/lwlock.c | 146 --
 src/backend/storage/lmgr/proc.c   |   2 -
 src/include/storage/lwlock.h  |   5 +-
 src/include/storage/proc.h|   3 +-
 5 files changed, 57 insertions(+), 100 deletions(-)

diff --git

[HACKERS] Commit timestamp abbreviations

I noticed this when looking at the allocated shared memory structures in
head:

shared memory alignment 64-byte of CommitTs Ctl:  0
shared memory alignment 64-byte of CommitTs shared:  0

I thought we got rid of the idea that 'Ts' means timestamp.  Was this
part forgotten?

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

  + Everyone has their own god. +


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

Re: [HACKERS] INSERT ... ON CONFLICT {UPDATE | IGNORE}

2014-12-23 Thread Peter Geoghegan

On Tue, Dec 23, 2014 at 11:30 AM, Peter Geoghegan p...@heroku.com wrote:
 I tend to agree. I think we should just live with the fact that not
 every conceivable use case will be covered, at least initially.

To be clear: I still think I should go and make the changes that will
make the feature play nice with all shipped non-default B-Tree
operator classes, and will make it work with partial unique indexes
[1]. That isn't difficult or controversial, AFAICT, and gets us very
close to satisfying every conceivable use case.

[1] 
http://www.postgresql.org/message-id/CAM3SWZQdv7GDLwPRv7=re-gg1qjlool3vcmaricbctyk8gw...@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] Turning recovery.conf into GUCs

2014-12-23 Thread Petr Jelinek


On 12/12/14 16:34, Alex Shulgin wrote:

Alex Shulgin a...@commandprompt.com writes:


Alex Shulgin a...@commandprompt.com writes:


Here's an attempt to revive this patch.


Here's the patch rebased against current HEAD, that is including the
recently committed action_at_recovery_target option.

The default for the new GUC is 'pause', as in HEAD, and
pause_at_recovery_target is removed completely in favor of it.

I've also taken the liberty to remove that part that errors out when
finding $PGDATA/recovery.conf.  Now get your rotten tomatoes ready. ;-)


This was rather short-sighted, so I've restored that part.

Also, rebased on current HEAD, following the rename of
action_at_recovery_target to recovery_target_action.



Hi,

I had a quick look, the patch does not apply cleanly anymore but it's 
just release notes so nothing too bad.


I did not do any testing yet, but I have few comments about the code:

- the patch mixes functionality change with the lowercasing of the 
config variables, I wonder if those could be separated into 2 separate 
diffs - it would make review somewhat easier, but I can live with it as 
it is if it's too much work do separate into 2 patches


- the StandbyModeRequested and StandbyMode should be lowercased like the 
rest of the GUCs


- I am wondering if StandbyMode and hot_standby should be merged into 
one GUC if we are breaking backwards compatibility anyway


- Could you explain the reasoning behind:
+   if ((*newval)[0] == 0)
+   xid = InvalidTransactionId;
+   else

in check_recovery_target_xid() (and same check in 
check_recovery_target_timeline()), isn't the strtoul which is called 
later enough?


- The whole CopyErrorData and memory context logic is not needed in 
check_recovery_target_time() as the FlushErrorState() is not called there


- The new code in StartupXLOG() like
+   if (recovery_target_xid_string != NULL)
+   InitRecoveryTarget(RECOVERY_TARGET_XID);
+
+   if (recovery_target_time_string != NULL)
+   InitRecoveryTarget(RECOVERY_TARGET_TIME);
+
+   if (recovery_target_name != NULL)
+   InitRecoveryTarget(RECOVERY_TARGET_NAME);
+
+   if (recovery_target_string != NULL)
+   InitRecoveryTarget(RECOVERY_TARGET_IMMEDIATE);

would probably be better in separate function that you then call 
StartupXLOG() as StartupXLOG() is crazy long already so I think it's 
preferable to not make it worse.


- I wonder if we should rename trigger_file to standby_tigger_file


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

* Peter Geoghegan (p...@heroku.com) wrote:
 On Tue, Dec 23, 2014 at 5:46 AM, Robert Haas robertmh...@gmail.com wrote:
  On Mon, Dec 22, 2014 at 5:04 PM, Peter Geoghegan p...@heroku.com wrote:
  If you're dead set on having an escape hatch, maybe we should just get
  over it and add a way of specifying a unique index by name. As I said,
  these under-served use cases are either exceedingly rare or entirely
  theoretical.
 
  I'm decidedly unenthusiastic about that.  People don't expect CREATE
  INDEX CONCURRENTLY + DROP INDEX CONCURRENTLY to break their DML.  I
  think the solution in this case would be a gateway to problems larger
  than the one we're trying to solve.
 
 I tend to agree. I think we should just live with the fact that not
 every conceivable use case will be covered, at least initially. Then,
 if an appreciable demand for even more flexibility emerges, we can
 revisit this. We already have a syntax that is significantly more
 flexible than the equivalent feature in any other system. Let's not
 lose sight of that.

+1

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Proposal: two new role attributes and/or capabilities?


On 12/23/2014 07:52 PM, Stephen Frost wrote:

[snip]
Manually performing VACUUM / VACUUM ANALYZE on the (few) affected
tables every 12h or so fixes the performance problem for the
particular queries without impacting the other users too much ---
the tables and indexes in question have been moved to a separate
tablespace/disk volume of their own.
Autovacuum can certainly run vacuum/analyze on a few tables every 12
hours, so I'm not really following where you see autovacuum being unable
to cope.  I agree that there *are* such cases, but getting more
information about those cases and exactly what solution *does* work
would really help us improve autovacuum to address those use-cases.


I'll try to. I don't have direct access, and the use case is quite edgy 
to be fair.

Plus, the configuration and hardware leaves quite a bit to be desired...

...but it's a real use case and the solution (even if only treating the 
symptoms) is quite straight-forward and easy.

In short, this addresses situations where some tables have a much
higher update rate than the rest of the database so that performance
degrades with time --- the application became unusable after about 6
days' worth of updates until the manual vacuums were setup

This really looks like a configuration issue with autovacuum..  Perhaps
you need to make it more aggressive than the default and have it run
more threads?


Yes to both. Up to something which actually affected performance a bit.
But basically only a few tables exhibited this behaviour among several 
hundreds in this particular situation.



Have you turned the autovacuum logging up all the way?
Is autovacuum giving up due to locking?
Not one of my systems, and I don't have access to it anymore, but I 
don't think this was the reason.


However, having some hundred million deleted rows piling every few hours 
quite increases the load. For the record, the (closed-source) 
application did issue the DELETEs on the table, so partitioning + 
TRUNCATE child_part was not applicable.



In any case, I was aiming at making this kind of operations possible and 
easier --- regardless of whether they are solving the right problem or 
not, or whether there exists an optimal solution --- since I have seen 
some real life solutions that could benefit from it.
I agree that routine index maintenance is a better match for this 
feature, though :)

 REINDEX CONCURRENTLY  (but not the blocking, regular, one)
 REFRESH MATERIALIZED VIEW CONCURRENTLY (but not the blocking one)

These are interesting, but would these make sense at the role level?
Both of these commands explicitly take specific relations to operate
against, after all.

Yup. Let's imagine a cron job invoking psql in order to perform
maintenance routine.

If they make sense at a relation level then they should be
relation-level GRANT'd permissions, not role-level attributes.


Same as before.
Let's imagine this coupled with REINDEX SCHEMA CONCURRENTLY ... or 
simply when constructing the list of tables dynamically and there is no 
other use for such a grant.
Arguably, this isn't that much of a problem if there exists a way to 
easily revoke all such permissions from all objects in one go (just like 
recently discussed in another thread)



The particular command(s) can be generated on-the-fly by querying
the catalog and then send them in one go to be run sequentially by
the one backend as a crude form of rate
limiting/quality-of-service of sorts (renice -p or even ionice
-p seems quite inadequate).

This sounds like it's something that we might want an autovacuum-like
background process to handle..  Some kind of auto-reindex-concurrently.
There are already plans to deal with updating of materialized views, as
I understand it.


While I can definitively see it for materialized views (they *are* 
views, after all), this pattern potentially gets us adding everything 
but the kitchen sink inside the database.
FWIW, it's only a matter of providing a mechanism for maintenance 
routines to use very unprivileged users to perform their duties on the 
whole cluster without having to explicitly grant permissions and/or 
include these into another, regular, role.
Please keep in mind that these  roles [having only LOGIN and 
MAINTENANCE] would NOT be able to perform any DML or DDL whatsoever, nor 
any queries (unless explicitly granted permission for SELECTs).



[snip]

Yes. That's the reason for the question marks  :-\
Some dump to csv then load somewhere else kind of jobs might
benefit from this feature, but I'm not sure the convenience is worth
the risk.

I've run into quite a few processes which would really benefit from
this, and would even be safe to use (the processes running the COPY
commands don't have any rights on the directories except through PG),
but it's not clear if that use-case is sufficiently broad for the
feature to be worthwhile..  At least, some feel it isn't.  Can you
describe your use-case more and perhaps the needle will move on

Re: [HACKERS] [COMMITTERS] pgsql: Use a bitmask to represent role attributes

2014-12-23 Thread David G Johnston

On Tue, Dec 23, 2014 at 11:44 AM, Stephen Frost [via PostgreSQL]
ml-node+s1045698n5831875...@n5.nabble.com wrote:

It would be great to figure out a way to get feedback like this earlier
on in the development. This patch has been floating around for quite a
while, with intentional breaks for feedback taken prior to incremental
improvements and documentation additions. Clearly that gets back to the
discussion around the commitfest situation.

There four possible situations here:

Seen, agreeable
Seen, not agreeable
Seen, abstain
Not Seen

Tracking, for the committers in particular, the not seen and directly
soliciting their agree/disagree/abstain opinion is really the only way to
avoid this situation where Tom probably saw the subject lines but ended up
filtering them out since his focus was elsewhere. However, something
getting committed definitely gets his attention.

FWIW my initial reaction to this idea of introducing bitmaps was why? but
I didn't have anything to go on but the feeling that bitmaps are not the
most obvious API in modern coding. I didn't have anything else to support
that, including coding experience, so I didn't voice it and figured when no
one else did that I likely was missing something. I'm not sure how an
email to Tom saying: hey, this doesn't smell right to me would have been
taken but changing the underlying authorization mechanisms does seem like
something Tom should comment on before development gets to far along - and
that input should be prompted for if not seen.

That's my current feeling as strictly a monitor of these lists and
observing a subset of the threads and new features that are being currently
developed - take it as you will.

David J.

--
View this message in context:
http://postgresql.nabble.com/Re-COMMITTERS-pgsql-Use-a-bitmask-to-represent-role-attributes-tp5831838p5831894.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

Re: [HACKERS] INSERT ... ON CONFLICT {UPDATE | IGNORE}

2014-12-23 Thread Peter Geoghegan

On Thu, Dec 18, 2014 at 9:20 AM, Jeff Janes jeff.ja...@gmail.com wrote:
 I've put this through an adaptation of my usual torture test, and it ran
 fine until wraparound shutdown.  I'll poke at it more later.

Could you elaborate, please? What are the details of the torture test
you're performing?

-- 
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] mysql with postgres

2014-12-23 Thread Mark Kirkwood


On 23/12/14 22:36, Ravi Kiran wrote:

hi all,


Is postgres source code compatible with mysql database?? If it is, could
someone could give me some links so that I can do that.

I want to hack into the postgres source code, but as I am comfortable
with mysql, I want to use the mysql database not postgres.

any references would be fine.



I'm wondering if you are thinking that you can use Postgres as a Mysql 
storage engine? While Mysql does has pluggable storage 
engines...Postgres is not designed to be able to be used in this way 
(that would be an interesting - but big and probably controversial 
project to undertake).


So if you are more familiar with Mysql, why not hack Innodb?

Regards

Mark



--
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] Lockless StrategyGetBuffer() clock sweep

On 2014-12-23 16:42:41 -0500, Robert Haas wrote:
 I don't think I have anything to say about the substance of the patch.
 If fetch-and-add is faster than a spinlock cycle, then it is.  And
 it's good to be fast.

I don't think the primary advantage is that it's fast (even though it
should be as fast as a single TAS on x86). It's that you can never sleep
while holding the spinlock when there's no such spinlock and that
everytime you transfer the cacheline from another cpu to you you'll also
make progress...

Will fix the other stuff.

Greetings,

Andres Freund

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


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

Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates

2014-12-23 Thread Andreas Karlsson


On 12/22/2014 11:47 PM, Oskari Saarenmaa wrote:

__int128_t and __uint128_t are GCC extensions and are not related to
stdint.h.


 [...]


These changes don't match what my autoconf does.  Not a big deal I guess,
but if this is merged as-is the next time someone runs autoreconf it'll
write these lines differently to a different location of the file and the
change will end up as a part of an unrelated commit.


Thanks for the feedback. These two issues will be fixed in the next version.


*** a/src/backend/utils/adt/numeric.c
--- b/src/backend/utils/adt/numeric.c
*** static void apply_typmod(NumericVar *var
*** 402,407 
--- 402,410 
   static int32 numericvar_to_int4(NumericVar *var);
   static bool numericvar_to_int8(NumericVar *var, int64 *result);
   static void int8_to_numericvar(int64 val, NumericVar *var);
+ #ifdef HAVE_INT128
+ static void int16_to_numericvar(int128 val, NumericVar *var);
+ #endif


Existing code uses int4 and int8 to refer to 32 and 64 bit integers as
they're also PG datatypes, but int16 isn't one and it looks a lot like
int16_t.  I think it would make sense to just call it int128 internally
everywhere instead of int16 which isn't used anywhere else to refer to 128
bit integers.


Perhaps. I switched opinion on this several times while coding. On one 
side there is consistency, on the other there is the risk of confusing 
the different meanings of int16. I am still not sure which of these I 
think is the least bad.



new file mode 100755


I guess src/tools/git-external-diff generated these bogus new file mode
lines?  I know the project policy says that context diffs should be used,
but it seems to me that most people just use unified diffs these days so I'd
just use git show or git format-patch -1 to generate the patches.  The
ones generated by git format-patch can be easily applied by reviewers
using git am.


At the time of submitting my patch I had not noticed the slow change 
from git-external-diff to regular git diffs. The change snuck up on me. 
The new version of the patch will be submitted in the standard git 
format which is what I am more used to work with.


--
Andreas Karlsson


--
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 VACUUM SCHEMA


On 12/23/14, 8:54 AM, Fabrízio de Royes Mello wrote:

  Right now a lot of people just work around this with things like DO blocks, 
but as mentioned elsewhere in the thread that fails for commands that can't be in 
a transaction.
 

I use dblink to solve it. :-)


So... how about instead of solving this only for vacuum we create something 
generic? :) Possibly using Robert's background worker work?
--
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] Proposal VACUUM SCHEMA


On 12/23/14, 7:44 AM, Robert Haas wrote:

On Mon, Dec 22, 2014 at 5:00 PM, Jim Nasby jim.na...@bluetreble.com wrote:

I would MUCH rather that we find a way to special-case executing
non-transactional commands dynamically, because VACUUM isn't the only one
that suffers from this problem.


Is pg_background a solution to this problem?


Yes, since it allows you to do autonomous transactions. It's probably not the 
most efficient way to solve this, but it should work.
--
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] Proposal VACUUM SCHEMA

2014-12-23 Thread Fabrízio de Royes Mello

Em terça-feira, 23 de dezembro de 2014, Jim Nasby jim.na...@bluetreble.com
escreveu:

 On 12/23/14, 8:54 AM, Fabrízio de Royes Mello wrote:

   Right now a lot of people just work around this with things like DO
 blocks, but as mentioned elsewhere in the thread that fails for commands
 that can't be in a transaction.
  

 I use dblink to solve it. :-)


 So... how about instead of solving this only for vacuum we create
 something generic? :) Possibly using Robert's background worker work?


 Interesting idea.

But and what about the idea of improve the --table option from clients:
vaccumdb and clusterdb?

Regards,

Fabrízio Mello



-- 
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] Using 128-bit integers for sum, avg and statistics aggregates

2014-12-23 Thread Andreas Karlsson

On 12/16/2014 11:04 AM, David Rowley wrote: These are some very 
promising performance increases.


I've done a quick pass of reading the patch. I currently don't have a
system with a 128bit int type, but I'm working on that.


Sorry for taking some time to get back. I have been busy before 
Christmas. A new version of the patch is attached.



This fragment needs fixed to put braces on new lines


Fixed!


It also looks like your OIDs have been nabbed by some jsonb stuff.


Fixed!


I'm also wondering why in numeric_int16_sum() you're doing:

#else
return numeric_sum(fcinfo);
#endif

but you're not doing return int8_accum() in the #else part
of int8_avg_accum()
The same goes for int8_accum_inv() and int8_avg_accum_inv(), though
perhaps you're doing it here because of the elog() showing the wrong
function name. Although that's a pretty much shouldn't ever happen
case that mightn't be worth worrying about.


No strong reason. I did it for symmetry with int2_accum() and int4_accum().


Also since I don't currently have a machine with a working int128, I
decided to benchmark master vs patched to see if there was any sort of
performance regression due to numeric_int16_sum calling numeric_sum, but
I'm a bit confused with the performance results as it seems there's
quite a good increase in performance with the patch, I'd have expected
there to be no change.


Weird, I noticed similar results when doing my benchmarks, but given 
that I did not change the accumulator function other than adding an 
ifdef I am not totally sure if this difference is real.


master
tps = 1.001984 (excluding connections establishing)

Without int128
tps = 1.014511 (excluding connections establishing)

With int128
tps = 3.185956 (excluding connections establishing)

--
Andreas Karlsson
diff --git a/configure b/configure
index 7594401..15f4eaf 100755
--- a/configure
+++ b/configure
@@ -13771,6 +13771,27 @@ _ACEOF
 fi
 
 
+# Check if platform support gcc style 128-bit integers.
+ac_fn_c_check_type $LINENO __int128_t ac_cv_type___int128_t $ac_includes_default
+if test x$ac_cv_type___int128_t = xyes; then :
+
+cat confdefs.h _ACEOF
+#define HAVE___INT128_T 1
+_ACEOF
+
+
+fi
+ac_fn_c_check_type $LINENO __uint128_t ac_cv_type___uint128_t $ac_includes_default
+if test x$ac_cv_type___uint128_t = xyes; then :
+
+cat confdefs.h _ACEOF
+#define HAVE___UINT128_T 1
+_ACEOF
+
+
+fi
+
+
 # We also check for sig_atomic_t, which *should* be defined per ANSI
 # C, but is missing on some old platforms.
 ac_fn_c_check_type $LINENO sig_atomic_t ac_cv_type_sig_atomic_t #include signal.h
diff --git a/configure.in b/configure.in
index 0dc3f18..1271682 100644
--- a/configure.in
+++ b/configure.in
@@ -1752,6 +1752,9 @@ AC_DEFINE_UNQUOTED(MAXIMUM_ALIGNOF, $MAX_ALIGNOF, [Define as the maximum alignme
 AC_CHECK_TYPES([int8, uint8, int64, uint64], [], [],
 [#include stdio.h])
 
+# Check if platform support gcc style 128-bit integers.
+AC_CHECK_TYPES([__int128_t, __uint128_t], [], [], [])
+
 # We also check for sig_atomic_t, which *should* be defined per ANSI
 # C, but is missing on some old platforms.
 AC_CHECK_TYPES(sig_atomic_t, [], [], [#include signal.h])
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index d841b6f..eb0fef4 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -402,6 +402,9 @@ static void apply_typmod(NumericVar *var, int32 typmod);
 static int32 numericvar_to_int4(NumericVar *var);
 static bool numericvar_to_int8(NumericVar *var, int64 *result);
 static void int8_to_numericvar(int64 val, NumericVar *var);
+#ifdef HAVE_INT128
+static void int16_to_numericvar(int128 val, NumericVar *var);
+#endif
 static double numeric_to_double_no_overflow(Numeric num);
 static double numericvar_to_double_no_overflow(NumericVar *var);
 
@@ -2659,6 +2662,9 @@ numeric_float4(PG_FUNCTION_ARGS)
  * Actually, it's a pointer to a NumericAggState allocated in the aggregate
  * context.  The digit buffers for the NumericVars will be there too.
  *
+ * On platforms which support 128-bit integers some aggergates instead use a
+ * 128-bit integer based transition datatype to speed up calculations.
+ *
  * --
  */
 
@@ -2917,6 +2923,65 @@ numeric_accum_inv(PG_FUNCTION_ARGS)
 	PG_RETURN_POINTER(state);
 }
 
+#ifdef HAVE_INT128
+typedef struct Int16AggState
+{
+	bool	calcSumX2;	/* if true, calculate sumX2 */
+	int64	N;			/* count of processed numbers */
+	int128	sumX;		/* sum of processed numbers */
+	int128	sumX2;		/* sum of squares of processed numbers */
+} Int16AggState;
+
+/*
+ * Prepare state data for a 128-bit aggregate function that needs to compute
+ * sum, count and optionally sum of squares of the input.
+ */
+static Int16AggState *
+makeInt16AggState(FunctionCallInfo fcinfo, bool calcSumX2)
+{
+	Int16AggState *state;
+	MemoryContext agg_context;
+	MemoryContext old_context;
+
+	if (!AggCheckCallContext(fcinfo, agg_context))
+

Re: [HACKERS] Misaligned BufferDescriptors causing major performance problems on AMD

On Thu, Apr 17, 2014 at 11:23:24AM +0200, Andres Freund wrote:
 On 2014-04-16 19:18:02 -0400, Bruce Momjian wrote:
  On Thu, Feb  6, 2014 at 09:40:32AM +0100, Andres Freund wrote:
   On 2014-02-05 12:36:42 -0500, Robert Haas wrote:
 It may well be that your proposal is spot on.  But I'd like to see 
 some
 data-structure-by-data-structure measurements, rather than assuming 
 that
 alignment must be a good thing.

 I am fine with just aligning BufferDescriptors properly. That has
 clearly shown massive improvements.

I thought your previous idea of increasing BUFFERALIGN to 64 bytes had
a lot to recommend it.
   
   Good.
   
   I wonder if we shouldn't move that bit of logic:
 if (size = BUFSIZ)
 newStart = BUFFERALIGN(newStart);
   out of ShmemAlloc() and instead have a ShmemAllocAligned() and
   ShmemInitStructAligned() that does it. So we can sensibly can control it
   per struct.
   
But that doesn't mean it doesn't need testing.
   
   I feel the need here, to say that I never said it doesn't need testing
   and never thought it didn't...
  
  Where are we on this?
 
 It needs somebody with time to evaluate possible performance regressions
 - I personally won't have time to look into this in detail before pgcon.

I am doing performance testing to try to complete this item.  I used the
first attached patch to report which structures are 64-byte aligned:

64-byte shared memory alignment of Control File:  0
64-byte shared memory alignment of XLOG Ctl:  1
64-byte shared memory alignment of CLOG Ctl:  0
64-byte shared memory alignment of CommitTs Ctl:  0
64-byte shared memory alignment of CommitTs shared:  0
64-byte shared memory alignment of SUBTRANS Ctl:  1
64-byte shared memory alignment of MultiXactOffset Ctl:  1
64-byte shared memory alignment of MultiXactMember Ctl:  1
64-byte shared memory alignment of Shared MultiXact State:  1
64-byte shared memory alignment of Buffer Descriptors:  1
64-byte shared memory alignment of Buffer Blocks:  1
64-byte shared memory alignment of Shared Buffer Lookup Table:  1
64-byte shared memory alignment of Buffer Strategy Status:  1
64-byte shared memory alignment of LOCK hash:  0
64-byte shared memory alignment of PROCLOCK hash:  0
64-byte shared memory alignment of Fast Path Strong Relation Lock Data: 
 0
64-byte shared memory alignment of PREDICATELOCKTARGET hash:  0
64-byte shared memory alignment of PREDICATELOCK hash:  0
64-byte shared memory alignment of PredXactList:  0
64-byte shared memory alignment of SERIALIZABLEXID hash:  1
64-byte shared memory alignment of RWConflictPool:  1
64-byte shared memory alignment of FinishedSerializableTransactions:  0
64-byte shared memory alignment of OldSerXid SLRU Ctl:  1
64-byte shared memory alignment of OldSerXidControlData:  1
64-byte shared memory alignment of Proc Header:  0
64-byte shared memory alignment of Proc Array:  0
64-byte shared memory alignment of Backend Status Array:  0
64-byte shared memory alignment of Backend Application Name Buffer:  0
64-byte shared memory alignment of Backend Client Host Name Buffer:  0
64-byte shared memory alignment of Backend Activity Buffer:  0
64-byte shared memory alignment of Prepared Transaction Table:  0
64-byte shared memory alignment of Background Worker Data:  0
64-byte shared memory alignment of shmInvalBuffer:  1
64-byte shared memory alignment of PMSignalState:  0
64-byte shared memory alignment of ProcSignalSlots:  0
64-byte shared memory alignment of Checkpointer Data:  0
64-byte shared memory alignment of AutoVacuum Data:  0
64-byte shared memory alignment of Wal Sender Ctl:  0
64-byte shared memory alignment of Wal Receiver Ctl:  0
64-byte shared memory alignment of BTree Vacuum State:  0
64-byte shared memory alignment of Sync Scan Locations List:  0
64-byte shared memory alignment of Async Queue Control:  0
64-byte shared memory alignment of Async Ctl:  0

Many of these are 64-byte aligned, including Buffer Descriptors.  I
tested pgbench with these commands:

$ pgbench -i -s 95 pgbench
$ pgbench -S -c 95 -j 95 -t 10 pgbench

on a 16-core Xeon server and got 84k tps.  I then applied another patch,
attached, which causes all the structures to be non-64-byte aligned, but
got the same tps number.

Can someone test these patches on an AMD CPU and see if you see a
difference?  Thanks.

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

  + Everyone has their own god. +
diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
new file mode 100644

Re: [HACKERS] install libpq.dll in bin directory on Windows / Cygwin

2014-12-23 Thread Michael Paquier

On Mon, Dec 22, 2014 at 2:05 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
 Peter, could it be possible to merge this patch with its MSVC portion
 for simplicity? I think that it would more readable to do all the
 changes at the same time once and for all. Also, that's still a bug,
 so are we still considering a backpatch? I wouldn't mind putting some
 time into a patch to get that fixed..

Attached are two patches, one for MinGW/cygwin, a slightly modified
version from Peter and the second implementing the same thing but for
the MSVC scripts. The method for MSVC is similar to what is done in
Peter's patch: roughly it checks if SO_MAJOR_VERSION is present in the
Makefile of a given library, the path of Makefile is found by looking
at the location of the .rc in the vcproj file (could be better but I
could not come up with a better method). TBH, it would be good to be
completely consistent in the way we build things on Windows, and we
may as well consider a backpatch to fix this long-standing bug. The
MSVC patch removes of course the hack copying libpq.dll from lib/ to
bin/.

I mentioned the fix for MSVC scripts as well here:
http://www.postgresql.org/message-id/cab7npqqiuepzphd3mmk+q7_cqqrkk_v1fvxknymri660z4d...@mail.gmail.com
Regards,
-- 
Michael
From 23c3fd58d65309e2ee6cc5ada9c3dee37110c70d Mon Sep 17 00:00:00 2001
From: Michael Paquier michael@otacoo.com
Date: Tue, 23 Dec 2014 05:01:38 -0800
Subject: [PATCH 1/2] Install shared libraries in bin/ and lib/ with
 MinGW/cygwin

Those libraries can be found by scanning for SO_MAJOR_VERSION in their
respective Makefiles.
---
 src/Makefile.shlib| 11 ++-
 src/interfaces/libpq/Makefile |  9 -
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/Makefile.shlib b/src/Makefile.shlib
index 674fe7e..c3af1fe 100644
--- a/src/Makefile.shlib
+++ b/src/Makefile.shlib
@@ -486,6 +486,9 @@ endif
 endif # not win32
 endif # not cygwin
 endif # not aix
+ifneq (,$(findstring $(PORTNAME),win32 cygwin))
+	$(INSTALL_SHLIB) $ '$(DESTDIR)$(bindir)/$(shlib)'
+endif
 else # no soname
 	$(INSTALL_SHLIB) $ '$(DESTDIR)$(pkglibdir)/$(shlib)'
 endif
@@ -494,7 +497,10 @@ endif
 installdirs-lib:
 ifdef soname
 	$(MKDIR_P) '$(DESTDIR)$(libdir)' '$(DESTDIR)$(pkgconfigdir)'
-else
+ifneq (,$(findstring $(PORTNAME),win32 cygwin))
+	$(MKDIR_P) '$(DESTDIR)$(bindir)'
+endif
+else # no soname
 	$(MKDIR_P) '$(DESTDIR)$(pkglibdir)'
 endif
 
@@ -511,6 +517,9 @@ ifdef soname
 	  '$(DESTDIR)$(libdir)/$(shlib_major)' \
 	  '$(DESTDIR)$(libdir)/$(shlib)' \
 	  '$(DESTDIR)$(pkgconfigdir)/lib$(NAME).pc'
+ifneq (,$(findstring $(PORTNAME),win32 cygwin))
+	rm -f '$(DESTDIR)$(bindir)/$(shlib)'
+endif
 else # no soname
 	rm -f '$(DESTDIR)$(pkglibdir)/$(shlib)'
 endif # no soname
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 18d9b85..5f0042e 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -125,18 +125,12 @@ install: all installdirs install-lib
 	$(INSTALL_DATA) $(srcdir)/libpq-int.h '$(DESTDIR)$(includedir_internal)'
 	$(INSTALL_DATA) $(srcdir)/pqexpbuffer.h '$(DESTDIR)$(includedir_internal)'
 	$(INSTALL_DATA) $(srcdir)/pg_service.conf.sample '$(DESTDIR)$(datadir)/pg_service.conf.sample'
-ifneq (,$(findstring $(PORTNAME), win32 cygwin))
-	$(INSTALL_SHLIB) $(shlib) '$(DESTDIR)$(bindir)/$(shlib)'
-endif
 
 installcheck:
 	$(MAKE) -C test $@
 
 installdirs: installdirs-lib
 	$(MKDIR_P) '$(DESTDIR)$(includedir)' '$(DESTDIR)$(includedir_internal)' '$(DESTDIR)$(datadir)'
-ifneq (,$(findstring $(PORTNAME), win32 cygwin))
-	$(MKDIR_P) '$(DESTDIR)$(bindir)'
-endif
 
 uninstall: uninstall-lib
 	rm -f '$(DESTDIR)$(includedir)/libpq-fe.h'
@@ -144,9 +138,6 @@ uninstall: uninstall-lib
 	rm -f '$(DESTDIR)$(includedir_internal)/libpq-int.h'
 	rm -f '$(DESTDIR)$(includedir_internal)/pqexpbuffer.h'
 	rm -f '$(DESTDIR)$(datadir)/pg_service.conf.sample'
-ifneq (,$(findstring $(PORTNAME), win32 cygwin))
-	rm -f '$(DESTDIR)$(bindir)/$(shlib)'
-endif
 
 clean distclean: clean-lib
 	$(MAKE) -C test $@
-- 
2.2.1

From 13ca64cb5cdae419d6ee1bbf7c7a04f6bd3d2388 Mon Sep 17 00:00:00 2001
From: Michael Paquier michael@otacoo.com
Date: Tue, 23 Dec 2014 21:58:50 -0800
Subject: [PATCH 2/2] Install shared libraries in bin/ and lib/ with MSVC

This is the MSVC part of the previous commit, to ensure that install is
completely consistent with the multiple methods supported.
---
 src/tools/msvc/Install.pm | 43 +++
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index eba9aa0..616cd9d 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -91,7 +91,6 @@ sub Install
 	}
 
 	CopySolutionOutput($conf, $target);
-	lcopy($target . '/lib/libpq.dll', $target . '/bin/libpq.dll');
 	my $sample_files = [];
 	my @top_dir  = (src);
 	@top_dir = (src\\bin, src\\interfaces) if ($insttype eq client);
@@ -236,8 +235,9 @@

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)


On 12/20/14, 2:13 PM, Jim Nasby wrote:

On 12/20/14, 11:51 AM, Tom Lane wrote:

Andres Freund and...@2ndquadrant.com writes:

On 2014-12-19 22:03:55 -0600, Jim Nasby wrote:

What I am thinking is not using all of those fields in their raw form to 
calculate the hash value. IE: something analogous to:
hash_any(SharedBufHash, (rot(forkNum, 2) | dbNode) ^ relNode)  32 | blockNum)

perhaps that actual code wouldn't work, but I don't see why we couldn't do 
something similar... am I missing something?



I don't think that'd improve anything. Jenkin's hash does have a quite
mixing properties, I don't believe that the above would improve the
quality of the hash.


I think what Jim is suggesting is to intentionally degrade the quality of
the hash in order to let it be calculated a tad faster.  We could do that
but I doubt it would be a win, especially in systems with lots of buffers.
IIRC, when we put in Jenkins hashing to replace the older homebrew hash
function, it improved performance even though the hash itself was slower.


Right. Now that you mention it, I vaguely recall the discussions about changing 
the hash function to reduce collisions.

I'll still take a look at fash-hash, but it's looking like there may not be 
anything we can do here unless we change how we identify relation files 
(combining dbid, tablespace id, fork number and file id, at least for 
searching). If we had 64bit hash support then maybe that'd be a significant 
win, since you wouldn't need to hash at all. But that certainly doesn't seem to 
be low-hanging fruit to me...


I've taken a look at a number of different hash algorithms, testing them with 
initially with SMHasher [1] and then with pgbench.

It's worth noting that a lot of work has been done in the area of hash algo's 
since we last updated the hash_any algorithm in 2009 [2]. It's probably worth 
revisiting this every other release or so.

Most of my SMHasher results are at [3]. I suspect SpookyHash is close to what 
we currently have, so that's what I used for a baseline. I determined that 
fash-hash (called superfast in results) was faster than Spooky, but not as fast 
as CityHash64[4] or xxhash[5]. CityHash and xxhash had similar performance, but 
xxhash seems to have slightly better characteristics according to SMHasher, and 
more important, it's in C, not C++. However, CityHash has been replaced by 
farmhash[7], which might be faster than xxhash.

I did a quick hack at using xxhash ONLY for shared buffer lookup [6]. I've 
attached the full patch, as well as a version that omits the new files. Note 
that currently xxhash is setup in such a way that we'd get different results 
depending on endian-ness, so we couldn't just drop this in as-is across the 
board. Of course, there's also the question of if we'd want to force a REINDEX 
on hash indexes.

pgbench results are below. Select-only testing was done first, then read-write. 
There were several select-only runs on both to warm shared_buffers (set to 
512MB for this test, and fsync is off). Database initialized with bin/pgbench 
-i -F 100 -s 10.

pgbench -S -T10 -c 4 -j 4
master:
tps = 9556.356145 (excluding connections establishing)
tps = 9897.324917 (excluding connections establishing)
tps = 9287.286907 (excluding connections establishing)
tps = 10210.130833 (excluding connections establishing)

XXH32:
tps = 32462.754437 (excluding connections establishing)
tps = 33232.144511 (excluding connections establishing)
tps = 33082.436760 (excluding connections establishing)
tps = 33597.904532 (excluding connections establishing)

pgbench -T10 -c 4 -j 4
master:
tps = 2299.314145 (excluding connections establishing)
tps = 2029.956749 (excluding connections establishing)
tps = 2067.462362 (excluding connections establishing)

XXH32:
tps = 2653.919321 (excluding connections establishing)
tps = 2615.764370 (excluding connections establishing)
tps = 2952.144461 (excluding connections establishing)

Questions:
Do we want to do what we've previously done and cut/paste/modify this code into 
our repo? Given how rapidly hash algorithms seem to be changing I'm inclined to 
minimize the amount of effort required to pull a new one in...

Can someone with a big-endian CPU see what the impact of 
XXH_FORCE_NATIVE_FORMAT 1 vs 0? If there's a notable difference we might want 
to do different things for on-disk vs in-memory hashes.

For that matter, assuming we adopt this, do we want to replace the index hash 
functions too? SMHasher shows that CityHash is ~50% faster than XXHash for 
262144 byte keys. Even SpookyHash is about 17% faster on that key size. So if 
we want to do something with hash indexes, we'd probably be better off with 
City or Farm hash than XXHash.

[1] https://code.google.com/p/smhasher/
[2] 
https://github.com/postgres/postgres/commit/8205258fa675115439017b626c4932d5fefe2ea8
[3] https://github.com/decibel/hash_testing/tree/master/results
[4] https://code.google.com/p/cityhash/
[5] https://code.google.com/p/xxhash/
[6]

Re: hash_create API changes (was Re: [HACKERS] speedup tidbitmap patch: hash BlockNumber)