Re: [HACKERS] W3C Specs: Web SQL
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Tue, Nov 09, 2010 at 12:14:06PM -0800, Charles Pritchard wrote: [...] as it reflects the current state of security. Which is... well, I haven't a word for *that*. Regards - -- tomás -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFM2lkYBcgs9XrR2kYRAnt7AJ4jI8qIz6BLlKnMXnj7h1AeWfXBcACfTOWI 8aMdXz0Y2CSGeFJA6WBxPnA= =MQ5R -END PGP SIGNATURE- -- 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] TODO Alter Table Rename Constraint
Thanks for your answer! I'm not really familiar with inheritance, but I wonder how this issue is handled in other cases, for instance renaming an index, which invokes internal a constraint rename too. Is that relevant or is the renaming of constraints so special? Is there a solution for the test-cases you have posted? Or is this yet a problem? Viktor 2010/11/9 Robert Haas robertmh...@gmail.com On Tue, Nov 9, 2010 at 10:50 AM, Viktor Valy vili0...@gmail.com wrote: Hi Everybody! I looked up this todo, and figured out a plan, how the implementation could be written. The main challenge is to keep constraints and indexes (for unique and PK constraints) consistent. Fortunately this is already realized in the case of an index. So at ALTER INDEX RENAME the consistency is given by an extra check in tablecmds.c (Line 2246), where a function is finally called, RenameConstraintById (pg_constraint.c Line 604). My idea is, to do that analog for ALTER CONSTRAINT RENAME too. So renaming the constraint is going to be done in tablecmds.c as for indexes, tables, sequences, views. And after checking whether the renametype is constraint, an extra rename has to be done for the index. Getting the index can be done with the function get_constraint_index (pg_depend.c Line 475). Now it should be possible to do the same as in RenameConstraintById. Is that so legal? Is anything else to be considered? I think the biggest problem is handling inherited tables properly, especially in complex inheritance hierarchies where there are multiple, separate paths from the top of the hierarchy to the bottom. See here for a couple of relevant test cases: http://archives.postgresql.org/pgsql-hackers/2010-07/msg01570.php I believe that the rename needs to fail if any table in the inheritance hierarchy rooted at the target table also inherits the constraint from someplace outside that hierarchy; or if any table in that hierarchy has a local copy of the constraint that got merged with the inherited one. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [HACKERS] Fix for seg picksplit function
Hmm, the second for loop in gseg_picksplit uses i maxoff whereas the other one uses =. The first is probably correct; if the second is also correct it merits a comment on the discrepancy (To be honest, I'd get rid of the -1 in computing maxoff and use in both places, given that offsets are 1-indexed). Also, the second one is using i++ to increment; probably should be OffsetNumberNext just to stay consistent with the rest of the code. The assignment to *left and *right at the end of the routine seem pretty useless (not to mention the comment talking about a routine that doesn't exist anywhere). -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 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] multi-platform, multi-locale regression tests
Peter Eisentraut wrote: On tis, 2010-11-09 at 14:00 -0800, David E. Wheeler wrote: I've been talking to Nasby and Dunstan about adding a Test::More/pgTAP-based test suite to the core. It wouldn't run with the usual core suite used by developers, which would continue to use pg_regress. But they could run it if they wanted (and had the prerequisites), and the build farm animals would run them regularly. I'd welcome something like that, but I'm not sure that that's the best overall solution to this particular problem in the short run. But it would be great to have anyway. For the Serializable Snapshot Isolation (SSI) patch I needed a test suite which would handle concurrent sessions which interleaved statements in predictable ways. I was told pgTAP wasn't a good choice for that and went with Markus Wanner's dtester package. The SSI patch adds a dcheck build target which is not included in any others to run the dtester tests. I don't know if dtester meets the other needs people have, or whether this is a complementary approach, but it seemed worth mentioning. -Kevin -- 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] security hooks on object creation
(2010/11/10 13:06), Robert Haas wrote: In this patch, we put InvokeObjectAccessHook0 on the following functions. - heap_create_with_catalog() for relations/attributes - ATExecAddColumn() for attributes - NamespaceCreate() for schemas - ProcedureCreate() for aggregates/functions - TypeCreate() and TypeShellMake() for types - create_proc_lang() for procedural languages - inv_create() for large objects I think you ought to try to arrange to avoid the overhead of a function call in the common case where nobody's using the hook. That's why I originally suggested making InvokeObjectAccessHook() a macro around the actual function call. Hmm. Although I have little preference here, the penalty to call an empty function (when no plugins are installed) is not visible, because frequency of DDL commands are not high. Even so, is it necessary to replace them by macros? I don't want to refer to this as a framework for enhanced security providers. Let's stick with the term object access hook. Calling it an enhanced security provider overspecifies; it could equally well be used for, say, logging. OK. As Itagaki-san also pointed out, we may be able to utilize the hooks in other purposes. Although I designed it in similar manner with security label provider, I'll revise it like as other hooks doing. Is there any compelling reason not to apply this to every object type in the system (e.g. all the ones COMMENT can apply to)? I don't see any reason to restrict it to the set of objects to which it's sensible to apply security labels. Because I thought too many hooks within one patch gives burden to reviewers, so I restricted it on a part of object classes in this version. However,it is not a compelling reason. OK, I'll try to revise the patch soon. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for seg picksplit function
Hmm, the second for loop in gseg_picksplit uses i maxoff whereas the other one uses =. The first is probably correct; if the second is also correct it merits a comment on the discrepancy (To be honest, I'd get rid of the -1 in computing maxoff and use in both places, given that offsets are 1-indexed). Also, the second one is using i++ to increment; probably should be OffsetNumberNext just to stay consistent with the rest of the code. Actually I can't understand the purpose of FirstOffsetNumber and OffsetNumberNext macros. When I wrote the patch I though about sortItems as about clean from all these strange things array, that's why I didn't use OffsetNumberNext there. :) I see only way to save logic of these macros is to use array starting from FirstOffsetNumber index like in gbt_num_picksplit. The assignment to *left and *right at the end of the routine seem pretty useless (not to mention the comment talking about a routine that doesn't exist anywhere). I found, that gtrgm_picksplit in pg_trgm and gtsvector_picksplit in core still use this assignment, while gist_box_picksplit and gbt_num_picksplit not. If this assignment is overall useless, than I think we should remove it from gtrgm_picksplit and gtsvector_picksplit in order to not mislead developers of gist implementations. With best regards, Alexander Korotkov.
[HACKERS] ECPG question about PREPARE and EXECUTE
Hi, a question came to us in the form of a code example, which I shortened. Say, we have this structure: EXEC SQL BEGIN DECLARE SECTION; struct t1 { int id; chart[80]; }; typedef struct t1 t1_t; t1_tt1; EXEC SQL END DECLARE SECTION; and a similar table in the database. The client wanted to use a PREPARE / EXECUTE pair this way: EXEC SQL PREPARE myquery AS SELECT * FROM t1 WHERE id = :t1.id; t1.id = 1; EXEC SQL EXECUTE myquery INTO :t1; Upon executing the EXECUTE query, we get an error: SQL error: too few arguments on line NNN The problem is that the input parameters given to the PREPARE are not preserved and carried to the EXECUTE. Any comment on why it isn't done? Best regards, Zoltán Böszörményi -- -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/ -- 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] Fix for seg picksplit function
On Wed, Nov 10, 2010 at 4:53 PM, Alexander Korotkov aekorot...@gmail.comwrote: Hmm, the second for loop in gseg_picksplit uses i maxoff whereas the other one uses =. The first is probably correct; if the second is also correct it merits a comment on the discrepancy (To be honest, I'd get rid of the -1 in computing maxoff and use in both places, given that offsets are 1-indexed). Also, the second one is using i++ to increment; probably should be OffsetNumberNext just to stay consistent with the rest of the code. Actually I can't understand the purpose of FirstOffsetNumber and OffsetNumberNext macros. When I wrote the patch I though about sortItems as about clean from all these strange things array, that's why I didn't use OffsetNumberNext there. :) I see only way to save logic of these macros is to use array starting from FirstOffsetNumber index like in gbt_num_picksplit. For example, if we assume, that OffsetNumberNext can do something other that just increment, that we shouldn't use it in loop on sortItems, but should do following things: 1) Do additional loop first to calculate actual items count. (Because we don't know how OffsetNumberNext increases the index. Probably it can increase it by various value.) 2) Fill sortItems using separate index. With best regards, Alexander Korotkov.
Re: [HACKERS] Fix for seg picksplit function
On 2010-11-10 14:27, Alvaro Herrera wrote: Hmm, the second for loop in gseg_picksplit uses i maxoff whereas the other one uses=. The first is probably correct; if the second is also correct it merits a comment on the discrepancy (To be honest, I'd get rid of the -1 in computing maxoff and use in both places, given that offsets are 1-indexed). Good point. The second loop walks over the sorted array, which is 0-indexed. Do you favor making the sort array 1-indexed, like done in e.g. gbt_num_picksplit? The downside is that the sort array is initialized with length maxoff + 1: puzzling on its own, even more since maxoff itself is initialized as entryvec-n -1. Note also that the qsort call for the 1-indexed variant is more complex since it must skip the first element. probably should be OffsetNumberNext just to stay consistent with the rest of the code. Yes. The assignment to *left and *right at the end of the routine seem pretty useless (not to mention the comment talking about a routine that doesn't exist anywhere). They are necessary and it is code untouched by this patch, and the same line occurs in other picksplit functions as well. The gbt_num_picksplit function shows that it can be avoided, by rewriting in the second loop *left++ = sortItems[i].index; into v-spl_left[v-spl_nleft] = sortItems[i].index Even though this is longer code, I prefer this variant over the shorter one. regards, Yeb Havinga -- 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: [COMMITTERS] pgsql: Don't unblock SIGQUIT in the SIGQUIT handler This was possibly
On Thu, Dec 17, 2009 at 8:05 AM, Peter Eisentraut pet...@postgresql.org wrote: Log Message: --- Don't unblock SIGQUIT in the SIGQUIT handler This was possibly linked to a deadlock-like situation in glibc syslog code invoked by the ereport call in quickdie(). In any case, a signal handler should not unblock its own signal unless there is a specific reason to. Modified Files: -- pgsql/src/backend/tcop: postgres.c (r1.577 - r1.578) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/tcop/postgres.c?r1=1.577r2=1.578) pgsql/src/include/libpq: pqsignal.h (r1.35 - r1.36) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/libpq/pqsignal.h?r1=1.35r2=1.36) Why wasn't this patch backported? Recently my customer encountered the bug which this patch fixed, in 8.3. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] Fix for seg picksplit function
On Wed, Nov 10, 2010 at 5:37 PM, Yeb Havinga yebhavi...@gmail.com wrote: They are necessary and it is code untouched by this patch, and the same line occurs in other picksplit functions as well. The gbt_num_picksplit function shows that it can be avoided, by rewriting in the second loop *left++ = sortItems[i].index; into v-spl_left[v-spl_nleft] = sortItems[i].index Even though this is longer code, I prefer this variant over the shorter one. I can't understand this point. How the way of spl_left and spl_right arrays filling is related with additional FirstOffsetNumber value at the end of array, which is added by *left = *right = FirstOffsetNumber; line? With best regards, Alexander Korotkov.
Re: [HACKERS] Fix for seg picksplit function
On 2010-11-10 15:46, Alexander Korotkov wrote: On Wed, Nov 10, 2010 at 5:37 PM, Yeb Havinga yebhavi...@gmail.com mailto:yebhavi...@gmail.com wrote: They are necessary and it is code untouched by this patch, and the same line occurs in other picksplit functions as well. The gbt_num_picksplit function shows that it can be avoided, by rewriting in the second loop *left++ = sortItems[i].index; into v-spl_left[v-spl_nleft] = sortItems[i].index Even though this is longer code, I prefer this variant over the shorter one. I can't understand this point. How the way of spl_left and spl_right arrays filling is related with additional FirstOffsetNumber value at the end of array, which is added by *left = *right = FirstOffsetNumber; line? You're right, they are not related. I'm no longer sure it is necessary, looking at gistUserPicksplit. regards, Yeb Havinga
Re: [HACKERS] Fix for seg picksplit function
On Wed, Nov 10, 2010 at 6:05 PM, Yeb Havinga yebhavi...@gmail.com wrote: On 2010-11-10 15:46, Alexander Korotkov wrote: On Wed, Nov 10, 2010 at 5:37 PM, Yeb Havinga yebhavi...@gmail.com wrote: They are necessary and it is code untouched by this patch, and the same line occurs in other picksplit functions as well. The gbt_num_picksplit function shows that it can be avoided, by rewriting in the second loop *left++ = sortItems[i].index; into v-spl_left[v-spl_nleft] = sortItems[i].index Even though this is longer code, I prefer this variant over the shorter one. I can't understand this point. How the way of spl_left and spl_right arrays filling is related with additional FirstOffsetNumber value at the end of array, which is added by *left = *right = FirstOffsetNumber; line? You're right, they are not related. I'm no longer sure it is necessary, looking at gistUserPicksplit. Teodor, Oleg, probably, you can help us. Is *left = *right = FirstOffsetNumber; line necessary in picksplit function or doing something useful? With best regards, Alexander Korotkov.
Re: [HACKERS] Fix for seg picksplit function
Alexander Korotkov aekorot...@gmail.com writes: On Wed, Nov 10, 2010 at 4:53 PM, Alexander Korotkov aekorot...@gmail.comwrote: Actually I can't understand the purpose of FirstOffsetNumber and OffsetNumberNext macros. For example, if we assume, that OffsetNumberNext can do something other that just increment, that we shouldn't use it in loop on sortItems, Right. Good style is to use FirstOffsetNumber/OffsetNumberNext if you are walking through the items on a page. They should *not* be used when you are just iterating over a local array. I'd go with for (i = 0; i nitems; i++) for the latter. 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] Fix for seg picksplit function
On 2010-11-10 14:53, Alexander Korotkov wrote: Actually I can't understand the purpose of FirstOffsetNumber and OffsetNumberNext macros. When I wrote the patch I though about sortItems as about clean from all these strange things array, that's why I didn't use OffsetNumberNext there. :) I see only way to save logic of these macros is to use array starting from FirstOffsetNumber index like in gbt_num_picksplit. Another reason for not using is FirstOffsetNumber and it's related macro's on the qsort array, is that InvalidOffsetNumber (0) is not invalid for the array. However all other sorts in picksplit functions already seem to do it this way. I'm not sure it's wise to introduce a different approach. The assignment to *left and *right at the end of the routine seem pretty useless (not to mention the comment talking about a routine that doesn't exist anywhere). I found, that gtrgm_picksplit in pg_trgm and gtsvector_picksplit in core still use this assignment, while gist_box_picksplit and gbt_num_picksplit not. If this assignment is overall useless, than I think we should remove it from gtrgm_picksplit and gtsvector_picksplit in order to not mislead developers of gist implementations. +1 regards, Yeb Havinga
Re: [HACKERS] improved parallel make support
On tis, 2010-11-09 at 03:54 -0800, Dave Page wrote: Narwhal should be OK now. The build has issues now, possibly related to the make upgrade. -- 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] multi-platform, multi-locale regression tests
On Nov 10, 2010, at 5:31 AM, Kevin Grittner wrote: For the Serializable Snapshot Isolation (SSI) patch I needed a test suite which would handle concurrent sessions which interleaved statements in predictable ways. I was told pgTAP wasn't a good choice for that and went with Markus Wanner's dtester package. The SSI patch adds a dcheck build target which is not included in any others to run the dtester tests. Right. pgTAP doesn't run tests, it's just a collection of assertion functions written in SQL and PL/pgSQL. It could have been used via a forking Perl script that would connect to the proper boxes, run the tests, collect the results, etc. But it clearly would have been a PITA, and the path of least resistance is often the best solution when hacking. Going with dcheck, which already did what you wanted, was clearly the right choice. Hopefully we can have the build farm animals run the dcheck target once SSI is committed. Best, David -- 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] multi-platform, multi-locale regression tests
On Wed, Nov 10, 2010 at 08:33:13AM -0800, David Wheeler wrote: On Nov 10, 2010, at 5:31 AM, Kevin Grittner wrote: For the Serializable Snapshot Isolation (SSI) patch I needed a test suite which would handle concurrent sessions which interleaved statements in predictable ways. I was told pgTAP wasn't a good choice for that and went with Markus Wanner's dtester package. The SSI patch adds a dcheck build target which is not included in any others to run the dtester tests. Right. pgTAP doesn't run tests, it's just a collection of assertion functions written in SQL and PL/pgSQL. It could have been used via a forking Perl script that would connect to the proper boxes, run the tests, collect the results, etc. But it clearly would have been a PITA, and the path of least resistance is often the best solution when hacking. Going with dcheck, which already did what you wanted, was clearly the right choice. Hopefully we can have the build farm animals run the dcheck target once SSI is committed. Does Perl have some kind of concurrency-controlled test framework? Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] multi-platform, multi-locale regression tests
On 11/10/2010 08:31 AM, Kevin Grittner wrote: I don't know if dtester meets the other needs people have, or whether this is a complementary approach, but it seemed worth mentioning. Where is this available? Is it self-contained? And what does it require? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] multi-platform, multi-locale regression tests
On Nov 10, 2010, at 9:48 AM, Andrew Dunstan wrote: I don't know if dtester meets the other needs people have, or whether this is a complementary approach, but it seemed worth mentioning. Where is this available? Is it self-contained? And what does it require? Python. http://www.bluegap.ch/projects/dtester/ Best, David -- 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] security hooks on object creation
On Wed, Nov 10, 2010 at 8:33 AM, KaiGai Kohei kai...@kaigai.gr.jp wrote: (2010/11/10 13:06), Robert Haas wrote: In this patch, we put InvokeObjectAccessHook0 on the following functions. - heap_create_with_catalog() for relations/attributes - ATExecAddColumn() for attributes - NamespaceCreate() for schemas - ProcedureCreate() for aggregates/functions - TypeCreate() and TypeShellMake() for types - create_proc_lang() for procedural languages - inv_create() for large objects I think you ought to try to arrange to avoid the overhead of a function call in the common case where nobody's using the hook. That's why I originally suggested making InvokeObjectAccessHook() a macro around the actual function call. Hmm. Although I have little preference here, the penalty to call an empty function (when no plugins are installed) is not visible, because frequency of DDL commands are not high. Even so, is it necessary to replace them by macros? It's a fair point. I'm open to other opinions but my vote is to shove a macro in there. A pointer test is cheaper than a function call, and doesn't really complicate things much. -- 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] improved parallel make support
On 11/10/2010 10:32 AM, Peter Eisentraut wrote: On tis, 2010-11-09 at 03:54 -0800, Dave Page wrote: Narwhal should be OK now. The build has issues now, possibly related to the make upgrade. Yeah, it's complaining about not finding bison, but configure managed to find bison just fine. Are you sure the right make was installed? It looks suspicious because it's not talking about msys virtual maths like the old make did. It needs to be make-3.81-3-msys-1.0.13 http://sourceforge.net/projects/mingw/files/MSYS/make/make-3.81-3/make-3.81-3-msys-1.0.13-bin.tar.lzma/download You'll need another couple of libraries as well (libiconv and libintl) if they are not already installed. Making this change took me a while to get right on dawn_bat. cheers andrew
Re: [HACKERS] multi-platform, multi-locale regression tests
David E. Wheeler da...@kineticode.com wrote: On Nov 10, 2010, at 9:48 AM, Andrew Dunstan wrote: Where is this available? Is it self-contained? And what does it require? Python. And some optional python packages, like twisted. http://www.bluegap.ch/projects/dtester/ It looks like I may have raised the issue at a particularly inopportune time -- it looks like maybe Markus is reloading his git repo based on the new official git repo for PostgreSQL. -Kevin -- 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] B-tree parent pointer and checkpoints
On 08.11.2010 15:40, Heikki Linnakangas wrote: Here's a first draft of this, using the inCommit flag as is. It works, but suffers from starvation if you have a lot of concurrent multi-WAL-record actions. I tested that by running INSERTs to a table with tsvector field with a GiST index on it from five concurrent sessions, and saw checkpoints regularly busy-waiting for over a minute. To avoid that, we need something a little bit more complicated than a boolean flag. I'm thinking of adding a counter beside the inCommit flag that's incremented every time a new multi-WAL-record action begins, so that the checkpoint process can distinguish between a new action that was started after deciding the REDO pointer and an old one that's still running. (inCommit is a misnomer now, of course. Will need to find a better name..) Here's a 2nd version, with an additional counter in PGPROC to avoid starving checkpoint in the face of a constant stream e.g GiST inserts. The new rule is that before you start a multi-WAL-record operation that needs to be completed at end of recovery if you crash in the middle, you call HoldCheckpoint(), and once you're finished, ResumeCheckpoint(). rm_safe_restartpoint() is gone. This is a pre-existing bug, but given the lack of field reports and the fact that it's pretty darn hard to run into this in real life, I'm inclined to not backpatch this. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com *** a/src/backend/access/gin/ginbtree.c --- b/src/backend/access/gin/ginbtree.c *** *** 17,22 --- 17,23 #include access/gin.h #include miscadmin.h #include storage/bufmgr.h + #include storage/proc.h #include utils/rel.h /* *** *** 281,286 ginInsertValue(GinBtree btree, GinBtreeStack *stack, GinStatsData *buildStats) --- 282,288 Page page, rpage, lpage; + bool splitInProgress = false; /* remember root BlockNumber */ while (parent) *** *** 318,324 ginInsertValue(GinBtree btree, GinBtreeStack *stack, GinStatsData *buildStats) freeGinBtreeStack(stack); ! return; } else { --- 320,326 freeGinBtreeStack(stack); ! break; /* don't need to recurse to parent */ } else { *** *** 326,331 ginInsertValue(GinBtree btree, GinBtreeStack *stack, GinStatsData *buildStats) --- 328,344 Page newlpage; /* + * Hold off checkpoints until we've finished this split by + * inserting the parent pointer. Replay might miss the incomplete + * split otherwise. + */ + if (!!btree-index-rd_istemp) + { + splitInProgress = true; + HoldCheckpoint(); + } + + /* * newlpage is a pointer to memory page, it doesn't associate with * buffer, stack-buffer should be untouched */ *** *** 402,408 ginInsertValue(GinBtree btree, GinBtreeStack *stack, GinStatsData *buildStats) buildStats-nEntryPages++; } ! return; } else { --- 415,421 buildStats-nEntryPages++; } ! break; /* don't need to recurse to parent */ } else { *** *** 472,475 ginInsertValue(GinBtree btree, GinBtreeStack *stack, GinStatsData *buildStats) --- 485,495 pfree(stack); stack = parent; } + + /* + * If we had to split a page, let checkpointer know that we're now + * finished with it. + */ + if (splitInProgress) + ResumeCheckpoint(); } *** a/src/backend/access/gin/ginxlog.c --- b/src/backend/access/gin/ginxlog.c *** *** 866,876 gin_xlog_cleanup(void) MemoryContextDelete(opCtx); incomplete_splits = NIL; } - - bool - gin_safe_restartpoint(void) - { - if (incomplete_splits) - return false; - return true; - } --- 866,868 *** a/src/backend/access/gist/gist.c --- b/src/backend/access/gist/gist.c *** *** 20,25 --- 20,26 #include miscadmin.h #include storage/bufmgr.h #include storage/indexfsm.h + #include storage/proc.h #include utils/memutils.h const XLogRecPtr XLogRecPtrForTemp = {1, 1}; *** *** 272,283 gistdoinsert(Relation r, IndexTuple itup, Size freespace, GISTSTATE *giststate) --- 273,290 state.r = r; state.key = itup-t_tid; state.needInsertComplete = true; + /* Hold off checkpoints until we've completed this insertion. */ + if (!r-rd_istemp) + HoldCheckpoint(); state.stack = (GISTInsertStack *) palloc0(sizeof(GISTInsertStack)); state.stack-blkno = GIST_ROOT_BLKNO; gistfindleaf(state, giststate); gistmakedeal(state, giststate); + + if (!r-rd_istemp) + ResumeCheckpoint(); } static bool *** a/src/backend/access/gist/gistxlog.c --- b/src/backend/access/gist/gistxlog.c *** *** 838,851 gist_xlog_cleanup(void) MemoryContextDelete(insertCtx); } - bool - gist_safe_restartpoint(void) - { - if (incomplete_inserts) - return
Re: [HACKERS] multi-platform, multi-locale regression tests
On Wed, Nov 10, 2010 at 15:31, Kevin Grittner kevin.gritt...@wicourts.gov wrote: For the Serializable Snapshot Isolation (SSI) patch I needed a test suite which would handle concurrent sessions which interleaved statements in predictable ways. I was told pgTAP wasn't a good choice for that and went with Markus Wanner's dtester package. Sounds like you could use pgTAP with dblink to do the same? :) Regards, Marti -- 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] TODO Alter Table Rename Constraint
On Wed, Nov 10, 2010 at 6:32 AM, Viktor Valy vili0...@gmail.com wrote: Thanks for your answer! I'm not really familiar with inheritance, but I wonder how this issue is handled in other cases, for instance renaming an index, which invokes internal a constraint rename too. Is that relevant or is the renaming of constraints so special? Indexes can't be inherited, so the problem doesn't arise in that case. Is there a solution for the test-cases you have posted? Or is this yet a problem? We had a bug related to the handling of ALTER TABLE .. ADD/DROP CONSTRAINT for those test cases, which I fixed. I think we still have a similar problem with ALTER TABLE .. ADD/DROP ATTRIBUTE, which I haven't fixed because it's hard and I haven't had time, and no one seems to care that much. My point was just that whatever patch you come up with for ALTER TABLE .. RENAME CONSTRAINT should probably be tested against those cases to see if it behaves correctly. -- 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] multi-platform, multi-locale regression tests
Hi, On 11/10/2010 07:28 PM, Kevin Grittner wrote: It looks like I may have raised the issue at a particularly inopportune time -- it looks like maybe Markus is reloading his git repo based on the new official git repo for PostgreSQL. Thanks for noticing me. The dtester repository should be there again. Sorry for the inconvenience. Regards Markus -- 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] multi-platform, multi-locale regression tests
On ons, 2010-11-10 at 07:31 -0600, Kevin Grittner wrote: I don't know if dtester meets the other needs people have, or whether this is a complementary approach, but it seemed worth mentioning. The right tool for the right job, I'd say. One thing to aim for, perhaps, would be to make all tools in use produce a common output format, at least optionally, so that creating a common test run dashboard or something like that is more easily possible. TAP and xUnit come to mind. -- 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] multi-platform, multi-locale regression tests
Marti Raudsepp ma...@juffo.org wrote: Sounds like you could use pgTAP with dblink to do the same? :) I had never read through the docs for dblink until you posted this. In fact, it appears that some testing of proper SSI behavior can be added to standard regression tests with dblink (without needing pgTAP) if there is some way to allow a contrib module like that to be used. Would I have to add the SSI tests to the dblink regression tests, or is there some more graceful way that might be made to work? I don't think this would be a sane way to *replace* the dcheck tests, but it might be a way to work *some* testing of SSI into a more frequently run test set. -Kevin -- 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] multi-platform, multi-locale regression tests
On 11/10/2010 05:06 PM, Kevin Grittner wrote: Marti Raudseppma...@juffo.org wrote: Sounds like you could use pgTAP with dblink to do the same? :) I had never read through the docs for dblink until you posted this. In fact, it appears that some testing of proper SSI behavior can be added to standard regression tests with dblink (without needing pgTAP) if there is some way to allow a contrib module like that to be used. Would I have to add the SSI tests to the dblink regression tests, or is there some more graceful way that might be made to work? I don't think this would be a sane way to *replace* the dcheck tests, but it might be a way to work *some* testing of SSI into a more frequently run test set. We already use some contrib stuff in the regression tests. (It really is time we stopped calling it contrib.) cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] B-tree parent pointer and checkpoints
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: The new rule is that before you start a multi-WAL-record operation that needs to be completed at end of recovery if you crash in the middle, you call HoldCheckpoint(), and once you're finished, ResumeCheckpoint(). What happens if you error out in between? Or is it assumed that the *entire* sequence is a critical section? If it has to be that way, one might wonder what's the point of trying to split it into multiple WAL records. 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] multi-platform, multi-locale regression tests
On Nov 10, 2010, at 2:15 PM, Andrew Dunstan wrote: We already use some contrib stuff in the regression tests. (It really is time we stopped calling it contrib.) Call them core extensions. Works well considering Dimitri's work, which explicitly makes them extensions. So maybe change the directory name to extensions or ext? Best, David -- 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] security hooks on object creation
(2010/11/11 3:00), Robert Haas wrote: On Wed, Nov 10, 2010 at 8:33 AM, KaiGai Koheikai...@kaigai.gr.jp wrote: (2010/11/10 13:06), Robert Haas wrote: In this patch, we put InvokeObjectAccessHook0 on the following functions. - heap_create_with_catalog() for relations/attributes - ATExecAddColumn() for attributes - NamespaceCreate() for schemas - ProcedureCreate() for aggregates/functions - TypeCreate() and TypeShellMake() for types - create_proc_lang() for procedural languages - inv_create() for large objects I think you ought to try to arrange to avoid the overhead of a function call in the common case where nobody's using the hook. That's why I originally suggested making InvokeObjectAccessHook() a macro around the actual function call. Hmm. Although I have little preference here, the penalty to call an empty function (when no plugins are installed) is not visible, because frequency of DDL commands are not high. Even so, is it necessary to replace them by macros? It's a fair point. I'm open to other opinions but my vote is to shove a macro in there. A pointer test is cheaper than a function call, and doesn't really complicate things much. Since I have no strong preference function call here, so, I'll revise my patch according to your vote. Thanks, -- KaiGai Kohei kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] B-tree parent pointer and checkpoints
I wrote: What happens if you error out in between? Or is it assumed that the *entire* sequence is a critical section? If it has to be that way, one might wonder what's the point of trying to split it into multiple WAL records. Or, to be more concrete: I'm wondering if this *entire* mechanism isn't a bad idea that we should just rip out. The question that ought to be asked here, I think, is whether it shouldn't be required that every inter-WAL-record state is a valid consistent state that doesn't require post-crash fixups. If that isn't the case, then a simple ERROR or FATAL exit out of the backend that was creating the sequence originally will leave the system in an unacceptable state. We could prevent such an exit by wrapping the whole sequence in a critical section, but if you have to do that then it's not apparent why you shouldn't fold it into one WAL record. IOW, forget this patch. Take out the logic that tries to complete pending splits during replay, instead. I believe this is perfectly safe for btree: loss of a parent record isn't fatal, as proven by the fact that searches don't have to be locked out while a split proceeds. (We might want to make btree_page_del not think that a missing parent record is an error, but it shouldn't think that anyway, because of the possibility of a non-crashing failure during the original split.) This approach might not be safe for GIST or GIN; but if it isn't, they need fixes anyway. 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] multi-platform, multi-locale regression tests
David E. Wheeler da...@kineticode.com writes: On Nov 10, 2010, at 2:15 PM, Andrew Dunstan wrote: We already use some contrib stuff in the regression tests. (It really is time we stopped calling it contrib.) Call them core extensions. Works well considering Dimitri's work, which explicitly makes them extensions. So maybe change the directory name to extensions or ext? We've been calling it contrib for a dozen years, so that name is pretty well baked in by now. IMO renaming it is pointless and will accomplish little beyond creating confusion and making back-patches harder. (And no, don't you dare breathe a word about git making that all automagically better. I have enough back-patching experience with git by now to be unimpressed; in fact, I notice that its rename-tracking feature falls over entirely when trying to back-patch further than 8.3. Apparently there's some hardwired limit on the number of files it can cope with.) 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] multi-platform, multi-locale regression tests
On Nov 10, 2010, at 3:17 PM, Tom Lane wrote: We've been calling it contrib for a dozen years, so that name is pretty well baked in by now. IMO renaming it is pointless and will accomplish little beyond creating confusion and making back-patches harder. *Shrug*. Just change the name in the docs, then. It's currently Additional Supplied Modules. Maybe just change that to Additional Supplied Extensions or, even better, Core Extensions? Best, David (And no, don't you dare breathe a word about git making that all automagically better. I have enough back-patching experience with git by now to be unimpressed; in fact, I notice that its rename-tracking feature falls over entirely when trying to back-patch further than 8.3. Apparently there's some hardwired limit on the number of files it can cope with.) How often do you have to back-patch contrib, anyway? David -- 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] renaming contrib. (was multi-platform, multi-locale regression tests)
On 11/10/2010 06:17 PM, Tom Lane wrote: David E. Wheelerda...@kineticode.com writes: On Nov 10, 2010, at 2:15 PM, Andrew Dunstan wrote: We already use some contrib stuff in the regression tests. (It really is time we stopped calling it contrib.) Call them core extensions. Works well considering Dimitri's work, which explicitly makes them extensions. So maybe change the directory name to extensions or ext? We've been calling it contrib for a dozen years, so that name is pretty well baked in by now. IMO renaming it is pointless and will accomplish little beyond creating confusion and making back-patches harder. The current name causes constant confusion. It's a significant misnomer, and leads people to distrust the code. There might be reasons not to change, but you should at least recognize why the suggestion is being made. (And no, don't you dare breathe a word about git making that all automagically better. I have enough back-patching experience with git by now to be unimpressed; in fact, I notice that its rename-tracking feature falls over entirely when trying to back-patch further than 8.3. Apparently there's some hardwired limit on the number of files it can cope with.) That's very sad. Did you file a bug? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] multi-platform, multi-locale regression tests
On Wed, Nov 10, 2010 at 6:39 PM, David E. Wheeler da...@kineticode.com wrote: On Nov 10, 2010, at 3:17 PM, Tom Lane wrote: We've been calling it contrib for a dozen years, so that name is pretty well baked in by now. IMO renaming it is pointless and will accomplish little beyond creating confusion and making back-patches harder. *Shrug*. Just change the name in the docs, then. It's currently Additional Supplied Modules. Maybe just change that to Additional Supplied Extensions or, even better, Core Extensions? I don't see any value to that change at all. Additional Supplied Modules is a fine name. If there's a problem here, it's with the name contrib, but I don't see that there's enough value in changing that to be worth the hassle. I think the big hurdle with contrib isn't that it's called contrib but that it's not part of the core server and, in many cases, enabling a contrib module means editing postgresql.conf and bouncing the server. Of course, there are certainly SOME people who wouldn't mind editing postgresql.conf and bouncing the server but are scared off by the name contrib, but I suspect the hassle-factor is the larger issue by a substantial margin. (And no, don't you dare breathe a word about git making that all automagically better. I have enough back-patching experience with git by now to be unimpressed; in fact, I notice that its rename-tracking feature falls over entirely when trying to back-patch further than 8.3. Apparently there's some hardwired limit on the number of files it can cope with.) How often do you have to back-patch contrib, anyway? [rhaas pgsql]$ git log --format=oneline `git merge-base REL9_0_STABLE master`..REL9_0_STABLE | wc -l 247 [rhaas pgsql]$ git log --format=oneline `git merge-base REL9_0_STABLE master`..REL9_0_STABLE contrib | wc -l 20 -- 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] renaming contrib. (was multi-platform, multi-locale regression tests)
On Wed, Nov 10, 2010 at 8:05 PM, Andrew Dunstan and...@dunslane.net wrote: On 11/10/2010 07:51 PM, Robert Haas wrote: On Wed, Nov 10, 2010 at 7:01 PM, Andrew Dunstanand...@dunslane.net wrote: The current name causes constant confusion. It's a significant misnomer, and leads people to distrust the code. There might be reasons not to change, but you should at least recognize why the suggestion is being made. Is it your position that contrib code is as well-vetted as core code? A damn sight more than it used to be. I claim a bit of credit for that - before the buildfarm existed it was quite poorly tested, but we can't get away with that any more. (Ditto PLs and ECPG once we added those into the buildfarm mix.) Of course, there are odd corners in the code. But hstore, for example, has just had a major makeover, and pgcrypto is pretty well maintained. Some other modules are less well loved. There are a few small bits of the core code that have cobwebs too. Fair enough. I think overall our code quality is good, and, over time, it's probably risen both within and outside core. Still, I think renaming contrib would likely be a lot more hassle than it's worth, and I don't think it would do much to remove the central issue, which is that installing extensions is a pain in the neck. Dimitri's work will help with that somewhat, but there's still that nasty business of needing to update shared_preload_libraries and bounce the server, at least for some modules. -- 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] Which file does the SELECT?
Yeah that is what seems to be the best way. The thing is that I am looking in the PostgreSQL code for the first time and I am not fully aware of the data structures or the methods / algos implemented in the project. This makes the work of finding out whats and whys much more difficult. So I asked it on the list. Thanks anyways for the reply. -Vaibhav (*_*) On Wed, Nov 10, 2010 at 9:38 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Nov 8, 2010 at 9:55 PM, Vaibhav Kaushal vaibhavkaushal...@gmail.com wrote: I have started with the work and am using Eclipse and it helps quite a lot. I can find the declarations quite easily. Thanks to open Source. BTW, I am encountering too many (just too many) data types as I try to understand the backend (specifically the executor). I do think that its normal because the executor has to consider almost everything that the other parts of the DB can possibly command it to do. Is there some documentation available on the data types / structures which are in use at the backend? There's less than one might hope. I think you pretty much have to look through the README files and source code comments. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
[HACKERS] wCTE behaviour
Hi all, The discussion around wCTE during the last week or so has brought to my attention that we don't actually have a consensus on how exactly wCTEs should behave. The question seems to be whether or not a statement should see the modifications of statements ran before it. While I think making the modifications visible would be a lot more intuitive, it's not clear how we'd optimize the execution in the future without changing the behaviour (triggers are a big concern). I've done some digging today and it seems that IBM's DB2 took the more intuitive approach: all statements are ran, in the order they're written in, to completion before the main statement, materializing the deltas into a temporary table and the modifications are made visible to the next statements. I have no idea how many complaints they have received about this behaviour, but I'd be in favor of matching it. Thoughts? Regards, Marko Tiikkaja -- 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] renaming contrib. (was multi-platform, multi-locale regression tests)
On 11/10/2010 07:51 PM, Robert Haas wrote: On Wed, Nov 10, 2010 at 7:01 PM, Andrew Dunstanand...@dunslane.net wrote: The current name causes constant confusion. It's a significant misnomer, and leads people to distrust the code. There might be reasons not to change, but you should at least recognize why the suggestion is being made. Is it your position that contrib code is as well-vetted as core code? A damn sight more than it used to be. I claim a bit of credit for that - before the buildfarm existed it was quite poorly tested, but we can't get away with that any more. (Ditto PLs and ECPG once we added those into the buildfarm mix.) Of course, there are odd corners in the code. But hstore, for example, has just had a major makeover, and pgcrypto is pretty well maintained. Some other modules are less well loved. There are a few small bits of the core code that have cobwebs too. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] renaming contrib. (was multi-platform, multi-locale regression tests)
On Wed, Nov 10, 2010 at 7:01 PM, Andrew Dunstan and...@dunslane.net wrote: The current name causes constant confusion. It's a significant misnomer, and leads people to distrust the code. There might be reasons not to change, but you should at least recognize why the suggestion is being made. Is it your position that contrib code is as well-vetted as core code? (And no, don't you dare breathe a word about git making that all automagically better. I have enough back-patching experience with git by now to be unimpressed; in fact, I notice that its rename-tracking feature falls over entirely when trying to back-patch further than 8.3. Apparently there's some hardwired limit on the number of files it can cope with.) That's very sad. Did you file a bug? It's intentional behavior. It gives up when there are too many differences to avoid being slow. -- 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] Exposing an installation's default value of unix_socket_directory
On tor, 2010-10-21 at 16:59 -0400, Tom Lane wrote: Actually, the only reason this is even up for discussion is that there's no configure option to set DEFAULT_PGSOCKET_DIR. If there were, and debian were using it, then pg_config --configure would tell what I wish to know. I thought for a bit about proposing we add such an option, but given the current state of play it might be more misleading than helpful: as long as distros are accustomed to changing this setting via a patch, you couldn't trust pg_config --configure to tell you what a given installation actually has compiled into it. Presumably, if a configure option were added, they couldn't change it via patch anymore. Of course, there will be a transition period, but there will have to be one of some kind anyway. Btw., a configure option for this was rejected years ago to discourage people from actually changing the default. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers