Re: [HACKERS] Patch Submission Guidelines

2006-02-14 Thread Neil Conway
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

2006-02-14 Thread Robert Treat
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

2006-02-14 Thread Andrew Dunstan
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

2006-02-14 Thread Simon Riggs
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

2006-02-14 Thread Tom Lane
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

2006-02-14 Thread Martijn van Oosterhout
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

2006-02-14 Thread Simon Riggs
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

2006-02-14 Thread Andrew Dunstan

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

2006-02-14 Thread Martijn van Oosterhout
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

2006-02-14 Thread James William Pye
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

2006-02-14 Thread Tom Lane
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

2006-02-14 Thread Simon Riggs
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

2006-02-14 Thread Martijn van Oosterhout
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

2006-02-14 Thread hector Corrada Bravo
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

2006-02-14 Thread Tom Lane
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

2006-02-14 Thread Tom Lane
"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

2006-02-14 Thread Dave Page
 

> -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

2006-02-14 Thread Mark Kirkwood

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

2006-02-14 Thread Dave Page
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