Thread: btree_gist (was: CommitFest progress - or lack thereof)
On Fri, Feb 4, 2011 at 12:02 PM, Oleg Bartunov <oleg@sai.msu.su> wrote: > Aha, > > Teodor sent it to the list Dec 28, see > http://archives.postgresql.org/message-id/4D1A1677.80300%40sigaev.ru > > After a month I didn't see any activity on this patch, so I I added it to > https://commitfest.postgresql.org/action/patch_view?id=350 Jan 21 > > Now, I realised it was too late. Added to current commitfest. I think this patch missed the deadline for the current CommitFest. It's true that it was posted to the list in time, but it's just madness to think we can do review in a meaningful way and get done in a reasonable time if every patch that's ever been posted is fair game to be added to the CommitFest at any point. I believe it's a generally accepted principle that adding things to the CommitFest properly is the submitter's responsibility. That having been said, this looks like a fairly mechanical change to a contrib module that you and Teodor wrote. So I'd say if you guys are confident that it's correct, go ahead and commit. If you need it reviewed, or if you can't commit it in the next week or so, I think it's going to have to wait for 9.2. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Fri, Feb 4, 2011 at 12:02 PM, Oleg Bartunov <oleg@sai.msu.su> wrote: > > Aha, > > > > Teodor sent it to the list Dec 28, see > > http://archives.postgresql.org/message-id/4D1A1677.80300%40sigaev.ru > > > > After a month I didn't see any activity on this patch, so I I added it to > > https://commitfest.postgresql.org/action/patch_view?id=350 Jan 21 > > > > Now, I realised it was too late. Added to current commitfest. > > I think this patch missed the deadline for the current CommitFest. > It's true that it was posted to the list in time, but it's just > madness to think we can do review in a meaningful way and get done in > a reasonable time if every patch that's ever been posted is fair game > to be added to the CommitFest at any point. I believe it's a > generally accepted principle that adding things to the CommitFest > properly is the submitter's responsibility. > > That having been said, this looks like a fairly mechanical change to a > contrib module that you and Teodor wrote. So I'd say if you guys are > confident that it's correct, go ahead and commit. If you need it > reviewed, or if you can't commit it in the next week or so, I think > it's going to have to wait for 9.2. If you need a +1, agreed. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
* Robert Haas (robertmhaas@gmail.com) wrote: > > Teodor sent it to the list Dec 28, see > > http://archives.postgresql.org/message-id/4D1A1677.80300%40sigaev.ru [...] > That having been said, this looks like a fairly mechanical change to a > contrib module that you and Teodor wrote. So I'd say if you guys are > confident that it's correct, go ahead and commit. If you need it > reviewed, or if you can't commit it in the next week or so, I think > it's going to have to wait for 9.2. Alright, I've gone through this patch and the main thing it's missing is documentation, as far as I can tell. It passes all the regression tests (and adds a number of them which are then tested with, which is always nice) and while there are quite a few changes, they're all pretty mechanical and simple. There are some really minor whitespace issues too, but overall I think this is ready to be committed, so long as we have a promise that someone will write up the documentation for it. I'd write the docs, but I'm not 100% sure that I know what's going on enough to really write them correctly. :) I'm also hoping that someone else is already working on them. If not, feel free to ping me and I'll work on writing up *something*, at least.Thanks, Stephen
Stephen, what do you need for documentation ? From users point of view we add just knn support and all examples are available in btree_gist.sql and sql subdirectory. Contact me directly, if you have questions. Oleg On Fri, 11 Feb 2011, Stephen Frost wrote: > * Robert Haas (robertmhaas@gmail.com) wrote: >>> Teodor sent it to the list Dec 28, see >>> http://archives.postgresql.org/message-id/4D1A1677.80300%40sigaev.ru > [...] >> That having been said, this looks like a fairly mechanical change to a >> contrib module that you and Teodor wrote. So I'd say if you guys are >> confident that it's correct, go ahead and commit. If you need it >> reviewed, or if you can't commit it in the next week or so, I think >> it's going to have to wait for 9.2. > > Alright, I've gone through this patch and the main thing it's missing is > documentation, as far as I can tell. It passes all the regression tests > (and adds a number of them which are then tested with, which is always > nice) and while there are quite a few changes, they're all pretty > mechanical and simple. There are some really minor whitespace issues > too, but overall I think this is ready to be committed, so long as we > have a promise that someone will write up the documentation for it. > > I'd write the docs, but I'm not 100% sure that I know what's going on > enough to really write them correctly. :) I'm also hoping that someone > else is already working on them. If not, feel free to ping me and I'll > work on writing up *something*, at least. > > Thanks, > > Stephen > 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, * Oleg Bartunov (oleg@sai.msu.su) wrote: > what do you need for documentation ? From users point of view we add just > knn support and all examples are available in btree_gist.sql and sql > subdirectory. Contact me directly, if you have questions. It sure seems like http://www.postgresql.org/docs/9.0/static/btree-gist.html could be and should be improved, in general.. If this module is really still just a test bed for GiST, then perhaps it's not a big deal.. Thanks, Stephen
On Sat, Feb 12, 2011 at 7:47 AM, Stephen Frost <sfrost@snowman.net> wrote: > Oleg, > > * Oleg Bartunov (oleg@sai.msu.su) wrote: >> what do you need for documentation ? From users point of view we add just >> knn support and all examples are available in btree_gist.sql and sql >> subdirectory. Contact me directly, if you have questions. > > It sure seems like > http://www.postgresql.org/docs/9.0/static/btree-gist.html could be and > should be improved, in general.. If this module is really still just a > test bed for GiST, then perhaps it's not a big deal.. I agree that the documentation there could be a lot better, but I don't think that's a commit-blocker for this patch. However, "us reaching beta" will be a commit-blocker. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Stephen, On Sat, 12 Feb 2011, Stephen Frost wrote: > Oleg, > > * Oleg Bartunov (oleg@sai.msu.su) wrote: >> what do you need for documentation ? From users point of view we add just >> knn support and all examples are available in btree_gist.sql and sql >> subdirectory. Contact me directly, if you have questions. > > It sure seems like > http://www.postgresql.org/docs/9.0/static/btree-gist.html could be and > should be improved, in general.. If this module is really still just a > test bed for GiST, then perhaps it's not a big deal.. No, it's quite useful and used in many projects, since it's the only way to create multicolumn gist indexes like (tsvector,date). 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 Sat, 2011-02-12 at 18:53 +0300, Oleg Bartunov wrote: > > It sure seems like > > http://www.postgresql.org/docs/9.0/static/btree-gist.html could be and > > should be improved, in general.. If this module is really still just a > > test bed for GiST, then perhaps it's not a big deal.. > > No, it's quite useful and used in many projects, since it's the only way > to create multicolumn gist indexes like (tsvector,date). +1 btree_gist is essentially required for exclusion constraints to be useful in a practical way. In fact, can you submit it for the next commitfest to be included in core? That would allow range types and exclusion constraints to be used out-of-the-box in 9.2. Only if you think it's reasonable to put it in core, of course. If extensions are easy enough to install, maybe that's not really necessary. Regards,Jeff Davis
Jeff Davis <pgsql@j-davis.com> writes: > btree_gist is essentially required for exclusion constraints to be > useful in a practical way. > In fact, can you submit it for the next commitfest to be included in > core? That would allow range types and exclusion constraints to be used > out-of-the-box in 9.2. > Only if you think it's reasonable to put it in core, of course. If > extensions are easy enough to install, maybe that's not really > necessary. btree_gist is entirely unready to be included in core. http://archives.postgresql.org/pgsql-bugs/2010-10/msg00104.php regards, tom lane
Oops, thank for remind ! Oleg On Sat, 12 Feb 2011, Tom Lane wrote: > Jeff Davis <pgsql@j-davis.com> writes: >> btree_gist is essentially required for exclusion constraints to be >> useful in a practical way. > >> In fact, can you submit it for the next commitfest to be included in >> core? That would allow range types and exclusion constraints to be used >> out-of-the-box in 9.2. > >> Only if you think it's reasonable to put it in core, of course. If >> extensions are easy enough to install, maybe that's not really >> necessary. > > btree_gist is entirely unready to be included in core. > http://archives.postgresql.org/pgsql-bugs/2010-10/msg00104.php > > regards, tom lane > > 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 Sat, Feb 12, 2011 at 8:19 AM, Robert Haas <robertmhaas@gmail.com> wrote: > On Sat, Feb 12, 2011 at 7:47 AM, Stephen Frost <sfrost@snowman.net> wrote: >> Oleg, >> >> * Oleg Bartunov (oleg@sai.msu.su) wrote: >>> what do you need for documentation ? From users point of view we add just >>> knn support and all examples are available in btree_gist.sql and sql >>> subdirectory. Contact me directly, if you have questions. >> >> It sure seems like >> http://www.postgresql.org/docs/9.0/static/btree-gist.html could be and >> should be improved, in general.. If this module is really still just a >> test bed for GiST, then perhaps it's not a big deal.. > > I agree that the documentation there could be a lot better, but I > don't think that's a commit-blocker for this patch. However, "us > reaching beta" will be a commit-blocker. Teodor, are you intending to commit this? If so, it needs to be soon. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
We need to fix inet support as Tom complained and then will commit. Oleg On Wed, 16 Feb 2011, Robert Haas wrote: > On Sat, Feb 12, 2011 at 8:19 AM, Robert Haas <robertmhaas@gmail.com> wrote: >> On Sat, Feb 12, 2011 at 7:47 AM, Stephen Frost <sfrost@snowman.net> wrote: >>> Oleg, >>> >>> * Oleg Bartunov (oleg@sai.msu.su) wrote: >>>> what do you need for documentation ? From users point of view we add just >>>> knn support and all examples are available in btree_gist.sql and sql >>>> subdirectory. Contact me directly, if you have questions. >>> >>> It sure seems like >>> http://www.postgresql.org/docs/9.0/static/btree-gist.html could be and >>> should be improved, in general.. If this module is really still just a >>> test bed for GiST, then perhaps it's not a big deal.. >> >> I agree that the documentation there could be a lot better, but I >> don't think that's a commit-blocker for this patch. However, "us >> reaching beta" will be a commit-blocker. > > Teodor, are you intending to commit this? If so, it needs to be soon. > > 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
2011/2/17 Oleg Bartunov <oleg@sai.msu.su>: > We need to fix inet support as Tom complained and then will commit. I think those are two separate issues. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> 2011/2/17 Oleg Bartunov <oleg@sai.msu.su>:
>> We need to fix inet support as Tom complained and then will commit.
> I think those are two separate issues.
Yeah ... also, fixing the inet problem looked to me like it'd probably
be a major compatibility break (change in on-disk index contents).
Not something to rush into at the end of a development cycle.
In any case, I was pointing to that as a reason that btree_gist wasn't
ready to be in core. It has nothing to do with KNN-ifying the module.
I would like to see that happen before 9.1, else KNN will go out with
not very many actual use-cases supported.
regards, tom lane
I wrote:
> In any case, I was pointing to that as a reason that btree_gist wasn't
> ready to be in core. It has nothing to do with KNN-ifying the module.
> I would like to see that happen before 9.1, else KNN will go out with
> not very many actual use-cases supported.
However, a larger reason for not applying that patch in a rush is that
it's almost certainly not up to speed for the recent extensions-related
changes. Please note in particular that we are now expecting the
update-from-unpackaged script to produce the same catalog state as the
install script. I just committed fixes to btree_gist's scripts to make
that actually true. Any new additions to the opclasses will need to be
done in the same style.
regards, tom lane