Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Friday, August 02, 2013 4:34 AM Dimitri Fontaine wrote: Andres Freund and...@2ndquadrant.com writes: They would need a setting that disables ALTER (DATABASE|USER) ... SET ... as well though. At least for some settings. I don't think enforcing things on that level makes much sense. If only we could trigger some actions when a command is about to be executed, in a way that it's easy for the user to implement whatever policy he fancies… Oh, maybe I should finish preparing those patches for Event Triggers to be fully usable in 9.4 then ;) I think this can be one useful way to disable. With Regards, Amit Kapila. -- 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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Friday, August 02, 2013 8:57 AM Stephen Frost wrote: * Andres Freund (and...@2ndquadrant.com) wrote: FWIW, I think you've just put the final nail in the coffin of this patch by raising the barriers unreasonably high. For my 2c, I don't think it's an unreasonable idea to actually *consider* what options are available through this mechanism rather than just presuming that it's a good idea to be able to modify anything, including things that you wouldn't be able to fix after a restart w/o hacking around in $PGDATA. I think if user has set any value wrong such that it doesn't allow server to start, we can provide an option similar to pg_resetxlog to reset the auto file. How about with such an option user might loose some settings? 1. We can think of providing reset for an particular parameter. 2. Suggestions in docs that in case of such a scenario, he can remember values from auto file and reset the settings. As this can happen only in rare scenario's, I think it can make sense to provide such option. With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] FOR [SHARE|UPDATE] NOWAIT may still block in EvalPlanQualFetch
FOR SHARE|UPDATE NOWAIT will still block if they have to follow a ctid
chain because the call to EvalPlanQualFetch doesn't take a param for
noWait, so it doesn't know not to block if the updated row can't be locked.
The attached patch against master includes an isolationtester spec to
demonstrate this issue and a proposed fix. Builds with the fix applied
pass make check and isolationtester make installcheck.
To reach this point you need to apply the patch in
http://www.postgresql.org/message-id/51fb4305.3070...@2ndquadrant.com
first, otherwise you'll get stuck there and won't touch the code changed
in this patch.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
From 76ff647f8997221de2a6981a51859d12a6b70276 Mon Sep 17 00:00:00 2001
From: Craig Ringer cr...@2ndquadrant.com
Date: Fri, 2 Aug 2013 14:18:34 +0800
Subject: [PATCH] Add noWait param to EvalPlanQualFetch
FOR SHARE|UPDATE NOWAIT would still wait if they had to follow a
ctid chain because EvalPlanQualFetch couldn't tell if a statement
was NOWAIT and would just assume it should block.
---
src/backend/executor/execMain.c| 7 +--
src/backend/executor/nodeLockRows.c| 2 +-
src/include/executor/executor.h| 2 +-
.../expected/for-updateshare-nowait-nonblock.out | 41 +++
src/test/isolation/isolation_schedule | 1 +
.../specs/for-updateshare-nowait-nonblock.spec | 58 ++
6 files changed, 106 insertions(+), 5 deletions(-)
create mode 100644 src/test/isolation/expected/for-updateshare-nowait-nonblock.out
create mode 100644 src/test/isolation/specs/for-updateshare-nowait-nonblock.spec
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 038f064..412dc9a 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1839,7 +1839,7 @@ EvalPlanQual(EState *estate, EPQState *epqstate,
/*
* Get and lock the updated version of the row; if fail, return NULL.
*/
- copyTuple = EvalPlanQualFetch(estate, relation, lockmode,
+ copyTuple = EvalPlanQualFetch(estate, relation, lockmode, false /* wait */,
tid, priorXmax);
if (copyTuple == NULL)
@@ -1898,6 +1898,7 @@ EvalPlanQual(EState *estate, EPQState *epqstate,
* estate - executor state data
* relation - table containing tuple
* lockmode - requested tuple lock mode
+ * noWait - wait mode to pass to heap_lock_tuple
* *tid - t_ctid from the outdated tuple (ie, next updated version)
* priorXmax - t_xmax from the outdated tuple
*
@@ -1910,7 +1911,7 @@ EvalPlanQual(EState *estate, EPQState *epqstate,
* but we use int to avoid having to include heapam.h in executor.h.
*/
HeapTuple
-EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
+EvalPlanQualFetch(EState *estate, Relation relation, int lockmode, bool noWait,
ItemPointer tid, TransactionId priorXmax)
{
HeapTuple copyTuple = NULL;
@@ -1986,7 +1987,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
*/
test = heap_lock_tuple(relation, tuple,
estate-es_output_cid,
- lockmode, false /* wait */ ,
+ lockmode, noWait,
false, buffer, hufd);
/* We now have two pins on the buffer, get rid of one */
ReleaseBuffer(buffer);
diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c
index 5b5c705..efee8a3 100644
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -170,7 +170,7 @@ lnext:
}
/* updated, so fetch and lock the updated version */
-copyTuple = EvalPlanQualFetch(estate, erm-relation, lockmode,
+copyTuple = EvalPlanQualFetch(estate, erm-relation, lockmode, erm-noWait,
hufd.ctid, hufd.xmax);
if (copyTuple == NULL)
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 75841c8..9c593a6 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -199,7 +199,7 @@ extern TupleTableSlot *EvalPlanQual(EState *estate, EPQState *epqstate,
Relation relation, Index rti, int lockmode,
ItemPointer tid, TransactionId priorXmax);
extern HeapTuple EvalPlanQualFetch(EState *estate, Relation relation,
- int lockmode, ItemPointer tid, TransactionId priorXmax);
+ int lockmode, bool noWait, ItemPointer tid, TransactionId priorXmax);
extern void EvalPlanQualInit(EPQState *epqstate, EState *estate,
Plan *subplan, List *auxrowmarks, int epqParam);
extern void EvalPlanQualSetPlan(EPQState *epqstate,
diff --git a/src/test/isolation/expected/for-updateshare-nowait-nonblock.out b/src/test/isolation/expected/for-updateshare-nowait-nonblock.out
new file mode 100644
index 000..d86b2e6
--- /dev/null
+++ b/src/test/isolation/expected/for-updateshare-nowait-nonblock.out
@@ -0,0 +1,41 @@
+Parsed test spec with 3 sessions
+
+starting
Re: [HACKERS] [9.3 bug] disk space in pg_xlog increases during archive recovery
Michael Paquier michael.paqu...@gmail.com writes: By reading this thread, -1 for the addition of a new GUC parameter related to cascading, it looks like an overkill for the possible gain. And +1 for the removal of WAL file once it is replayed in archive recovery if cascading replication is not allowed. However, what about Well, we still don't register standby servers, so I vote against early removal of files that you have no possible way to know if they are still useful or not. We need something smarter here. Please. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] [GENERAL] postgres FDW cost estimation options unrecognized in 9.3-beta1
On Fri, Jul 26, 2013 at 6:28 PM, Tom Lane t...@sss.pgh.pa.us wrote: snip I think we could do with both more documentation, and better error messages for these cases. In the SET-where-you-should-use-ADD case, perhaps ERROR: option use_remote_estimate has not been set HINT: Use ADD not SET to define an option that wasn't already set. In the ADD-where-you-should-use-SET case, perhaps ERROR: option use_remote_estimate is already set HINT: Use SET not ADD to change an option's value. snip Thoughts, better wordings? Since SET is more or less a keyword in this context and there's already not some obvious things about it, it might be better to avoid using it with a slightly different meaning in the error messages. Maybe defined would be clearer? That would be consistent with your usage of define in the first error message as well. ERROR: option use_remote_estimate has not been defined HINT: Use ADD not SET to define an option that wasn't already defined. ERROR: option use_remote_estimate is already defined HINT: Use SET not ADD to change an option's value. Just a thought.
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Le vendredi 2 août 2013 09:23:17, Amit Kapila a écrit : On Friday, August 02, 2013 8:57 AM Stephen Frost wrote: * Andres Freund (and...@2ndquadrant.com) wrote: FWIW, I think you've just put the final nail in the coffin of this patch by raising the barriers unreasonably high. For my 2c, I don't think it's an unreasonable idea to actually *consider* what options are available through this mechanism rather than just presuming that it's a good idea to be able to modify anything, including things that you wouldn't be able to fix after a restart w/o hacking around in $PGDATA. I think if user has set any value wrong such that it doesn't allow server to start, we can provide an option similar to pg_resetxlog to reset the auto file. While guessing what changes are safe is hard, it is easy to discover the GUCs preventing PostgreSQL from restarting correctly. pg_ctl might be able to expose a clear message like : MSG: Params X and Y set by ALTER SYSTEM prevent PostgreSQL from starting HINT: Issue pg_ctl --ignore-bad-GUC start Note that the same may also allow postgresql to start with bad GUC value in postgresql.conf So this is another topic (IMHO). With the current idea of one-GUC-one-file in a PGDATA subdirectory it is *easy* to fix for any DBA or admin. However, note that I do prefer a simple 'include_auto_conf=here|start|end|off' in postgresql.conf (by default at end of file, with value 'end' set). -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Thursday, August 01, 2013 8:37 PM Andres Freund wrote: Hi, On 2013-08-01 15:40:22 +0100, Greg Stark wrote: Why isn't it enough to just dump out all variables with a source of alter system to a text file? You can either have a single global lock around that operation or write it to a new file and move it into place. It saves you from several complications: * No need to iterate over all GUCs, figuring out which come from which source, when writing out the file. Not all GUC's, only which are in auto file. * Less logic required when writing out a value, since you have an acceptable input ready. * No need to make sure the autogenerated file is written out in the same order when adding/changing a setting (to make sure you can diff/version control it sensibly) * No locking necessary, without locking concurrent changes can vanish. * Way easier to delete a setting if it ends up stopping postgres from starting. The logic needed in current patch for above points is not all that complex that it needs to be thought of redesign until the basic idea of one-file-per-setting scores high over one-file-all-setting. Below are some points in my mind due to which I have supported/implemented one-file-all-setting approach: a. I had heard quite a few times that Postgres has lot of files (each relation has separate file) as compare to Oracle. Users feel that Oracle's table space approach is better. b. While server start/Sighup, we needs to open/read/close each file separately which in-itself seems to be overhead. I believe what Greg Stark has said in his below mail link is the more appropriate way and the current patch has done that way. http://www.postgresql.org/message-id/CAM-w4HP7=a2VowyJUD0CAZL5b8FsaHymdQeouL udsohdncw...@mail.gmail.com Also when other commercial database (Oracle) can do it with single file, users will try to compare with it. I understand that our views are not matching on this point and I totally respect your views, but not able to convince myself for one-file-per-setting approach. Can you please once again think and see if there is a viable way for moving forward. I had modified the patch for many suggestions which had made it simpler and if you have any idea to make one-file-all-settings implementation better, then I can address it. With Regards, Amit Kapila. -- 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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Friday, August 02, 2013 4:17 PM Cédric Villemain wrote: Le vendredi 2 août 2013 09:23:17, Amit Kapila a écrit : On Friday, August 02, 2013 8:57 AM Stephen Frost wrote: * Andres Freund (and...@2ndquadrant.com) wrote: FWIW, I think you've just put the final nail in the coffin of this patch by raising the barriers unreasonably high. For my 2c, I don't think it's an unreasonable idea to actually *consider* what options are available through this mechanism rather than just presuming that it's a good idea to be able to modify anything, including things that you wouldn't be able to fix after a restart w/o hacking around in $PGDATA. I think if user has set any value wrong such that it doesn't allow server to start, we can provide an option similar to pg_resetxlog to reset the auto file. While guessing what changes are safe is hard, it is easy to discover the GUCs preventing PostgreSQL from restarting correctly. pg_ctl might be able to expose a clear message like : MSG: Params X and Y set by ALTER SYSTEM prevent PostgreSQL from starting HINT: Issue pg_ctl --ignore-bad-GUC start Note that the same may also allow postgresql to start with bad GUC value in postgresql.conf So this is another topic (IMHO). Yes, this can be viable option to ignore variable values that don't allow server to start, also I agree with you that this can be a separate patch. With the current idea of one-GUC-one-file in a PGDATA subdirectory it is *easy* to fix for any DBA or admin. However, note that I do prefer a simple 'include_auto_conf=here|start|end|off' in postgresql.conf (by default at end of file, with value 'end' set). Earlier patch has this implementation, but later based on suggestions, I made it default. With Regards, Amit Kapila. -- 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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Amit Kapila (amit.kap...@huawei.com) wrote: Below are some points in my mind due to which I have supported/implemented one-file-all-setting approach: a. I had heard quite a few times that Postgres has lot of files (each relation has separate file) as compare to Oracle. Users feel that Oracle's table space approach is better. This is completely unrelated to this discussion, imv. b. While server start/Sighup, we needs to open/read/close each file separately which in-itself seems to be overhead. I also don't think performance of this particular operation should be a high priority. I believe what Greg Stark has said in his below mail link is the more appropriate way and the current patch has done that way. http://www.postgresql.org/message-id/CAM-w4HP7=a2VowyJUD0CAZL5b8FsaHymdQeouL udsohdncw...@mail.gmail.com He doesn't actually provide any reasoning for it. That said, I've not heard any particularly good reason for having a setting per file either. This is an internal-to-PG data file and we should really implement it in whichever way makes the most sense for us. My general feeling is that one file is simpler and sufficient for the postgresql.conf-like parameters, but I wonder what we're going to do for pg_hba/pg_ident. Those would be good to have multiple files for because (as we saw with pg_authid) they could get quite large because they can have per-user entries in them and rewriting a large file for every change would be quite painful. Also when other commercial database (Oracle) can do it with single file, users will try to compare with it. To make it clear- this isn't justification for this design. Also, the notion that Oracle's *configuration* is all done with a *single-file* is.. laughable. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Amit Kapila (amit.kap...@huawei.com) wrote: Yes, this can be viable option to ignore variable values that don't allow server to start, also I agree with you that this can be a separate patch. I disagree that this can be a separate patch. Adding an option to not allow certain GUCs from being modified through this mechanism should be trivial. Then, *after* we have such utility that will allow users to fix a bad situation, we can consider relaxing those restrictions to allow more values to be set. I still contend that there will be some GUCs that simply don't make any sense to have in the auto-conf area of PGDATA. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] inconsistent state after crash recovery
On Fri, Jul 26, 2013 at 8:27 AM, Tom Lane t...@sss.pgh.pa.us wrote: Andres Freund and...@2ndquadrant.com writes: On 2013-07-26 13:33:13 +0900, Satoshi Nagayasu wrote: Is this expected or acceptable? I'd say it's both. Postgres is built on the assumption that the underlying filesystem is reliable, ie, once you've successfully fsync'd some data that data won't disappear. If the filesystem fails to honor that contract, it's a filesystem bug not a Postgres bug. Nor is it reasonable to expect Postgres to be able to detect every such violation. As an example, would you expect crash recovery to notice the disappearance of a file that was touched nowhere in the replayed actions? Eh, maybe not. But should we try harder to detect the unexpected disappearance of one that is? -- 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] Add json_typeof() and json_is_*() functions.
On Mon, Jul 29, 2013 at 5:36 PM, Merlin Moncure mmonc...@gmail.com wrote: On Mon, Jul 29, 2013 at 2:16 AM, Andrew Tipton and...@kiwidrew.com wrote: The attached patch adds four new SQL functions for the JSON type: json_typeof(json) RETURNS text json_is_object(json) RETURNS boolean json_is_array(json) RETURNS boolean json_is_scalar(json) RETURNS boolean The motivating use-case for this patch is the ability to easily create a domain type for what RFC 4627 calls json text, where the top-level value must be either an object or array. An example of this usage is: CREATE DOMAIN json_document AS json CHECK (NOT json_is_scalar(VALUE)); An additional use-case arises when writing functions which can handle arbitrary JSON values. This can be difficult when nested objects or arrays are present or when the input may be either an array or an object. Many of the built-in functions will raise an error when presented with an invalid value, such as when giving an array to json_object_keys(). The json_typeof() and json_is_*() functions should make it easier to call the correct function in these cases, e.g.: CASE json_typeof($1) WHEN 'object' THEN json_object_keys($1) WHEN 'array' THEN json_array_elements($1) ELSE $1 END These new functions operate by making a single call to json_lex() to get the first token of the JSON value; this token uniquely determines the value's type. (Thanks to Merlin Moncure for suggesting this approach.) The patch also updates the JSON Functions and Operators section of the docs to ensure that the words value, object, and array are used in a consistent manner. JSON object and JSON array refer to parameters which must be an object or an array or to results which are always an object or an array. JSON value refers to parameters or results which may be any kind of JSON. you're welcome! :-). small point: Personally I would prune the supplied functions to json_typeof() and json_is_scalar(). These functions are in the public namespace so there is a certain minimum bang/buck ratio which IMNSHO json_is_object() and json_is_array() don't meet -- just call json_typeof() to get that info. +1, but I'm wondering why we need anything more than just json_typeof(). Doesn't that pretty much cover it? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Tue, Jul 30, 2013 at 3:10 AM, Gibheer gibh...@zero-knowledge.org wrote: here is an update off my patch based on the discussion with Marko Tiikkaja and Andres Freund. Marko and I had the idea of introducing reserved connections based on roles as it would create a way to garantuee specific roles to connect when other roles use up all connections for whatever reason. But Andreas said, that it would make connecting take much too long. So to just fix the issue at hand, we decided that adding max_wal_senders to the pool of reserved connections is better. With that, we are sure that streaming replication can connect to the master. So instead of creating a new configuration option I added max_wal_senders to the reserved connections and changed the check for new connections. The test.pl is a small script to test, if the patch does what it should. Hmm. It seems like this match is making MaxConnections no longer mean the maximum number of connections, but rather the maximum number of non-replication connections. I don't think I support that definitional change, and I'm kinda surprised if this is sufficient to implement it anyway (e.g. see InitProcGlobal()). -- 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] inconsistent state after crash recovery
Robert Haas robertmh...@gmail.com writes: On Fri, Jul 26, 2013 at 8:27 AM, Tom Lane t...@sss.pgh.pa.us wrote: would you expect crash recovery to notice the disappearance of a file that was touched nowhere in the replayed actions? Eh, maybe not. But should we try harder to detect the unexpected disappearance of one that is? We do, don't we? The replay stuff should complain unless it sees a drop or truncate covering any unaccounted-for pages. 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] inconsistent state after crash recovery
On Fri, Aug 2, 2013 at 8:17 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Jul 26, 2013 at 8:27 AM, Tom Lane t...@sss.pgh.pa.us wrote: would you expect crash recovery to notice the disappearance of a file that was touched nowhere in the replayed actions? Eh, maybe not. But should we try harder to detect the unexpected disappearance of one that is? We do, don't we? The replay stuff should complain unless it sees a drop or truncate covering any unaccounted-for pages. Hmm. Yeah. But the OP seems to think it doesn't work. -- 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] Immediate shutdown causes the assertion failure in 9.4dev
On Wed, Jul 31, 2013 at 1:26 PM, Fujii Masao masao.fu...@gmail.com wrote: I encountered the following assertion failure when I executed an immediate shutdown. LOG: received immediate shutdown request WARNING: terminating connection because of crash of another server process DETAIL: The postmaster has commanded this server process to roll back the current transaction and exit, because another server process exited abnormally and possibly corrupted shared memory. HINT: In a moment you should be able to reconnect to the database and repeat your command. TRAP: FailedAssertion(!(CheckpointerPID == 0), File: postmaster.c, Line: 3440) The cause of this problem seems to be that PostmasterStateMachine() may fail to wait for the checkpointer to exit. Attached patch fixes this. What commit broke this? -- 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] Add json_typeof() and json_is_*() functions.
On Fri, Aug 2, 2013 at 8:12 PM, Robert Haas robertmh...@gmail.com wrote:
+1, but I'm wondering why we need anything more than just
json_typeof(). Doesn't that pretty much cover it?
I agree with Merlin that json_is_object() is superfluous, since it can just
be replaced with json_typeof() = 'object'. Likewise for json_is_array().
But without json_is_scalar(), the choice is one of these two forms:
json_typeof() NOT IN ('object', 'array')
json_typeof() IN ('string', 'number', 'boolean', 'null')
And it protects the user against forgetting about, say, the 'null' typeof()
when constructing their check expression.
Regards,
Andrew Tipton
Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Friday, August 02, 2013 5:19 PM Stephen Frost wrote: * Amit Kapila (amit.kap...@huawei.com) wrote: Below are some points in my mind due to which I have supported/implemented one-file-all-setting approach: a. I had heard quite a few times that Postgres has lot of files (each relation has separate file) as compare to Oracle. Users feel that Oracle's table space approach is better. This is completely unrelated to this discussion, imv. The point I wanted to convey is that having more files for database in general is not a great idea. b. While server start/Sighup, we needs to open/read/close each file separately which in-itself seems to be overhead. I also don't think performance of this particular operation should be a high priority. If it makes startup taking more time, then isn't it a performance critical path? I believe what Greg Stark has said in his below mail link is the more appropriate way and the current patch has done that way. http://www.postgresql.org/message-id/CAM- w4HP7=a2VowyJUD0CAZL5b8FsaHym dQeouL udsohdncw...@mail.gmail.com He doesn't actually provide any reasoning for it. That said, I've not heard any particularly good reason for having a setting per file either. This is an internal-to-PG data file and we should really implement it in whichever way makes the most sense for us. My general feeling is that one file is simpler and sufficient for the postgresql.conf-like parameters, Sure, I also feel the same that if it can be addressed with single file, then lets do that way only. but I wonder what we're going to do for pg_hba/pg_ident. Those would be good to have multiple files for because (as we saw with pg_authid) they could get quite large because they can have per-user entries in them and rewriting a large file for every change would be quite painful. Also when other commercial database (Oracle) can do it with single file, users will try to compare with it. To make it clear- this isn't justification for this design. Also, the notion that Oracle's *configuration* is all done with a *single-file* is.. laughable. Not all Oracle's configuration, but Change of configuration parameters. IIRC, before starting this feature I had checked Oracle's specs and it seems to be not doing with multiple files for Alter System. If you have doubt, I can once again Verify it. With Regards, Amit Kapila. -- 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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Amit Kapila (amit.kap...@huawei.com) wrote: This is an internal-to-PG data file and we should really implement it in whichever way makes the most sense for us. My general feeling is that one file is simpler and sufficient for the postgresql.conf-like parameters, Sure, I also feel the same that if it can be addressed with single file, then lets do that way only. We need to settle on one choice and then implement it, yes. It certainly doesn't make any sense to have two different ways to deal with an internal-to-PG data structure. Of course, that might argue for making this file actually *be* like pg_authid is today; has that been considered? I'm guessing it's not practical because the point where we need to read the config is before certain things have been set up to allow reading from heap files, but it seems like something which should at least be considered. If we can make it work, then that may also solve the pg_hba/pg_ident issue, which is about a bazillion times more interesting than the mostly set-and-forget postgresql.conf settings. Perhaps having the file be a heap file instead of anything a sysadmin can be asked to go hack would also make it more clear that this is an internal PG file which is to be managed only through PG and stop all this arguing about how oh, they can just fix it by twiddling things in $PGDATA is considered by some to be an acceptable solution. Heck, it'd also keep the number of files down while allowing more fine-grained modifications and writes (iow, we wouldn't have to rewrite the whole file every time..). Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Add json_typeof() and json_is_*() functions.
On Fri, Aug 2, 2013 at 7:22 AM, Andrew Tipton and...@kiwidrew.com wrote:
On Fri, Aug 2, 2013 at 8:12 PM, Robert Haas robertmh...@gmail.com wrote:
+1, but I'm wondering why we need anything more than just
json_typeof(). Doesn't that pretty much cover it?
I agree with Merlin that json_is_object() is superfluous, since it can just
be replaced with json_typeof() = 'object'. Likewise for json_is_array().
But without json_is_scalar(), the choice is one of these two forms:
json_typeof() NOT IN ('object', 'array')
json_typeof() IN ('string', 'number', 'boolean', 'null')
And it protects the user against forgetting about, say, the 'null' typeof()
when constructing their check expression.
right: I was thinking also that if/when json were ever to get new
types, you'd appreciate that function.
merlin
--
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] inconsistent state after crash recovery
Hi, I'm very new here on this mailing list, but I've been using PostgreSQL for a while, and it scares me a little, that it's a real pain to try to recover data from corrupted table. Situations like file being lost following server crash (after fsck) or page corruption happens quite often. Having corruption at the row level for example, system could mark the page as corrupted in the system catalog. Giving that page knows last change to this page, can we use archived WAL-s to recover the page? We can have a table inside pg_catalog like pg_corrupted_pages with information of page corruption detected by backend. Similar to tables, in a case of lost file, once system notice that, we should have a column in pg_class called relneedrecovery to record that. Will it be possible to recover this file from last hot backup and apply redo to it based on WAL records? Similar functionality is provider by Oracle (media and block recovery). I assume we will need some extra DDL commands to handle file/block recovery. Also, It would be nice to have a command to quickly cross check files between pg_class and filesystem - just simple open/close system call for each relation - it is always faster that to run vaccum to check that. Tomasz On 2 August 2013 13:19, Robert Haas robertmh...@gmail.com wrote: On Fri, Aug 2, 2013 at 8:17 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Jul 26, 2013 at 8:27 AM, Tom Lane t...@sss.pgh.pa.us wrote: would you expect crash recovery to notice the disappearance of a file that was touched nowhere in the replayed actions? Eh, maybe not. But should we try harder to detect the unexpected disappearance of one that is? We do, don't we? The replay stuff should complain unless it sees a drop or truncate covering any unaccounted-for pages. Hmm. Yeah. But the OP seems to think it doesn't work. -- 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 On 2 August 2013 13:19, Robert Haas robertmh...@gmail.com wrote: On Fri, Aug 2, 2013 at 8:17 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Jul 26, 2013 at 8:27 AM, Tom Lane t...@sss.pgh.pa.us wrote: would you expect crash recovery to notice the disappearance of a file that was touched nowhere in the replayed actions? Eh, maybe not. But should we try harder to detect the unexpected disappearance of one that is? We do, don't we? The replay stuff should complain unless it sees a drop or truncate covering any unaccounted-for pages. Hmm. Yeah. But the OP seems to think it doesn't work. -- 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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost sfr...@snowman.net writes: Perhaps having the file be a heap file instead of anything a sysadmin can be asked to go hack would also make it more clear that this is an internal PG file which is to be managed only through PG and stop all this arguing about how oh, they can just fix it by twiddling things in $PGDATA is considered by some to be an acceptable solution. Whether you think it's acceptable or not, sometimes it's *necessary*. What if you set a combination of parameters that prevents Postgres from starting? Even if we could make config-in-a-catalog work, which I'm very dubious of, I would think it far too unsafe to use. We could of course fix that problem by only storing safe parameters in a catalog, but then you lose the supposed appeal of a single-file solution. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Typo fix in bufmgr.c
On Wed, Jul 31, 2013 at 5:50 AM, Etsuro Fujita fujita.ets...@lab.ntt.co.jp wrote: Attached is a small typo fix patch. Committed. Thanks. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 7/29/13 3:46 PM, Josh Berkus wrote: Based on the ongoing discussion of this patch, I have moved it to 9.4CF2 (9-2013). Mind you, it would be good to commit some version of it before September. Quite a bit of the patch adds some refactored GUC parameter validation code that seems necessary for this feature, and that's now where the controversial parts are either. That's the part I thought should be looked at for commit now. I'd like to get the change size Amit is carrying around and (and other people are reviewing) reduced here. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-02 08:41:09 -0400, Stephen Frost wrote: Perhaps having the file be a heap file instead of anything a sysadmin can be asked to go hack would also make it more clear that this is an internal PG file which is to be managed only through PG and stop all this arguing about how oh, they can just fix it by twiddling things in $PGDATA is considered by some to be an acceptable solution. Heck, it'd also keep the number of files down while allowing more fine-grained modifications and writes (iow, we wouldn't have to rewrite the whole file every time..). Crash recovery PGC_SIGHUP|PGC_POSTMASTER… 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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Writing out each guc in a separate file is a singularly bad idea. It's going out of our way to confuse users about what's going on and how they're expected to interact with the settings files and it actively makes it harder or nearly impossible to protect against simple failures. 1) The whole reason for conf.d directories for things like Apache or cron or whatever is so that other software can drop in snippets without having to parse and edit a file in place. We *do not* want users doing that inside PGDATA. I'm not even clear we do want this in /etc since none of our GUC options are repeatable things like Apache virtual servers. It actually makes *more* sense for pg_hba than it does for gucs. I think we can assume that in the future we'll have something like it however. 2) Directories are notoriously hard to version control, most version control systems either don't do it at all or do a weak form of version control on directories. Even if users tried to track the changes in these files they'll likely find it difficult to tell when two settings were changed together or in separate changes. On the other hand if all the settings are in a single file then even the simplest form of version control -- backup files -- would suffice. If we just keep a backup copy of the settings file for each change then it would be easy for people to diff from one version to another and see all the changes together and easy for them to restore an old copy if the current one isn't starting up. If they're in a million tiny files then users would have to keep a backup copy of the whole directory and dig thorugh a recursive diff of the whole directory. Or they would have tons of backup files for different settings at different times and need to figure out which ones were in effect at a given time. -- 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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 8/1/13 10:47 AM, David Johnston wrote: Minor request: could someone enlighten me as to why making the directory location a compile-time option is undesirable. The ongoing argument here is whether to allow moving the directory at all, or to make it fixed to $PGDATA the way recovery.conf is. If you accept that it should float, then it actually needs to be a start time option. Software like Debian moves around the postgresql.conf like this: pg_ctl -c config_file=/etc/postgresql/9.3/main/postgresql.conf ... The way this argument is going the last few days, I'm starting to think that it's worth breaking this style of config directory setup out into its own feature now. Whether or not ALTER SYSTEM SET takes advantage of the config directory or not seems a still raging question. I've been coupling the two together because I think the design of ALTER SYSTEM SET should consider a config directory based approach. But from the perspective of what can get committed first, the config directory really should go first. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.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] Should we automatically run duplicate_oids?
Sent from my iPad On 02-Aug-2013, at 10:30, Bruce Momjian br...@momjian.us wrote: On Mon, Jul 8, 2013 at 06:25:44PM -0700, Peter Geoghegan wrote: When rebasing a patch that I'm working on, I occasionally forget to update the oid of any pg_proc.h entries I may have created. Of course this isn't a real problem; when I go to initdb, I immediately recognize what has happened. All the same, it seems like there is a case to be made for having this run automatically at build time, and having the build fail on the basis of there being a duplicate - this is something that fails reliably, but only when someone has added another pg_proc.h entry, and only when that other person happened to choose an oid in a range of free-in-git-tip oids that I myself fancied. Sure, I ought to remember to check this anyway, but it seems preferable to make this process more mechanical. I can point to commit 55c1687a as a kind of precedent, where the process of running check_keywords.pl was made to run automatically any time gram.c is rebuilt. Granted, that's a more subtle problem than the one I'm proposing to solve, but I still see this as a modest improvement. FYI, attached is the pgtest script I always run before I do a commit; it also calls src/tools/pgtest. It has saved me from erroneous commits many times. +1,a much needed thing.Duplicate oids is a pain enough to deserve its own solution. Regards, Atri -- 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 message for CREATE VIEW is confusing
On Wed, Jul 31, 2013 at 6:49 AM, Pavel Golub pa...@microolap.com wrote: Hello, PostgreSQL. Let's assume we have created MATERIALIZED VIEW, e.g. CREATE MATERIALIZED VIEW customer_v AS SELECT ; Then one wants to redefine this view as a regular view, e.g. CREATE OR REPLACE VIEW customer_v AS ; Error is rising: ERROR: customer_v is not a view ** Error ** ERROR: customer_v is not a view SQL-state: 42809 Should we change error message to something like customer_v has wrong object type (according to errcode appendix)? Or should we change word view to regular view since we have materialized already, e.g. customer_v is not a regular view? Well, this is another instance of the general problem that some people think that view ought to mean materialized view, but it doesn't. I'm not inclined to go too crazy trying to clear up all possible ambiguities in this area, because I think it's a rat's nest that will never really work out well as long as people think those two things are somehow the same. One idea is to add a hint: HINT: It is a materialized view. But I'm not sure whether that's a good idea or not. -- 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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Tom Lane (t...@sss.pgh.pa.us) wrote: What if you set a combination of parameters that prevents Postgres from starting? This was what I was trying to get at up-thread. Things that prevent PG from being able to start (or, really, which cause PG to be started in a completely different mode, ala recovery.conf) need to be able to be modified outside of PG and therefore should, imv, be considered configuration parameters and therefore live outside of $PGDATA (when installed from a distro, blah, blah). We could of course fix that problem by only storing safe parameters in a catalog, but then you lose the supposed appeal of a single-file solution. I don't see having them all in one file is more convenient for the admin as being a justification for the single-file approach, simply because I don't consider the 'auto' file to be something that the admin would be modifying. Curiously, I've not heard any argument about what parameters are safe and what aren't, though I was asked which ones I thought were safe and which weren't. Perhaps looking at the specific options that would likely cause PG to not start would be useful to this discussion. Off-hand, I see: data_directory- Clearly we need to know this before starting, so it has to be defined somewhere and then passed to PG in some way. Currently this is done in Ubuntu by having the init script read the directory out of the postgresql.conf, but it could also be passed through Ubuntu's pg_ctl.conf, which takes a set of parameters to pass. I will note here, though it may apply in other places also, that this part of the configuration is necessairly the distro's responsibility. hba_file- Won't start without one. ident_file- We *will* start without one. We'll even start if it's got garbage in it. I have to admit, I was a bit surprised that this behaves so differently from hba_file wrt dealing with existance / errors. listen_addresses- Won't start if it's invalid, but if it's not there we'll just try to listen on localhost:5432, but if that fails we won't start. port- Similar to listen_addresses unix_socket_directories, unix_socket_group- Won't start if it's invalid, will start w/ a default if not set. ssl_cert_file, ssl_key_file, ssl_ca_file, ssl_crl_file, krb_server_keyfile, shared_preload_libraries- Won't start if it's invalid, not used if not set. shared_buffers- Older versions won't start if it's set above the SHM limits on the system. Even in 9.3+ though, if set too high, will either cause it to not start or to possible crash very quickly (eg: user set it to however much real memory they have in the system). log_directory, log_filename- Won't start if set such that PG can't create the necessary directories or open its log file. log_timezone, timezone, lc_messages, lc_monetary, etc- Won't start if set invalid- can we check validity of this when set through ALTER SYSTEM? local_preload_libraries- Will start if it's set to something invalid, but then you can't connect because new backend starts will fail. Now, there's certainly a whole slew of things which *can* be modified w/o too much risk to being able to get PG started again. Also, many of the things above could probably be changed to deal with error cases and keep starting up (eg: ssl_*). Andrew pointed out that we can use command-line arguments to override badly configured parameters, but I'm worried that'd simply end up making those items be configured through the distro scripts, and probably in a way that isn't as nice as postgresql.conf, eg: Ubuntu's pg_ctl.conf might become: pg_ctl_options = '-o -c listen_addresses=127.0.01.1 -o -c port=5433' etc, or, more likely, a *new* config file being created to manage these things which is completely distribution-specific... Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Add json_typeof() and json_is_*() functions.
Merlin Moncure escribió:
On Fri, Aug 2, 2013 at 7:22 AM, Andrew Tipton and...@kiwidrew.com wrote:
On Fri, Aug 2, 2013 at 8:12 PM, Robert Haas robertmh...@gmail.com wrote:
+1, but I'm wondering why we need anything more than just
json_typeof(). Doesn't that pretty much cover it?
I agree with Merlin that json_is_object() is superfluous, since it can just
be replaced with json_typeof() = 'object'. Likewise for json_is_array().
But without json_is_scalar(), the choice is one of these two forms:
json_typeof() NOT IN ('object', 'array')
json_typeof() IN ('string', 'number', 'boolean', 'null')
And it protects the user against forgetting about, say, the 'null' typeof()
when constructing their check expression.
right: I was thinking also that if/when json were ever to get new
types, you'd appreciate that function.
That was what I thought as well upon seen the code.
--
Á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] Immediate shutdown causes the assertion failure in 9.4dev
Robert Haas escribió: On Wed, Jul 31, 2013 at 1:26 PM, Fujii Masao masao.fu...@gmail.com wrote: I encountered the following assertion failure when I executed an immediate shutdown. LOG: received immediate shutdown request WARNING: terminating connection because of crash of another server process DETAIL: The postmaster has commanded this server process to roll back the current transaction and exit, because another server process exited abnormally and possibly corrupted shared memory. HINT: In a moment you should be able to reconnect to the database and repeat your command. TRAP: FailedAssertion(!(CheckpointerPID == 0), File: postmaster.c, Line: 3440) The cause of this problem seems to be that PostmasterStateMachine() may fail to wait for the checkpointer to exit. Attached patch fixes this. What commit broke this? The one that introduced SIGKILL in immediate-mode stop. -- Á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
[HACKERS] WIP: Partial match using range key entries
While looking at the GIN's partial match logic, I got an idea to let the generic
index code do what opclass-specific comparePartial() functions do. It can be
achieved if range type is accepted as key entry.
In this patch I add ANYRANGEARRAY pseudotype (note that changes in
parse_coerce.c are rather ad hoc so far) and a set of cross-type operators like
ANYARRAY @ ANYRANGEARRAY
The semantics is that the range array is contained in the array iff each range
element has a matching (non-range) element in the left array. For example:
postgres=# SELECT ARRAY[-2, 5, 0.1, 10]::numeric[] @ ARRAY['[-10,-1]',
'[7,10]']::numrange[];
?column?
--
t
(1 row)
postgres=# SELECT ARRAY[-2, 5, 0.1, 10]::numeric[] @ ARRAY['[-10,-1]',
'[7,10)']::numrange[];
?column?
--
f
(1 row)
The other operators also correspond to those (ANYARRAY, ANYARRAY), except that
array elements are matched using
ANYRANGE @ ANYELEMENT
The patch just adds the matching logic to GIN. It does not remove the original
partial match because text search depends on it.
Subtopic: GIN and cross-type operators
--
So far all the in-core operators in the GIN's opfamilies have oprleft equal to
oprright. When I tried to implement the (ANYARRAY, ANYRANGEARRAY) operators I
had to do some changes in the core code:
1. While GIN_COMPARE_PROC and GIN_EXTRACTVALUE_PROC support functions depend on
pg_opclass(opckeytype) and pg_opclass(opcintype) respectively (and thus are
universial for the whole opclass), the other ones can be specific for
pg_amproc(amproclefttype, amprocrighttype). That's why I moved some code from
ginbeginscan() to ginrescan().
(I think it'd make sense to only store GIN_COMPARE_PROC and
GIN_EXTRACTVALUE_PROC once per opclass, but that would require changes in CREATE
OPERATOR CLASS command.)
2. To let the GIN code find the appropriate support functions for cross-type
operators, I had to ensure that scan key's sk_subtype contains OID of particular
type as opposed to that of the pseudotype.
Is there any misconception in this patch proposal?
// Antonin Houska (Tony)
diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index cb17d38..9e1c665 100644
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -194,7 +194,7 @@ collectMatchBitmap(GinBtreeData *btree, GinBtreeStack *stack,
/* Initialize empty bitmap result */
scanEntry-matchBitmap = tbm_create(work_mem * 1024L);
- /* Null query cannot partial-match anything */
+ /* Null query cannot partial/range-match anything */
if (scanEntry-isPartialMatch
scanEntry-queryCategory != GIN_CAT_NORM_KEY)
return true;
@@ -263,8 +263,34 @@ collectMatchBitmap(GinBtreeData *btree, GinBtreeStack *stack,
stack-off++;
continue;
}
- }
- else if (scanEntry-searchMode == GIN_SEARCH_MODE_ALL)
+ } else if (scanEntry-isRange) {
+ int32 cmp;
+ Datum lower = scanEntry-rangeLower.val;
+ bool lowerIncl = scanEntry-rangeLower.inclusive;
+ Datum upper = scanEntry-rangeUpper.val;
+ bool upperIncl = scanEntry-rangeUpper.inclusive;
+
+ cmp = DatumGetInt32(FunctionCall2Coll(btree-ginstate-compareFn[attnum - 1],
+ btree-ginstate-supportCollation[attnum - 1],
+ idatum, lower));
+
+ if ((lowerIncl cmp 0) || (!lowerIncl cmp = 0)) {
+/* Matching idatum not reached yet. */
+stack-off++;
+continue;
+ }
+
+ cmp = DatumGetInt32(FunctionCall2Coll(btree-ginstate-compareFn[attnum - 1],
+ btree-ginstate-supportCollation[attnum - 1],
+ idatum, upper));
+
+ if ((upperIncl cmp 0) || (!upperIncl cmp = 0))
+/*
+ * We're past the upper bound, no more matches
+ * for the current range.
+ */
+return true;
+ } else if (scanEntry-searchMode == GIN_SEARCH_MODE_ALL)
{
/*
* In ALL mode, we are not interested in null items, so we can
@@ -392,7 +418,7 @@ restartScanEntry:
entry-isFinished = TRUE;
- if (entry-isPartialMatch ||
+ if (entry-isPartialMatch || entry-isRange ||
entry-queryCategory == GIN_CAT_EMPTY_QUERY)
{
/*
@@ -1531,7 +1557,11 @@ gingetbitmap(PG_FUNCTION_ARGS)
* to scan the main index before the pending list, since concurrent
* cleanup could then make us miss entries entirely.
*/
- scanPendingInsert(scan, tbm, ntids);
+ /*
+ * TODO
+ * enable when the match logic is adjusted.
+ */
+ //scanPendingInsert(scan, tbm, ntids);
/*
* Now scan the main index.
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index afee2db..b085f75 100644
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -17,6 +17,7 @@
#include access/gin_private.h
#include access/relscan.h
#include pgstat.h
+#include utils/lsyscache.h
#include utils/memutils.h
#include utils/rel.h
@@ -59,11 +60,98 @@ static GinScanEntry
ginFillScanEntry(GinScanOpaque so, OffsetNumber attnum,
StrategyNumber strategy, int32 searchMode,
Datum queryKey,
[HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Greg, * Greg Stark (st...@mit.edu) wrote: Writing out each guc in a separate file is a singularly bad idea. It's going out of our way to confuse users about what's going on and how they're expected to interact with the settings files and it actively makes it harder or nearly impossible to protect against simple failures. I agree that having a separate file for each GUC is a bad idea. That said, I still contend that there's a difference between files in $PGDATA and files found in /etc. 1) The whole reason for conf.d directories for things like Apache or cron or whatever is so that other software can drop in snippets without having to parse and edit a file in place. We *do not* want users doing that inside PGDATA. Agreed. That said, if users *want* a separate file per GUC in their /etc, we can let them do that with the conf.d structure. I'm not even clear we do want this in /etc since none of our GUC options are repeatable things like Apache virtual servers. It actually makes *more* sense for pg_hba than it does for gucs. I think we can assume that in the future we'll have something like it however. I tend to agree with this also, though I can imagine wanting to separate things in a conf.d directory ala exim's conf.d directories, to allow tools like puppet to manage certain things environment-wide (perhaps krb_server_keyfile) while other configuration options are managed locally. If we just keep a backup copy of the settings file for each change then it would be easy for people to diff from one version to another and see all the changes together and easy for them to restore an old copy if the current one isn't starting up. If they're in a million tiny files then users would have to keep a backup copy of the whole directory and dig thorugh a recursive diff of the whole directory. Or they would have tons of backup files for different settings at different times and need to figure out which ones were in effect at a given time. This I don't entirely follow though. Above, you don't want users monkeying around in PGDATA dropping files, but it's ok for them to be diffing them and, presumably, rewritting them with older version when they feel the need to? I agree that we should provide a way for users to get old versions of their config and know when things changed, but, to be honest, I'd like to see that kind of audit log information for various catalog tables too- eg: pg_database. If we had event triggers and they could be fired for ALTER SYSTEM along with ALTER DATABASE, then we could have an audit system with an audit table which takes who did what when, old value, new value, etc.. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Dimitri Fontaine escribió: Andres Freund and...@2ndquadrant.com writes: They would need a setting that disables ALTER (DATABASE|USER) ... SET ... as well though. At least for some settings. I don't think enforcing things on that level makes much sense. If only we could trigger some actions when a command is about to be executed, in a way that it's easy for the user to implement whatever policy he fancies… Oh, maybe I should finish preparing those patches for Event Triggers to be fully usable in 9.4 then ;) I remind you that event triggers are not fired for global objects such as databases and roles. Do you intend to lift that restriction? -- Á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
[HACKERS] Need help to begin contribution in PostgreSQL Development - Specifically XML module
Dear pgsql-hackers, We students of International Institute of Information Technology Bangalore India, are interested to contribute to PostgreSQL development. We identified some modules from ToDo list to which we want to contribute.We want to begin with an simple module with less dependency like 'Add pretty-printed XML output option'. If our work is satisfactory we would like to further contribute for module 'Add XML Schema validation and xmlvalidate functions (SQL:2008)'. If the ToDo items chosen are okay, will you please help us by elaborating more details on requirements of module 'Add pretty-printed XML output option', we want to begin with this module so as to quick overview of complete process. Thanks Regards, Kodamasimham Pridhvi Bisen Vikrantsingh
[HACKERS] CREATE MATERIALIZED VIEW .. FOR UPDATE
Does the combination in $SUBJECT make sense? It is currently allowed, but of course the underlying locks only last while the creating transaction is open, and they are reacquired during a refresh. Somewhat related is that the error message they emit is a bit nonstandard: cannot lock rows in materialized view \%s\ After checking the reason for this, I noticed that it doesn't even match what the code thinks it should (CheckValidRowMarkRel()): case RELKIND_MATVIEW: /* Should not get here */ ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg(cannot lock rows in materialized view \%s\, RelationGetRelationName(rel; apparently this function believes that the check should be applied earlier, but it isn't. I think we ought to either add a check to the parser stage; *or* we should remove the should not get here comment. I also propose we make these errors consistent with the wording of the other related errors, i.e. FOR UPDATE is not allowed with materialized views, and of course change it for all the other cases in that function. Opinions? -- Á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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost escribió: * Tom Lane (t...@sss.pgh.pa.us) wrote: What if you set a combination of parameters that prevents Postgres from starting? This was what I was trying to get at up-thread. Things that prevent PG from being able to start (or, really, which cause PG to be started in a completely different mode, ala recovery.conf) need to be able to be modified outside of PG and therefore should, imv, be considered configuration parameters and therefore live outside of $PGDATA (when installed from a distro, blah, blah). I think the way out of this situation is to have a postmaster and/or pg_ctl switch that disables reading of ALTER SYSTEM settings. Then the DBA can do ALTER SYSTEM RESET until it works again. No need to mess with data files. -- Á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] CREATE MATERIALIZED VIEW .. FOR UPDATE
Alvaro Herrera alvhe...@2ndquadrant.com wrote: Does the combination in $SUBJECT make sense? I don't think so; I don't know what it would mean. It is currently allowed, I will take a look. -- 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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 08/02/2013 07:54 AM, Stephen Frost wrote: Curiously, I've not heard any argument about what parameters are safe and what aren't, though I was asked which ones I thought were safe and which weren't. Perhaps looking at the specific options that would likely cause PG to not start would be useful to this discussion. I really think this is the wrong approach. If we start removing unsafe parameters from ALTER SYSTEM SET, we basically hobble the feature to the point of uselessness. Out of the 15 or so parameters 80% of our users touch, half of them are on your unsafe list. A much simpler solution to the issue Stephen proposes is to have a way to start up the server with all settings from ALTER SYSTEM SET disabled, just like some software allows you to start it up in safe mode. Of course, automatically disabling the *individual* parameters would be even better from a usability perspective. This would be equally useful for a manually-written postgresql.conf, i.e.: PostgreSQL is unable to start because we couldn't allocate all of the memory you asked for. Starting up with shared_buffers set to 32MB.. ... However, I'm not confident that we'll always be able to do that. We'd also need to have a way to kick and scream so that sysadmins would actually SEE it when the system disables a parameter. -- 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] CREATE MATERIALIZED VIEW .. FOR UPDATE
I just realized I mixed two different (but related) cases in my previous email: Alvaro Herrera wrote: Does the combination in $SUBJECT make sense? It is currently allowed, but of course the underlying locks only last while the creating transaction is open, and they are reacquired during a refresh. This paragraph is talking about a FOR UPDATE clause in the CREATE MATERIALIZED VIEW command, as in the email subject. Somewhat related is that the error message they emit is a bit nonstandard: cannot lock rows in materialized view \%s\ This other paragraph, and everything below it, is talking about a SELECT .. FROM matview FOR UPDATE command. -- Á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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Josh Berkus (j...@agliodbs.com) wrote: On 08/02/2013 07:54 AM, Stephen Frost wrote: Curiously, I've not heard any argument about what parameters are safe and what aren't, though I was asked which ones I thought were safe and which weren't. Perhaps looking at the specific options that would likely cause PG to not start would be useful to this discussion. I really think this is the wrong approach. If we start removing unsafe parameters from ALTER SYSTEM SET, we basically hobble the feature to the point of uselessness. Out of the 15 or so parameters 80% of our users touch, half of them are on your unsafe list. They're on the 'unsafe' list because they're likely to *break* things. I'm not at *all* surprised that the list comprises 80% of the parameters that people actually modify/use, but that doesn't mean it's smart to let them hack at those parameters remotely w/ no access to the host system to fix things if they screw it up, which is really the case that I'm thinking about here because it's what we're enabling through this mechanism. If the users already had access to the host system to go modify the parameters in the config on the filesystem, they'd be likely to just go *do* that, no? A much simpler solution to the issue Stephen proposes is to have a way to start up the server with all settings from ALTER SYSTEM SET disabled, just like some software allows you to start it up in safe mode. See above for why I'm not thrilled wih this approach, unless it was set up to happen automatically, but you couldn't simply ignore *all* the ALTER SYSTEM SET parameters because then you might not be able to connect in due to some ALTER SYSTEM SET parameter being necessary for remote connectivity or authentication. Of course, automatically disabling the *individual* parameters would be even better from a usability perspective. This would be equally useful for a manually-written postgresql.conf, i.e.: PostgreSQL is unable to start because we couldn't allocate all of the memory you asked for. Starting up with shared_buffers set to 32MB.. Right; this is part of what I was getting at with the list- there are definitely items on that list that we could start up without, were they misconfigured. The one question remaining from *that* however is if there would be such a security impact from the config change that it wouldn't be *safe* for us to do so (consider if we used our 'default' pg_hba.conf, with 'trust' all over the place, in the event that we couldn't load the system pg_hba.conf..). ... However, I'm not confident that we'll always be able to do that. We'd also need to have a way to kick and scream so that sysadmins would actually SEE it when the system disables a parameter. Yes, this is also true. When it comes to a 'safe mode', my original inclination was to only allow connections over a unix socket from a user running as the same Unix UID as the database is running as... That's not a solution for the Windows folks though, I'm afraid. :/ Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Patch for removng unused targets
Etsuro Fujita fujita.ets...@lab.ntt.co.jp writes: Thank you for the adjustments and comments! In addition to adding comments to the function, I've improved the code in the function a little bit. Please find attached an updated version of the patch. I started looking at this patch (finally). I'm not terribly satisfied with it, because it addresses only a very small part of what we really need to do in this area, and I'm afraid we might end up throwing it away in toto once we make the larger changes needed. I carped about this a bit back in 15642.1354650...@sss.pgh.pa.us, but to try to fill in some background, consider a query like select expensive_function(x) from t; where, since the DBA is smart, t has an index on expensive_function(x). Ideally we'd just scan the index and return values out of it, without recomputing the expensive_function(). The planner is physically able to produce such a plan, but only in very limited cases, and an even bigger problem is that its cost accounting doesn't recognize the potential savings from not evaluating expensive_function(x); therefore, even if it can generate the right plan, it might discard it in favor of a plan that doesn't use the index. This patch has got that same problem: it makes a useful improvement in the finished plan if that plan is of the right form, but it does nothing to push the planner to produce that form in the first place. Basically these problems stem from the assumption that we can treat all scan/join paths as producing the same flat tlist (containing only Vars) and only worry about tlist evaluation at the top level. I think the fix will have to involve recognizing that certain paths can produce some expressions more cheaply than others can, and explicitly including those expressions in the returned tlists in such cases. That's going to be a pretty invasive change. (Of course, the executor already works that way, but the planner has never included such considerations at the Path stage.) Now, the connection to the patch at hand is that if the query is select x,y,z from t order by expensive_function(x); this patch will successfully suppress calculation of the expensive function, *if* we were lucky enough to make the right choice of plan without considering the cost of the function. It's perfectly capable of making the wrong choice though. This will lead to bug reports about the planner chooses a dumb plan, even though it knows the right plan is cheaper when I force it to choose that one. I think it's possible to revise the patch so that we do take the cost savings into account, at least at the point in grouping_planner where it chooses between the cheapest_path and the sorted_path returned by query_planner. (I'm not sure if there are cases where query_planner would discard the best choice at an earlier stage, but that seems possible in join queries.) But this won't do anything for cases where the expensive function appears in the SELECT list. So as I said, I'm worried that this will be mostly bogus once we address the larger problem. With the larger fix in place, the expensive_function value could come out of the indexscan, and then the resjunk expression would be nothing more than a Var referencing it, and hence hardly worth suppressing. Having said all that, there is one situation where this type of approach might still be useful even after such a fix, and that's KNNGist-style queries: select a,b,c from t order by col - constant limit 10; In a KNNGist search, there's no provision for the index AM to return the actual value of the ORDER BY expression, and in fact it's theoretically possible that that value is never even explicitly computed inside the index AM. So we couldn't suppress the useless evaluation of - by dint of requiring the physical scan to return that value as a Var. Reading between the lines of the original submission at CAPpHfdtG5qoHoD+w=Tz3wC3fZ=b8i21=V5xandBFM=DTo-Yg=q...@mail.gmail.com, I gather that it's the KNNGist-style case that worries you, so maybe it's worth applying this type of patch anyway. I'd want to rejigger it to be aware of the cost implications though, at least for grouping_planner's choices. Comments? 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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost sfr...@snowman.net writes: * Josh Berkus (j...@agliodbs.com) wrote: A much simpler solution to the issue Stephen proposes is to have a way to start up the server with all settings from ALTER SYSTEM SET disabled, just like some software allows you to start it up in safe mode. See above for why I'm not thrilled wih this approach, unless it was set up to happen automatically, but you couldn't simply ignore *all* the ALTER SYSTEM SET parameters because then you might not be able to connect in due to some ALTER SYSTEM SET parameter being necessary for remote connectivity or authentication. Yeah, this approach is a nonstarter because there's no reason to assume that a postmaster started with default parameters will start successfully, or will be connectable-to if it does start. Maybe there's another postmaster hogging the default port, for instance. 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] Kudos for Reviewers -- wrapping it up
On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote: On 07/12/2013 10:49 AM, Andrew Dunstan wrote: On 07/12/2013 01:28 PM, Alvaro Herrera wrote: Josh Berkus wrote: -- a couple of compromise proposals were made: a) that reviewers who do actual code modification of the patch get credited on the feature, and those who just review it get credited at the bottom of the release notes, or I'd probably say substantial or non-trivial, but otherwise +1 Right cause if a reviewer ends up writing (or cleaning up) all the docs, I would say they deserve very close to equal credit. As an example. I can do whatever we agree to in the release notes. The big question is whether committers can properly document these people. I do think the names are going to overwhelm the release note items and we will _again_ remove some or all names. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Kudos for Reviewers -- wrapping it up
Bruce Momjian wrote: On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote: Right cause if a reviewer ends up writing (or cleaning up) all the docs, I would say they deserve very close to equal credit. As an example. I can do whatever we agree to in the release notes. The big question is whether committers can properly document these people. I don't see why not. Most of them, if not all, already do. I do think the names are going to overwhelm the release note items and we will _again_ remove some or all names. There's plenty of opinion to the contrary; but then it's just opinion. I think the idea of trying it at least once has merit. -- Á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] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Josh Berkus (j...@agliodbs.com) wrote: I really think this is the wrong approach. If we start removing unsafe parameters from ALTER SYSTEM SET, we basically hobble the feature to the point of uselessness. Out of the 15 or so parameters 80% of our users touch, half of them are on your unsafe list. Reflecting on this a bit more, I'm curious what your list-of-15 is. Many of the items on my list were file locations or other things which generally require coordination with other groups (like the unix admins or network admins) to change, eg: listen_addresses, port, SSL or Kerberos file locations, etc. There's quite a few parameters which I've changed that are safe and internal-to-PG which weren't on my list- work_mem, maint_work_mem, vacuum / autovacuum settings, effective_io_concurrency, wal_level, sync_commit, checkpoint settings, max_wal_senders, planner costs, logging settings (though this might have to be coordinated w/ the unix admins unless splunk or similar is being used), etc. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Kudos for Reviewers -- wrapping it up
On Fri, Aug 2, 2013 at 04:43:30PM -0400, Alvaro Herrera wrote: Bruce Momjian wrote: On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote: Right cause if a reviewer ends up writing (or cleaning up) all the docs, I would say they deserve very close to equal credit. As an example. I can do whatever we agree to in the release notes. The big question is whether committers can properly document these people. I don't see why not. Most of them, if not all, already do. Do they record which reviewers changed code and which just gave feedback? I do think the names are going to overwhelm the release note items and we will _again_ remove some or all names. There's plenty of opinion to the contrary; but then it's just opinion. I think the idea of trying it at least once has merit. This is what the 9.2 release notes looked like before I remove the reviewers: http://momjian.us/expire/release-9-2.html Most items had 2-3 names, and it was widely rejected. Of course, these were all reviewers, not just those that changed the code. I did not have details of which reviewers changed code and which just gave feedback. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Kudos for Reviewers -- wrapping it up
On 08/02/2013 01:56 PM, Bruce Momjian wrote: On Fri, Aug 2, 2013 at 04:43:30PM -0400, Alvaro Herrera wrote: Bruce Momjian wrote: On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote: Right cause if a reviewer ends up writing (or cleaning up) all the docs, I would say they deserve very close to equal credit. As an example. I can do whatever we agree to in the release notes. The big question is whether committers can properly document these people. I don't see why not. Most of them, if not all, already do. It is also my thinking that it is the job of the CommitFestManager to re-enforce this list by looking through the review list. If we do this on a per-CF basis, the workload won't become substantial; it's only if we wait until beta that it gets overwhelming. The CFM needs to supply the list of reviewers at the end anyway. Most items had 2-3 names, and it was widely rejected. Of course, these were all reviewers, not just those that changed the code. I did not have details of which reviewers changed code and which just gave feedback. I think widely rejected is an exaggeration; a few people objected stenuously. And the primary objection voiced was that people who did it compiles! shouldn't get equal credit with the original author of the patch. Which we're not proposing to do. BTW, all of this I'm talking about the 9.4 release notes, where we have the opportunity to start from the first CF. There's the question of what to do about the *9.3* release notes, which I'll address in a seperate email. -- 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] 9.3 Reviewer Credit WAS: Kudos for Reviewers -- wrapping it up
Bruce, all: Per previous email, I wanted to make a specific proposal for what to do on the 9.3 release notes. This is because, without policy set, we have not been tracking which reviewers make substantial changes in 9.3, and listing some-but-not-all of them would cause a lot of unhappiness among the omitted reviewers. Therefore, I propose for 9.3 only that we just have the list at the end of the release notes, including all reviewers. That way we can make sure to include everyone equally. -- 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] 9.3 Reviewer Credit WAS: Kudos for Reviewers -- wrapping it up
On Fri, Aug 2, 2013 at 02:10:17PM -0700, Josh Berkus wrote: Bruce, all: Per previous email, I wanted to make a specific proposal for what to do on the 9.3 release notes. This is because, without policy set, we have not been tracking which reviewers make substantial changes in 9.3, and listing some-but-not-all of them would cause a lot of unhappiness among the omitted reviewers. Therefore, I propose for 9.3 only that we just have the list at the end of the release notes, including all reviewers. That way we can make sure to include everyone equally. Yes, there is no way to add people to the 9.3 release note items at this point --- my assumption is this is all 9.4 discussion, or maybe 9.5 as we have already committed quite a bit to 9.4. I don't think dumping reviewer names at the bottom of the 9.3 release notes is what the majority want, and it seems like an ugly short-term solution. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Kudos for Reviewers -- wrapping it up
On Fri, Aug 2, 2013 at 02:07:53PM -0700, Josh Berkus wrote: On 08/02/2013 01:56 PM, Bruce Momjian wrote: On Fri, Aug 2, 2013 at 04:43:30PM -0400, Alvaro Herrera wrote: Bruce Momjian wrote: On Fri, Jul 12, 2013 at 12:18:15PM -0700, Joshua D. Drake wrote: Right cause if a reviewer ends up writing (or cleaning up) all the docs, I would say they deserve very close to equal credit. As an example. I can do whatever we agree to in the release notes. The big question is whether committers can properly document these people. I don't see why not. Most of them, if not all, already do. It is also my thinking that it is the job of the CommitFestManager to re-enforce this list by looking through the review list. If we do this on a per-CF basis, the workload won't become substantial; it's only if we wait until beta that it gets overwhelming. Based on existing workflow, we need those reviewer names in the commit message. I don't see how the CommitFestManager can help with that. The CFM needs to supply the list of reviewers at the end anyway. Why? Most items had 2-3 names, and it was widely rejected. Of course, these were all reviewers, not just those that changed the code. I did not have details of which reviewers changed code and which just gave feedback. I think widely rejected is an exaggeration; a few people objected stenuously. And the primary objection voiced was that people who did it compiles! shouldn't get equal credit with the original author of the patch. Which we're not proposing to do. Well, I had to remove it pretty quickly, so that is my recolletion. BTW, all of this I'm talking about the 9.4 release notes, where we have the opportunity to start from the first CF. There's the question of what to do about the *9.3* release notes, which I'll address in a seperate email. I am worried we are talking about 9.5 as we have already committed quite a bit to 9.4. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] HeapTupleSatisfiesDirty fails to test HEAP_XMAX_IS_LOCKED_ONLY for TransactionIdIsInProgress(...)
Craig Ringer wrote: A SELECT ... FOR SHARE will incorrectly block on another open transaction that ran SELECT ... FOR SHARE and still holds the locks if it has to follow a ctid chain from the current snapshot through a committed update to a share-locked tuple. This also affects uniqueness checks in btrees, where it can cause unnecessary waiting. It's also an issue with FOR KEY UPDATE, in that it can cause an update to block when it doesn't have to. Interesting bug. Thanks for the patch. I have applied it all the way back to 8.4 (with adjustments for 9.2 and beyond). The attached test case runs under isolationtester to exersise the problem. I've tested it against 9.2, 9.3, and HEAD, but Andres looked over the code and says the underlying bug goes back to the commit of MVCC, it's just become easier to trigger. To reliably test this with isolationtester I had to horribly abuse pg_advisory_lock(...) so that I could force the first SELECT ... FOR UPDATE to acquire its snapshot before the UPDATE runs. I didn't apply the test case. I think if we want to test tqual.c behavior we will need to introduce a large battery of tests. I would like to see more opinions on whether that's something we want. A backtrace of the point where it's incorrectly blocked is attached. What's happening is that the test for TransactionIdIsInProgress unconditionally sets snapshot-xmax, even if xmax was only set for locking purposes. This will cause the caller to wait for the xid in xmax when it doesn't need to. Yeah, after actually going over the code I think the bug is clear. (I was initially unsure about SatisfiesDirty returning true not false for this case; but the return value was correct, only snapshot-xmax was being set incorrectly. If you examine the callers they would misbehave if the return value were changed; for example EvalPlanQualFetch would completely skip the tuple if SatisfiesDirty returned false, which is not what we want; we want the tuple to be processed.) I think the comments on what exactly SatisfiesDirty does about in-progress transactions ought to be more specific. -- Á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] Kudos for Reviewers -- wrapping it up
On 08/02/2013 02:24 PM, Bruce Momjian wrote: Based on existing workflow, we need those reviewer names in the commit message. I don't see how the CommitFestManager can help with that. We can change the workflow. It's ours, there's no government agency mandating it. Anyway, the list from the CFM would just be to make sure nobody got missed; it's a double-check on the commit messages. The CFM needs to supply the list of reviewers at the end anyway. Why? Who else would do it? BTW, all of this I'm talking about the 9.4 release notes, where we have the opportunity to start from the first CF. There's the question of what to do about the *9.3* release notes, which I'll address in a seperate email. I am worried we are talking about 9.5 as we have already committed quite a bit to 9.4. You're making a big deal out of what's a minor clerical detail. Don't let minutia which any secretary could take care of get in the way of an important project goal, that is, rewarding reviewers so that lack of reviewers stops being a major project bottleneck. -- 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] 9.3 Reviewer Credit WAS: Kudos for Reviewers -- wrapping it up
On 08/02/2013 02:23 PM, Bruce Momjian wrote: I don't think dumping reviewer names at the bottom of the 9.3 release notes is what the majority want, and it seems like an ugly short-term solution. It's better than not crediting the reviewers *at all*, which is the only alternative I can think of. -- 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] how to pass data (tuples) to worker processes?
Tomas Vondra wrote: I'm learning how to use the background worker processes commited in 9.3. The usage basics are quite nicely illustrated in the worker_spi extension (kudos to those who designed the feature / extension). Thanks! I'm not quite sure how to pass data between the regular backend and a worker. Implementing the channel (socket/pipe/...) itself is not a big deal, that's IPC 101, but deciding which data to copy (and how) is. Say I need to forward a tuple to the worker process - e.g. from a nodeAgg node, so that the worker can build the hash table. Is there something (a rule of a thumb, method, ...) that would help me to identify the pieces of data that need to be copied? Or do I need to do the go through the objects and decide what to copy and how on my own? Were you able to figure it out? If so, would you share? -- Á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 for removng unused targets
Reading between the lines of the original submission at CAPpHfdtG5qoHoD+w=Tz3wC3fZ=b8i21=V5xandBFM=DTo-Yg=q...@mail.gmail.com, I gather that it's the KNNGist-style case that worries you, so maybe it's worth applying this type of patch anyway. I'd want to rejigger it to be aware of the cost implications though, at least for grouping_planner's choices. Hmm. Can we optimize for the KNN case, without causing the issues which you warned about earlier in your post? I'm really wary of any optimization which operates completely outside of the cost model; the ones we have (abort-early plans, for example) are already among our primary sources of bad plan issues. Comments? So, Returned With Feedback, or move it to September? -- 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] [BUGS] 9.3beta2: Failure to pg_upgrade
Alvaro Herrera escribió:
As it turns out, I have a patched slru.c that adds a new function to
verify whether a page exists on disk. I created this for the commit
timestamp module, for the BDR branch, but I think it's what we need
here.
Here's a patch that should fix the problem. Jesse, if you're able to
test it, please give it a run and let me know if it works for you. I
was able to upgrade an installation containing a problem that should
reproduce yours.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
*** a/src/backend/access/transam/multixact.c
--- b/src/backend/access/transam/multixact.c
***
*** 1719,1724 ZeroMultiXactMemberPage(int pageno, bool writeXlog)
--- 1719,1756
}
/*
+ * After a binary upgrade from = 9.2, the pg_multixact/offset SLRU area might
+ * contain files that are shorter than necessary; this would occur if the old
+ * installation had used multixacts beyond the first page (files cannot be
+ * copied, because the on-disk representation is different). pg_upgrade would
+ * update pg_control to set the next offset value to be at that position, so
+ * that tuples marked as locked by such MultiXacts would be seen as visible
+ * without having to consult multixact. However, trying to create a use a new
+ * MultiXactId would result in an error because the page on which the new value
+ * would reside does not exist. This routine is in charge of creating such
+ * pages.
+ */
+ static void
+ MaybeExtendOffsetSlru(void)
+ {
+ int pageno;
+
+ pageno = MultiXactIdToOffsetPage(MultiXactState-nextMXact);
+
+ LWLockAcquire(MultiXactOffsetControlLock, LW_EXCLUSIVE);
+
+ if (!SimpleLruDoesPhysicalPageExist(MultiXactOffsetCtl, pageno))
+ {
+ int slotno;
+
+ slotno = ZeroMultiXactOffsetPage(pageno, false);
+ SimpleLruWritePage(MultiXactOffsetCtl, slotno);
+ }
+
+ LWLockRelease(MultiXactOffsetControlLock);
+ }
+
+ /*
* This must be called ONCE during postmaster or standalone-backend startup.
*
* StartupXLOG has already established nextMXact/nextOffset by calling
***
*** 1738,1743 StartupMultiXact(void)
--- 1770,1782
int entryno;
int flagsoff;
+ /*
+ * During a binary upgrade, make sure that the offsets SLRU is large
+ * enough to contain the next value that would be created.
+ */
+ if (IsBinaryUpgrade)
+ MaybeExtendOffsetSlru();
+
/* Clean up offsets state */
LWLockAcquire(MultiXactOffsetControlLock, LW_EXCLUSIVE);
*** a/src/backend/access/transam/slru.c
--- b/src/backend/access/transam/slru.c
***
*** 563,568 SimpleLruWritePage(SlruCtl ctl, int slotno)
--- 563,612
SlruInternalWritePage(ctl, slotno, NULL);
}
+ /*
+ * Return whether the given page exists on disk.
+ *
+ * A false return means that either the file does not exist, or that it's not
+ * large enough to contain the given page.
+ */
+ bool
+ SimpleLruDoesPhysicalPageExist(SlruCtl ctl, int pageno)
+ {
+ int segno = pageno / SLRU_PAGES_PER_SEGMENT;
+ int rpageno = pageno % SLRU_PAGES_PER_SEGMENT;
+ int offset = rpageno * BLCKSZ;
+ char path[MAXPGPATH];
+ int fd;
+ bool result;
+ off_t endpos;
+
+ SlruFileName(ctl, path, segno);
+
+ fd = OpenTransientFile(path, O_RDWR | PG_BINARY, S_IRUSR | S_IWUSR);
+ if (fd 0)
+ {
+ /* expected: file doesn't exist */
+ if (errno == ENOENT)
+ return false;
+
+ /* report error normally */
+ slru_errcause = SLRU_OPEN_FAILED;
+ slru_errno = errno;
+ SlruReportIOError(ctl, pageno, 0);
+ }
+
+ if ((endpos = lseek(fd, 0, SEEK_END)) 0)
+ {
+ slru_errcause = SLRU_OPEN_FAILED;
+ slru_errno = errno;
+ SlruReportIOError(ctl, pageno, 0);
+ }
+
+ result = endpos = (off_t) (offset + BLCKSZ);
+
+ CloseTransientFile(fd);
+ return result;
+ }
/*
* Physical read of a (previously existing) page into a buffer slot
*** a/src/include/access/slru.h
--- b/src/include/access/slru.h
***
*** 145,150 extern int SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno,
--- 145,151
extern void SimpleLruWritePage(SlruCtl ctl, int slotno);
extern void SimpleLruFlush(SlruCtl ctl, bool checkpoint);
extern void SimpleLruTruncate(SlruCtl ctl, int cutoffPage);
+ extern bool SimpleLruDoesPhysicalPageExist(SlruCtl ctl, int pageno);
typedef bool (*SlruScanCallback) (SlruCtl ctl, char *filename, int segpage,
void *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] Kudos for Reviewers -- wrapping it up
On Fri, Aug 2, 2013 at 02:36:42PM -0700, Josh Berkus wrote: On 08/02/2013 02:24 PM, Bruce Momjian wrote: Based on existing workflow, we need those reviewer names in the commit message. I don't see how the CommitFestManager can help with that. We can change the workflow. It's ours, there's no government agency mandating it. Anyway, the list from the CFM would just be to make sure nobody got missed; it's a double-check on the commit messages. The CFM needs to supply the list of reviewers at the end anyway. Why? Who else would do it? BTW, all of this I'm talking about the 9.4 release notes, where we have the opportunity to start from the first CF. There's the question of what to do about the *9.3* release notes, which I'll address in a seperate email. I am worried we are talking about 9.5 as we have already committed quite a bit to 9.4. You're making a big deal out of what's a minor clerical detail. Don't let minutia which any secretary could take care of get in the way of an important project goal, that is, rewarding reviewers so that lack of reviewers stops being a major project bottleneck. You are approaching this like it is a done deal and everyone agrees to it. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for removng unused targets
Josh Berkus j...@agliodbs.com writes: Reading between the lines of the original submission at CAPpHfdtG5qoHoD+w=Tz3wC3fZ=b8i21=V5xandBFM=DTo-Yg=q...@mail.gmail.com, I gather that it's the KNNGist-style case that worries you, so maybe it's worth applying this type of patch anyway. I'd want to rejigger it to be aware of the cost implications though, at least for grouping_planner's choices. Hmm. Can we optimize for the KNN case, without causing the issues which you warned about earlier in your post? Those are pre-existing issues, not something that would be made any worse by this patch. The main thing I think is really wrong with the patch as it stands is that the cost savings from suppressing the ORDER BY expressions should enter into the cheapest_path-vs-sorted_path decision, which it doesn't, in fact the total cost the plan is labeled with isn't corrected either. (Not that that matters for the current level of plan, but it could matter at an outer level if this is a subquery.) I think that is fixable but am just wondering whether to bother. So, Returned With Feedback, or move it to September? The patch is certainly not getting committed as-is (at least not by me), so it would likely be fair to mark it RWF so we can close the commitfest. I'll still work on a revised version after the fest if people think that improving the KNN-search case is worth a patch that's a bit larger than this one currently is. 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] Kudos for Reviewers -- wrapping it up
On 08/02/2013 03:18 PM, Bruce Momjian wrote: You're making a big deal out of what's a minor clerical detail. Don't let minutia which any secretary could take care of get in the way of an important project goal, that is, rewarding reviewers so that lack of reviewers stops being a major project bottleneck. You are approaching this like it is a done deal and everyone agrees to it. We already discussed it in the thread ad nauseum, and arrived at a compromise which everyone could live with. So from that perspective, it *is* a done deal, at least as far as 9.4 is concerned. At some point, we need to make a decision and move forward, instead of rehashing the same arguments forever. So if you're raising an objection to the compromise which many people already agreed to, then raise an objection and back it up. But don't sandbag. -- 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] Patch for removng unused targets
On 08/02/2013 03:45 PM, Tom Lane wrote: So, Returned With Feedback, or move it to September? The patch is certainly not getting committed as-is (at least not by me), so it would likely be fair to mark it RWF so we can close the commitfest. I'll still work on a revised version after the fest if people think that improving the KNN-search case is worth a patch that's a bit larger than this one currently is. Ok, marking it returned with feedback. Thanks! -- 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
[HACKERS] [9.4 CF 1]Commitfest ... over!
Folks, The first CF for the 9.4 development cycle is officially over. In all, 49 patches were committed, 47 were returned with feedback, 6 were rejected outright, and 6 were punted to CF2. We're 17 days over the CF deadline at this point, but that's unsurprising considering that this CF included a record number of patches -- 108 patches at peak, compared with 59 for last year's CF1, and 101 for even CF4. So this was, measured strictly by patch count, the biggest CF ever (of course, that's not the only measure). See my blog at http://www.databasesoup.com/2013/08/94-commitfest-1-wrap-up.html for some additional details. Given that we can expect to be dealing with more patches per CF in the future, I'd like some feedback about what things would make the CF process more efficient. For that matter, for the first time we tried enforcing some of the rules of CFs this time, and I'd like to hear if people think that helped. -- 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] [9.4 CF 1]Commitfest ... over!
On 08/02/2013 04:47 PM, Josh Berkus wrote: Folks, The first CF for the 9.4 development cycle is officially over. Also, I wanted to say thank you to: - Mike Blackwell, assistant CFM - all 30+ reviewers and committers (list to come) -- 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] Kudos for Reviewers -- wrapping it up
On Fri, Aug 2, 2013 at 03:55:27PM -0700, Josh Berkus wrote: On 08/02/2013 03:18 PM, Bruce Momjian wrote: You're making a big deal out of what's a minor clerical detail. Don't let minutia which any secretary could take care of get in the way of an important project goal, that is, rewarding reviewers so that lack of reviewers stops being a major project bottleneck. You are approaching this like it is a done deal and everyone agrees to it. We already discussed it in the thread ad nauseum, and arrived at a compromise which everyone could live with. So from that perspective, it *is* a done deal, at least as far as 9.4 is concerned. At some point, we need to make a decision and move forward, instead of rehashing the same arguments forever. So if you're raising an objection to the compromise which many people already agreed to, then raise an objection and back it up. But don't sandbag. There are three issues here: 1. What will best motive reviewers? 2. What is a reasonable effort to accomplish #1? 3. What is acceptable for release note readers? You seem to be only focused on #1, and you don't want to address the other items --- that's fine --- I will still be around if people lose interest or the system becomes unworkable. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Extension Templates S03E11
On 1 August 2013 18:01, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Hi, Please find attached to this email the latest and greatest version of in-line SQL only extensions support, known as Extension Templates and which could be renamed In-Catalog Extension Templates. I've included a high-level description of the patch in a style that targets the detailed commit messages for features of that source code impact level. The attached patch is known to address all points raised in the previous reviews and to implement the best design we could come up with, thanks to immense helping from Tom, Heikki and Markus. Of course, bugs are all my precious. I'm going to register that patch to the next commitfest. It's not the only patch I intend to register for september though, as I want to get to a usable situation with Event Triggers, so you can expect a series of patches for that, covering what couldn't make it previously. As I think this WIP is about as ready-for-committer as it will ever be, it would be fantastic if we could do a single committer review before CF2013-09 so that I know that it's going to be accepted… or not. Well at least it's in the queue already, we'll see what can be done. Regards, --- Implement in-catalog Extension Template facility. Previously, the only way to CREATE EXTENSION involved installing file system templates in a place generally owned by root: creation scripts, upgrade scripts, main control file and auxilliary control files. This patch implements a way to upload all those resources into the catalogs, so that a PostgreSQL connection is all you need to make an extension available. By design and for security concerns the current Extension Template facility is not able to deal with extensions that need to load a DSO module into the backend. Using any other PL is supported though. An extension created from a template depends on it, and the templates are part of any backup script taken with pg_dump. So that at pg_restore time, when CREATE EXTENSION is executed the templates are already in place. To be able to do that, though, we need a difference in behavior in between the classic file system level templates and the catalog templates: there's no dependency tracking happening at all with file system templates and those can be changed at will even if an extension has been already instanciated from the templates, or even removed. Apart from the dependency tracking, the only other difference between file system templates and catalog templates for extensions is that the later are managed per-database. The file system level templates being managed per major version of PostgreSQL is considered a drawback of that method and not to be immitated by the in-catalog system, more flexible by design. At CREATE EXTENSION time, the file system templates are always prefered to the catalog templates. Also, it's prohibited to make available an extension in the catalogs if an extension of the same name is already available from file system templates. That said, some race conditions make it still possible to have the same extension name available as a file system template and a catalog template. Even if only the former will ever get installed, it's been deemed prudent to restrict the in-catalog templates for extensions to superusers only. Could you please resubmit this without using SnapshotNow as it's no longer supported? Thanks Thom
Re: [HACKERS] [BUGS] 9.3beta2: Failure to pg_upgrade
On 2013-08-02 18:17:43 -0400, Alvaro Herrera wrote: Alvaro Herrera escribió: As it turns out, I have a patched slru.c that adds a new function to verify whether a page exists on disk. I created this for the commit timestamp module, for the BDR branch, but I think it's what we need here. Here's a patch that should fix the problem. Jesse, if you're able to test it, please give it a run and let me know if it works for you. I was able to upgrade an installation containing a problem that should reproduce yours. Wouldn't it be easier to make pg_upgrade fudge pg_control to have a safe NextMultiXactId/Offset using pg_resetxlog? 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] [BUGS] 9.3beta2: Failure to pg_upgrade
Andres Freund escribió: On 2013-08-02 18:17:43 -0400, Alvaro Herrera wrote: Alvaro Herrera escribió: As it turns out, I have a patched slru.c that adds a new function to verify whether a page exists on disk. I created this for the commit timestamp module, for the BDR branch, but I think it's what we need here. Here's a patch that should fix the problem. Jesse, if you're able to test it, please give it a run and let me know if it works for you. I was able to upgrade an installation containing a problem that should reproduce yours. Wouldn't it be easier to make pg_upgrade fudge pg_control to have a safe NextMultiXactId/Offset using pg_resetxlog? I don't understand. pg_upgrade already fudges pg_control to have a safe next multi, namely the same value used by the old cluster. The reason to preserve this value is that we must ensure no older value is consulted in pg_multixact: those might be present in tuples that were locked in the old cluster. (To be precise, this is the value to set as oldest multi, not next multi. But of course, the next multi must be greater than that one.) -- Á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] [9.4 CF 1]Commitfest ... over!
(2013/08/03 8:47), Josh Berkus wrote: Given that we can expect to be dealing with more patches per CF in the future, I'd like some feedback about what things would make the CF process more efficient. For that matter, for the first time we tried enforcing some of the rules of CFs this time, and I'd like to hear if people think that helped. The 5-day rule (and the notifications from CFM) seemed to be working for me. It helped me focus on the patch review. Regards, -- Satoshi Nagayasu sn...@uptime.jp Uptime Technologies, LLC. http://www.uptime.jp -- 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] 9.3beta2: Failure to pg_upgrade
On Fri, Aug 2, 2013 at 11:20:37PM -0400, Jesse Denardo wrote: Alvaro, I applied the patch and tried upgrading again, and everything seemed to work as expected. We are now up and running the beta! Yeah, great, thanks everyone! -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] 9.3beta2: Failure to pg_upgrade
On 2013-08-02 22:25:36 -0400, Alvaro Herrera wrote: Andres Freund escribió: On 2013-08-02 18:17:43 -0400, Alvaro Herrera wrote: Alvaro Herrera escribió: As it turns out, I have a patched slru.c that adds a new function to verify whether a page exists on disk. I created this for the commit timestamp module, for the BDR branch, but I think it's what we need here. Here's a patch that should fix the problem. Jesse, if you're able to test it, please give it a run and let me know if it works for you. I was able to upgrade an installation containing a problem that should reproduce yours. Wouldn't it be easier to make pg_upgrade fudge pg_control to have a safe NextMultiXactId/Offset using pg_resetxlog? I don't understand. pg_upgrade already fudges pg_control to have a safe next multi, namely the same value used by the old cluster. The reason to preserve this value is that we must ensure no older value is consulted in pg_multixact: those might be present in tuples that were locked in the old cluster. (To be precise, this is the value to set as oldest multi, not next multi. But of course, the next multi must be greater than that one.) I am suggesting to set them to a greater value than in the old cluster, computed so it's guaranteed that they are proper page boundaries. Then the situation described upthread shouldn't occur anymore, right? 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] [BUGS] 9.3beta2: Failure to pg_upgrade
Alvaro, I applied the patch and tried upgrading again, and everything seemed to work as expected. We are now up and running the beta! -- Jesse Denardo On Fri, Aug 2, 2013 at 10:25 PM, Alvaro Herrera alvhe...@2ndquadrant.comwrote: Andres Freund escribió: On 2013-08-02 18:17:43 -0400, Alvaro Herrera wrote: Alvaro Herrera escribió: As it turns out, I have a patched slru.c that adds a new function to verify whether a page exists on disk. I created this for the commit timestamp module, for the BDR branch, but I think it's what we need here. Here's a patch that should fix the problem. Jesse, if you're able to test it, please give it a run and let me know if it works for you. I was able to upgrade an installation containing a problem that should reproduce yours. Wouldn't it be easier to make pg_upgrade fudge pg_control to have a safe NextMultiXactId/Offset using pg_resetxlog? I don't understand. pg_upgrade already fudges pg_control to have a safe next multi, namely the same value used by the old cluster. The reason to preserve this value is that we must ensure no older value is consulted in pg_multixact: those might be present in tuples that were locked in the old cluster. (To be precise, this is the value to set as oldest multi, not next multi. But of course, the next multi must be greater than that one.) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
