Re: B-Tree support function number 3 (strxfrm() optimization)

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Fri, Apr 4, 2014 at 4:29 PM, Greg Stark <stark@mit.edu> wrote:
> > I've been meaning to do more review for a while and just took a skim through
> > the queue. There are only a couple I feel I can contribute with so I'm going
> > to work on those and then if it's still before the feature freeze I would
> > like to go ahead with Peter's patch. I think it's generally a good patch.
>
> To be honest, I think that's just flat-out inappropriate.  There were
> over 100 patches in this CommitFest and there's not a single committed
> patch that has your name on it even as a reviewer, let alone a
> committer.

I haven't got any either (except for my little one), which frustrates
me greatly.  Not because I'm looking for credit on the time that I've
spent in discussions, doing reviews, and I could have sworn there was
some patch that I did commit, but because I've not been able to find
the larger chunks of time required to get the more complex patches in.

> When a committer says, hey, I'm going to commit XYZ, that
> basically forces anybody who might have an objection to it to drop
> what they're doing and object fast, before it's too late.  In other
> words, the people who just said that they are too busy reviewing
> patches that were timely submitted and don't want to divert effort
> from that to handle patches that weren't are going to have to do that
> anyway, or lose their right to object.

I don't agree that this is the case.  We do revert patches from time to
time, when necessary, and issues with this particular patch seem likely
to be found during testing, well in advance of any release, and it's
self contained enough to be reverted pretty easily.  Perhaps it's not
fair to expect everyone to have realized that, but at least any
committer looking at it would, I believe, weigh those against it being a
late patch.

> I think that's unfair.  You're
> essentially leveraging a commit bit that you haven't used in more than
> three years to try to push a patch that was submitted months too late
> to the head of the review queue - and, just to put icing on the cake,
> it just so happens that you and the patch author work for the same
> employer.  I have no objection to people committing patches written by
> others who work at the same company, but only if those patches have
> gone through a full, fair, and public review, with ample opportunity
> for other people to complain if they don't like it.  That is obviously
> not the case here.

As for this- it's disingenuous at best and outright accusatory at worst.
The reason for this discussion is entirely because of PGConf.NYC, imv,
where Peter brought this patch up with at least Greg, Magnus and I.
Also during PGConf.NYC, Greg was specifically asking me about patches
which were in the commitfest that could be reviewed, ideally without
having to go back through threads hundreds of messages long and dealing
with complex parts of the code.  Sadly, as is often the case, the "easy"
ones get handled pretty quickly and the difficult ones get left behind.

One bad part of the commitfest, imv, is that when a "clearly good,
small" patch does manage to show up, we don't formally take any of that
into consideration when we are prioritizing patches.  They end up being
informally prioritized and get in quickly at the beginning, but that
doesn't address the reality that those smaller patches likely would get
in without any kind of commitfest process and not letting them in
because of the commitfest process doesn't generally mean that the larger
patches are any more likely to get in- iow, it's not a zero-sum game.
We have quite a few "part-time" committers (or at least, committers who
have disproportionate amounts of time, or perhaps the difference is in
the size of the blocks of time which can be dedicated to PG) and saying
"no, you can't help unless you tackle the hard problems" doesn't
particularly move us forward.

All that said, I don't have any particularly good idea of how to "fix"
any of this- it's not fair to tell the committers who have more time (or
larger blocks of time, etc) "you must work the hard problems only"
either.  I don't feel Greg's interest in this patch has anything to do
with his current employment and everything to do with the side-talks in
NYC, and to that point, I'm very tempted to go look at this patch myself
because it sounds like an exciting improvement with minimal effort.
Would I feel bad for doing so, with the CustomScan API and Updatable
Security Barrier Views patches still pending?  Sure, but it's the
difference between finding an hour and finding 8.  The hour will come
pretty easily (probably spent half that on this email..), while an
8-hour block, which would likely turn into more, is neigh-on impossible
til at least this weekend.  And, no, 8x one-hour blocks would not be
worthwhile; I've tried that before.
Thanks,
    Stephen