Re: [HACKERS] ALTER SYSTEM SET command to change postgresql.conf parameters
On Fri, Dec 20, 2013 at 2:35 AM, Fujii Masao masao.fu...@gmail.com wrote: On Thu, Dec 19, 2013 at 2:21 PM, Tatsuo Ishii is...@postgresql.org wrote: I found that the psql tab-completion for ALTER SYSTEM SET has not been implemented yet. Attached patch does that. Barring any objections, I will commit this patch. Good catch! Committed. I found the bug of ALTER SYSTEM SET patch. The procedure to reproduce it is here. $ psql =# ALTER SYSTEM SET shared_buffers = '512MB'; ALTER SYSTEM =# \q $ pg_ctl -D data reload server signaled 2013-12-22 18:24:13 JST LOG: received SIGHUP, reloading configuration files 2013-12-22 18:24:13 JST LOG: parameter shared_buffers cannot be changed without restarting the server 2013-12-22 18:24:13 JST LOG: configuration file X?? contains errors; unaffected changes were applied The configuration file name in the last log message is broken. This problem was introduced by the ALTER SYSTEM SET patch. FreeConfigVariables(head); snip else if (apply) ereport(elevel, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg(configuration file \%s\ contains errors; unaffected changes were applied, ErrorConfFile))); The cause of the problem is that, in ProcessConfigFile(), the log message including the 'ErrorConfFile' is emitted after 'head' is free'd even though 'ErrorConfFile' points to one of entry of the 'head'. Regards, -- Fujii Masao -- 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] multibyte messages are displayed incorrectly on the client
From: Noah Misch n...@leadboat.com Better to attack that directly. Arrange to apply any client_encoding named in the startup packet earlier, before authentication. This relates to the TODO item Let the client indicate character encoding of database names, user names, and passwords. (I expect such an endeavor to be tricky.) Unfortunately, character set conversion is not possible until the database session is established, since it requires system catalog access. Please the comment in src/backend/utils/mb/mbutils.c: * During backend startup we can't set client encoding because we (a) * can't look up the conversion functions, and (b) may not know the database * encoding yet either. So SetClientEncoding() just accepts anything and * remembers it for InitializeClientEncoding() to apply later. I guess that's why Tom-san suggested the same solution as my patch (as a compromise) in the below thread, which is also a TODO item: Re: encoding of PostgreSQL messages http://www.postgresql.org/message-id/19896.1234107...@sss.pgh.pa.us From: Alvaro Herrera alvhe...@2ndquadrant.com The problem is that if there's an encoding mismatch, the message might be impossible to figure out. If the message is in english, at least it can be searched for in the web, or something -- the user might even find a page in which the english error string appears, with a native language explanation. I feel like this, too. Being readable in English is better than being unrecognizable. Regards MauMau -- 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] WITHIN GROUP patch
Tom == Tom Lane t...@sss.pgh.pa.us writes: Tom I eventually decided that we were overthinking this problem. At Tom least for regular ordered-set aggregates, we can just deem that Tom the collation of the aggregate is indeterminate unless all the Tom inputs (both direct and aggregated) agree on the collation to Tom use. This gives us the right answer for all the standard Tom aggregates, which have at most one collatable input, and it's Tom very unclear that anything more complicated would be an Tom improvement. I definitely didn't like the hack that was in Tom there that'd force a sort column to absorb a possibly-unrelated Tom collation. Yeah, I can go along with that, but see below. Tom For hypotheticals, I agree after reading the spec text that Tom we're supposed to unify the collations of matching hypothetical Tom and aggregated arguments to determine the collation to use for Tom sorting that column. Yeah, the spec seemed clear enough on that. Tom I see that the patch just leaves these columns out of the Tom determination of the aggregate's result collation. That's okay Tom for the time being at least, since we have no hypotheticals with Tom collatable output types, but I wonder if anyone wants to argue Tom for some other rule (and if so, what)? Any alternative seems a bit ad-hoc to me. The examples I've thought of which would return collatable types are all ones that would be implemented as plain ordered set functions even if their logic was in some sense hypothetical. For example you could envisage a value_prec(x) within group (order by y) that returns the value of y which sorts immediately before x, but this would just be declared as value_prec(anyelement) within group (anyelement) rather than engaging the hypothetical argument stuff. (It's this sort of thing that suggested pushing down the collation into the sort column even for non-hypothetical ordered set functions.) -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ALTER SYSTEM SET command to change postgresql.conf parameters
On Sun, Dec 22, 2013 at 3:01 PM, Fujii Masao masao.fu...@gmail.com wrote: I found the bug of ALTER SYSTEM SET patch. The procedure to reproduce it is here. $ psql =# ALTER SYSTEM SET shared_buffers = '512MB'; ALTER SYSTEM =# \q $ pg_ctl -D data reload server signaled 2013-12-22 18:24:13 JST LOG: received SIGHUP, reloading configuration files 2013-12-22 18:24:13 JST LOG: parameter shared_buffers cannot be changed without restarting the server 2013-12-22 18:24:13 JST LOG: configuration file X?? contains errors; unaffected changes were applied The configuration file name in the last log message is broken. This problem was introduced by the ALTER SYSTEM SET patch. FreeConfigVariables(head); snip else if (apply) ereport(elevel, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg(configuration file \%s\ contains errors; unaffected changes were applied, ErrorConfFile))); The cause of the problem is that, in ProcessConfigFile(), the log message including the 'ErrorConfFile' is emitted after 'head' is free'd even though 'ErrorConfFile' points to one of entry of the 'head'. Your analysis is absolutely right. The reason for this issue is that we want to display filename to which erroneous parameter belongs and in this case we are freeing the parameter info before actual error. To fix, we need to save the filename value before freeing parameter list. Please find the attached patch to fix this issue. Note - In my m/c, I could not reproduce the issue. I think this is not surprising because we are not setting freed memory to NULL, so it can display anything (sometimes right value as well) With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com display_error_conf_filename.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PoC: Partial sort
On Sun, Dec 22, 2013 at 07:38:05PM +0400, Alexander Korotkov wrote: Hi! Next revision. It expected to do better work with optimizer. It introduces presorted_keys argument of cost_sort function which represent number of keys already sorted in Path. Then this function uses estimate_num_groups to estimate number of groups with different values of presorted keys and assumes that dataset is uniformly divided by groups. get_cheapest_fractional_path_for_pathkeys tries to select the path matching most part of path keys. You can see it's working pretty good on single table queries. Nice work! The plans look good and the calculated costs seem sane also. I suppose the problem with the joins is generating the pathkeys? Have a nice day, -- Martijn van Oosterhout klep...@svana.org http://svana.org/kleptog/ He who writes carelessly confesses thereby at the very outset that he does not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature
Re: [HACKERS] WITHIN GROUP patch
Andrew Gierth and...@tao11.riddles.org.uk writes: The examples I've thought of which would return collatable types are all ones that would be implemented as plain ordered set functions even if their logic was in some sense hypothetical. For example you could envisage a value_prec(x) within group (order by y) that returns the value of y which sorts immediately before x, but this would just be declared as value_prec(anyelement) within group (anyelement) rather than engaging the hypothetical argument stuff. (It's this sort of thing that suggested pushing down the collation into the sort column even for non-hypothetical ordered set functions.) Meh. I see no very good reason that we shouldn't insist that the collation be set on the sort column rather than the other input. That is, if the alternatives are value_prec(x collate foo) within group (order by y) value_prec(x) within group (order by y collate foo) which one makes it clearer that the ordering is to use collation foo? The first one is at best unnatural, and at worst action-at-a-distance. If you think of the sorting as an operation done in advance of applying value_prec(), which is something the syntax rather encourages you to think, there's no reason that it should absorb a collation from a collate clause attached to a higher-level operation. So I think the spec authors arguably got it wrong for hypotheticals, and I'm uneager to extrapolate their choice of behavior into situations where we don't know for certain that the collations of two arguments must get unified. More practically, I'm dubious about your assumption that an aggregate like this needn't be marked hypothetical. I see that use of anyelement would be enough to constrain the types to be the same, but it doesn't ordinarily constrain collations, and I don't think it should start doing so. So my reaction to this example is not that we should hack the behavior for plain ordered-set aggregates, but that we ought to find a rule that allows result-collation determination for hypotheticals. We speculated upthread about merge the collations normally, but ignore inputs declared ANY and merge the collations normally, but ignore variadic inputs. Either of those would get the job done in this example. I kinda think we should pick one of these rules and move on. 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] WITHIN GROUP patch
I wrote: ... So my reaction to this example is not that we should hack the behavior for plain ordered-set aggregates, but that we ought to find a rule that allows result-collation determination for hypotheticals. We speculated upthread about merge the collations normally, but ignore inputs declared ANY and merge the collations normally, but ignore variadic inputs. Either of those would get the job done in this example. I kinda think we should pick one of these rules and move on. Or, really, why don't we just do the same thing I'm advocating for the plain-ordered-set case? That is, if there's a single collation applying to all the collatable inputs, that's the collation to use for the aggregate; otherwise it has no determinate collation, and it'll throw an error at runtime if it needs one. We realized long ago that we can't throw most need-a-collation errors at parse time, because the parser lacks information about which functions need to know a collation to use. This seems to be in the same category. 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] PoC: Partial sort
On Sun, Dec 22, 2013 at 8:12 PM, Martijn van Oosterhout klep...@svana.orgwrote: On Sun, Dec 22, 2013 at 07:38:05PM +0400, Alexander Korotkov wrote: Hi! Next revision. It expected to do better work with optimizer. It introduces presorted_keys argument of cost_sort function which represent number of keys already sorted in Path. Then this function uses estimate_num_groups to estimate number of groups with different values of presorted keys and assumes that dataset is uniformly divided by groups. get_cheapest_fractional_path_for_pathkeys tries to select the path matching most part of path keys. You can see it's working pretty good on single table queries. Nice work! The plans look good and the calculated costs seem sane also. I suppose the problem with the joins is generating the pathkeys? In general, problem is that partial sort is alternative to do less restrictive merge join and filter it's results. As far as I can see, taking care about it require some rework of merge optimization. For now, I didn't get what it's going to look like. I'll try to dig more into details. -- With best regards, Alexander Korotkov.
Re: [HACKERS] PoC: Partial sort
On Sat, Dec 14, 2013 at 6:30 PM, Jeremy Harris j...@wizmail.org wrote: On 14/12/13 12:54, Andres Freund wrote: On 2013-12-14 13:59:02 +0400, Alexander Korotkov wrote: Currently when we need to get ordered result from table we have to choose one of two approaches: get results from index in exact order we need or do sort of tuples. However, it could be useful to mix both methods: get results from index in order which partially meets our requirements and do rest of work from heap. -- Limit (cost=69214.06..69214.08 rows=10 width=16) (actual time=0.097..0.099 rows=10 loops=1) - Sort (cost=69214.06..71714.06 rows=100 width=16) (actual time=0.096..0.097 rows=10 loops=1) Sort Key: v1, v2 Sort Method: top-N heapsort Memory: 25kB - Index Scan using test_v1_idx on test (cost=0.42..47604.42 rows=100 width=16) (actual time=0.017..0.066 rows=56 loops=1) Total runtime: 0.125 ms (6 rows) Is that actually all that beneficial when sorting with a bog standard qsort() since that doesn't generally benefit from data being pre-sorted? I think we might need to switch to a different algorithm to really benefit from mostly pre-sorted input. Eg: http://www.postgresql.org/message-id/5291467e.6070...@wizmail.org Maybe Alexander and I should bash our heads together. Partial sort patch is mostly optimizer/executor improvement rather than improvement of sort algorithm itself. But I would appreciate using enchantments of sorting algorithms in my work. -- With best regards, Alexander Korotkov.
Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
On Fri, Dec 20, 2013 at 11:59 PM, Peter Geoghegan p...@heroku.com wrote: I think that the way forward is to refine my design in order to upgrade locks from exclusive buffer locks to something else, managed by the lock manager but perhaps through an additional layer of indirection. As previously outlined, I'm thinking of a new SLRU-based granular value locking infrastructure built for this purpose, with btree inserters marking pages as having an entry in this table. I'm working on a revision that holds lmgr page-level exclusive locks (and buffer pins) across multiple operations. This isn't too different to what you've already seen, since they are still only held for an instant. Notably, hash indexes currently quickly grab and release lmgr page-level locks, though they're the only existing clients of that infrastructure. I think on reflection that fully-fledged value locking may be overkill, given the fact that these locks are only held for an instant, and only need to function as a choke point for unique index insertion, and only when upserting occurs. This approach seems promising. It didn't take me very long to get it to a place where it passed a few prior test-cases of mine, with fairly varied input, though the patch isn't likely to be posted for another few days. I think I can get it to a place where it doesn't regress regular insertion at all. I think that that will tick all of the many boxes, without unwieldy complexity and without compromising conceptual integrity. I mention this now because obviously time is a factor. If you think there's something I need to do, or that there's some way that I can more usefully coordinate with you, please let me know. Likewise for anyone else following. -- 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] preserving forensic information when we freeze
On Fri, Dec 20, 2013 at 8:01 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-12-20 07:58:46 -0500, Robert Haas wrote: I think the immediate problem is to decide whether this patch ought to make the xmin column display the result of GetXmin() or GetRawXmin(). Thoughts on that? I slightly favor GetRawXmin(). Committed that way for now. It seems more consistent with what we do elsewhere. We neglected to write documentation for this change, so here's a patch for that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 8f9f61d..0f2f2bf 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -5264,8 +5264,8 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; listitem para Specifies the cutoff age (in transactions) that commandVACUUM/ -should use to decide whether to replace transaction IDs with -literalFrozenXID/ while scanning a table. +should use to decide whether to freeze row versions +while scanning a table. The default is 50 million transactions. Although users can set this value anywhere from zero to one billion, commandVACUUM/ will silently limit the effective value to half diff --git a/doc/src/sgml/maintenance.sgml b/doc/src/sgml/maintenance.sgml index fb94596..324ed0e 100644 --- a/doc/src/sgml/maintenance.sgml +++ b/doc/src/sgml/maintenance.sgml @@ -397,8 +397,12 @@ para The reason that periodic vacuuming solves the problem is that -productnamePostgreSQL/productname reserves a special XID -as literalFrozenXID/. This XID does not follow the normal XID +commandVACUUM/ will mark rows as emphasisfrozen/, indicating that +they were inserted by a transaction which committed sufficiently far in +the past that the effects of the inserting transaction is certain to be +visible, from an MVCC perspective, to all current and future transactions. +productnamePostgreSQL/ reserves a special XID, +literalFrozenTransactionId/, which does not follow the normal XID comparison rules and is always considered older than every normal XID. Normal XIDs are compared using modulo-2superscript32/ arithmetic. This means @@ -410,20 +414,19 @@ the next two billion transactions, no matter which normal XID we are talking about. If the row version still exists after more than two billion transactions, it will suddenly appear to be in the future. To -prevent this, old row versions must be reassigned the XID -literalFrozenXID/ sometime before they reach the -two-billion-transactions-old mark. Once they are assigned this -special XID, they will appear to be quotein the past/ to all -normal transactions regardless of wraparound issues, and so such -row versions will be valid until deleted, no matter how long that is. -This reassignment of old XIDs is handled by commandVACUUM/. +prevent this, frozen row versions are treated as if the inserting XID were +literalFrozenTransactionId/, so that they will appear to be +quotein the past/ to all normal transactions regardless of wraparound +issues, and so such row versions will be valid until deleted, no matter +how long that is. /para para xref linkend=guc-vacuum-freeze-min-age -controls how old an XID value has to be before it's replaced with -literalFrozenXID/. Larger values of this setting -preserve transactional information longer, while smaller values increase +controls how old an XID value has to be before its row version will be +frozen. Increasing this setting may avoid unnecessary work if the +rows that would otherwise be frozen will soon be modified again, +but decreasing this setting increases the number of transactions that can elapse before the table must be vacuumed again. /para @@ -431,8 +434,8 @@ para commandVACUUM/ normally skips pages that don't have any dead row versions, but those pages might still have row versions with old XID -values. To ensure all old XIDs have been replaced by -literalFrozenXID/, a scan of the whole table is needed. +values. To ensure all old row versions have been frozen, a +scan of the whole table is needed. xref linkend=guc-vacuum-freeze-table-age controls when commandVACUUM/ does that: a whole table sweep is forced if the table hasn't been fully scanned for varnamevacuum_freeze_table_age/ @@ -447,8 +450,8 @@ the time commandVACUUM/ last scanned the whole table. If it were to go unvacuumed for longer than that, data loss could result. To ensure that this does not happen, -autovacuum is invoked on any table that might contain XIDs older than the -age specified by the configuration parameter xref +autovacuum is invoked on any table that might contain unfrozen rows
Re: [HACKERS] CLUSTER FREEZE
On Sun, Dec 8, 2013 at 10:51 AM, Peter Eisentraut pete...@gmx.net wrote:
On Tue, 2013-11-19 at 18:24 +0100, Andres Freund wrote:
On 2013-11-19 12:23:30 -0500, Robert Haas wrote:
On Mon, Nov 18, 2013 at 11:45 AM, Andres Freund and...@2ndquadrant.com
wrote:
Yes, we probably should make a decision, unless Robert's idea can be
implemented. We have to balance the ease of debugging MVCC failures
with the interface we give to the user community.
Imo that patch really doesn't need too much further work.
Would you care to submit a version you're happy with?
I'll give it a spin sometime this week.
I'm setting the CLUSTER FREEZE patch as returned with feedback, until
this other patch has been considered.
The other patch in question is now committed. Here's a patch for
this, which does somewhat more extensive surgery than previously
proposed (actually, I wrote it from scratch, before looking at the
prior submission). It's basically the same idea, though. Note that
both versions of the patch affect not only CLUSTER, but also VACUUM
FULL.
I suspect we ought to extend this to rewriting variants of ALTER TABLE
as well, but a little thought is needed there. ATRewriteTables()
appears to just call heap_insert() for each updated row, which if I'm
not mistaken is an MVCC violation - offhand, it seems like a
transaction with an older MVCC snapshot could access the table for
this first time after the rewriter commits, and fail to see rows which
would have appeared to be there before the rewrite. (I haven't
actually tested this, so watch me be wrong.) If we're OK with an MVCC
violation here, we could just pass HEAP_INSERT_FROZEN and have a
slightly different flavor of violation; if not, this needs some kind
of more extensive surgery quite apart from what we do about freezing.
Review of the attached appreciated...
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index eb71581..1e98473 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -102,7 +102,9 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] ANALYZE [ replaceable class=PARAMETER
Specifying literalFREEZE/literal is equivalent to performing
commandVACUUM/command with the
xref linkend=guc-vacuum-freeze-min-age parameter
- set to zero.
+ set to zero. Aggressive freezing is always performed when the
+ table is rewritten, so this option is redundant when literalFULL/
+ is specified.
/para
/listitem
/varlistentry
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index deec77d..634754c 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -345,7 +345,7 @@ rewrite_heap_tuple(RewriteState state,
/*
* While we have our hands on the tuple, we may as well freeze any
- * very-old xmin or xmax, so that future VACUUM effort can be saved.
+ * eligible xmin or xmax, so that future VACUUM effort can be saved.
*/
heap_freeze_tuple(new_tuple-t_data, state-rs_freeze_xid,
state-rs_cutoff_multi);
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 0b8ac8c..9f41278 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -64,12 +64,10 @@ typedef struct
} RelToCluster;
-static void rebuild_relation(Relation OldHeap, Oid indexOid,
- int freeze_min_age, int freeze_table_age, bool verbose);
+static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose);
static void copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
- int freeze_min_age, int freeze_table_age, bool verbose,
- bool *pSwapToastByContent, TransactionId *pFreezeXid,
- MultiXactId *pCutoffMulti);
+ bool verbose, bool *pSwapToastByContent,
+ TransactionId *pFreezeXid, MultiXactId *pCutoffMulti);
static List *get_tables_to_cluster(MemoryContext cluster_context);
static void reform_and_rewrite_tuple(HeapTuple tuple,
TupleDesc oldTupDesc, TupleDesc newTupDesc,
@@ -176,11 +174,8 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
/* close relation, keep lock till commit */
heap_close(rel, NoLock);
- /*
- * Do the job. We use a -1 freeze_min_age to avoid having CLUSTER
- * freeze tuples earlier than a plain VACUUM would.
- */
- cluster_rel(tableOid, indexOid, false, stmt-verbose, -1, -1);
+ /* Do the job. */
+ cluster_rel(tableOid, indexOid, false, stmt-verbose);
}
else
{
@@ -229,9 +224,8 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
StartTransactionCommand();
/* functions in indexes may want a snapshot set */
PushActiveSnapshot(GetTransactionSnapshot());
- /* Do the job. As above, use a -1 freeze_min_age. */
- cluster_rel(rvtc-tableOid, rvtc-indexOid, true, stmt-verbose,
- -1, -1);
+ /* Do the job. */
+ cluster_rel(rvtc-tableOid,
Re: [HACKERS] row security roadmap proposal
On Wed, Dec 18, 2013 at 10:21 PM, Craig Ringer cr...@2ndquadrant.com wrote: My main worry was that it requires the user to build everything manually, and is potentially error prone as a result. To address that we can build convenience features (label security, ACL types and operators, etc) on top of the same infrastructure later - it doesn't have to go in right away. So putting that side, the concerns I have are: - The current RLS design is restricted to one predicate per table with no contextual control over when that predicate applies. That means we can't implement anything like policy groups or overlapping sets of predicates, anything like that has to be built by the user. We don't need multiple predicates to start with but I want to make sure there's room for them later, particularly so that different apps / extensions that use row-security don't have to fight with each other. - You can't declare a predicate then apply it to a bunch of tables with consistent security columns. Updating/changing predicates will be a pain. We might be able to get around that by recommending that people use an inlineable SQL function to declare their predicates, but inlineable can be hard to pin down sometimes. If not, we need something akin to GRANT ... ALL TABLES IN SCHEMA ... for row security, and ALTER DEFAULT PRIVILEGES ... too. - It's too hard to see what tables have row-security and what impact it has. Needs psql improvements. - There's no way to control whether or not a client can see the row-security policy definition. The other one I want to deal with is secure session variables, but that's mostly a performance optimisation we can add later. What's your list? I don't have a lot of specific concerns apart from what I mentioned here: http://www.postgresql.org/message-id/ca+tgmoyc37qwnqkkqx3p3gu3psfh3tcox8ue06nnuiqn_4r...@mail.gmail.com One thing we do need to think about is our good friend search_path, and making sure that it's relatively easy to implement RLS in a way that's secure against malicious manipulation thereof. I do also agree with some of your concerns, particularly the first two (is it too manual? and insufficient contextual control). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Make various variables read-only (const)
On Fri, Dec 20, 2013 at 12:01 PM, Oskari Saarenmaa o...@ohmu.fi wrote:
This allows the variables to be moved from .data to .rodata section which
means that more data can be shared by processes and makes sure that nothing
can accidentally overwrite the read-only definitions. On a x86-64 Linux
system this moves roughly 9 kilobytes of previously writable data to the
read-only data segment in the backend and 4 kilobytes in libpq.
https://github.com/saaros/postgres/compare/constify
24 files changed, 108 insertions(+), 137 deletions(-)
This sounds like a broadly good thing, but I've had enough painful
experiences with const to be a little wary. And how much does this
really affect data sharing? Doesn't copy-on-write do the same thing
for writable data? Could we get most of the benefit by const-ifying
one or two large data structures and forget the rest?
Other comments:
- The first hunk of the patch mutilates the comment it modifies for no
apparent reason. Please revert.
- Why change the API of transformRelOptions()?
-#define DEF_ENC2NAME(name, codepage) { #name, PG_##name }
+/* The extra NUL-terminator will make sure a warning is raised if the
+ * storage space for name is too small, otherwise when strlen(name) ==
+ * sizeof(pg_enc2name.name) the NUL-terminator would be silently dropped.
+ */
+#define DEF_ENC2NAME(name, codepage) { #name \0, PG_##name }
- The above hunk is not related to the primary purpose of this patch.
--
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] XML Issue with DTDs
On Fri, Dec 20, 2013 at 8:16 PM, Florian Pflug f...@phlo.org wrote: On Dec20, 2013, at 18:52 , Robert Haas robertmh...@gmail.com wrote: On Thu, Dec 19, 2013 at 6:40 PM, Florian Pflug f...@phlo.org wrote: Solving this seems a bit messy, unfortunately. First, I think we need to have some XMLOPTION value which is a superset of all the others - otherwise, dump restore won't work reliably. That means either allowing DTDs if XMLOPTION is CONTENT, or inventing a third XMLOPTION, say ANY. Or we can just decide that it was a bug that this was ever allowed, and if you upgrade to $FIXEDVERSION you'll need to sanitize your data. This is roughly what we did with encoding checks. What exactly do you suggest we outlaw? !DOCTYPE anywhere but at the beginning. -- 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] Issue with PGC_BACKEND parameters
I had observed one problem with PGC_BACKEND parameters while testing patch
for ALTER SYSTEM command.
Problem statement: If I change PGC_BACKEND parameters directly in
postgresql.conf and then do pg_reload_conf() and reconnect, it will
still show the old value.
Detailed steps
1. Start server with default settings
2. Connect Client
3. show log_connections; -- it will show as off, this is correct.
4. Change log_connections in postgresql.conf to on
5. issue command select pg_reload_conf() in client (which is started in
step-2)
6. Connect a new client
7. show log_connections; -- it will show as off, this is in-correct.
The problem is in step-7, it should show as on.
This problem occur only in Windows.
The reason for this problem is that in WINDOWS, when a new session is
started it will load the changed parameters in new backend by
global/config_exec_params file. The flow is in
SubPostmasterMain()-read_nondefault_variables()-set_config_option().
In below code in function set_config_option(), it will not allow to change
PGC_BACKEND variable and even in comments it has mentioned that only
postmaster will be allowed to change and the same will propagate to
subsequently started backends, but this is not TRUE for Windows.
switch (record-context)
{
..
..
case PGC_BACKEND:
if (context == PGC_SIGHUP)
{
/*
* If a PGC_BACKEND parameter is changed in
the config file,
* we want to accept the new value in the
postmaster (whence
* it will propagate to subsequently-started
backends), but
* ignore it in existing backends.
This is a tad klugy, but
* necessary because we don't re-read the
config file during
* backend start.
*/
if (IsUnderPostmaster)
return -1;
}
}
I think to fix the issue we need to pass the information whether PGC_BACKEND
parameter is allowed to change in set_config_option() function.
One way is to pass a new parameter.
Yesterday, I again thought about this issue and found that we can handle it by
checking IsInitProcessingMode() which will be True only during backend startup
which is what we need here.
Please find the attached patch to fix this issue.
I think that this issue should be fixed in PostgreSQL, because
currently PGC_BACKEND
parameters doesn't work on Windows.
I will upload this patch to next CF, so that it can be tracked.
Kindly let me know your suggestions or if you have any objections?
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
set_guc_backend_params.patch
Description: Binary data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [bug fix] pg_ctl always uses the same event source
On Fri, Dec 20, 2013 at 5:26 PM, MauMau maumau...@gmail.com wrote: From: Amit Kapila amit.kapil...@gmail.com Few other points: - 1. #ifdef WIN32 /* Get event source from postgresql.conf for eventlog output */ get_config_value(event_source, event_source, sizeof(event_source)); #endif event logging is done for both win32 and cygwin env. under hash define (Win32 || cygwin), so event source name should also be retrieved for both environments. Refer below in code: #if defined(WIN32) || defined(__CYGWIN__) static void write_eventlog(int level, const char *line) 2. Docs needs to be updated for default value: http://www.postgresql.org/docs/devel/static/event-log-registration.html http://www.postgresql.org/docs/devel/static/runtime-config-logging.html#GUC-EVENT-SOURCE Okay, done. Thanks. I'll update the commitfest entry this weekend. Your changes are fine. The main part left from myside is test of this patch. I will do that in next CF or If I get time before that, I will try to complete it. 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
Re: [HACKERS] GiST support for inet datatypes
On Tue, Dec 17, 2013 at 08:58:13PM +0200, Emre Hasegeli wrote: Hi, Attached patch adds GiST support to the inet datatypes with two new operators. Overlaps operator can be used with exclusion constraints. Is adjacent to operator is just the negator of it. Index uses only the network bits of the addresses. Except for the new operators and is contained within, contains; basic comparison operators are also supported. Please add this patch to the upcoming Commitfest at https://commitfest.postgresql.org/action/commitfest_view?id=21 Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
