Re: Indirect indexes

On Tue, Dec 13, 2016 at 11:11 PM, Pavan Deolasee
<pavan(dot)deolasee(at)gmail(dot)com> wrote:
> To be fair to myself, I did try to find patches with equal or more
> complexity. But most of them had (multiple) reviewers assigned and were
> being discussed for weeks and months. I did not think I could contribute
> positively to those discussions, mostly because meaningful review at that
> point would require much more in-depth knowledge of the area. So I picked
> couple of patches which did not have any reviewers. May be not the best
> decision, but I did what I thought was correct.

I'm a little tired right now because it's almost 2am here, so maybe I
should go to bed instead of writing emails when I might be excessively
grumpy, but I just don't buy this. There are indeed a number of
patches which have provoked lots of discussion already, but there are
also many that have had virtually no review at all. Many of the
parallel query patches fall into this category, and there are lots of
others. CommitFest 2017-01 currently has 62 patches in the "Needs
Review" state, and it is just not the case that all 62 of those are
under active discussion and review. I bet there are a dozen that have
had no replies at all and another dozen that have had only a handful
of fairly casual replies without any in-depth discussion. Maybe more.

Now, I will agree that diving into a patch in an area that you've
never seen before can be challenging and intimidating. But you're not
asking any less for your own patch. You would like someone to dive
into your patch and figure it out, whether they know that area well or
not, and let's face it: most people don't. I don't. I've read the
email threads about WARM, but the concept hasn't sunk in for me yet.
I understand HOT and I understand the heap and indexes pretty well so
I'll figure it out eventually, but I don't have it figured out now and
that's only going to get solved if I go put in a bunch of time to go
learn. I imagine virtually everyone is in the same position.
Similarly, I just spent a ton of time reviewing Amit Kapila's work on
hash indexes and I don't know a heck of a lot about hash indexes --
virtually nobody does, because that area has been largely ignored for
the last decade or so. Yet if everybody uses that as an excuse, then
we don't get anywhere.

To put that another way, reviewing patches is hard, but we still have
to do it if we want to produce a quality release with good features.
If we don't do a good job reviewing and therefore don't commit things,
we'll have a disappointing release. If we don't do a good job
reviewing but commit things anyway, then we'll have a horribly buggy
release. If the patch we fail to review adequately happens to be
WARM, then those will very possibly be data-corrupting bugs, which are
particularly bad for our project's reputation for reliability. Those
aren't good options, so we had better try to make sure that we do lots
of high-quality review of that patch and all the others. If we can't
turn to the people who are writing the patches to help review them,
even though that's difficult, then I think that much-needed review
won't happen.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company