Re: IF (NOT) EXISTS in psql-completion

2017-02-26 19:43 GMT+01:00 Robert Haas <robertmhaas(at)gmail(dot)com>:

> On Wed, Feb 22, 2017 at 12:38 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>