2017-02-26 19:43 GMT+01:00 Robert Haas <robertmh...@gmail.com>: > On Wed, Feb 22, 2017 at 12:38 AM, Pavel Stehule <pavel.steh...@gmail.com> > wrote: > > Now first patch is broken :( > > > > It is pretty sensitive to any changes. Isn't possible to commit first > four > > patches first and separately maybe out of commitfest window? > > Yeah, maybe, but we'd need a committer to take more of an interest in > this patch series. Personally, I'm wondering why we need a series of > 19 patches to add tab completion support for IF NOT EXISTS. The > feature which is the subject of this thread arrives in patch 0017, and > a lot of the patches which come before that seem to change a lot of > stuff without actually improving much that would really benefit users. > > 0001 seems like a lot of churn for no real benefit that I can immediately > see. > 0002 is a real feature, and probably a good one, though unrelated to > the subject of this thread. In the process, it changes many lines of > code in fairly mechanical ways; does it need to do that? > 0003 is infrastructure. > 0004 adds a README. Do we really need that? It seems to be > explaining things which are mostly fairly clear from just looking at > the code. If we add a README, we have to update it when we change > things. That's worthwhile if it helps people write code better, I'm > not sure if it will do that. >
it needs a separation to refactoring part and to new features part. The refactoring looks well and I am sure so has sense. about README - there are described fundamental things - that should be stable. With last changes and this set of patches, the autocomplete is not trivial and I am sure, so any documentation is better than nothing. Not all developers has years of experience with PostgreSQL hacking. > 0005 extends 0002. > 0006 prevents incorrect completions in obscure circumstances. > 0007 adds some kind of tab completion for CREATE RULE; I'm fuzzy on the > details. > 0008 improves tab completion after EXPLAIN. > 0009-0014 uses the infrastructure from 0003 to improve tab completion > for various commands. They say they're merely simplifying tab > completion for those things, but actually they're extending it to some > obscure situations that aren't currently covered. > 0015 adds completion for magic keywords like CURRENT_USER when role > commands are used. > 0016 refactors tab completion for ALTER DEFAULT PRIVILEGES, possibly > improving it somehow. > 0017 implements the titular feature. > 0018 adds optional debugging output. > 0019 improves things for CREATE OR REPLACE completion. > > Phew. That's a lot of work for relatively obscure improvements to tab > completion. I grant that the result is probably better, but it's a > lot of code change for what we get out of it. I'm not saying we > should reject it on that basis, but it may be the reason why nobody's > jumped in to work on getting this committed. > These patches are big - but in the end it cleaning tab complete code, and open a doors for more smarter completion. Some features can be interesting for users too - repeated writing IF EXISTS, IF NOT EXISTS or OR REPLACE is really scary - mainly so some other parts of tab complete are friendly enough now. Can be solution a splitting this set of patches to more independent parts? We should to start with refactoring. Other patches can be processed individually - with individual discussion. Regards Pavel > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >