On Tue, Apr 12, 2016 at 9:14 AM, Anastasia Lubennikova <a.lubennik...@postgrespro.ru> wrote: > Sooner or later, I'd like to see this patch finished.
Me, too. > For now, it has two complaints: > - support of expressions as included columns. > Frankly, I don't understand, why it's a problem of the patch. > The patch is already big enough and it will be much easier to add > expressions support in the following patch, after the first one will be > stable. > I wonder, if someone has objections to that? Probably. If we limit the scope of something, it's always in a way that limits the functionality available to users, rather than limits how generalized the new functionality is, and so cutting scope sometimes isn't possible. There is a very high value placed on features working well together. A user ought to be able to rely on the intuition that features work well together. Preserving that general ability for users to guess correctly what will work based on what they already know is seen as important. For example, notice that the INSERT documentation allows UPSERT unique index inference to optionally accept an opclass or collation. So far, the need for this functionality is totally theoretical (in practice all B-Tree opclasses have the same idea about equality across a given type, and we have no case insensitive collations), but it's still there. Making that work was not a small effort (there was a follow-up bugfix commit just for that, too). This approach is mostly about making the implementation theoretically sound (or demonstrating that it is) by considering edge-cases up-front. Often, there will be benefits to a maximally generalized approach that were not initially anticipated by the patch author, or anyone else. I agree that it is difficult to uphold this standard at all times, but there is something to be said for it. Postgres development must have a very long term outlook, and this approach tends to make things easier for future patch authors by making the code more maintainable. Even if this is the wrong thing in specific cases, it's sometimes easier to just do it than to convince others that their concern is misplaced in this one instance. > Yes, it's a kind of delayed feature. But should we wait for every patch when > it will be entirely completed? I think that knowing where and how to cut scope is an important skill. If this question is asked as a general question, then the answer must be "yes". I suggest asking a more specific question. :-) > - lack of review and testing > Obviously I did as much testing as I could. > So, if reviewers have any concerns about the patch, I'm waiting forward to > see them. For what it's worth, I agree that you put a great deal of effort into this patch, and it did not get in to 9.6 because of a collective failure to focus minds on the patch. Your patch was a credible attempt, which is impressive when you consider that the B-Tree code is so complicated. There is also the fact that there is now a very small list of credible reviewers for B-Tree patches; you must have noticed that not even amcheck was committed, even though I was asked to produce a polished version in February during the FOSDEM dev meeting, and even though it's just a contrib module that is totally orientated around finding bugs and so on. I'm not happy about that either, but that's just something I have to swallow. I fancy myself as am expert on the B-Tree code, but I've never managed to make an impact in improving its performance at all (I've never made a serious effort, but have had many ideas). So, in case it needs to be said, I'll say it: You've chosen a very ambitious set of projects to work on, by any standard. I think it's a good thing that you've been ambitious, and I don't suggest changing that, since I think that you have commensurate skill. But, in order to be successful in these projects, patience and resolve are very important. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
