Re: [HACKERS] Patch Submission Guidelines
On Tue, 2006-02-14 at 22:54 +, Simon Riggs wrote: > On Tue, 2006-02-14 at 17:28 -0500, Tom Lane wrote: > > IMHO the thing we are really seriously short of is patch reviewers. [...] > Well that was the basis of my original suggestion. Publish some > guidelines and everybody becomes a patch reviewer. I agree guidelines would be help, but I hope (and doubt!) that is not what is stopping people from reviewing patches. Anyone with the time and inclination can review patches, guidelines or not -- reviewing patches is actually a good way to learn more about Postgres internals. The method I personally use for reviewing patches is trivial: for each hunk in the patch what is the intent of the hunk? is there a better way to accomplish that? (Actually applying the patch to a local tree and then browsing the tree can be helpful to understand the context each hunk is modifying.) Of course, the first few patches you review, you'll probably spend more time on step 1 than on step 2, and you might not produce very many useful review comments. But that's what practice is for :) Newbie patch reviewers might also try reviewing patches for client applications (e.g. psql, pg_dump) that do not require as much knowledge of the rest of the source tree. If you are competent at C, you can probably hack on psql/pg_dump/etc. with little additional knowledge. Similarly, reviewing documentation patches is another easy way to get involved -- SGML style fixes, spelling/grammar improvements and the like require no knowledge of PG at all. -Neil ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Patch Submission Guidelines
On Tuesday 14 February 2006 16:00, Martijn van Oosterhout wrote:
> > I would like to suggest that we increase substantially the FAQ entries
> > relating to patch submission. By we, I actually mean please could the
> > committers sit down and agree some clarified written guidelines?
>
> As I remember, there is a disinclination to increase the size of the
> FAQ very much. This suggests maintaining it as a seperate document. Or
> alternatively attach it as an appendix to the main documentation.
>
Huh? The current developers FAQ is at least 1/2 the size of the main FAQ. I
think adding a submission on patch submission guidelines is a great idea.
I'll have a patch based on Simon's post to -patches ready in the next 24
hours unless someone is really going to object.
--
Robert Treat
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [HACKERS] Patch Submission Guidelines
Tom Lane said: > Simon Riggs <[EMAIL PROTECTED]> writes: >> How much time would you need? I think having every patch built before >> anyone even looks at the code would sort out most of the issues I >> mentioned. > > If I ran a buildfarm machine, I'd turn it off immediately if anyone > proposed setting up a system that would cause it to run code no one had > vetted... so I don't think the above will fly. It might or might not > be worth doing something with patches that have passed some kind of > initial review but aren't yet applied. > Yes, I agree. Whast I had in mind was adding some sort of experimental branch to CVS. > IMHO the thing we are really seriously short of is patch reviewers. > Neil and Bruce and I seem to be the only ones doing that much at all, > and the main burden is falling on Bruce. More eyeballs would help much > more than throwing machines at the problem. > Unfortunately, demands from my real job increased enormously right at the time I was given commit privileges. I don't know when that will change. People can review without having commit privs, though. cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Patch Submission Guidelines
On Tue, 2006-02-14 at 17:28 -0500, Tom Lane wrote: > IMHO the thing we are really seriously short of is patch reviewers. > Neil and Bruce and I seem to be the only ones doing that much at all, > and the main burden is falling on Bruce. More eyeballs would help > much more than throwing machines at the problem. Well that was the basis of my original suggestion. Publish some guidelines and everybody becomes a patch reviewer. Best Regards, Simon Riggs ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Patch Submission Guidelines
Simon Riggs <[EMAIL PROTECTED]> writes: > How much time would you need? I think having every patch built before > anyone even looks at the code would sort out most of the issues I > mentioned. If I ran a buildfarm machine, I'd turn it off immediately if anyone proposed setting up a system that would cause it to run code no one had vetted... so I don't think the above will fly. It might or might not be worth doing something with patches that have passed some kind of initial review but aren't yet applied. IMHO the thing we are really seriously short of is patch reviewers. Neil and Bruce and I seem to be the only ones doing that much at all, and the main burden is falling on Bruce. More eyeballs would help much more than throwing machines at the problem. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Patch Submission Guidelines
On Tue, Feb 14, 2006 at 09:54:12PM +, Simon Riggs wrote: > On Tue, 2006-02-14 at 16:17 -0500, Andrew Dunstan wrote: > > > If I had enough time there are all sorts of things like this I'd love to > > set up. A fetchable url that says "try these experimental CVS branches" > > or something like that would be great. > > How much time would you need? I think having every patch built before > anyone even looks at the code would sort out most of the issues I > mentioned. Indeed. I was thinking of downloading the pgbuildfarm code, setting up an autoresponder for -patches and have it autocompile patches on submission. I've never looked at the code so I have no idea how hard it is to set it up, but it doesn't seem that difficult. > I'm thinking in that direction for performance testing. Is there any standard stuff (besides maybe pgbench) that could be run? Have a nice day, -- Martijn van Oosterhout http://svana.org/kleptog/ > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a > tool for doing 5% of the work and then sitting around waiting for someone > else to do the other 95% so you can sue them. signature.asc Description: Digital signature
Re: [HACKERS] Patch Submission Guidelines
On Tue, 2006-02-14 at 16:17 -0500, Andrew Dunstan wrote: > If I had enough time there are all sorts of things like this I'd love to > set up. A fetchable url that says "try these experimental CVS branches" > or something like that would be great. How much time would you need? I think having every patch built before anyone even looks at the code would sort out most of the issues I mentioned. I'm thinking in that direction for performance testing. Best Regards, Simon Riggs ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] Patch Submission Guidelines
Martijn van Oosterhout wrote: Finally, several of the patches committed the last few days have been fixing minor bugs or platform specific issues with various patches. One thing that would be really nice is a real patch queue and have the buildfarm machines occasionally apply one of the patches and try to run with it. For people that don't have access to all sorts of architechtures, it would be a great way of getting feedback on the portability of the patch prior to actual submission to -HEAD. This would probably be fairly trivial to arrange. There is nothing magical about the branches we build against on buildfarm - it just happens to be HEAD and the REL_foo_STABLE branches. They are just names in a config file. If I had enough time there are all sorts of things like this I'd love to set up. A fetchable url that says "try these experimental CVS branches" or something like that would be great. cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Patch Submission Guidelines
Note: People following this should probably read this post on -patches in the archive: http://archives.postgresql.org/pgsql-patches/2006-02/msg00207.php On Tue, Feb 14, 2006 at 05:20:55PM +, Simon Riggs wrote: > Many patch submitters discover that they fall foul of various "you > should have done"s at a late stage of the patch review process. > I would like to suggest that we increase substantially the FAQ entries > relating to patch submission. By we, I actually mean please could the > committers sit down and agree some clarified written guidelines? As I remember, there is a disinclination to increase the size of the FAQ very much. This suggests maintaining it as a seperate document. Or alternatively attach it as an appendix to the main documentation. I liked your list BTW. It covers most of the common issues. I think you missed SQL standards related issues. If you're submitting a patch to increase SQL conformence, you need to say so. > I believe if we do this we will have more patches produced, reviewed and > committed from our available resources, as well as more hackers more > regularly willing to face the challenges of getting a quality patch > accepted. In the end we will live and die by the number of people > submitting and how many of those go on to become regular contributors > (should I say "serial hackers"?) One real big issue is feedback. Some patches are obviously going to receive much more feedback than others. But in some places there are really very few people that can give meaningful feedback. I honestly don't know what we can do about that other than try to grow the number of people :( Related to that is testing. I get the impression that very few patches are tested by people other than the author before submission. This is one thing the linux kernel does well. There exist trees that will take almost any patch and people who download that and hammer it. Great for testing stability before accepting it into the real tree. Finally, several of the patches committed the last few days have been fixing minor bugs or platform specific issues with various patches. One thing that would be really nice is a real patch queue and have the buildfarm machines occasionally apply one of the patches and try to run with it. For people that don't have access to all sorts of architechtures, it would be a great way of getting feedback on the portability of the patch prior to actual submission to -HEAD. Does give you ideas, doesn't it? Have a nice day, -- Martijn van Oosterhout http://svana.org/kleptog/ > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a > tool for doing 5% of the work and then sitting around waiting for someone > else to do the other 95% so you can sue them. signature.asc Description: Digital signature
Re: [HACKERS] Copy From & Insert UNLESS
On Mon, Feb 06, 2006 at 05:08:38PM -0500, Alon Goldshuv wrote: > The proposal is neat, however, I am not too > excited about handling errors in such high granularity, as far as the user > is concerned. I am more on the same line with Tom Lane's statement in > Simon's thread (Practical error logging for very large COPY statements): > > "The general problem that needs to be solved is "trap any error that > occurs during attempted insertion of a COPY row, and instead of aborting > the copy, record the data and the error message someplace else". Seen > in that light, implementing a special path for uniqueness violations is > pretty pointless." I think I would be inclined to actually agree with that, which is why I proposed a special path for constraint violations in general as opposed to just uniqueness. However, I can understand if you remain unmoved. ;) > But, I definitely share your struggle to finding a good way to handle those > unique/FK constraints... Aye, :-( > As far as UNIQUE goes, maybe there is a good way to do a bt scan against the > index table right before the simple_heap_insert call? Yes, but I believe some additional locking is required in order to make that safe. Not that that would kill it, but I think there is a better way; I'm cooking up a general proposal for refactoring unique constraints, so I'm hoping something along those lines will aid any patch attempting to solving this problem[copy error/violation management]. -- Regards, James William Pye ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] optimizer questions
hector Corrada Bravo <[EMAIL PROTECTED]> writes: > 1) Regardless of the optimization problem, is the executor able to > execute aggregate nodes within join trees (that is, not as the result > of subqueries)? Sure. > 3) For debugging purposes: Has anyone figured out a way to feed > hand-crafted plans to the executor? Setting up some of the data > structures (PlannerInfo, target lists) etc. does not look trivial. By > this I mean, beyond giving explicit join clauses in queries. It's not really very practical --- the data structures are too complex to create by hand in any reasonable way. You can probably test by entering modified queries that do the aggregations in sub-selects, though. The most practical way to implement something like this is probably to restructure the query during the "prep" stage, pushing the aggregates down into sub-queries automatically. The 8.1 min/max optimization code does something related, although I'll freely admit that's a bit of a hack. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
[HACKERS] Patch Submission Guidelines
Many patch submitters discover that they fall foul of various "you should have done"s at a late stage of the patch review process. These include the usual: - major feature change not discussed on -hackers or elsewhere first - patch in wrong format - performance patch, yet no performance test results to prove benefit - no accompanying doc patch - won't work on various ports (and it needs to) etc.. In contrast, the documentation and translation process is extremely well documented; this may be by design. I would like to suggest that we increase substantially the FAQ entries relating to patch submission. By we, I actually mean please could the committers sit down and agree some clarified written guidelines? There is nothing wrong right now with the level of quality of patches that get accepted, so I do not wish to discuss lowering or increasing the quality bar. What I do want to discuss is how to increase the efficiency of the patch submission process so that senior committers spend less of their time (our most critical resource) on poor quality submissions (however that is judged) and also that patch submitters also have fast feedback on missing requirements. A clear FAQ entry or checklist can be applied easily by more casual readers of the -patches list, allowing errors to be pointed out quickly by non-committers and any missing requirements rectified. Written guidelines are also much more easily translated than no guidelines at all, benefiting non-native English speakers considerably. Some of the above guidelines are clearly explained in FAQ, others not. I would also want to add to the Developer page of the website something along the lines of "Interested in developing for PostgreSQL? Please read the patch submission guidelines before you begin work since only the highest quality patches will be accepted." I believe if we do this we will have more patches produced, reviewed and committed from our available resources, as well as more hackers more regularly willing to face the challenges of getting a quality patch accepted. In the end we will live and die by the number of people submitting and how many of those go on to become regular contributors (should I say "serial hackers"?) Bruce currently maintains much of this material, so I want it to be known that this is specifically not a criticism of his work. This is just an earnest attempt to increase the efficiency of the current process, so patch authors can move quickly onto their next patch. [Increasing the quality of my own submissions is a necessary act in this process, though I hope these thoughts can be considered outside of my own involvement and experience.] It's probably also time for the annual discussion about when is the next patch submission deadline. ;-) Best Regards, Simon Riggs ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] optimizer questions
On Tue, Feb 14, 2006 at 10:35:12AM -0600, hector Corrada Bravo wrote: > Hello everyone, > > I am working with the Postgres optimizer for the first time, so bear with > me... > > I want to extend the optimizer to deal with aggregate queries a bit > better. The idea is from an old paper by Chaudhuri and Shim in VLDB > 94. The gist of it is that when computing aggregates over the result > of joining multiple tables, under some conditions the aggregate can be > pushed into the join tree to reduce the size of join operands making > resulting plans cheaper. > > So here is my problem, due to the path/plan separation of the Postgres > optimizer, this is not trivial (joins are decided in path, aggregates > decided in plan). As it stands, aggregate nodes can only appear as the > top node of subqueries. Well, the basic approach in the beginning should probably be to change the planner to modify the plan so you get a new plan with the aggregation in the right place. > Before I start trying this (creating aggregate paths seems the > reasonable thing to do) I would like your counsel. > > 1) Regardless of the optimization problem, is the executor able to > execute aggregate nodes within join trees (that is, not as the result > of subqueries)? An Aggregate is just a node type that can be stacked above any other node. Ofcourse, it requires the input rows to be in the appropriate order but other than that. > 2) Has anyone tried something like this before? No idea. > 3) For debugging purposes: Has anyone figured out a way to feed > hand-crafted plans to the executor? Setting up some of the data > structures (PlannerInfo, target lists) etc. does not look trivial. By > this I mean, beyond giving explicit join clauses in queries. Nope sorry. > 4) Any other suggestions? Well, I honestly have no idea what kind of transformations you have in mind so not really. However, you should probably realise that in PostgreSQL aggregates are not special and users can make their own. Whatever you think of needs to take that into account. Have a ncie day, -- Martijn van Oosterhout http://svana.org/kleptog/ > Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a > tool for doing 5% of the work and then sitting around waiting for someone > else to do the other 95% so you can sue them. signature.asc Description: Digital signature
[HACKERS] optimizer questions
Hello everyone, I am working with the Postgres optimizer for the first time, so bear with me... I want to extend the optimizer to deal with aggregate queries a bit better. The idea is from an old paper by Chaudhuri and Shim in VLDB 94. The gist of it is that when computing aggregates over the result of joining multiple tables, under some conditions the aggregate can be pushed into the join tree to reduce the size of join operands making resulting plans cheaper. So here is my problem, due to the path/plan separation of the Postgres optimizer, this is not trivial (joins are decided in path, aggregates decided in plan). As it stands, aggregate nodes can only appear as the top node of subqueries. Before I start trying this (creating aggregate paths seems the reasonable thing to do) I would like your counsel. 1) Regardless of the optimization problem, is the executor able to execute aggregate nodes within join trees (that is, not as the result of subqueries)? 2) Has anyone tried something like this before? 3) For debugging purposes: Has anyone figured out a way to feed hand-crafted plans to the executor? Setting up some of the data structures (PlannerInfo, target lists) etc. does not look trivial. By this I mean, beyond giving explicit join clauses in queries. 4) Any other suggestions? Thank you very much, Hector ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] FW: PGBuildfarm member snake Branch HEAD Status changed
Mark Kirkwood <[EMAIL PROTECTED]> writes: > Oh dear - looks like my pg_freespacemap patch is getting its Windows > testing :-( > Dave - are you able to try out the attached patch? Already committed an equivalent patch before seeing your message ... regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] FW: PGBuildfarm member snake Branch HEAD Status changed from Make failure to Contrib failure
"Dave Page" writes: > And the fun continues :-) > Info: resolving _MaxFSMPages by linking to __imp__MaxFSMPages (auto-import) Fixed. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] FW: PGBuildfarm member snake Branch HEAD Status changed from Make failure to Contrib failure
> -Original Message- > From: Mark Kirkwood [mailto:[EMAIL PROTECTED] > Sent: 14 February 2006 10:17 > To: Dave Page > Cc: pgsql-hackers@postgresql.org > Subject: Re: [HACKERS] FW: PGBuildfarm member snake Branch > HEAD Status changed from Make failure to Contrib failure > > Oh dear - looks like my pg_freespacemap patch is getting its Windows > testing :-( > > Dave - are you able to try out the attached patch? Yup, looks good here. Cheers, Dave. ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] FW: PGBuildfarm member snake Branch HEAD Status changed
Dave Page wrote: And the fun continues :-) -Original Message- From: PG Build Farm [mailto:[EMAIL PROTECTED] Sent: 14 February 2006 02:16 To: [EMAIL PROTECTED] Subject: PGBuildfarm member snake Branch HEAD Status changed from Make failure to Contrib failure The PGBuildfarm member snake had the following event on branch HEAD: Status changed from Make failure to Contrib failure Oh dear - looks like my pg_freespacemap patch is getting its Windows testing :-( Dave - are you able to try out the attached patch? Thanks Mark Index: pg_freespacemap.c === RCS file: /projects/cvsroot/pgsql/contrib/pg_freespacemap/pg_freespacemap.c,v retrieving revision 1.1 diff -c -r1.1 pg_freespacemap.c *** pg_freespacemap.c 12 Feb 2006 03:55:53 - 1.1 --- pg_freespacemap.c 14 Feb 2006 10:14:28 - *** *** 16,21 --- 16,22 #if defined(WIN32) || defined(__CYGWIN__) extern DLLIMPORT volatile uint32 InterruptHoldoffCount; + extern DLLIMPORT int MaxFSMPages; #endif Datum pg_freespacemap(PG_FUNCTION_ARGS); ---(end of broadcast)--- TIP 6: explain analyze is your friend
[HACKERS] FW: PGBuildfarm member snake Branch HEAD Status changed from Make failure to Contrib failure
And the fun continues :-) > -Original Message- > From: PG Build Farm > [mailto:[EMAIL PROTECTED] > Sent: 14 February 2006 02:16 > To: [EMAIL PROTECTED] > Subject: PGBuildfarm member snake Branch HEAD Status changed > from Make failure to Contrib failure > > > The PGBuildfarm member snake had the following event on branch HEAD: > > Status changed from Make failure to Contrib failure > > The snapshot timestamp for the build that triggered this > notification is: 2006-02-14 02:00:00 > > The specs of this machine are: > OS: Windows / Server 2003 SP1 > Arch: i686 > Comp: gcc / 3.4.2 > > For more information, see > http://www.pgbuildfarm.org/cgi-bin/show_history.pl?nm=snake&br=HEAD > > > ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
