Re: KNNGiST for knn-search (WIP)

On Wed, Dec 30, 2009 at 9:20 AM, Oleg Bartunov <oleg(at)sai(dot)msu(dot)su> wrote:
> From metodological point of view I don't quite understand how to measure
> the value of development, I mean what'is a "big patch", "invasive patch".

I want to speak specifically to this question because I think it's a
good one. Of course, I also want to make clear that I have nothing
against you or your patch and that it sounds like a really nice
feature.

From my point of view, what makes a patch invasive is the likelihood
that it might break something other than itself. For example, your
patch touches the core planner code and the core GIST code, so it
seems possible that adding support for this feature might break
something else in one of those areas. All things being equal, we
would prefer to take that risk at the beginning of a development cycle
rather than the end. If your patch was the same size, but consisted
mostly of new code with very few changes to what is there now, it
might still be difficult to properly review and verify - but any bugs
we missed would likely affect only the NEW functionality, not any
EXISTING functionality.

Please understand that the previous paragraph is intended to be a
general statement about software development in general more than a
specific commentary on your particular patch. Whether applying your
patch in particular will break anything is, of course, something
that's difficult to know until we do it and see what happens, and at
this point I haven't even reviewed it. It's also possible that I'm
doing a poor job estimating the risk of breakage, and I certainly
welcome other opinions from other people in a position to make a
technical judgement on that point. I might also have a different
opinion myself after I review the patch in more detail, so please do
post an updated version.

> Should we prefer cosmetic pathces, spelling fixes, etc ? Of course, they are
> easy for refering, but people are waiting from us not just fixes, but new
> features. For example, KNN-GiST is a big improvement for PostGIS community,
> which is a big part of postgres users. Actually, it's PostGIS community,
> which
> supported our work. Now, what we should say them ? The patch was too big and
> invasive, so, sorry, wait one year more ? I think it's not good.

Well, I understand your point, but there is obviously some deadline
for patches to be submitted for any particular release. Clearly,
after the last CommitFest is over, that deadline is past. However, we
have previously discussed having a policy that no new large patches
will be accepted for the last CommitFest that were not also submitted
for the second-to-last CommitFest. Hopefully it's obvious that I have
no desire to keep cool new features away from the PostGIS community,
the PostgreSQL community, or anyone else, but we have to weigh that
against the desire to have a stable and bug-free release, and applying
big patches at the last minute makes that less likely.

As an example, the change to run the background writer during recovery
and the changes in semi/anti join planning for 8.4 have both resulted
in multiple bug reports. The former was half the footprint of your
patch and applied at the very end of the release cycle; the latter was
slightly larger and applied in August 2008, so considerably earlier in
the cycle than this one could possibly be - and there were still
things we did not catch before release.

...Robert