Thread: Triaging the remaining open commitfest items
Looking at what remains open in the current commitfest:
* fsync $PGDATA recursively at startup
Andres is the reviewer of record on this one. He should either commit it
if he feels it's ready, or bounce it to next CF if not.
* EvalPlanQual behaves oddly for FDW queries involving system columns
I've been working this one and will deal with any mop-up needed, but
I think probably what ought to happen now is just to commit the small
ctid-transmission hack I posted yesterday and call it good.
* BRIN inclusion operator class
This is Alvaro's turf, obviously.
* PageRepairFragmentation optimization
Heikki's patch, so his to commit or not, though since it's marked
Waiting on Author I'm guessing it's not ready?
* Abbreviated key support for Datum sorts
I've been assuming Robert would either commit this or not, since he's
been the committer for the predecessor patches.
* KNN-GiST with recheck
Heikki's been dealing with this thread so far, and is surely the best
qualified to decide if it's ready or not.
* GIN fillfactor
I'd like to put this one on Heikki's plate as well, since he's touched
the GIN code more than anyone else lately.
* Manipulating complex types as non-contiguous structures in-memory
This one's mine of course. I've been hoping to get more independent
performance testing than it's gotten, but time grows short. I'm inclined
to just go ahead and push it in.
* Optimization for updating foreign tables in Postgres FDW
I concur with Stephen's assessment that this doesn't look well designed.
I think we should just mark it RWF for now.
* iteration over record in plpgsql
I fear this one slipped through the cracks --- it's marked Waiting on
Author in the CF app, but it looks like Pavel did submit an updated
version after that marking was made. However, it's not a terribly
significant feature and there was doubt as to whether we want it at
all anyway. Suggest we just punt it to next CF at this point.
* Sequence Access Method
Heikki's marked as reviewer, so it's his call as to whether this is ready,
but the impression I have is that there's not really consensus as to
whether the API is good. If not, it's something I think we should push
to 9.6.
* archive_mode=shared/always
Heikki's patch, so his call.
* Sending WAL receiver feedback regularly even if the receiver is under heavy load
This seems to not be ready, but it also seems to be a bug fix, so when
it is ready I think we could commit it.
* Auditing extension
I do not get the impression that there is consensus on this. Push to 9.6?
* ctidscan as an example of custom-scan
This basically hasn't gotten any attention, which may mean nobody cares
enough to justify putting it in the tree. We need to either push it to
next CF or reject altogether.
* parallel mode/contexts
Robert's patch, his to deal with (likewise for "assessing parallel-safety").
* RLS: row-level security, more review
Stephen's baby.
* Cube extension kNN support
This is still marked as "needs review", I'm afraid we have to push to 9.6.
* compress method for spgist
* Join pushdown support for foreign tables
There doesn't seem to be any current patch linked to this CF item.
If there is a patch to get postgres_fdw to make use of the recently
committed join-path support, I assume it's in need of a rebase anyway.
* Grouping Sets
I had originally promised to be committer for this one, and still want
to go look at it, but Robert's nearby message about not committing stuff
in haste definitely seems to apply.
* TABLESAMPLE clause
I assume we'd better push this to 9.6 at this point.
* REINDEX xxx VERBOSE
Seems like we just need somebody to make a decision on syntax.
* Additional role attributes
Is this ready to commit? Stephen's call.
* catalog view to pg_hba.conf file
Greg Stark is marked as committer of record on this.
* Add pg_settings.pending_restart column
Peter's patch, his call.
So there you have it. If everyone would go make decisions on the patches
that they are the most obvious committer for, we could get those taken
care of one way or the other pretty quickly. As for the ones I proposed
pushing to 9.6, any committer who feels differently can pick those up,
else I'll go change their status in the CF app tomorrow or so.
regards, tom lane
I wrote:
> * Cube extension kNN support
> This is still marked as "needs review", I'm afraid we have to push to 9.6.
> * compress method for spgist
Sigh. There was a "Ditto" under this one, but somehow it disappeared
in editing :-(
regards, tom lane
On Wed, May 13, 2015 at 11:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > * fsync $PGDATA recursively at startup > > Andres is the reviewer of record on this one. He should either commit it > if he feels it's ready, or bounce it to next CF if not. I committed the first part of this as 2ce439f3379aed857517c8ce207485655000fc8e. I think that we do not have design consensus on the rest. I think we should mark this committed, and if the folks who proposed the further work here still want to press their case, that should wait for 9.6. > * Abbreviated key support for Datum sorts > > I've been assuming Robert would either commit this or not, since he's > been the committer for the predecessor patches. I'll deal with this. > * Sequence Access Method > > Heikki's marked as reviewer, so it's his call as to whether this is ready, > but the impression I have is that there's not really consensus as to > whether the API is good. If not, it's something I think we should push > to 9.6. I share your concern on this one. > * ctidscan as an example of custom-scan > > This basically hasn't gotten any attention, which may mean nobody cares > enough to justify putting it in the tree. We need to either push it to > next CF or reject altogether. Agreed. I was fine with never committing this. I don't think we have a requirement that every hook or bit of functionality we expose at the C level must have an example in core. But other people (you? Simon?) seemed to want a demonstration in the core repository. If that's still a priority, I am willing to work on it more for 9.6, but there is not time now. > * parallel mode/contexts > > Robert's patch, his to deal with (likewise for "assessing parallel-safety"). Most of the parallel mode stuff is committed. What's left will have to wait for 9.6. Assessing parallel-safety will also need to wait for 9.6. > * Join pushdown support for foreign tables > > There doesn't seem to be any current patch linked to this CF item. > If there is a patch to get postgres_fdw to make use of the recently > committed join-path support, I assume it's in need of a rebase anyway. I think there is a rebased patch around. I think it's just not linked into the CF thread. I don't think it's committable as is. > * Grouping Sets > > I had originally promised to be committer for this one, and still want > to go look at it, but Robert's nearby message about not committing stuff > in haste definitely seems to apply. I really feel we didn't give this a fair shake. I'm not saying we should make up for that by committing it in haste, but not giving things a fair shake is bad for the project regardless of anything else. > * TABLESAMPLE clause > > I assume we'd better push this to 9.6 at this point. +1 > * REINDEX xxx VERBOSE > > Seems like we just need somebody to make a decision on syntax. I just posted a review of this raising minor points only. If it is timely revised, I will commit it. > * Additional role attributes > > Is this ready to commit? Stephen's call. -1 for committing this, per discussion earlier today on a thread that's probably not linked into the CF app. > * catalog view to pg_hba.conf file > > Greg Stark is marked as committer of record on this. I am doubtful whether this is ready. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2015-05-13 11:38:27 -0400, Tom Lane wrote: > Looking at what remains open in the current commitfest: > > * fsync $PGDATA recursively at startup > > Andres is the reviewer of record on this one. He should either commit it > if he feels it's ready, or bounce it to next CF if not. The more important part of this has been committed by Robert. The other part, while also important in my opinion, has by now slipped 9.5. I've moved it. > * Manipulating complex types as non-contiguous structures in-memory > > This one's mine of course. I've been hoping to get more independent > performance testing than it's gotten, but time grows short. I'm inclined > to just go ahead and push it in. I'm a bit hesitant about performance regressions around it. And I'd obviously rather not see the macros but the inline version ;). But I think overall we're in a better position with it, than without. If it turns out to have bad edge cases performancewise, we can still "turn it off" in plpgsql without much problems. If we, preferrably, can't find a better solution for the performance problem. > * Grouping Sets > > I had originally promised to be committer for this one, and still want > to go look at it, but Robert's nearby message about not committing stuff > in haste definitely seems to apply. This has been in a limbo for a very long time. I'm right now working with Andrew getting it into a a committable shape. I'm not 100% sure we'll succeed for 9.5, but it deserves a chance. If it doesn't make it into 9.6, I plan to get into 9.6 soon after the branch opens. > * Additional role attributes > > Is this ready to commit? Stephen's call. Not yet ready in my opinion. Greetings, Andres Freund
Tom, all,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> * Optimization for updating foreign tables in Postgres FDW
>
> I concur with Stephen's assessment that this doesn't look well designed.
> I think we should just mark it RWF for now.
I had meant to do that already, sorry about that, done now.
> * iteration over record in plpgsql
>
> I fear this one slipped through the cracks --- it's marked Waiting on
> Author in the CF app, but it looks like Pavel did submit an updated
> version after that marking was made. However, it's not a terribly
> significant feature and there was doubt as to whether we want it at
> all anyway. Suggest we just punt it to next CF at this point.
I had been interested but also thought it was waiting for a new
version. Doesn't look like I'll have time now.
> * Auditing extension
>
> I do not get the impression that there is consensus on this. Push to 9.6?
I've not seen much disagreement about what it provides recently, those
were dealt with a while ago. I agree with Robert that it needs more
work on documentation and comments and I've sent my thoughts about what
to improve in those areas over to David. I've reviewed it in various
forms and am hoping to commit it, unless there are objections.
> * RLS: row-level security, more review
>
> Stephen's baby.
I don't have anything pending on this at the moment. Post
feature-freeze I'm planning to spend time on bug hunting, documentation
improvements, etc, for this, UPSERT, and other things. If anyone is
aware of any outstanding issues here, please let me know.
> * Additional role attributes
>
> Is this ready to commit? Stephen's call.
I'm pretty happy with the latest version. Would be great for others to
weigh in on if they have any concerns about reserving the 'pg_' prefix
for system roles (or if they're fine with it, that'd be useful to know
too..). I'll also be improving the documentation for it.
> * catalog view to pg_hba.conf file
>
> Greg Stark is marked as committer of record on this.
I was hoping to look at that also, as I do think it'd be valuable to
have. The current patch needs rework though. I agree with Peter that
using "keyword_*" arrays is not a good approach and that it'd really be
better to have this in conjunction with a function that users could use
to see what row is returned. I might have time to work on it tomorrow,
if other things fall into place, but it's not going to be without
changes and perhaps that means it has to punt to 9.6.
> So there you have it. If everyone would go make decisions on the patches
> that they are the most obvious committer for, we could get those taken
> care of one way or the other pretty quickly. As for the ones I proposed
> pushing to 9.6, any committer who feels differently can pick those up,
> else I'll go change their status in the CF app tomorrow or so.
Done, for my part.
Thanks!
Stephen
Andres Freund <andres@anarazel.de> writes:
> On 2015-05-13 11:38:27 -0400, Tom Lane wrote:
>> * Manipulating complex types as non-contiguous structures in-memory
>>
>> This one's mine of course. I've been hoping to get more independent
>> performance testing than it's gotten, but time grows short. I'm inclined
>> to just go ahead and push it in.
> I'm a bit hesitant about performance regressions around it. And I'd
> obviously rather not see the macros but the inline version ;). But I
> think overall we're in a better position with it, than without. If it
> turns out to have bad edge cases performancewise, we can still "turn it
> off" in plpgsql without much problems. If we, preferrably, can't find a
> better solution for the performance problem.
Right, I should have said "absorb Andres' input and then commit". What
I wanted to know was whether there would be objections to committing
this at all.
regards, tom lane
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Wed, May 13, 2015 at 11:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > * Additional role attributes
> >
> > Is this ready to commit? Stephen's call.
>
> -1 for committing this, per discussion earlier today on a thread
> that's probably not linked into the CF app.
Linked it into the CF, sorry for not doing so earlier.
I'm not going to push it over objections, but I'm certainly hopeful that
there will be further discussion on that thread. For my part, at least,
it's a really nice capability to have that we've been missing for a long
time- I can remember back to the first time I was setting up Nagios and
wondering how in the world I could monitor the system without giving the
Nagios user full superuser rights. I know there are instances out in
the wild that I've set up which are still hacked up to deal with this
lack of capability.
Further, if we don't have something like this, then I'm worried about
people who use the XLOG functions today for monitoring, as we'd end up
having to lock those down to superuser-only, since there isn't anything
between 'public' and 'superuser'.
Thanks!
Stephen
On 05/13/2015 11:38 AM, Tom Lane wrote: > * Grouping Sets > > I had originally promised to be committer for this one, and still want > to go look at it, but Robert's nearby message about not committing stuff > in haste definitely seems to apply. > > That makes me sad. I wish you would still try. Sadly, my plate is full with other stuff, the last few days absorbed more than my available time. cheers andrew
On 13/05/15 17:38, Tom Lane wrote: > > * Sequence Access Method > > Heikki's marked as reviewer, so it's his call as to whether this is ready, > but the impression I have is that there's not really consensus as to > whether the API is good. If not, it's something I think we should push > to 9.6. > Heikki said he won't have time for this one before freeze so I guess it can be pushed to 9.6. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 2015-05-13 11:38:27 -0400, Tom Lane wrote: > * parallel mode/contexts > > Robert's patch, his to deal with (likewise for "assessing parallel-safety"). Just as a note, a large part of this has been committed.
Andres Freund <andres@anarazel.de> writes:
> On 2015-05-13 11:38:27 -0400, Tom Lane wrote:
>> * parallel mode/contexts
>>
>> Robert's patch, his to deal with (likewise for "assessing parallel-safety").
> Just as a note, a large part of this has been committed.
Right, and Robert commented that he isn't planning to do more here
for 9.5, so probably these CF items should be closed as committed?
regards, tom lane
On Wed, May 13, 2015 at 1:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Andres Freund <andres@anarazel.de> writes: >> On 2015-05-13 11:38:27 -0400, Tom Lane wrote: >>> * parallel mode/contexts >>> >>> Robert's patch, his to deal with (likewise for "assessing parallel-safety"). > >> Just as a note, a large part of this has been committed. > > Right, and Robert commented that he isn't planning to do more here > for 9.5, so probably these CF items should be closed as committed? Already done. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
> > * ctidscan as an example of custom-scan
> >
> > This basically hasn't gotten any attention, which may mean nobody cares
> > enough to justify putting it in the tree. We need to either push it to
> > next CF or reject altogether.
>
> Agreed. I was fine with never committing this. I don't think we have
> a requirement that every hook or bit of functionality we expose at the
> C level must have an example in core. But other people (you? Simon?)
> seemed to want a demonstration in the core repository. If that's
> still a priority, I am willing to work on it more for 9.6, but there
> is not time now.
>
If no other people required it again, I don't think this module should
be kept in core and also I'm not favor to push ctidscan to v9.6 development
cycle. It intends to demonstrate custom-scan interface, however, it is
not certain an example always needs to be in-core.
If we split the patch partially, definition below makes sense to implement
out-of-core example module
+#define TIDNotEqualOperator 402DATA(insert OID = 2799 ( "<" PGNSP PGUID b f f 27 27 16
2800DESCR("lessthan");#define TIDLessOperator 2799DATA(insert OID = 2800 ( ">" PGNSP PGUID b f f 27 27
16 2799DESCR("greater than");
+#define TIDGreaterOperator 2800DATA(insert OID = 2801 ( "<=" PGNSP PGUID b f f 27 27 16
2802DESCR("lessthan or equal");
+#define TIDLessEqualOperator 2801DATA(insert OID = 2802 ( ">=" PGNSP PGUID b f f 27 27 16
2801DESCR("greaterthan or equal");
+#define TIDGreaterEqualOperator 2802
> > * Join pushdown support for foreign tables
> >
> > There doesn't seem to be any current patch linked to this CF item.
> > If there is a patch to get postgres_fdw to make use of the recently
> > committed join-path support, I assume it's in need of a rebase anyway.
>
> I think there is a rebased patch around. I think it's just not linked
> into the CF thread. I don't think it's committable as is.
>
I believe he is working to follow up the latest foreign/custom-join
interface on which postgres_fdw expected. One point we like to clarify
is how create_plan_recurse() shall be dealt with.
As Hanada-san posted yesterday, postgres_fdw internally uses
create_plan_recurse() to fetch SQL statement associated with inner /
outer sub-plan, so it will take additional adjustment work even
though he already begin to work.
| IMO it isn't necessary to apply the change onto
| ForeignPath/ForeignScan. FDW would have a way to keep-and-use sub
| paths as private information. In fact, I wrote postgres_fdw to use
| create_plan_recurse to generate SQL statements of inner/outer
| relations to be joined, but as of now SQL construction for join
| push-down is accomplished by calling private function deparseSelectSql
| recursively at the top of a join tree.
Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei <kaigai@ak.jp.nec.com>
Andrew Dunstan <andrew@dunslane.net> writes:
> On 05/13/2015 11:38 AM, Tom Lane wrote:
>> * Grouping Sets
>>
>> I had originally promised to be committer for this one, and still want
>> to go look at it, but Robert's nearby message about not committing stuff
>> in haste definitely seems to apply.
> That makes me sad. I wish you would still try.
FWIW, I did go look at this patch, and concluded it was not close enough
to ready to try to squeeze it in now. (I think Andres isn't convinced
of that yet, but time grows short, and I quite agree with Robert that
committing almost-ready patches at this stage of the game is a bad idea.)
The good news on this front is that Salesforce has recently taken an
interest in having GROUPING SETS capability, so I should be able to
find more time to work on this over the next month or two. What I am
now hoping for is to work it over and have something ready to push as
soon as the 9.6 branch opens.
What I intend to spend my time on over the next day or so is fixing
the GB18030 conversion problem (bug #12845), which looks like a small
enough task to finish before feature freeze.
regards, tom lane
On Thu, May 14, 2015 at 05:10:29PM -0400, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > > On 05/13/2015 11:38 AM, Tom Lane wrote: > >> * Grouping Sets > >> > >> I had originally promised to be committer for this one, and still want > >> to go look at it, but Robert's nearby message about not committing stuff > >> in haste definitely seems to apply. > > > That makes me sad. I wish you would still try. > > FWIW, I did go look at this patch, and concluded it was not close enough > to ready to try to squeeze it in now. (I think Andres isn't convinced > of that yet, but time grows short, and I quite agree with Robert that > committing almost-ready patches at this stage of the game is a bad idea.) > > The good news on this front is that Salesforce has recently taken an > interest in having GROUPING SETS capability, so I should be able to > find more time to work on this over the next month or two. What I am > now hoping for is to work it over and have something ready to push as > soon as the 9.6 branch opens. > > What I intend to spend my time on over the next day or so is fixing > the GB18030 conversion problem (bug #12845), which looks like a small > enough task to finish before feature freeze. So you claim the item on the commitfest in the Fall, which effectively prevents other committers from getting involved, then two days before the freeze you encourage others to work on it, and a day before the freeze you say it is too late to apply? And now, all of a sudden, you are interested in working on this because your employer is interested? How do I measure the amount of unfairness here? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
On 2015-05-14 17:10:29 -0400, Tom Lane wrote: > FWIW, I did go look at this patch, and concluded it was not close enough > to ready to try to squeeze it in now. (I think Andres isn't convinced > of that yet, but time grows short, and I quite agree with Robert that > committing almost-ready patches at this stage of the game is a bad idea.) I think if there's any patch that deserves some leeway it's this one. It's been been forced into a limbo for nearly half a year; without leaving Andrew many options. I've removed the use of GroupedVars and Andrew is right now working on structural changes. I'm not ready at this point to make a judgement.
On Thu, May 14, 2015 at 11:28:33PM +0200, Andres Freund wrote: > On 2015-05-14 17:10:29 -0400, Tom Lane wrote: > > FWIW, I did go look at this patch, and concluded it was not close enough > > to ready to try to squeeze it in now. (I think Andres isn't convinced > > of that yet, but time grows short, and I quite agree with Robert that > > committing almost-ready patches at this stage of the game is a bad idea.) > > I think if there's any patch that deserves some leeway it's this > one. It's been been forced into a limbo for nearly half a year; without > leaving Andrew many options. > > I've removed the use of GroupedVars and Andrew is right now working on > structural changes. I'm not ready at this point to make a judgement. I will call for a vote that the freeze deadline be changed if this patch is rejected to due to time. I might lose the vote, but I am going to try because if we lose our reputation for fairness, we have lost a lot more than a week/month of release time. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Bruce Momjian <bruce@momjian.us> writes:
> On Thu, May 14, 2015 at 05:10:29PM -0400, Tom Lane wrote:
>> The good news on this front is that Salesforce has recently taken an
>> interest in having GROUPING SETS capability, so I should be able to
>> find more time to work on this over the next month or two. What I am
>> now hoping for is to work it over and have something ready to push as
>> soon as the 9.6 branch opens.
> So you claim the item on the commitfest in the Fall, which effectively
> prevents other committers from getting involved, then two days before
> the freeze you encourage others to work on it, and a day before the
> freeze you say it is too late to apply? And now, all of a sudden, you
> are interested in working on this because your employer is interested?
[ shrug... ] Andrew had unilaterally removed me as committer from that
patch back in January or so, so it dropped way down my priority list.
I'm willing to move it back up now, but I could do without people
expressing a sense of entitlement to my time. In any case, Andres is
currently the committer of record, and if he decides to push it in the
next 24 hours, I'm not doing anything more to stand in his way than
Robert already did.
> How do I measure the amount of unfairness here?
Life is unfair.
regards, tom lane
On Thu, May 14, 2015 at 05:37:07PM -0400, Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > On Thu, May 14, 2015 at 05:10:29PM -0400, Tom Lane wrote: > >> The good news on this front is that Salesforce has recently taken an > >> interest in having GROUPING SETS capability, so I should be able to > >> find more time to work on this over the next month or two. What I am > >> now hoping for is to work it over and have something ready to push as > >> soon as the 9.6 branch opens. > > > So you claim the item on the commitfest in the Fall, which effectively > > prevents other committers from getting involved, then two days before > > the freeze you encourage others to work on it, and a day before the > > freeze you say it is too late to apply? And now, all of a sudden, you > > are interested in working on this because your employer is interested? > > [ shrug... ] Andrew had unilaterally removed me as committer from that > patch back in January or so, so it dropped way down my priority list. > I'm willing to move it back up now, but I could do without people > expressing a sense of entitlement to my time. In any case, Andres is I am trying not to express any entitlement. I do have a problem with people claiming things that block others. In fact, the reason your name was taken off was because you were inactive on the patch. Unfortunately, even though your name was removed, people still thought of you as owning the patch, and your formidable reputation cemented that. I feel if you had not gotten involved, that patch would have been applied long ago, When someone's involvement _prevents_ a patch from being reviewed or applied, that is the opposite of what we want. I think this effect is indisputable in this case, which is why I am saying if we let this patch slip due to time, we are being unfair. > currently the committer of record, and if he decides to push it in the > next 24 hours, I'm not doing anything more to stand in his way than > Robert already did. Uh, did Robert delay work on the patch in any way? > > How do I measure the amount of unfairness here? > > Life is unfair. True, but I have problems with leaders acting in a way that is unfair to those with less power. Have you considered how demoralizing it is to work in an unfair environment? Unfairness happens, but as leaders, we are supposed to try to avoid it, not cause it. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
On Thu, May 14, 2015 at 2:28 PM, Andres Freund <andres@anarazel.de> wrote: > On 2015-05-14 17:10:29 -0400, Tom Lane wrote: >> FWIW, I did go look at this patch, and concluded it was not close enough >> to ready to try to squeeze it in now. (I think Andres isn't convinced >> of that yet, but time grows short, and I quite agree with Robert that >> committing almost-ready patches at this stage of the game is a bad idea.) > > I think if there's any patch that deserves some leeway it's this > one. It's been been forced into a limbo for nearly half a year; without > leaving Andrew many options. +1. If Andres is going to try and find a way of getting the patch into 9.5, then I think he deserves at least a few extra days. It wasn't as if Andrew wasn't all over this from early in the cycle. -- Peter Geoghegan
* Bruce Momjian (bruce@momjian.us) wrote:
> On Thu, May 14, 2015 at 11:28:33PM +0200, Andres Freund wrote:
> > On 2015-05-14 17:10:29 -0400, Tom Lane wrote:
> > > FWIW, I did go look at this patch, and concluded it was not close enough
> > > to ready to try to squeeze it in now. (I think Andres isn't convinced
> > > of that yet, but time grows short, and I quite agree with Robert that
> > > committing almost-ready patches at this stage of the game is a bad idea.)
> >
> > I think if there's any patch that deserves some leeway it's this
> > one. It's been been forced into a limbo for nearly half a year; without
> > leaving Andrew many options.
> >
> > I've removed the use of GroupedVars and Andrew is right now working on
> > structural changes. I'm not ready at this point to make a judgement.
>
> I will call for a vote that the freeze deadline be changed if this patch
> is rejected to due to time. I might lose the vote, but I am going to
> try because if we lose our reputation for fairness, we have lost a lot
> more than a week/month of release time.
I'm guessing the vote is core-only, but +1 from me in any case. I fully
agree that this patch has had a serious measure of effort put behind it
from the author and is absolutely a capability we desire and need to
have in core.
I'd be happy to jump in and help tonight or tomorrow, but I don't think
there's much I could really contribute right now beyond my support.
Still, please ping me if there's something I can do.
Thanks!
Stephen
On Thu, May 14, 2015 at 5:54 PM, Bruce Momjian <bruce@momjian.us> wrote: >> Life is unfair. > > True, but I have problems with leaders acting in a way that is unfair to > those with less power. Have you considered how demoralizing it is to > work in an unfair environment? Unfairness happens, but as leaders, we > are supposed to try to avoid it, not cause it. +1. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Thu, May 14, 2015 at 6:03 PM, Peter Geoghegan <pg@heroku.com> wrote: > On Thu, May 14, 2015 at 2:28 PM, Andres Freund <andres@anarazel.de> wrote: >> On 2015-05-14 17:10:29 -0400, Tom Lane wrote: >>> FWIW, I did go look at this patch, and concluded it was not close enough >>> to ready to try to squeeze it in now. (I think Andres isn't convinced >>> of that yet, but time grows short, and I quite agree with Robert that >>> committing almost-ready patches at this stage of the game is a bad idea.) >> >> I think if there's any patch that deserves some leeway it's this >> one. It's been been forced into a limbo for nearly half a year; without >> leaving Andrew many options. > > +1. > > If Andres is going to try and find a way of getting the patch into > 9.5, then I think he deserves at least a few extra days. It wasn't as > if Andrew wasn't all over this from early in the cycle. +1. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Bruce Momjian <bruce@momjian.us> writes:
> On Thu, May 14, 2015 at 05:37:07PM -0400, Tom Lane wrote:
>> [ shrug... ] Andrew had unilaterally removed me as committer from that
>> patch back in January or so, so it dropped way down my priority list.
>> I'm willing to move it back up now, but I could do without people
>> expressing a sense of entitlement to my time. In any case, Andres is
> I am trying not to express any entitlement. I do have a problem with
> people claiming things that block others.
Well, I was and remain concerned that the patch would do a great deal of
violence to basic system structure. If I'd had adequate time to work on
it I would have attempted to fix it, but I have not had that kind of time.
In the meantime, the patch was not claimed and I was not particularly
blocking anyone else from claiming it. Plenty of stuff gets committed
around here without my blessing.
>> currently the committer of record, and if he decides to push it in the
>> next 24 hours, I'm not doing anything more to stand in his way than
>> Robert already did.
> Uh, did Robert delay work on the patch in any way?
I was merely agreeing with the concerns Robert expressed in
<CA+TgmoZt0Q=Odx-pL+9Zc6Qyf1A5_2hMBWgaEUdoWgW=JveP6Q@mail.gmail.com>
that we'd do well to avoid committing any more large-and-not-quite-
fully-baked patches at this point. If Andres decides it's baked enough,
that's his responsibility and prerogative as a committer.
> True, but I have problems with leaders acting in a way that is unfair to
> those with less power. Have you considered how demoralizing it is to
> work in an unfair environment? Unfairness happens, but as leaders, we
> are supposed to try to avoid it, not cause it.
TBH, every time somebody beats me up about not having dropped everything
else to spend a month on this patch, it just makes me want to back away
further. I haven't had the time, and I really could do without
accusations of that being unfair.
regards, tom lane
Stephen Frost <sfrost@snowman.net> writes:
> * Bruce Momjian (bruce@momjian.us) wrote:
>> I will call for a vote that the freeze deadline be changed if this patch
>> is rejected to due to time. I might lose the vote, but I am going to
>> try because if we lose our reputation for fairness, we have lost a lot
>> more than a week/month of release time.
> I'm guessing the vote is core-only, but +1 from me in any case. I fully
> agree that this patch has had a serious measure of effort put behind it
> from the author and is absolutely a capability we desire and need to
> have in core.
I should think we'd have learned by now what happens when we delay a
release date to get in some extra feature. It hasn't worked well in
the past and I see no reason to believe the results would be any more
desirable this time.
regards, tom lane
On Thu, May 14, 2015 at 06:57:24PM -0400, Tom Lane wrote: > Stephen Frost <sfrost@snowman.net> writes: > > * Bruce Momjian (bruce@momjian.us) wrote: > >> I will call for a vote that the freeze deadline be changed if this patch > >> is rejected to due to time. I might lose the vote, but I am going to > >> try because if we lose our reputation for fairness, we have lost a lot > >> more than a week/month of release time. > > > I'm guessing the vote is core-only, but +1 from me in any case. I fully > > agree that this patch has had a serious measure of effort put behind it > > from the author and is absolutely a capability we desire and need to > > have in core. > > I should think we'd have learned by now what happens when we delay a > release date to get in some extra feature. It hasn't worked well in > the past and I see no reason to believe the results would be any more > desirable this time. Right, the importance of the feature is not a reason to delay the feature freeze. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
On 15/05/15 10:58, Bruce Momjian wrote: > On Thu, May 14, 2015 at 06:57:24PM -0400, Tom Lane wrote: >> Stephen Frost <sfrost@snowman.net> writes: >>> * Bruce Momjian (bruce@momjian.us) wrote: >>>> I will call for a vote that the freeze deadline be changed if this patch >>>> is rejected to due to time. I might lose the vote, but I am going to >>>> try because if we lose our reputation for fairness, we have lost a lot >>>> more than a week/month of release time. >>> I'm guessing the vote is core-only, but +1 from me in any case. I fully >>> agree that this patch has had a serious measure of effort put behind it >>> from the author and is absolutely a capability we desire and need to >>> have in core. >> I should think we'd have learned by now what happens when we delay a >> release date to get in some extra feature. It hasn't worked well in >> the past and I see no reason to believe the results would be any more >> desirable this time. > Right, the importance of the feature is not a reason to delay the > feature freeze. > Following rules like this is very important, but so is making valid exceptions. Though I'm in no position to judge the importance of this patch, so I won't attempt to! Cheers, Gavin
On 2015-05-14 23:28:33 +0200, Andres Freund wrote: > I've removed the use of GroupedVars and Andrew is right now working on > structural changes. I'm not ready at this point to make a judgement. Andrew worked really hard and addressed the voiced concerns with the way chaining was done. In my last read through I found a bunch of stylistic quibbles and a question about behaviour where reading the spec confirmed that the current implementation is actually correct ( grouping sets + functional dependencies => weird). I plan to post a squashed patches from what's in git now in a couple hours and then, unless something major (issues, protest) comes up, push PDT late afternoon.
On 2015-05-15 18:00:49 +0200, Andres Freund wrote: > On 2015-05-14 23:28:33 +0200, Andres Freund wrote: > > I've removed the use of GroupedVars and Andrew is right now working on > > structural changes. I'm not ready at this point to make a judgement. > > Andrew worked really hard and addressed the voiced concerns with the way > chaining was done. In my last read through I found a bunch of stylistic > quibbles and a question about behaviour where reading the spec confirmed > that the current implementation is actually correct ( grouping sets + > functional dependencies => weird). > > I plan to post a squashed patches from what's in git now in a couple > hours and then, unless something major (issues, protest) comes up, push > PDT late afternoon. Here's why I think it's somewhat important that we make progress on the issue, and why I want to go ahead with this: In my eye Tom has, I'll assume unintentionally, stalled progress on this patch for nearly half a year. Due to Tom's profile it's unlikely that somebody else is going to seriously tackle a nontrivial patch that Tom has laid claims to. Especially if fundamental objections have been made, without also painting a way forward. In addition, by commenting on various triage emails that he'll get to it at some point, Tom in my view essentially has cemented that claim. Due to that I think the issue can't be characterized, as done nearby, as one of unduly laying claims to Tom's time, which obviously is his own to manage. The way it turned out people were forced to do that. I do think that part of the problem is that Andrew (Gierth, not Dunstan) isn't always easy to work with. Particularly there seems to be a very unfortunate dynamic between both Tom and Andrew. But if one is annoyed about someone's communication style I think it's much better to ignore that person or publically lash out about it. Essentially blocking progress of a patch for months is in my opinion a poor way of handling it. If Tom had said a couple months, or even weeks, ago that he doesn't have time to look into the patch in the 9.5 cycle, I'd not even think about pressing forward with the patch at this time of the cycle after it had just undergone significant changes. I like to think that it would have already gotten in at that point, but who knows. But either way, we're not in normal circumstances with regard to this patch. Our community has a reputation, and increasingly so, of being very painful to work with. Given the growth in adoption, without a corresponding growth in experienced long term contributors, I don't think we can afford feeding that reputation with more justified causes. We have difficulties keeping up even today. If the patch were in a bad shape I wouldn't even consider pressing ahead. But I don't think it is anymore. It's also not a patch that has the danger to destabilize postgres in the longer term. The code is fairly specific to grouping sets. Doesn't change the disk format. If we in hindsight discover the implementation wasn't using the right approach we can just change it. The worst that will happen is that explain output changes. I think many, much more dangerous, patches have been integreated into 9.5 with less review. Andres
On 05/14/2015 03:58 PM, Bruce Momjian wrote: > On Thu, May 14, 2015 at 06:57:24PM -0400, Tom Lane wrote: >> Stephen Frost <sfrost@snowman.net> writes: >>> * Bruce Momjian (bruce@momjian.us) wrote: >>>> I will call for a vote that the freeze deadline be changed if this patch >>>> is rejected to due to time. I might lose the vote, but I am going to >>>> try because if we lose our reputation for fairness, we have lost a lot >>>> more than a week/month of release time. >> >>> I'm guessing the vote is core-only, but +1 from me in any case. I fully >>> agree that this patch has had a serious measure of effort put behind it >>> from the author and is absolutely a capability we desire and need to >>> have in core. >> >> I should think we'd have learned by now what happens when we delay a >> release date to get in some extra feature. It hasn't worked well in >> the past and I see no reason to believe the results would be any more >> desirable this time. > > Right, the importance of the feature is not a reason to delay the > feature freeze. It has nothing to do with the importance of the feature. It has everything to do with fairness. Regardless of what Tom did or didn't do, what we have here is a major feature patch which was submitted in a timely fashion, and then *not reviewed* for multiple commitfests, and now in danger of being bounced because it's "late". Considering that many other things have been committed which were submitted significantly later than Grouping Sets, including some which have been committed with the acknowledgement that there is more work do do during beta, this would have the appearance of being prejudicial against Gierth. Grouping Sets has been working, at least in demo form, since November. I really don't think we can, as a project, afford to have the appearance of prejudice in the review process. Things are bad enough already; if contributors feel that the review process is blatantly unfair, they will resort to underhanded means to get their patches in, and things will break down completely. We're only holding stuff together despite short resources because contributors believe in the inherent fairness of the process and the committers and that scarce resources will be allocated evenly. Note that I am not proposing a general delay in feature freeze. I am specifically proposing an additional week for Grouping Sets and *only* for Grouping Sets. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 05/15/2015 12:32 PM, Josh Berkus wrote: > Note that I am not proposing a general delay in feature freeze. I am > specifically proposing an additional week for Grouping Sets and *only* > for Grouping Sets. Core is in charge of releases. I believe like the other semi and formal organizations around this community it makes sense for Core to present a motion and for that motion to be voted upon. The vote can take place publicly (and there is an argument that it should) but it is a vote for core not hackers or general. In short, Josh, Bruce, you are both core members. If you want to call a vote, do so. Something like this should suffice: WHEREAS 1. The Core Committee of PGDG is responsible for releases of PostgreSQL 2. The feature "Grouping Sets" is a major feature for the upcoming release of PostgreSQL THE CORE COMMITTEE RESOLVES THAT 3. In an effort to produce a fair and equitable return for the efforts put in by contributors of the Grouping Sets patch, the core committee will extend feature freeze for the Grouping Sets patch for exactly 7 days. 4. This extension is for the Grouping Sets patch and the Grouping Sets patch only. 5. Should a committable patch not be produced within 7 days, the patch shall be pushed back into the queue for the production release of PostgreSQL. Sincerely, JD P.S. Note that I have seen the "final" patch that hit the list about 45 minutes ago. -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Centered full stack support, consulting and development. Announcing "I'm offended" is basically telling the world you can't control your own emotions, so everyone else should do it for you.
On 5/13/15 7:46 PM, Kouhei Kaigai wrote: >>> * ctidscan as an example of custom-scan >>> > > >>> > >This basically hasn't gotten any attention, which may mean nobody cares >>> > >enough to justify putting it in the tree. We need to either push it to >>> > >next CF or reject altogether. >> > >> >Agreed. I was fine with never committing this. I don't think we have >> >a requirement that every hook or bit of functionality we expose at the >> >C level must have an example in core. But other people (you? Simon?) >> >seemed to want a demonstration in the core repository. If that's >> >still a priority, I am willing to work on it more for 9.6, but there >> >is not time now. >> > > If no other people required it again, I don't think this module should > be kept in core and also I'm not favor to push ctidscan to v9.6 development > cycle. It intends to demonstrate custom-scan interface, however, it is > not certain an example always needs to be in-core. FWIW, having TIDGreaterOperator would be very useful for anyone trying to un-bloat a table, so it'd be nice if this was at least available as a PGXN extension. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
On 5/14/15 5:48 PM, Tom Lane wrote: >> True, but I have problems with leaders acting in a way that is unfair to >> >those with less power. Have you considered how demoralizing it is to >> >work in an unfair environment? Unfairness happens, but as leaders, we >> >are supposed to try to avoid it, not cause it. > TBH, every time somebody beats me up about not having dropped everything > else to spend a month on this patch, it just makes me want to back away > further. I haven't had the time, and I really could do without > accusations of that being unfair. FWIW, I don't think that's what people are expressing an issue with. Rather, while you were marked as committer no one else was working on it even thought they might have had you not been marked. I think (or at least hope) people understand that things happen, especially when $JOB intervenes. OTOH, once you were no longer marked as committer I don't think it's fair to hold you accountable for people not stepping back up. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
On 2015-05-13 11:38:27 -0400, Tom Lane wrote: > Looking at what remains open in the current commitfest: As of now the remaining items !bugfix entries are: > * GIN fillfactor > > I'd like to put this one on Heikki's plate as well, since he's touched > the GIN code more than anyone else lately. While sad, I think this is going to have to be moved. > * Additional role attributes > > Is this ready to commit? Stephen's call. This was still being discussed/spec'ed recently, so I think it's not too bad to move this now. > * catalog view to pg_hba.conf file > > Greg Stark is marked as committer of record on this. A bit sad again. I think we can close the commitfest now? Moving these three entries to the next one? Andres
Andres Freund <andres@anarazel.de> writes:
> I think we can close the commitfest now? Moving these three entries to
> the next one?
Yeah, I don't think any of the remaining entries are committable.
regards, tom lane
On Sat, May 16, 2015 at 11:00 AM, Andres Freund wrote: > On 2015-05-13 11:38:27 -0400, Tom Lane wrote: >> * GIN fillfactor >> >> I'd like to put this one on Heikki's plate as well, since he's touched >> the GIN code more than anyone else lately. > > While sad, I think this is going to have to be moved. Yeah, I was rather confident in what Alexander did here. But life is life, and so is deadline. >> * Additional role attributes >> >> Is this ready to commit? Stephen's call. > > This was still being discussed/spec'ed recently, so I think it's not too > bad to move this now. Moved to next CF. > >> * catalog view to pg_hba.conf file >> >> Greg Stark is marked as committer of record on this. > > A bit sad again. Moved to next CF. > I think we can close the commitfest now? Moving these three entries to > the next one? And CF closed. -- Michael