knngist patch support
Hello,
I noticed this morning that the k nearest neighbor gist patch
https://commitfest.postgresql.org/action/patch_view?id=230 was still being
considered for inclusion in 9. Sadly, this feature appears to have been
dropped from 9.
It seems to me that the functionality this brings is one of the most common
use cases for mobile applications (or even web in some cases) that are
location enabled. How many times do we find ourselves "Finding the 10
nearest restaurants" in our smart phones? Well, this patch solves exactly
that in the most efficient manner.
Granted, one can currently solve this problem with PostgreSQL/PostGIS as it
stands, however the performance improvements that one can gain for those
types of (extremely common) queries leveraging the Gist enhancements are,
well, exciting.
300x time improvements? Sounds great to me.
I would love if you guys could consider applying this patch for this
upcoming release.
Best Regards,
- Ragi Burhum
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Wed, Feb 10, 2010 at 04:49:59PM -0800, Ragi Y. Burhum wrote:
Hello,
I noticed this morning that the k nearest neighbor gist patch
https://commitfest.postgresql.org/action/patch_view?id=230 was still being
considered for inclusion in 9. Sadly, this feature appears to have been
dropped from 9.
This has been discussed recently on this list. Seems the patch would
need more review to be considered stable. So it's the hard choice of
letting the schedule for 9.0 slip or not letting this patch in.
But some prerequisites will go in, that's the good news.
(BTW: I tried to find this discussion in the Web archives, but had no
luck. It's in my mailbox, though --
e.g.
message-ID 603c8f071002070527j1dada7cdseb42e7cbc71bf71a@mail.gmail.com
part of the long thread "Damage control mode", starting on Jan 8, 2010;
this one mail is from Feb 7 -- but that might be me)
Regards
- -- tomás
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
iD8DBQFLc5YoBcgs9XrR2kYRAoW3AJ94tYWPenLOjH4B4GHD9DCYSSWYOQCeOcoM
RYDhINv+k9YeD23xFHyj9yw=
=K1E0
-----END PGP SIGNATURE-----
This is very disgraceful from my point of view and reflects real problem
in scheduling of CF. The patch was submitted Nov 23 2009, discussed and
reworked Nov 25. Long holidays in December-January, probably are reason why
there were no any movement on reviewing the patch. People with
inspiration spent time to discuss rbtree, while it was clear, that rbtree is
a minor issue. Now we have no review and great feature is missing. I understand
that some healthy resistance is useful to let developers more accurate and
discipline, but, hey, not dropping great feature ! I'd understand if developer
is missing, or just not willing to contact, but I and Teodor are here and
we readily answer any questions.
I failed to find any documents about commitfest to understand if we already
discussed all possible scenario of feature drop. If we say 'A', when started
to formalize our development process instead of old way discuss&vote
in -hackers, then we should say 'B' - formalize procedure and possible
collision of interests.
Oleg
On Thu, 11 Feb 2010, tomas@tuxteam.de wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1On Wed, Feb 10, 2010 at 04:49:59PM -0800, Ragi Y. Burhum wrote:
Hello,
I noticed this morning that the k nearest neighbor gist patch
https://commitfest.postgresql.org/action/patch_view?id=230 was still being
considered for inclusion in 9. Sadly, this feature appears to have been
dropped from 9.This has been discussed recently on this list. Seems the patch would
need more review to be considered stable. So it's the hard choice of
letting the schedule for 9.0 slip or not letting this patch in.But some prerequisites will go in, that's the good news.
(BTW: I tried to find this discussion in the Web archives, but had no
luck. It's in my mailbox, though --e.g.
message-ID 603c8f071002070527j1dada7cdseb42e7cbc71bf71a@mail.gmail.com
part of the long thread "Damage control mode", starting on Jan 8, 2010;
this one mail is from Feb 7 -- but that might be me)Regards
- -- tomЪЪs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)iD8DBQFLc5YoBcgs9XrR2kYRAoW3AJ94tYWPenLOjH4B4GHD9DCYSSWYOQCeOcoM
RYDhINv+k9YeD23xFHyj9yw=
=K1E0
-----END PGP SIGNATURE-----
Regards,
Oleg
_____________________________________________________________
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83
Oleg Bartunov <oleg@sai.msu.su> writes:
This is very disgraceful from my point of view and reflects real problem
in scheduling of CF. The patch was submitted Nov 23 2009, discussed and
reworked Nov 25. Long holidays in December-January, probably are reason why
there were no any movement on reviewing the patch.
There was a scheduling problem all right, which was that this patch *did
not make* the deadline for the November CF. The fact that it got any
review at all in November was more than expected under the CF process.
And I remind you that we stated more than once that we didn't want major
feature patches to show up only at the last CF. If it had come from
anyone other than you and Teodor, there would have not been even a
moment's consideration of letting it into 9.0.
My own feeling about it is that I much preferred the original proposal
of a contrib module with little or no change to core code. I don't want
to be changing core code for this at this late hour. If it were only
touching GIST I'd be willing to rely on your and Teodor's expertise in
that module, but it's not. It whacks around the planner, it makes
questionable changes in the operator class structure, and the last
version I saw hadn't any documentation whatever. It's not committable
on documentation grounds alone, even if everybody was satisfied about
the code.
How do you feel about going back to the original contrib module for now
and resubmitting the builtin version for 9.1?
regards, tom lane
2010/2/11 Oleg Bartunov <oleg@sai.msu.su>:
This is very disgraceful from my point of view and reflects real problem
in scheduling of CF. The patch was submitted Nov 23 2009, discussed and
reworked Nov 25. Long holidays in December-January, probably are reason why
there were no any movement on reviewing the patch. People with
So... I think the reason why there was no movement between November
25th and January 15th is because no CommitFest started between
November 25th and January 15th. Had you submitted the patch on
November 14th, you would have gotten a lot more feedback in November;
I agree that we don't have a lot of formal documentation about the
CommitFest process, but I would think that much would be pretty clear,
but maybe not. The reason there was no movement after January 15th is
because (1) I couldn't get anyone to volunteer to review it, except
Mark Cave-Ayland who didn't actually do so (or anyway didn't post
anything publicly), and (2) we were still working on rbtree.
Personally, I am a little irritated about the whole way this situation
has unfolded. I devoted a substantial amount of time over my
Christmas vacation to patch review, and many of those patches went on
to be committed. Some of the patches I reviewed were yours. I did
not get paid one dime for any of that work. I expressed candidly,
from the very beginning, that getting such a large patch done by the
end of this CommitFest would likely be difficult, especially given
that it had two precursor patches. In exchange for giving you my
honest opinions about your patches two weeks before the scheduled
start of the CommitFest, over my Christmas vacation, and for free, I
got a long stream of complaints from you and others about how the
process is unfair, and as nearly zero help making the prerequisite
patches committable as it is possible for anyone to achieve. It
regularly took 4-6 days for a new version of the patch to appear, and
as often as not questions in my reviews were ignored for days, if not
weeks. It took a LOT of iterations before my performance concerns
were addressed; and I believe that process could have been done MUCH
more quickly.
Now, it is possible that as you are sitting there reading this email,
you are thinking to yourself "well, your feedback didn't actually make
that patch any better, so this whole thing is just pure
obstructionism." I don't believe that's the case, but obviously I'm
biased and everyone is entitled to their own opinion. What I can tell
you for sure is that all of my reviewing was done with the best of
motivations and in a sincere attempt to do the right thing.
You may be right that January 15th was a bad time to start a
CommitFest, although it's very unclear to me why that might be. At
least in the US, the holidays are over long before January 15th, but
we had a very small crop of reviewers this time around, and a number
of them failed to review the patches they picked up, or did only a
very cursory review. It might be mentioned that if you have concerns
about getting your own patches reviewed, you might want to think about
reviewing some patches by other people. Of the 60 patches currently
in the 2010-01 CommitFest, I'm listed as a reviewer on 12 of them.
Needless to say, if someone else had volunteered to do some or all of
the review work on some of those patches, I would have had more time
to work on other patches.
...Robert
I have to say that as a 3rd party observer it is quite obvious to
understand why the PostgreSQL software is so good - people are very
passionate about the work they are doing. However, in this instance,
as a by-stander, it seems that there is a lot of energy being spent on
pointing fingers. At the end, the only people that loose are users
like me who would love to have a feature like this since it would
literally make one of the most common types of spatial queries, for
lack of better wording, ridiculously fast. I sincerely apologize if I
triggered any kind of trouble by asking a questions about this
feature.
- Ragi
Show quoted text
On Wed, Feb 10, 2010 at 11:28 PM, Robert Haas <robertmhaas@gmail.com> wrote:
2010/2/11 Oleg Bartunov <oleg@sai.msu.su>:
This is very disgraceful from my point of view and reflects real problem
in scheduling of CF. The patch was submitted Nov 23 2009, discussed and
reworked Nov 25. Long holidays in December-January, probably are reason why
there were no any movement on reviewing the patch. People withSo... I think the reason why there was no movement between November
25th and January 15th is because no CommitFest started between
November 25th and January 15th. Had you submitted the patch on
November 14th, you would have gotten a lot more feedback in November;
I agree that we don't have a lot of formal documentation about the
CommitFest process, but I would think that much would be pretty clear,
but maybe not. The reason there was no movement after January 15th is
because (1) I couldn't get anyone to volunteer to review it, except
Mark Cave-Ayland who didn't actually do so (or anyway didn't post
anything publicly), and (2) we were still working on rbtree.Personally, I am a little irritated about the whole way this situation
has unfolded. I devoted a substantial amount of time over my
Christmas vacation to patch review, and many of those patches went on
to be committed. Some of the patches I reviewed were yours. I did
not get paid one dime for any of that work. I expressed candidly,
from the very beginning, that getting such a large patch done by the
end of this CommitFest would likely be difficult, especially given
that it had two precursor patches. In exchange for giving you my
honest opinions about your patches two weeks before the scheduled
start of the CommitFest, over my Christmas vacation, and for free, I
got a long stream of complaints from you and others about how the
process is unfair, and as nearly zero help making the prerequisite
patches committable as it is possible for anyone to achieve. It
regularly took 4-6 days for a new version of the patch to appear, and
as often as not questions in my reviews were ignored for days, if not
weeks. It took a LOT of iterations before my performance concerns
were addressed; and I believe that process could have been done MUCH
more quickly.Now, it is possible that as you are sitting there reading this email,
you are thinking to yourself "well, your feedback didn't actually make
that patch any better, so this whole thing is just pure
obstructionism." I don't believe that's the case, but obviously I'm
biased and everyone is entitled to their own opinion. What I can tell
you for sure is that all of my reviewing was done with the best of
motivations and in a sincere attempt to do the right thing.You may be right that January 15th was a bad time to start a
CommitFest, although it's very unclear to me why that might be. At
least in the US, the holidays are over long before January 15th, but
we had a very small crop of reviewers this time around, and a number
of them failed to review the patches they picked up, or did only a
very cursory review. It might be mentioned that if you have concerns
about getting your own patches reviewed, you might want to think about
reviewing some patches by other people. Of the 60 patches currently
in the 2010-01 CommitFest, I'm listed as a reviewer on 12 of them.
Needless to say, if someone else had volunteered to do some or all of
the review work on some of those patches, I would have had more time
to work on other patches....Robert
On Thu, 11 Feb 2010, Tom Lane wrote:
Oleg Bartunov <oleg@sai.msu.su> writes:
This is very disgraceful from my point of view and reflects real problem
in scheduling of CF. The patch was submitted Nov 23 2009, discussed and
reworked Nov 25. Long holidays in December-January, probably are reason why
there were no any movement on reviewing the patch.There was a scheduling problem all right, which was that this patch *did
not make* the deadline for the November CF. The fact that it got any
review at all in November was more than expected under the CF process.
And I remind you that we stated more than once that we didn't want major
feature patches to show up only at the last CF. If it had come from
anyone other than you and Teodor, there would have not been even a
moment's consideration of letting it into 9.0.
there were several long threads, which I have no possibility to follow,
so we relied on the wisdom of people, who can discuss. So, it's true we
didn't track all nuances of our development schedule. We just developed.
Looked on commitfest page we didn't find any summary and it's hard to
understand what is the final word.
In the old-good time we also discussed
a lot, we release faster and we always had tolerance of time, since many
things were not formalized, we were developers and reviewed each other.
Now, the whole process of development redesigned to be more enterprize,
but we still have problem with resources - developers, reviewers. And I don't
see, how all changes try to solve this problem. We have problem with long
release cycle, it's getting worse and worse, in spite of CF. The main problem
is not in scheduling - we have little delta here, the problem is in human
resources and unclear regulations make it worse.
My own feeling about it is that I much preferred the original proposal
of a contrib module with little or no change to core code. I don't want
to be changing core code for this at this late hour. If it were only
touching GIST I'd be willing to rely on your and Teodor's expertise in
that module, but it's not. It whacks around the planner, it makes
questionable changes in the operator class structure, and the last
aha, we originally submit contrib module, which didn't touch anything you
mentioned, we improve stuff to follow discussion and now we are out of luck %(
version I saw hadn't any documentation whatever. It's not committable
on documentation grounds alone, even if everybody was satisfied about
the code.
well, there is enough documentation to review patch.
In my understanding this was always enough to submit code.
User's documentation is depend on discussion and review and can be added later
before releasing beta.
How do you feel about going back to the original contrib module for now
and resubmitting the builtin version for 9.1?
Hmm, one good thing is that rbtree seems ok for submisson. We need to discuss
this, if it's good for PostGIS community. I'd not complain about this decision
if it touch my interests only, I could live with closed-source patch.
Regards,
Oleg
_____________________________________________________________
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83
On Thu, 11 Feb 2010, Robert Haas wrote:
2010/2/11 Oleg Bartunov <oleg@sai.msu.su>:
This is very disgraceful from my point of view and reflects real problem
in scheduling of CF. The patch was submitted Nov 23 2009, discussed and
reworked Nov 25. Long holidays in December-January, probably are reason why
there were no any movement on reviewing the patch. People withSo... I think the reason why there was no movement between November
25th and January 15th is because no CommitFest started between
November 25th and January 15th. Had you submitted the patch on
November 14th, you would have gotten a lot more feedback in November;
I agree that we don't have a lot of formal documentation about the
CommitFest process, but I would think that much would be pretty clear,
but maybe not. The reason there was no movement after January 15th is
because (1) I couldn't get anyone to volunteer to review it, except
Mark Cave-Ayland who didn't actually do so (or anyway didn't post
anything publicly), and (2) we were still working on rbtree.Personally, I am a little irritated about the whole way this situation
has unfolded. I devoted a substantial amount of time over my
Robert, please accept my public apology, if you feel I offense you. There are
nothing against you. Your contribution is very important and I really don't
understand why on the Earth you're not paid ! I remember discussion
to paid you from our foundation. That's shame.
Does nybody ever got support for development from our foundation ?
Christmas vacation to patch review, and many of those patches went on
to be committed. Some of the patches I reviewed were yours. I did
not get paid one dime for any of that work. I expressed candidly,
from the very beginning, that getting such a large patch done by the
end of this CommitFest would likely be difficult, especially given
that it had two precursor patches. In exchange for giving you my
honest opinions about your patches two weeks before the scheduled
start of the CommitFest, over my Christmas vacation, and for free, I
got a long stream of complaints from you and others about how the
process is unfair, and as nearly zero help making the prerequisite
patches committable as it is possible for anyone to achieve. It
regularly took 4-6 days for a new version of the patch to appear, and
as often as not questions in my reviews were ignored for days, if not
weeks. It took a LOT of iterations before my performance concerns
were addressed; and I believe that process could have been done MUCH
more quickly.
Robert, it's very hard to marshal all developers, who are not-paid people
with their regular duties and problems and their own interests in postgres.
You just discovered we have long-long
holidays in Russia, when people try to spend somewhere. I always beaten with
Christmas in December, when I tried to communicate with business people un US.
Earlier, we lived with this and our releases were faster. I'd not say, CF is
a step back, but our system should have tolerance in time if we're
open-source community, or go enterprize way - we are all paid, we follow
business plan, ... etc. Something is really wrong, that's what I can say.
Now, it is possible that as you are sitting there reading this email,
you are thinking to yourself "well, your feedback didn't actually make
that patch any better, so this whole thing is just pure
obstructionism." I don't believe that's the case, but obviously I'm
biased and everyone is entitled to their own opinion. What I can tell
you for sure is that all of my reviewing was done with the best of
motivations and in a sincere attempt to do the right thing.You may be right that January 15th was a bad time to start a
CommitFest, although it's very unclear to me why that might be. At
least in the US, the holidays are over long before January 15th, but
we had a very small crop of reviewers this time around, and a number
of them failed to review the patches they picked up, or did only a
very cursory review. It might be mentioned that if you have concerns
about getting your own patches reviewed, you might want to think about
reviewing some patches by other people. Of the 60 patches currently
in the 2010-01 CommitFest, I'm listed as a reviewer on 12 of them.
Needless to say, if someone else had volunteered to do some or all of
the review work on some of those patches, I would have had more time
to work on other patches.
Robert, human resources are the main problem and, first of all,
our system should work for developers ! If we will not understand each other
and follow only some unclear rules, we'll lost current developers and will
not attract new. We, probably, in our particulary case, will follow our
original suggestion -just contrib module, but I concern about future. Now I
have to think not just about algorithms and implementation, but about
reviewer and current regulation.
Regards,
Oleg
_____________________________________________________________
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83
On Thu, Feb 11, 2010 at 3:38 AM, Oleg Bartunov <oleg@sai.msu.su> wrote:
Robert, please accept my public apology, if you feel I offense you. There
are
nothing against you. Your contribution is very important and I really don't
understand why on the Earth you're not paid ! I remember discussion to paid
you from our foundation. That's shame. Does nybody ever got support for
development from our foundation ?
No, I don't feel like you offended me. It's more that, from my point
of view, it seems like all the things you're complaining about are
things that you more or less have control over, or at least could have
foreseen. I have only been involved in this project for a year and a
half, so the CommitFest process is the only process that I know or
understand. On the whole, I've found it to be a pretty good process.
I get my patches in; I help other people get their patches in (and
hopefully improve them along the way). It's particularly appealing
when you're a non-committer, as it gives you a formal structure to
make sure your work gets looked at.
It seems that you're sort of frustrated with the system and the need
to go through a process before committing a patch; and that you feel
that the rules are unclear. I don't think it's a bad thing to go
through a process before committing a patch, especially a large patch
like knngist, but of course that's just my opinion. I agree that the
fact that the rules are unclear is a problem, though I'm not sure what
to do about it. I am not sure they are so unclear as you are making
them out to be, but again, I'm biased by being a relative newcomer, as
well as someone who has been in the middle of many of the process
discussions.
Robert, human resources are the main problem and, first of all,
our system should work for developers ! If we will not understand each other
and follow only some unclear rules, we'll lost current developers and will
not attract new. We, probably, in our particulary case, will follow our
original suggestion -just contrib module, but I concern about future. Now I
have to think not just about algorithms and implementation, but about
reviewer and current regulation.
IMHO, our system has to work for both developers and users, and it has
to work for both committers and non-committers.
...Robert
On Thu, Feb 11, 2010 at 3:00 AM, Oleg Bartunov <oleg@sai.msu.su> wrote:
version I saw hadn't any documentation whatever. It's not committable
on documentation grounds alone, even if everybody was satisfied about
the code.well, there is enough documentation to review patch.
Where is there any documentation at all? There are no changes to doc/
at all; no README; and not even a lengthy comment block anywhere that
I saw. Nor did the email in which the patch was submitted clearly lay
out the design of the feature.
In my understanding
this was always enough to submit code. User's documentation is depend on
discussion and review and can be added later
before releasing beta.
Several people have said this lately, but it doesn't match what I've
seen of our practice over the last year and a half; Tom regularly
boots patches that lack documentation (or necessary regression test
updates). Sure, people often submit small patches without
documentation thinking to fill it in later, but anything major pretty
much has to have it, AFAICS. From my own point of view, I would never
commit anything that lacked documentation, for fear of being asked to
write it myself if the patch author didn't. Of course it's a bit
different for committers, who can presumably be counted on to clean up
their own mess, but I still think it's fair to expect at least some
effort to be put into the docs before commit.
...Robert
Robert Haas <robertmhaas@gmail.com> writes:
It seems that you're sort of frustrated with the system and the need
to go through a process before committing a patch;
I've been handling arround here for years (since 2005 or before) and I
think there always was a process. The only change is it's getting more
and more formal. But still not any clearer.
It's good to try to keep the major releases one year apart, but until
now we had some flexibility towards developpers. They could have their
own agenda then appear with a big patch and it was getting considered.
We never asked contributors to arrange for being able to find a
sponsor, do the closed source version, prepare for publishing, and then
send a patch in a timely maneer so that to ease the integration and
release.
Before there was a Commit Fest process, we took some weeks then months
at the end of the cycle to consider what had been accumulated.
The new process is there for giving more feedback to developpers, and is
being considered now as a way to get better control about the release
agenda. I'm not sure it's a good tool for that. I'm not sure insisting
that much on the release schedule is a good idea.
Once more making compromises is easy. What's hard and challenging is
making *good* compromises.
IMHO, our system has to work for both developers and users, and it has
to work for both committers and non-committers.
That's an easy goal to share. The question is how you get there without
losing existing developpers and possibly attracting new developpers on
the way.
--
dim
On Thu, Feb 11, 2010 at 1:18 PM, Robert Haas <robertmhaas@gmail.com> wrote:
In my understanding
this was always enough to submit code. User's documentation is depend on
discussion and review and can be added later
before releasing beta.Several people have said this lately, but it doesn't match what I've
seen of our practice over the last year and a half;
Perhaps the confusion is that we often say not to worry about the
quality of the English in the documentation. That's because it's easy
for a reviewer to fix up the English but not so easy to figure out
what you intend the behaviour to be.
--
greg
On Thu, 11 Feb 2010, Robert Haas wrote:
On Thu, Feb 11, 2010 at 3:00 AM, Oleg Bartunov <oleg@sai.msu.su> wrote:
version I saw hadn't any documentation whatever. It's not committable
on documentation grounds alone, even if everybody was satisfied about
the code.well, there is enough documentation to review patch.
Where is there any documentation at all? There are no changes to doc/
at all; no README; and not even a lengthy comment block anywhere that
I saw. Nor did the email in which the patch was submitted clearly lay
out the design of the feature.
Well, initial knngist announce
http://archives.postgresql.org/pgsql-hackers/2009-11/msg01547.php
isn't enough to review ? We made test data available to reproduce
results, see http://www.sai.msu.su/~megera/wiki/2009-11-25
We are here and open to any reviewer's question.
In my understanding
this was always enough to submit code. User's documentation is depend on
discussion and review and can be added later
before releasing beta.Several people have said this lately, but it doesn't match what I've
seen of our practice over the last year and a half; Tom regularly
boots patches that lack documentation (or necessary regression test
updates). Sure, people often submit small patches without
documentation thinking to fill it in later, but anything major pretty
much has to have it, AFAICS. From my own point of view, I would never
commit anything that lacked documentation, for fear of being asked to
write it myself if the patch author didn't. Of course it's a bit
different for committers, who can presumably be counted on to clean up
their own mess, but I still think it's fair to expect at least some
effort to be put into the docs before commit.
I think nobody will spend his time to write sgml code for user's
documentation for fear his patch will be rejected/moved/getting rewritten,
so his time will be just wasted.
Regards,
Oleg
_____________________________________________________________
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83
On Thu, 11 Feb 2010, Greg Stark wrote:
On Thu, Feb 11, 2010 at 1:18 PM, Robert Haas <robertmhaas@gmail.com> wrote:
In my understanding
this was always enough to submit code. User's documentation is depend on
discussion and review and can be added later
before releasing beta.Several people have said this lately, but it doesn't match what I've
seen of our practice over the last year and a half;Perhaps the confusion is that we often say not to worry about the
quality of the English in the documentation. That's because it's easy
for a reviewer to fix up the English but not so easy to figure out
what you intend the behaviour to be.
English + SGML stuff. We usually provide information in plain text, posted
in -hackers and published in my wiki. I don't remember, that there were
no information about patches.
Regards,
Oleg
_____________________________________________________________
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83
On Thu, 11 Feb 2010, Oleg Bartunov wrote:
On Thu, 11 Feb 2010, Tom Lane wrote:
My own feeling about it is that I much preferred the original proposal
of a contrib module with little or no change to core code. I don't want
to be changing core code for this at this late hour. If it were only
touching GIST I'd be willing to rely on your and Teodor's expertise in
that module, but it's not. It whacks around the planner, it makes
questionable changes in the operator class structure, and the last
We splitted patch to make review easy, probably by several reviewers,
since we touched several subsystems.
http://archives.postgresql.org/message-id/4B4CCB9F.8080708@sigaev.ru
Patch for planner is 5600 bytes long, not so big.
aha, we originally submit contrib module, which didn't touch anything you
mentioned, we improve stuff to follow discussion and now we are out of luck
%(version I saw hadn't any documentation whatever. It's not committable
on documentation grounds alone, even if everybody was satisfied about
the code.well, there is enough documentation to review patch. In my understanding this
was always enough to submit code. User's documentation is depend on
discussion and review and can be added later
before releasing beta.How do you feel about going back to the original contrib module for now
and resubmitting the builtin version for 9.1?Hmm, one good thing is that rbtree seems ok for submisson. We need to discuss
this, if it's good for PostGIS community. I'd not complain about this
decision
if it touch my interests only, I could live with closed-source patch.
Contrib module is a big step backward and will produce compatibility problem,
since we'll have to use awkward operation ><, which works different
with/without index. Also, it'd be very difficult to add support to other
contrib modules (btree_gist, pg_trgm).
Links:
Heikki complaint - http://archives.postgresql.org/message-id/4B0B8C30.2080400@enterprisedb.com
Simon - http://archives.postgresql.org/message-id/1259115190.27757.11194.camel@ebony
Regards,
Oleg
_____________________________________________________________
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Thu, Feb 11, 2010 at 03:19:14PM +0100, Dimitri Fontaine wrote:
Robert Haas <robertmhaas@gmail.com> writes:
It seems that you're sort of frustrated with the system and the need
to go through a process before committing a patch;I've been handling arround here for years (since 2005 or before) and I
think there always was a process. The only change is it's getting more
and more formal. But still not any clearer.
I didn't want to give the impression that I see the process as too
heavy. Granted, knngist is a cool feature, and I'm a big fan of Oleg and
Teodor (since they pulled the hstore trick :)
Still I think Robert is doing a terrific job. The weight of the big
patches has increased considerably in the last years, and without the
heroic efforts of some people (adn Robert is definitely among them!),
PostgreSQL's release process wouldn't have been as smooth.
It's good to try to keep the major releases one year apart, but until
now we had some flexibility towards developpers. They could have their
own agenda then appear with a big patch and it was getting considered.We never asked contributors to arrange for being able to find a
sponsor, do the closed source version, prepare for publishing, and then
send a patch in a timely maneer so that to ease the integration and
release.Before there was a Commit Fest process, we took some weeks then months
at the end of the cycle to consider what had been accumulated.
Yes. But remember HOT. Things seem smoother nowadays (disclaimer: I'm
just a bystander).
The new process is there for giving more feedback to developpers, and is
being considered now as a way to get better control about the release
agenda. I'm not sure it's a good tool for that. I'm not sure insisting
that much on the release schedule is a good idea.
What's your proposal then? Allow more slippage in the release schedule?
How much? When?
Once more making compromises is easy. What's hard and challenging is
making *good* compromises.
Agreed
Regards
- -- tomás
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
iD8DBQFLdHkNBcgs9XrR2kYRAhSsAJ4n899St+aPYkRA7XBX78vF/xZh9wCfZ3T0
h226KRarnFrEEIcOY+zBG9E=
=X9fa
-----END PGP SIGNATURE-----
2010/2/11 Oleg Bartunov <oleg@sai.msu.su>:
On Thu, 11 Feb 2010, Robert Haas wrote:
On Thu, Feb 11, 2010 at 3:00 AM, Oleg Bartunov <oleg@sai.msu.su> wrote:
version I saw hadn't any documentation whatever. It's not committable
on documentation grounds alone, even if everybody was satisfied about
the code.well, there is enough documentation to review patch.
Where is there any documentation at all? There are no changes to doc/
at all; no README; and not even a lengthy comment block anywhere that
I saw. Nor did the email in which the patch was submitted clearly lay
out the design of the feature.Well, initial knngist announce
http://archives.postgresql.org/pgsql-hackers/2009-11/msg01547.php
isn't enough to review ? We made test data available to reproduce results,
see http://www.sai.msu.su/~megera/wiki/2009-11-25
We are here and open to any reviewer's question.
Well, just for example, that doesn't document the changes you made to
the opclass infrastructure, and the reasons for those decisions.
Actually, I've been working on an email on that topic but haven't
gotten around to finishing it. There's some description of the
overall goal of the feature but not much about how you got there. Is
it enough to review? Perhaps, but it would certainly be nice to have
some more discussion of the overall design. IMHO, anyway.
...Robert
On Fri, 12 Feb 2010, Robert Haas wrote:
2010/2/11 Oleg Bartunov <oleg@sai.msu.su>:
On Thu, 11 Feb 2010, Robert Haas wrote:
On Thu, Feb 11, 2010 at 3:00 AM, Oleg Bartunov <oleg@sai.msu.su> wrote:
version I saw hadn't any documentation whatever. =A0It's not committab=
le
on documentation grounds alone, even if everybody was satisfied about
the code.well, there is enough documentation to review patch.
Where is there any documentation at all? =A0There are no changes to doc/
at all; no README; and not even a lengthy comment block anywhere that
I saw. =A0Nor did the email in which the patch was submitted clearly lay
out the design of the feature.Well, initial knngist announce
http://archives.postgresql.org/pgsql-hackers/2009-11/msg01547.php
isn't enough to review ? We made test data available to reproduce results=,
see http://www.sai.msu.su/~megera/wiki/2009-11-25
We are here and open to any reviewer's question.Well, just for example, that doesn't document the changes you made to
the opclass infrastructure, and the reasons for those decisions.
Actually, I've been working on an email on that topic but haven't
gotten around to finishing it. There's some description of the
overall goal of the feature but not much about how you got there. Is
it enough to review? Perhaps, but it would certainly be nice to have
some more discussion of the overall design. IMHO, anyway.
This is not fair,Robert. Everything was discussed in -hackers.I assume reviewer
should follow discussion at least, he is a member of our community. Mailing
list archive was/is/will our the best knowledge base. For example, regarding
changes in the opclass infrastructure you complain, you can see your reply
to Teodor's message http://archives.postgresql.org/message-id/603c8f070912292255u30e1983bi22ed5778bd2ce890@mail.gmail.com
which contains description of amcanorderbyop flag.
Frankly, I think we see here limit of our resources. Let me explain this.
We splitted patch by several parts - 2 parts are about contrib modules
(rather trivial), 1 - is a gist changes, 1 - planner changes and 1 part -
some proc changes. The most serious are gist and planner patches. We develop
GiST for many years and know almost everything there and could say that we're
responsible for GiST. I don't know if anybody from -hackers could review our
patch for planner better than Tom, but he is busy and will be busy.
So, any serious feature, which touch planner doomed to be rejected because of
lack of reviewer.
We tried to find compromise for 9.0 (Tom suggests contrib module), but all
variants are ugly and bring incompatibility in future. If there are no hackers
willing/capable to review our patches, then, please, help us how to save
neighbourhood search without incompatibility in future.
Regards,
Oleg
_____________________________________________________________
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: oleg@sai.msu.su, http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83
On Fri, Feb 12, 2010 at 3:44 PM, Oleg Bartunov <oleg@sai.msu.su> wrote:
This is not fair,Robert. Everything was discussed in -hackers.I assume
reviewer
should follow discussion at least, he is a member of our community. Mailing
list archive was/is/will our the best knowledge base.
Dude, there's no fair or unfair here; I'm just telling you what I
think. There are not six people who follow this mailing list more
closely than I do, and when I started looking at this patch it took me
two hours to figure out why you made those changes to the opclass
machinery. It's not documented anywhere either in the patch or in the
email in which the patch was submitted. That made it hard for me. If
that makes me stupid, then I'm stupid, but then probably there are a
lot of stupid people around here.
For example, regarding
changes in the opclass infrastructure you complain, you can see your reply
to Teodor's message
http://archives.postgresql.org/message-id/603c8f070912292255u30e1983bi22ed5778bd2ce890@mail.gmail.com
which contains description of amcanorderbyop flag.
OK, so in one email message that is not the email in which you
submitted the patch there is one sentence of explanation that I failed
to remember six weeks later when I looked at the patch.
Frankly, I think we see here limit of our resources. Let me explain this.
We splitted patch by several parts - 2 parts are about contrib modules
(rather trivial), 1 - is a gist changes, 1 - planner changes and 1 part -
some proc changes. The most serious are gist and planner patches. We develop
GiST for many years and know almost everything there and could say that
we're
responsible for GiST. I don't know if anybody from -hackers could review our
patch for planner better than Tom, but he is busy and will be busy.
So, any serious feature, which touch planner doomed to be rejected because
of
lack of reviewer.
I do think resource limitations play a role. For at least as long as
I have been involved in the community, we have relied very heavily on
Tom Lane to review nearly every patch and commit more than half of
them. As our group of developers grows, that is unsustainable, which
I believe to be part of the reason that -core just created four new
committers; as well as part of the reason for the CommitFest process,
which tries to enlist non-committer reviewers. But none of us are Tom
Lane. The part of your patch that took me two hours to figure out
probably would have taken Tom twenty minutes (maybe less). But even
if we assume that I am as smart as Tom Lane and that I will spend as
much time working on PostgreSQL as Tom Lane, both of which may well be
false, I won't know the code base as well as he does now until ~2020.
The only way we can get past that is, first, by splitting up the work
across as many people as possible, and second, by making it as easy as
possible for the reviewer to understand what the code is about. And
at least if the reviewer is me, *documentation helps*.
I'm going to respond to the part of this email that's about moving
this patch forward with a separate email.
...Robert
On Fri, Feb 12, 2010 at 3:44 PM, Oleg Bartunov <oleg@sai.msu.su> wrote:
We tried to find compromise for 9.0 (Tom suggests contrib module), but all
variants are ugly and bring incompatibility in future. If there are no
hackers
willing/capable to review our patches, then, please, help us how to save
neighbourhood search without incompatibility in future.
Here's what I've gathered from my read through this patch. Let me
know if it's right:
In CVS HEAD, if an AM is marked amcanorder, that means that the index
defines some kind of intrinsic order over the tuples so that, from a
given starting point, it makes sense to talk about scanning either
forward or backward. GIST indices don't have an intrinsic notion of
ordering, but the structure of the index does lend itself to finding
index tuples that are "nearby" to some specified point. So this patch
wants to introduce the notion of an AM that is marked amcanorderbyop
to accelerate queries that order by <indexed column> <op> <constant>.
To do that, we need some way of recognizing which operators are
candidates for this optimization. This patch implements that by
putting the relevant operators into the operator class. That requires
relaxing the rule that operator class operators must return bool; so
instead when the AM is marked amcanorderbyop we allow the operator
class operators to return any type with a default btree operator
class.
Does that sound right?
Tom remarked in another email that he wasn't too happy with the
opclass changes. They seem kind of grotty to me, too, but I don't
immediately have a better idea. My fear is that there may be places
in the code that rely on opclass operators only ever returning bool,
and that changing that may break things. It also feels like allowing
non-bool-returning opclass members only for this one specific case is
kind of a hack: is this an instance of some more general problem that
we ought to be solving in some more general way? Not sure.
In any case, it seems to me that figuring out how we're going to solve
the problem of marking the operators (or otherwise identifying the
expression trees) that are index-optimizable is the key issue for this
patch. If we can agree on that, the rest should be a SMOP.
...Robert