Re: [HACKERS] GIN improvements part 1: additional information
On 04.11.2013 23:44, Alexander Korotkov wrote: Attached version of patch has support of old page format. Patch still needs more documentation and probably refactoring, but I believe idea is clear and it can be reviewed. In the patch I have to revert change of null category placement for compatibility with old posting list format. I committed some of the refactorings and cleanup included in this patch. Attached is a rebased version that applies over current head. I hope I didn't joggle your elbow.. - Heikki gin-packed-postinglists-12-rebased.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.4] row level security
On 11/06/2013 05:02 PM, Dean Rasheed wrote: The basic idea is to have rewriteTargetView() collect up any quals from SB views in a new list on the target RTE, instead of adding them to the main query's predicates (it needs to be a list of SB quals, in case there are SB views on top of other SB views, in which case they need to be kept separate from one another). Then at the end of the rewriting process (after any views referenced in the SB quals have been expanded), a new piece of code kicks in to process any RTEs with SB quals, turning them into (possibly nested) subquery RTEs. That makes sense, though presumably you face the same problem that the existing RLS code does with references to system columns that don't normally exist in subqueries? Since this happens during query rewrite, what prevents the optimizer from pushing outer quals down into the subqueries? The complication is that the query's resultRelation RTE mustn't be a subquery. I think this is what Robert was alluding to earlier with his comments about join relations: Robert Haas wrote: I don't really see why. AIUI, the ModifyTable node just needs to get the right TIDs. It's not like that has to be stacked directly on top of a scan; indeed, in cases like UPDATE .. FROM and DELETE .. USING it already isn't. Maybe there's some reason why the intervening level can be a Join but not a SubqueryScan, but if so I'd expect we could find some way of lifting that limitation without suffering too much pain. (http://www.postgresql.org/message-id/ca+tgmoyr1phw3x9vnvuwdcfxkzk2p_jhtwc0fv2q58negcx...@mail.gmail.com) Maybe we just need to make a subquery scan a valid target for an update, so those fixups aren't required anymore? This is handled this in a similar way to the trigger-updatable views code, producing 2 RTEs --- the resultRelation RTE becomes a direct reference to the base relation, and a separate subquery RTE acts as the source of rows to update. Some of the complexity of the current RLS code is caused by the need to do similar fixups to handle the case where the input relation isn't the same as the target relation, but is a subquery over it instead. Anyway, feel free to do what you like with this. I wasn't planning on submitting it to the next commitfest myself, because my non-PG workload is too high, and I don't expect to get much time to hack on postgresql during the next couple of months. Thanks for sending what you have. It's informative, and it shows that some of the same issues must be solved for writable security barrier views and for RLS. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.4] row level security
On 6 November 2013 09:23, Craig Ringer cr...@2ndquadrant.com wrote: On 11/06/2013 05:02 PM, Dean Rasheed wrote: The basic idea is to have rewriteTargetView() collect up any quals from SB views in a new list on the target RTE, instead of adding them to the main query's predicates (it needs to be a list of SB quals, in case there are SB views on top of other SB views, in which case they need to be kept separate from one another). Then at the end of the rewriting process (after any views referenced in the SB quals have been expanded), a new piece of code kicks in to process any RTEs with SB quals, turning them into (possibly nested) subquery RTEs. That makes sense, though presumably you face the same problem that the existing RLS code does with references to system columns that don't normally exist in subqueries? Yeah, that feels like an ugly hack. Since this happens during query rewrite, what prevents the optimizer from pushing outer quals down into the subqueries? The subquery RTE is marked with the security_barrier flag, which prevents quals from being pushed down in the presence of leaky functions (see set_subquery_pathlist). The complication is that the query's resultRelation RTE mustn't be a subquery. I think this is what Robert was alluding to earlier with his comments about join relations: Robert Haas wrote: I don't really see why. AIUI, the ModifyTable node just needs to get the right TIDs. It's not like that has to be stacked directly on top of a scan; indeed, in cases like UPDATE .. FROM and DELETE .. USING it already isn't. Maybe there's some reason why the intervening level can be a Join but not a SubqueryScan, but if so I'd expect we could find some way of lifting that limitation without suffering too much pain. (http://www.postgresql.org/message-id/ca+tgmoyr1phw3x9vnvuwdcfxkzk2p_jhtwc0fv2q58negcx...@mail.gmail.com) Yeah. We already do a similar thing for trigger-updatable views. So with this approach you end up with similar plans, for example: Update on base_tbl - Subquery Scan on base_tbl ... Maybe we just need to make a subquery scan a valid target for an update, so those fixups aren't required anymore? Possibly. That feels like it would require much more extensive surgery on the planner though. I've not explored that idea, but I suspect it would quickly turn into a whole new can of worms. This is handled this in a similar way to the trigger-updatable views code, producing 2 RTEs --- the resultRelation RTE becomes a direct reference to the base relation, and a separate subquery RTE acts as the source of rows to update. Some of the complexity of the current RLS code is caused by the need to do similar fixups to handle the case where the input relation isn't the same as the target relation, but is a subquery over it instead. Anyway, feel free to do what you like with this. I wasn't planning on submitting it to the next commitfest myself, because my non-PG workload is too high, and I don't expect to get much time to hack on postgresql during the next couple of months. Thanks for sending what you have. It's informative, and it shows that some of the same issues must be solved for writable security barrier views and for RLS. Agreed. I'm not sure what the best way to fix those issues is though. The currently proposed approach feels pretty ugly, but I can't see a better way at the moment. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK
Andres Freund and...@2ndquadrant.com Looks fine to me Any thoughts on whether this should be back-patched to 9.3? I can see arguments both ways, and don't have a particularly strong feeling one way or the other. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #8573: int4range memory consumption
On Mon, Nov 4, 2013 at 10:51 PM, Amit Kapila amit.kapil...@gmail.com wrote: On Tue, Nov 5, 2013 at 12:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: Amit Kapila amit.kapil...@gmail.com writes: On Mon, Nov 4, 2013 at 8:15 PM, Tom Lane t...@sss.pgh.pa.us wrote: Really I'd like to see standalone mode, in its current form, go away completely. I had a prototype patch for allowing psql and other clients to interface to a standalone backend. I think getting that finished would be a way more productive use of time than improving debugtup. The last patch including Windows implementation was posted at below link sometime back. http://www.postgresql.org/message-id/6C0B27F7206C9E4CA54AE035729E9C382853263C@szxeml509-mbs If this is the right thing, I can rebase the patch and make it ready. IIRC, the discussion stalled out because people had security concerns and/or there wasn't consensus about how it should look at the user level. There's probably not much point in polishing a patch until we have more agreement about the high-level design. Okay. However +1 for working on that idea as lot of people have shown interest in it. Agreed. I remember the sticking point as being mostly, like, whether we should just make it work, or whether we had to do a huge amount of additional work to turn it into something quite different from what you had in mind. That question answers itself. -- 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] better atomics
On Tue, Nov 5, 2013 at 10:51 AM, Andres Freund and...@2ndquadrant.com wrote:
Hi,
(Coming back to this now)
On 2013-10-28 21:55:22 +0100, Andres Freund wrote:
The list I have previously suggested was:
* pg_atomic_load_u32(uint32 *)
* uint32 pg_atomic_store_u32(uint32 *)
To be able to write code without spreading volatiles all over while
making it very clear that that part of the code is worthy of attention.
* uint32 pg_atomic_exchange_u32(uint32 *ptr, uint32 val)
* bool pg_atomic_compare_exchange_u32(uint32 *ptr, uint32 *expected, uint32
newval)
So what I've previously proposed did use plain types for the
to-be-manipulated atomic integers. I am not sure about that anymore
though, C11 includes support for atomic operations, but it assumes that
the to-be-manipulated variable is of a special atomic type. While I am
not propose that we try to emulate C11's API completely (basically
impossible as it uses type agnostic functions, it also exposes too
much), it seems to make sense to allow PG's atomics to be implemented by
C11's.
The API is described at: http://en.cppreference.com/w/c/atomic
The fundamental difference to what I've suggested so far is that the
atomic variable isn't a plain integer/type but a distinct datatype that
can *only* be manipulated via the atomic operators. That has the nice
property that it cannot be accidentally manipulated via plain operators.
E.g. it wouldn't be
uint32 pg_atomic_fetch_add_u32(uint32 *ptr, uint32 add_);
but
uint32 pg_atomic_fetch_add_u32(pg_atomic_uint32 *ptr, uint32 add_);
where, for now, atomic_uint32 is something like:
typedef struct pg_atomic_uint32
{
volatile uint32 value;
} pg_atomic_uint32;
a future C11 implementation would just typedef C11's respective atomic
type to pg_atomic_uint32.
Comments?
Sadly, I don't have a clear feeling for how well or poorly that's
going to work out.
--
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] git diff --check whitespace checks, gitattributes
On 11/5/13, 10:31 PM, Tom Lane wrote: I always (well, almost always) do git diff --check, so making it stronger sounds good to me. But it sounds like this still leaves it to the committer to remember to run it. Can we do anything about that? Sure, you could install an update hook on the server. I'm generally not in favor of such things, but that's a separate discussion. Build servers could also do this checking. Maybe we should think about fixing psql to not generate that whitespace. Not easy, see http://www.postgresql.org/message-id/1285093687.5468.18.ca...@vanquo.pezone.net -- 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] git diff --check whitespace checks, gitattributes
On 11/5/13, 11:17 PM, Alvaro Herrera wrote: I think pasting psql output verbatim isn't such a great idea anyway. Maybe it's okay for certain specific examples, but in most cases I think it'd be better to produce native SGML tables instead of programoutput stuff. After all, it's the result set that's interesting, not the way psql presents it. I'm not convinced that that would be better, but it's worth a try. -- 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] exit_horribly vs exit_nicely in pg_dump
On 11/5/13, 8:46 AM, Pavel Golub wrote: I suppose this should be call to exit_nicely() for all possible cases. The only need for calling exit_horribly() is when we are deep down in the multithreaded code, AFAIK. Doesn't hurt either, though. But it would be OK to make this more consistent. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] FDW: possible resjunk columns in AddForeignUpdateTargets
I have a question concerning the Foreign Data Wrapper API: I find no mention of this in the documentation, but I remember that you can only add a resjunk column that matches an existing attribute of the foreign table and not one with an arbitrary name or definition. Ist that right? Yours, Laurenz Albe -- 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] git diff --check whitespace checks, gitattributes
On 11/05/2013 10:31 PM, Tom Lane wrote: Peter Eisentraut pete...@gmx.net writes: Attached is a patch that - Adds a .gitattributes file to configure appropriate whitespace checks for git diff --check. - Cleans up all whitespace errors found in this way in existing code. Most of that is in files not covered by pgindent, some in new code since the last pgindent. This makes the entire tree git diff --check clean. After this, future patches can be inspected for whitespace errors with git diff --check, something that has been discussed on occasion. I always (well, almost always) do git diff --check, so making it stronger sounds good to me. But it sounds like this still leaves it to the committer to remember to run it. Can we do anything about that? Personally I don't get the mania about trailing whitespace, but maybe that's just me. We could certainly implement a hook on the server that would reject cases that fail this test, but we might find it hamstrings us a bit in future. I'm not sure what else could be done to remedy committer forgetfulness, though. 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] FDW: possible resjunk columns in AddForeignUpdateTargets
2013/11/6 Albe Laurenz laurenz.a...@wien.gv.at: I have a question concerning the Foreign Data Wrapper API: I find no mention of this in the documentation, but I remember that you can only add a resjunk column that matches an existing attribute of the foreign table and not one with an arbitrary name or definition. Ist that right? My understanding (having recently had a crack at getting an FDW working) is that the name can be arbitrary within reason - at least that's what I get from this bit of the documentation: To do that, add TargetEntry items to parsetree-targetList, containing expressions for the extra values to be fetched. Each such entry must be marked resjunk = true, and must have a distinct resname that will identify it at execution time. Avoid using names matching ctidN or wholerowN, as the core system can generate junk columns of these names. http://www.postgresql.org/docs/9.3/interactive/fdw-callbacks.html#FDW-CALLBACKS-UPDATE Someone more knowledgeable than myself will know better, I hope. Regards Ian Barwick -- 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] ERROR during end-of-xact/FATAL
Noah Misch wrote: Incomplete list: - If smgrDoPendingDeletes() finds files to delete, mdunlink() and its callee relpathbackend() call palloc(); this is true in all supported branches. In 9.3, due to commit 279628a0, smgrDoPendingDeletes() itself calls palloc(). (In fact, it does so even when the pending list is empty -- this is the only palloc() during a trivial transaction commit.) palloc() failure there yields a PANIC during commit. I think we should fix this routine to avoid the palloc when not necessary. That initial palloc is pointless. Also, there have been previous discussions about having relpathbackend not use palloc at all. That was only because we wanted to use it in pg_xlogdump which didn't have palloc support at the time, so it's no longer as pressing; but perhaps it's still worthy of consideration. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] better atomics
On 2013-11-06 16:48:17 +0200, Ants Aasma wrote: C11 atomics need to be initialized through atomic_init(), so a simple typedef will not be correct here. Also, C11 atomics are designed to have the compiler take care of memory barriers - so they might not always be a perfect match to our API's, or any API implementable without compiler support. Yea, I think we'll always need to have our layer above C11 atomics, but it still will be useful to allow them to be used to implement our atomics. FWIW, I find the requirement for atomic_init() really, really annoying. Not that that will change anything ;) However I'm mildly supportive on having a separate type for variables accessed by atomics. It can result in some unnecessary code churn, but in general if an atomic access to a variable is added, then all other accesses to it need to be audited for memory barriers, even if they were previously correctly synchronized by a lock. Ok, that's what I am writing right now. I guess the best approach for deciding would be to try to convert a couple of the existing unlocked accesses to the API and see what the patch looks like. I don't think there really exist any interesting ones? I am using my lwlock reimplementation as a testbed so far. 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] better atomics
On Tue, Nov 5, 2013 at 5:51 PM, Andres Freund and...@2ndquadrant.com wrote:
So what I've previously proposed did use plain types for the
to-be-manipulated atomic integers. I am not sure about that anymore
though, C11 includes support for atomic operations, but it assumes that
the to-be-manipulated variable is of a special atomic type. While I am
not propose that we try to emulate C11's API completely (basically
impossible as it uses type agnostic functions, it also exposes too
much), it seems to make sense to allow PG's atomics to be implemented by
C11's.
The API is described at: http://en.cppreference.com/w/c/atomic
The fundamental difference to what I've suggested so far is that the
atomic variable isn't a plain integer/type but a distinct datatype that
can *only* be manipulated via the atomic operators. That has the nice
property that it cannot be accidentally manipulated via plain operators.
E.g. it wouldn't be
uint32 pg_atomic_fetch_add_u32(uint32 *ptr, uint32 add_);
but
uint32 pg_atomic_fetch_add_u32(pg_atomic_uint32 *ptr, uint32 add_);
where, for now, atomic_uint32 is something like:
typedef struct pg_atomic_uint32
{
volatile uint32 value;
} pg_atomic_uint32;
a future C11 implementation would just typedef C11's respective atomic
type to pg_atomic_uint32.
Comments?
C11 atomics need to be initialized through atomic_init(), so a simple
typedef will not be correct here. Also, C11 atomics are designed to
have the compiler take care of memory barriers - so they might not
always be a perfect match to our API's, or any API implementable
without compiler support.
However I'm mildly supportive on having a separate type for variables
accessed by atomics. It can result in some unnecessary code churn, but
in general if an atomic access to a variable is added, then all other
accesses to it need to be audited for memory barriers, even if they
were previously correctly synchronized by a lock.
I guess the best approach for deciding would be to try to convert a
couple of the existing unlocked accesses to the API and see what the
patch looks like.
Regards,
Ants Aasma
--
Cybertec Schönig Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] shared memory message queues
This patch needs to be rebased. -- 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] UNION ALL on partitioned tables won't use indices.
On 10/29/13, 11:05 PM, Kyotaro HORIGUCHI wrote: Hello, Please add your patches to the currently-open CommitFest so that we don't lose track of them. https://commitfest.postgresql.org/action/commitfest_view/open I'm not sure which approach to this problem is best, but I agree that it is worth solving. Thank you, I've regsitered this on CF3. Your patch doesn't apply anymore. Please rebase it. -- 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] FDW: possible resjunk columns in AddForeignUpdateTargets
Albe Laurenz laurenz.a...@wien.gv.at writes: I have a question concerning the Foreign Data Wrapper API: I find no mention of this in the documentation, but I remember that you can only add a resjunk column that matches an existing attribute of the foreign table and not one with an arbitrary name or definition. Ist that right? I don't recall such a limitation offhand. It's probably true that you can't create Vars that don't match some declared column of the table, but that doesn't restrict what you put into resjunk columns AFAIR. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Recent matview push broken with -DCLOBBER_CACHE_ALWAYS
I am investigating. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] git diff --check whitespace checks, gitattributes
Peter Eisentraut pete...@gmx.net writes: On 11/5/13, 10:31 PM, Tom Lane wrote: Maybe we should think about fixing psql to not generate that whitespace. Not easy, see http://www.postgresql.org/message-id/1285093687.5468.18.ca...@vanquo.pezone.net Ah, thanks for the reminder. One killer point in that discussion seemed to be that even if we got rid of the bogus whitespace in result header lines, there might be legitimate trailing whitespace *in the result data* --- consider SELECT 'foo '::text as an example. So we can't install a blanket prohibition on trailing whitespace in *.out files. (Not that we need one anyway, since those aren't hand-edited source; but at the time, it was not clear --- at least not to me --- that we could apply different check rules to different files.) But, returning to the question of psql output pasted into SGML, it's less clear to me that we shouldn't complain about trailing whitespace in SGML files. If we suppressed psql's header-line whitespace, we'd greatly ease copying-and-pasting example output without triggering such warnings. So that might be a use-case that's sufficient to justify changing what psql does. On the third hand, there were also claims in that discussion that third-party extensions would be annoyed if we changed psql's header formatting, because they couldn't use the same regression output files across PG versions. Don't know how much weight to put on that argument. 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] git diff --check whitespace checks, gitattributes
Andrew Dunstan and...@dunslane.net writes: Personally I don't get the mania about trailing whitespace, but maybe that's just me. For me, it's an easily-checked thing that will reduce pgindent noise later. (Actually I tend to pgindent stuff before committing, these days, but sometimes that's impractical because somebody's already committed some not-well-indented stuff elsewhere in the same file.) We could certainly implement a hook on the server that would reject cases that fail this test, but we might find it hamstrings us a bit in future. I'm not sure what else could be done to remedy committer forgetfulness, though. I agree that that would not be 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] GIN improvements part 1: additional information
Heikki Linnakangas escribió: On 04.11.2013 23:44, Alexander Korotkov wrote: Attached version of patch has support of old page format. Patch still needs more documentation and probably refactoring, but I believe idea is clear and it can be reviewed. In the patch I have to revert change of null category placement for compatibility with old posting list format. I committed some of the refactorings and cleanup included in this patch. Attached is a rebased version that applies over current head. I hope I didn't joggle your elbow.. Just for my own illumination, can someone explain this bit? + If a posting list is too large to store in-line in a key entry, a posting tree + is created. A posting tree is a B-tree structure, where the ItemPointer is + used as the key. At the leaf-level, item pointers are stored compressed, in + varbyte encoding. I think the first ItemPointer mentioned (the key) refers to a TID pointing to the index, and item pointers stored compressed refers to the TIDs pointing to the heap (the data). Is that correct? I'm also interested in the FIXME explain varbyte encoding explanation currently missing, if somebody can write it down ... Thanks, -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] better atomics
On Wed, Nov 6, 2013 at 4:54 PM, Andres Freund and...@2ndquadrant.com wrote: FWIW, I find the requirement for atomic_init() really, really annoying. Not that that will change anything ;) Me too, but I guess this allows them to emulate atomics on platforms that don't have native support. Whether that is a good idea is a separate question. However I'm mildly supportive on having a separate type for variables accessed by atomics. It can result in some unnecessary code churn, but in general if an atomic access to a variable is added, then all other accesses to it need to be audited for memory barriers, even if they were previously correctly synchronized by a lock. Ok, that's what I am writing right now. I guess the best approach for deciding would be to try to convert a couple of the existing unlocked accesses to the API and see what the patch looks like. I don't think there really exist any interesting ones? I am using my lwlock reimplementation as a testbed so far. BufferDesc management in src/backend/storage/buffer/bufmgr.c. The seqlock like thing used to provide consistent reads from PgBackendStatus src/backend/postmaster/pgstat.c (this code looks like it's broken on weakly ordered machines) The unlocked reads that are done from various procarray variables. The XLogCtl accesses in xlog.c. IMO the volatile keyword should be confined to the code actually implementing atomics, locking. The use volatile pointer to prevent code rearrangement comment is evil. If there are data races in the code, they need comments pointing this out and probably memory barriers too. If there are no data races, then the volatile declaration is useless and a pessimization. Currently it's more of a catch all here be dragons declaration for data structures. Regards, Ants Aasma -- Cybertec Schönig Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt Web: http://www.postgresql-support.de -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] git diff --check whitespace checks, gitattributes
Tom Lane wrote: (Actually I tend to pgindent stuff before committing, these days, but sometimes that's impractical because somebody's already committed some not-well-indented stuff elsewhere in the same file.) What I normally do is commit the changes, then pgindent, then manually review the pgindent changes (which are normally tiny because my editor is configured to produce very similar results to pgindent, as I'm sure yours is.) Then I manually revert the parts not in my commit (also eased by editor support). Then I squash the commits. This doesn't take very long. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_fallocate
On 10/31/13, 9:16 AM, Mitsumasa KONDO wrote: I'l like to add fallocate() system call to improve sequential read/write peformance. fallocate() system call is different from posix_fallocate() that is zero-fille algorithm to reserve continues disk space. fallocate() is almost less overhead alogotithm to reserve continues disk space than posix_fallocate(). Your patch seems to be missing a bit that defines HAVE_FALLOCATE, probably something in configure.in. -- 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] better atomics
On 2013-11-06 08:15:59 -0500, Robert Haas wrote:
The API is described at: http://en.cppreference.com/w/c/atomic
The fundamental difference to what I've suggested so far is that the
atomic variable isn't a plain integer/type but a distinct datatype that
can *only* be manipulated via the atomic operators. That has the nice
property that it cannot be accidentally manipulated via plain operators.
E.g. it wouldn't be
uint32 pg_atomic_fetch_add_u32(uint32 *ptr, uint32 add_);
but
uint32 pg_atomic_fetch_add_u32(pg_atomic_uint32 *ptr, uint32 add_);
where, for now, atomic_uint32 is something like:
typedef struct pg_atomic_uint32
{
volatile uint32 value;
} pg_atomic_uint32;
a future C11 implementation would just typedef C11's respective atomic
type to pg_atomic_uint32.
Comments?
Sadly, I don't have a clear feeling for how well or poorly that's
going to work out.
I've implemented it that way and it imo looks sensible. I dislike the
pg_atomic_init_(flag|u32|u64) calls that are forced on us by wanting to
be compatible with C11, but I think that doesn't end up being too bad.
So, attached is a very first draft of an atomics implementation. There's
lots to be done, but this is how I think it should roughly look like and
what things we should implement in the beginning.
The API should be understandable from looking at
src/include/storage/atomics.h
I haven't tested the IBM xlc, HPUX IA64 acc, Sunpro fallback at all, but
the important part is that those provide the necessary tools to
implement everything. Also, even the gcc implementation falls back to
compare_and_swap() based implementations for the operations, but that
shouldn't matter for now. I haven't looked for other compilers yet.
Questions:
* Fundamental issues with the API?
* should we split of compiler/arch specific parts of into include/ports
or such? -0.5 from me.
* should we add src/include/storage/atomics subdirectory? +0.5 from me.
* Do we really want to continue to cater to compilers not supporting
PG_USE_INLINE? I've tried to add support for them, but it does make
things considerably more #ifdef-y.
Afaik it's only HPUX's acc that has problems, and it seems to work but
generate warnings, so maybe that's ok?
* Which additional compilers do we want to support? icc (might be gcc
compatible)?
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
From 04a8dc8516323eb404a7a3392f950763497a76ec Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Wed, 6 Nov 2013 10:32:53 +0100
Subject: [PATCH 2/3] Very basic atomic ops implementation
Only gcc has been tested, stub implementations for
* msvc
* HUPX acc
* IBM xlc
* Sun/Oracle Sunpro compiler
are included.
All implementations only provide the bare minimum of operations for now and
rely on emulating more complex operations via compare_and_swap().
---
config/c-compiler.m4 | 77 +
configure.in | 19 +-
src/backend/storage/lmgr/Makefile| 2 +-
src/backend/storage/lmgr/atomics.c | 23 ++
src/include/c.h | 7 +
src/include/pg_config.h.in | 11 +-
src/include/storage/atomics-arch-arm.h | 29 ++
src/include/storage/atomics-generic-acc.h| 99 +++
src/include/storage/atomics-generic-gcc.h| 154 ++
src/include/storage/atomics-generic-msvc.h | 58
src/include/storage/atomics-generic-sunpro.h | 66 +
src/include/storage/atomics-generic-xlc.h| 84 ++
src/include/storage/atomics-generic.h| 290 +++
src/include/storage/atomics.h| 415 +++
src/include/storage/s_lock.h | 10 +-
15 files changed, 1325 insertions(+), 19 deletions(-)
create mode 100644 src/backend/storage/lmgr/atomics.c
create mode 100644 src/include/storage/atomics-arch-arm.h
create mode 100644 src/include/storage/atomics-generic-acc.h
create mode 100644 src/include/storage/atomics-generic-gcc.h
create mode 100644 src/include/storage/atomics-generic-msvc.h
create mode 100644 src/include/storage/atomics-generic-sunpro.h
create mode 100644 src/include/storage/atomics-generic-xlc.h
create mode 100644 src/include/storage/atomics-generic.h
create mode 100644 src/include/storage/atomics.h
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index 4ba3236..ae92281 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -289,3 +289,80 @@ if test x$Ac_cachevar = xyes; then
fi
undefine([Ac_cachevar])dnl
])# PGAC_PROG_CC_LDFLAGS_OPT
+
+AC_DEFUN([PGAC_HAVE_GCC__SYNC_INT32_ATOMICS],
+[AC_CACHE_CHECK(for __builtin_constant_p, pgac_cv__builtin_constant_p,
+[AC_TRY_COMPILE([static int x; static int y[__builtin_constant_p(x) ? x : 1];],
+[],
+[pgac_cv__builtin_constant_p=yes],
+[pgac_cv__builtin_constant_p=no])])
+if
Re: [HACKERS] better atomics
On 2013-11-06 17:47:45 +0200, Ants Aasma wrote: On Wed, Nov 6, 2013 at 4:54 PM, Andres Freund and...@2ndquadrant.com wrote: FWIW, I find the requirement for atomic_init() really, really annoying. Not that that will change anything ;) Me too, but I guess this allows them to emulate atomics on platforms that don't have native support. Whether that is a good idea is a separate question. Since static initialization is supported that would require quite some dirty hacks, but well, we're talking about compiler makers ;) I don't think there really exist any interesting ones? I am using my lwlock reimplementation as a testbed so far. BufferDesc management in src/backend/storage/buffer/bufmgr.c. The unlocked reads that are done from various procarray variables. The XLogCtl accesses in xlog.c. Those are actually only modified under spinlocks afair, so there isn't too much interesting happening. But yes, it'd be better if we used more explicit means to manipulate/query them. The seqlock like thing used to provide consistent reads from PgBackendStatus src/backend/postmaster/pgstat.c (this code looks like it's broken on weakly ordered machines) Whoa. Never noticed that bit. The consequences of races are quite low, but still... IMO the volatile keyword should be confined to the code actually implementing atomics, locking. The use volatile pointer to prevent code rearrangement comment is evil. If there are data races in the code, they need comments pointing this out and probably memory barriers too. If there are no data races, then the volatile declaration is useless and a pessimization. Currently it's more of a catch all here be dragons declaration for data structures. Agreed. But I don't think we can replace them all at once. Or well, I won't ;) 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] [v9.4] row level security
All, Just a comment: I'm really glad to see the serious work on this. If RLS doesn't make it into 9.4, it'll be because the problems of RLS are fundamentally unsolvable, not because we didn't give it our best. Great work, all! -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Recent matview push broken with -DCLOBBER_CACHE_ALWAYS
Kevin Grittner kgri...@ymail.com wrote: I am investigating. Information from the syscache entry for the table was being referenced after the relation was closed. We were generally getting away with it until the new locking caused a check for invalidation messages at a new spot. Moving the heap_close() call down a few lines fixed it by keeping the syscache entry valid until we were through with it. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [v9.4] row level security
2013/11/6 Craig Ringer cr...@2ndquadrant.com: On 11/05/2013 09:36 PM, Robert Haas wrote: I haven't studied this patch in detail, but I see why there's some unhappiness about that code: it's an RLS-specific kluge. Just shooting from the hip here, maybe we should attack the problem of making security-barrier views updatable first, as a separate patch. That's the approach I've been considering. There are a few wrinkles with it, though: (a) Updatable views are implemented in the rewriter, not the planner. The rewriter is not re-run when plans are invalidated or when the session authorization changes, etc. This means that we can't simply omit the RLS predicate for superuser because the same rewritten parse tree might get used for both superuser and non-superuser queries. Options: * Store the before-rewrite parse tree when RLS is discovered on one of the rels in the tree. Re-run the rewriter when re-planning. Ensure a change in current user always invalidates plans. * Declare that it's not legal to run a query originally parsed and rewritten as superuser as a non-superuser or vice versa. This would cause a great deal of pain with PL/PgSQL. * Always add the RLS predicate and solve the problem of reliably short-circuiting the user-supplied predicate for RLS-exempt users. We'd need a way to allow direct (not query-based) COPY TO for tables with RLS applied, preventing the rewriting of direct table access into subqueries for COPY, but since we never save plans for COPY that may be fine. * ... ? How about an idea that uses two different type of rules: the existing one is expanded prior to planner stage as we are doing now, and the newer one is expanded on the head of planner stage. The argument of planner() is still parse tree, so it seems to me here is no serious problem to call rewriter again to handle second stage rules. If we go on this approach, ALTER TABLE ... SET ROW SECURITY will become a synonym to declare a rule with special attribute. (b) Inheritance is a problem when RLS is done in the rewriter. As I understood it from Kohei KaiGai's description to me earlier, there was a strong preference on -hackers to enforce RLS predicates for child and parent tables completely independently. That's how RLS currently works, but it might be hard to get the same effect when applying RLS in the rewriter. We'd need to solve that, or redefine RLS's behaviour so that the predicate on a parent table applies to any child tables too. Personally I'd prefer the latter. I'm not certain whether it was a strong preference, even though I followed the consensus at that time. So, I think it makes sense to discuss how RLS policy shall be enforced on the child tables. As long as we can have consistent view on child tables even if it is referenced without parent tables, I don't have any arguments to your preference. Also, it makes implementation simple than the approach I tried to have; that enforces RLS policy of tables individually, because of utilization of existing rule mechanism. It is not difficult to enforce parent's RLS policy on the child relation even if it is referenced individually. All we need to do special is append RLS policy of its parent, not only child's one, if referenced table has parent. (c) RLS might interact differently with rules declared on tables if implemented in the rewriter, so some investigation into that would be needed. I would think that if we make that work, this will also work without, hopefully, any special hackery. And we'd get a separate, independently useful feature out of it, too. I tend to agree. I'm just a bit concerned about dealing with the issues around RLS-exempt operations and users. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] alter_table regression test problem
In checking things out with CLOBBER_CACHE_ALWAYS, I was getting
this problem, which seems to be unrelated to my changes:
*** /home/kgrittn/pg/master/src/test/regress/expected/alter_table.out
2013-11-01 09:07:35.418829105 -0500
--- /home/kgrittn/pg/master/src/test/regress/results/alter_table.out
2013-11-06 11:06:29.785830560 -0600
***
*** 2320,2325
) mapped;
incorrectly_mapped | have_mappings
+---
! 0 | t
(1 row)
--- 2320,2325
) mapped;
incorrectly_mapped | have_mappings
+---
! 1 | t
(1 row)
==
I looked for detail with this query:
SELECT
mapped_oid, oid
FROM (
SELECT
oid, reltablespace, relfilenode, relname,
pg_filenode_relation(reltablespace, pg_relation_filenode(oid))
mapped_oid
FROM pg_class
WHERE relkind IN ('r', 'i', 'S', 't', 'm')
) mapped
WHERE (mapped_oid != oid OR mapped_oid IS NULL);
... with this result:
mapped_oid | oid
+--
2139062143 | 2619
(1 row)
2619 is the oid for pg_statistic, and the mapped_oid value matches
what we use to clobber the cache. Any ideas before I start
digging?
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] alter_table regression test problem
Hi, Kevin Grittner kgri...@ymail.com schrieb: In checking things out with CLOBBER_CACHE_ALWAYS, I was getting this problem, which seems to be unrelated to my changes: ... with this result: mapped_oid | oid +-- 2139062143 | 2619 (1 row) 2619 is the oid for pg_statistic, and the mapped_oid value matches what we use to clobber the cache. Any ideas before I start digging? Interesting. That's new code of mine, but it hasn't shown up in the clobber animals so far. I'll have a look. Thanks, Andred --- Please excuse brevity and formatting - I am writing this on my mobile phone. -- 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] alter_table regression test problem
Kevin Grittner kgri...@ymail.com wrote:
In checking things out with CLOBBER_CACHE_ALWAYS, I was getting
this problem, which seems to be unrelated to my changes:
On a CLOBBER_CACHE_ALWAYS build I did a fresh initdb, started the
cluster, and immediately tested this query on both the postgres and
template1 databases, with the same result:
SELECT
*
FROM (
SELECT
oid, reltablespace, relfilenode, relname,
pg_filenode_relation(reltablespace,
pg_relation_filenode(oid)) mapped_oid
FROM pg_class
WHERE relkind IN ('r', 'i', 'S', 't', 'm')
) mapped
WHERE (mapped_oid != oid OR mapped_oid IS NULL);
oid | reltablespace | relfilenode | relname | mapped_oid
--+---+-+--+
2619 | 0 | 11828 | pg_statistic | 2139062143
(1 row)
That makes for a pretty simple test for git bisect, even if
everything including initdb is painfully slow with
CLOBBER_CACHE_ALWAYS.
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] alter_table regression test problem
Kevin Grittner wrote: That makes for a pretty simple test for git bisect, even if everything including initdb is painfully slow with CLOBBER_CACHE_ALWAYS. Most likely culprit is f01d1ae3a104019d6d68aeff85c4816a275130b3 -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [bug fix] strerror() returns ??? in a UTF-8/C database with LC_MESSAGES=non-ASCII
MauMau maumau...@gmail.com writes: I did as Tom san suggested. Please review the attached patch. I chose as common errnos by selecting those which are used in PosttgreSQL source code out of the error numbers defined in POSIX 2013. I've committed this with some editorialization (mostly, I used a case statement not a constant array, because that's more like the other places that switch on errnos in this file). As I said, lack of %m string has been making troubleshooting difficult, so I wish this to be backported at least 9.2. I'm waiting to see whether the buildfarm likes this before considering back-patching. 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] alter_table regression test problem
On 2013-11-06 17:00:40 -0300, Alvaro Herrera wrote: Kevin Grittner wrote: That makes for a pretty simple test for git bisect, even if everything including initdb is painfully slow with CLOBBER_CACHE_ALWAYS. Most likely culprit is f01d1ae3a104019d6d68aeff85c4816a275130b3 Well, that test tests the functionality added in that commit, so sure, it can't be before that. What confuses me is that relfilenodemap has survived quite some CLOBBER_CACHE_ALWAYS runs in the buildfarm since: http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=jaguarundibr=HEAD Did you compile with CLOBBER_CACHE_ALWAYS or CLOBBER_CACHE_RECURSIVELY? 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] alter_table regression test problem
On 2013-11-07 00:17:34 +0100, Andres Freund wrote: On 2013-11-06 17:00:40 -0300, Alvaro Herrera wrote: Kevin Grittner wrote: That makes for a pretty simple test for git bisect, even if everything including initdb is painfully slow with CLOBBER_CACHE_ALWAYS. Most likely culprit is f01d1ae3a104019d6d68aeff85c4816a275130b3 Well, that test tests the functionality added in that commit, so sure, it can't be before that. What confuses me is that relfilenodemap has survived quite some CLOBBER_CACHE_ALWAYS runs in the buildfarm since: http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=jaguarundibr=HEAD Did you compile with CLOBBER_CACHE_ALWAYS or CLOBBER_CACHE_RECURSIVELY? Either way, the code is completely and utterly broken in the face of cache invalidations that are received when it does its internal heap_open(RelationRelationId). I can't have been thinking straight when I wrote the invalidation logic. Will send a fix. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH 1/2] SSL: GUC option to prefer server cipher order
By default OpenSSL (and SSL/TLS in general) lets client cipher
order take priority. This is OK for browsers where the ciphers
were tuned, but few Postgres client libraries make cipher order
configurable. So it makes sense to make cipher order in
postgresql.conf take priority over client defaults.
This patch adds setting 'ssl_prefer_server_ciphers' which can be
turned on so that server cipher order is preferred.
The setting SSL_OP_CIPHER_SERVER_PREFERENCE appeared in
OpenSSL 0.9.7 (31 Dec 2002), not sure if #ifdef is required
for conditional compilation.
---
doc/src/sgml/config.sgml | 12
src/backend/libpq/be-secure.c | 7 +++
src/backend/utils/misc/guc.c | 10 ++
3 files changed, 29 insertions(+)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 77a9303..56bfa01 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -883,6 +883,18 @@ include 'filename'
/listitem
/varlistentry
+ varlistentry id=guc-ssl-prefer-server-ciphers xreflabel=ssl_prefer_server_ciphers
+ termvarnamessl_prefer_server_ciphers/varname (typebool/type)/term
+ indexterm
+ primaryvarnamessl_prefer_server_ciphers/ configuration parameter/primary
+ /indexterm
+ listitem
+ para
+Specifies whether to prefer client or server ciphersuite.
+ /para
+ /listitem
+ /varlistentry
+
varlistentry id=guc-password-encryption xreflabel=password_encryption
termvarnamepassword_encryption/varname (typeboolean/type)/term
indexterm
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 7f01a78..2094674 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -112,6 +112,9 @@ static bool ssl_loaded_verify_locations = false;
/* GUC variable controlling SSL cipher list */
char *SSLCipherSuites = NULL;
+/* GUC variable: if false, prefer client ciphers */
+bool SSLPreferServerCiphers;
+
/* */
/* Hardcoded values */
/* */
@@ -845,6 +848,10 @@ initialize_SSL(void)
if (SSL_CTX_set_cipher_list(SSL_context, SSLCipherSuites) != 1)
elog(FATAL, could not set the cipher list (no valid ciphers available));
+ /* Let server choose order */
+ if (SSLPreferServerCiphers)
+ SSL_CTX_set_options(SSL_context, SSL_OP_CIPHER_SERVER_PREFERENCE);
+
/*
* Load CA store, so we can verify client certificates if needed.
*/
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 538d027..7f1771a 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -127,6 +127,7 @@ extern char *temp_tablespaces;
extern bool ignore_checksum_failure;
extern bool synchronize_seqscans;
extern char *SSLCipherSuites;
+extern bool SSLPreferServerCiphers;
#ifdef TRACE_SORT
extern bool trace_sort;
@@ -801,6 +802,15 @@ static struct config_bool ConfigureNamesBool[] =
check_ssl, NULL, NULL
},
{
+ {ssl_prefer_server_ciphers, PGC_POSTMASTER, CONN_AUTH_SECURITY,
+ gettext_noop(Give priority to server ciphersuite order.),
+ NULL
+ },
+ SSLPreferServerCiphers,
+ false,
+ NULL, NULL, NULL
+ },
+ {
{fsync, PGC_SIGHUP, WAL_SETTINGS,
gettext_noop(Forces synchronization of updates to disk.),
gettext_noop(The server will use the fsync() system call in several places to make
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH 2/2] SSL: Support ECDH key excange.
This sets up ECDH key exchange, when compiling against OpenSSL
that supports EC. Then ECDHE-RSA and ECDHE-ECDSA ciphersuites
can be used for SSL connections. Latter one means that EC keys
are now usable.
The reason for EC key exchange is that it's faster than DHE
and it allows to go to higher security levels where RSA will
be horribly slow.
Quick test with single-threaded client connecting repeatedly
to server on same machine, then closes connection. Measured
is connections-per-second.
Key DHE ECDHE
RSA-1024177.5 278.1 (x 1.56)
RSA-2048140.5 191.1 (x 1.36)
RSA-409659.567.3(x 1.13)
ECDSA-256 280.7 (~ RSA-3072)
ECDSA-384 128.9 (~ RSA-7680)
There is also new GUC option - ssl_ecdh_curve - that specifies
curve name used for ECDH. It defaults to prime256v1, which
is the most common curve in use in HTTPS. According to NIST
should be securitywise similar to ~3072 bit RSA/DH.
(http://www.keylength.com / NIST Recommendations).
Other commonly-implemented curves are secp384r1 and secp521r1
(OpenSSL names). The rest are not recommended as EC curves
needed to be exchanged by name and need to be explicitly
supprted by both client and server. TLS does have free-form
curve exchange, but few client libraries implement that,
at least OpenSSL does not.
Full list can be seen with openssl ecparam -list_curves.
It does not tune ECDH curve with key size automatically,
like DHE does. The reason is the curve naming situation.
---
doc/src/sgml/config.sgml | 13 +
src/backend/libpq/be-secure.c | 32
src/backend/utils/misc/guc.c | 16
3 files changed, 61 insertions(+)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 56bfa01..3785052 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -895,6 +895,19 @@ include 'filename'
/listitem
/varlistentry
+ varlistentry id=guc-ssl-ecdh-curve xreflabel=ssl_ecdh_curve
+ termvarnamessl_ecdh_curve/varname (typestring/type)/term
+ indexterm
+ primaryvarnamessl_ecdh_curve/ configuration parameter/primary
+ /indexterm
+ listitem
+ para
+Specifies name of EC curve which will be used in ECDH key excanges.
+Default is literalprime256p1/.
+ /para
+ /listitem
+ /varlistentry
+
varlistentry id=guc-password-encryption xreflabel=password_encryption
termvarnamepassword_encryption/varname (typeboolean/type)/term
indexterm
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 2094674..8d688f2 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -69,6 +69,9 @@
#if SSLEAY_VERSION_NUMBER = 0x0907000L
#include openssl/conf.h
#endif
+#if (OPENSSL_VERSION_NUMBER = 0x0090800fL) !defined(OPENSSL_NO_ECDH)
+#include openssl/ec.h
+#endif
#endif /* USE_SSL */
#include libpq/libpq.h
@@ -112,6 +115,9 @@ static bool ssl_loaded_verify_locations = false;
/* GUC variable controlling SSL cipher list */
char *SSLCipherSuites = NULL;
+/* GUC variable for default ECHD curve. */
+char *SSLECDHCurve;
+
/* GUC variable: if false, prefer client ciphers */
bool SSLPreferServerCiphers;
@@ -765,6 +771,29 @@ info_cb(const SSL *ssl, int type, int args)
}
}
+#if (OPENSSL_VERSION_NUMBER = 0x0090800fL) !defined(OPENSSL_NO_ECDH)
+static void
+initialize_ecdh(void)
+{
+ EC_KEY *ecdh;
+ int nid;
+
+ nid = OBJ_sn2nid(SSLECDHCurve);
+ if (!nid)
+ elog(FATAL, ECDH: curve not known: %s, SSLECDHCurve);
+
+ ecdh = EC_KEY_new_by_curve_name(nid);
+ if (!ecdh)
+ elog(FATAL, ECDH: failed to allocate key);
+
+ SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_ECDH_USE);
+ SSL_CTX_set_tmp_ecdh(SSL_context, ecdh);
+ EC_KEY_free(ecdh);
+}
+#else
+#define initialize_ecdh()
+#endif
+
/*
* Initialize global SSL context.
*/
@@ -844,6 +873,9 @@ initialize_SSL(void)
SSL_CTX_set_tmp_dh_callback(SSL_context, tmp_dh_cb);
SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE | SSL_OP_NO_SSLv2);
+ /* set up ephemeral ECDH keys */
+ initialize_ecdh();
+
/* set up the allowed cipher list */
if (SSL_CTX_set_cipher_list(SSL_context, SSLCipherSuites) != 1)
elog(FATAL, could not set the cipher list (no valid ciphers available));
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 7f1771a..defd44a 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -127,6 +127,7 @@ extern char *temp_tablespaces;
extern bool ignore_checksum_failure;
extern bool synchronize_seqscans;
extern char *SSLCipherSuites;
+extern char *SSLECDHCurve;
extern bool SSLPreferServerCiphers;
#ifdef TRACE_SORT
@@ -3151,6 +3152,21 @@ static struct config_string ConfigureNamesString[] =
},
{
+ {ssl_ecdh_curve, PGC_POSTMASTER, CONN_AUTH_SECURITY,
+ gettext_noop(Sets the list of EC curve used for ECDH.),
+ NULL,
+
[HACKERS] Documentation patch for date/time formatting functions
Due to a variety of messages over time regarding perceived weirdness in
to_timestamp and to_date, this patch adds (see notes) in the
description column for to_date and to_timestamp in the Formatting
Functions table and adds the following text to the opening of the usage
notes for date/time conversion:
The to_date and to_timestamp functions exist to handle unusual input
formats that cannot be converted by simple casting. These functions
interpret input liberally and with minimal error checking and while they
will produce valid output, the conversion has the potential to yield
unexpected results. Read the following notes and test carefully before
use. Casting is the preferred method of conversion wherever possible.
It also adds the following usage note:
Input to to_date and to_timestamp is not restricted to normal ranges
thus to_date('20096040','MMDD') returns 2014-01-17 rather than
causing an error.
This is the first patch I have submitted directly. Please advise if I
have made any errors in method, style, etc.
Cheers,
Steve
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index a1d3aee..6f5eee0 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -5426,7 +5426,7 @@ SELECT SUBSTRING('XY1234Z', 'Y*?([0-9]{1,3})');
literalfunctionto_date(typetext/type,
typetext/type)/function/literal
/entry
entrytypedate/type/entry
-entryconvert string to date/entry
+entryconvert string to date (see notes)/entry
entryliteralto_date('05nbsp;Decnbsp;2000',
'DDnbsp;Monnbsp;')/literal/entry
/row
row
@@ -5448,7 +5448,7 @@ SELECT SUBSTRING('XY1234Z', 'Y*?([0-9]{1,3})');
literalfunctionto_timestamp(typetext/type,
typetext/type)/function/literal
/entry
entrytypetimestamp with time zone/type/entry
-entryconvert string to time stamp/entry
+entryconvert string to time stamp (see notes)/entry
entryliteralto_timestamp('05nbsp;Decnbsp;2000',
'DDnbsp;Monnbsp;')/literal/entry
/row
row
@@ -5750,10 +5750,27 @@ SELECT SUBSTRING('XY1234Z', 'Y*?([0-9]{1,3})');
para
Usage notes for date/time formatting:
-
+ /para
+ para
+The functionto_date/function and functionto_timestampfunction
+functions exist to handle unusual input formats that cannot be
+converted by simple casting. These functions interpret input
liberally
+and with minimal error checking and while they will produce valid
output,
+the conversion has the potential to yield unexpected results. Read the
+following notes and test carefully before use. Casting is the
+preferred method of conversion wherever possible.
itemizedlist
listitem
para
+ Input to functionto_date/function and
+ functionto_timestampfunction is not restricted to normal ranges
+ thus literalto_date('20096040','MMDD')/literal returns
+ literal2014-01-17/literal rather than causing an error.
+ /para
+ /listitem
+
+ listitem
+ para
literalFM/literal suppresses leading zeroes and trailing
blanks
that would otherwise be added to make the output of a pattern be
fixed-width. In productnamePostgreSQL/productname,
--
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 1/2] SSL: GUC option to prefer server cipher order
Marko Kreen escribió: By default OpenSSL (and SSL/TLS in general) lets client cipher order take priority. This is OK for browsers where the ciphers were tuned, but few Postgres client libraries make cipher order configurable. So it makes sense to make cipher order in postgresql.conf take priority over client defaults. This patch adds setting 'ssl_prefer_server_ciphers' which can be turned on so that server cipher order is preferred. Wouldn't it make more sense to have this enabled by default? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH 1/2] SSL: GUC option to prefer server cipher order
On Wed, Nov 06, 2013 at 09:57:32PM -0300, Alvaro Herrera wrote: Marko Kreen escribió: By default OpenSSL (and SSL/TLS in general) lets client cipher order take priority. This is OK for browsers where the ciphers were tuned, but few Postgres client libraries make cipher order configurable. So it makes sense to make cipher order in postgresql.conf take priority over client defaults. This patch adds setting 'ssl_prefer_server_ciphers' which can be turned on so that server cipher order is preferred. Wouldn't it make more sense to have this enabled by default? Well, yes. :) I would even drop the GUC setting, but hypothetically there could be some sort of backwards compatiblity concerns, so I added it to patch and kept old default. But if noone has strong need for it, the setting can be removed. -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation patch for date/time formatting functions
On Thu, Nov 7, 2013 at 9:14 AM, Steve Crawford
scrawf...@pinpointresearch.com wrote:
Due to a variety of messages over time regarding perceived weirdness in
to_timestamp and to_date, this patch adds (see notes) in the description
column for to_date and to_timestamp in the Formatting Functions table and
adds the following text to the opening of the usage notes for date/time
conversion:
The to_date and to_timestamp functions exist to handle unusual input formats
that cannot be converted by simple casting. These functions interpret input
liberally and with minimal error checking and while they will produce valid
output, the conversion has the potential to yield unexpected results. Read
the following notes and test carefully before use. Casting is the preferred
method of conversion wherever possible.
It also adds the following usage note:
Input to to_date and to_timestamp is not restricted to normal ranges thus
to_date('20096040','MMDD') returns 2014-01-17 rather than causing an
error.
This is the first patch I have submitted directly. Please advise if I have
made any errors in method, style, etc.
Sure. You should do the following things:
- attach a proper patch to the email you are sending such as people
can easily download it and have a look at it without having to
regenerate it themselves
- submit it to the next commit fest if you want to have it reviewed:
https://commitfest.postgresql.org/action/commitfest_view?id=20. Next
commit fest begins on the 15th of November and your patch is simple,
so for sure somebody will show up and have a closer look at it.
Here are as well general guidelines about patch submission:
http://wiki.postgresql.org/wiki/Submitting_a_Patch
Regards,
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Heavily modified big table bloat even in auto vacuum is running
On Tue, Oct 22, 2013 at 2:09 PM, Haribabu kommi haribabu.ko...@huawei.com wrote: On 22 October 2013 10:15 Amit Kapila wrote: On Mon, Oct 21, 2013 at 10:54 AM, Haribabu kommi haribabu.ko...@huawei.com wrote: Actually what I had in mind was to use nkeep to estimate n_dead_tuples similar to how num_tuples is used to estimate n_live_tuples. I think it will match what Tom had pointed in his response (What would make more sense to me is for VACUUM to estimate the number of remaining dead tuples somehow and send that in its message. However, since the whole point here is that we aren't accounting for transactions that commit while VACUUM runs, it's not very clear how to do that.) I changed the patch as passing the nkeep counter data as the new dead tuples in the relation to stats like the new_rel_tuples. The nkeep counter is an approximation of dead tuples data of a relation. Instead of resetting dead tuples stats as zero, used this value to set n_dead_tuples same as n_live_tuples. Directly using nkeep might not work as it is not guaranteed that Vacuum will scan all the pages, we need to estimate the value similar to new_rel_tuples, something like as done in below function: /* now we can compute the new value for pg_class.reltuples */ vacrelstats-new_rel_tuples = vac_estimate_reltuples(onerel, false, nblocks, vacrelstats-scanned_pages, num_tuples); I am not sure whether the same calculation as done for new_rel_tuples works for new_dead_tuples, you can once check it. I am thinking that if we have to do estimation anyway, then wouldn't it be better to do the way Tom had initially suggested (Maybe we could have VACUUM copy the n_dead_tuples value as it exists when VACUUM starts, and then send that as the value to subtract when it's done?) I think the reason you gave that due to tuple visibility check the number of dead tuples calculated by above logic is not accurate is right but still it will make the value of dead tuples more appropriate than it's current value. You can check if there is a way to do estimation of dead tuples similar to new tuples, and it will be as solid as current logic of vac_estimate_reltuples(), then it's okay, otherwise use the other solution (using the value of n_dead_tuples at start of Vacuum) to solve the problem. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] patch to fix unused variable warning on windows build
Attached is a small patch which fixes the unused variable warning in the visual studios build. Seems like VS does not support __attribute__((unused)) but looks like all other places we must assign to the variable. Regards David Rowley unused_variable.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
