Re: IF (NOT) EXISTS in psql-completion

On Mon, Feb 27, 2017 at 5:21 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> 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.
>
> FWIW, one reason this committer hasn't jumped in is that we already
> rewrote tab-complete.c pretty completely in 9.6. If we accept a patch
> that completely rewrites it again, we're going to be faced with
> maintaining three fundamentally different implementations for the next
> three-plus years (until 9.5 dies). Admittedly, we don't back-patch
> fixes in tab-complete.c every week, but a look at the git history says
> we do need to do that several times a year.

Indeed, having worked on the 9.6 refactoring a bit as well... I'll
vote for not doing this again as HEAD is in a more readable shape
compared to the pre-9.5 area, and I am not convinced that it is worth
the trouble. There are a couple of things that can be extracted from
this set of patches, but I would vote for not doing the same level of
refactoring.

> Also, the nature of the primary refactoring (changing the big else-chain
> into standalone ifs, if I read it correctly) is particularly bad from a
> back-patching standpoint because all you have to do is insert an "else",
> or fail to insert one, to silently and almost completely break either
> one or the other branch. And I don't really understand why that's a good
> idea anyway: surely we can return at most one set of completions, so how
> is turning the function into a lot of independent actions a win?
>
> So I'd be a whole lot happier if it didn't do that. Can we really not
> add the desired features in a more localized fashion?

As "if not exists" is defined after the object type if would not be
that complicated to add completion for IE/INE after the object type
with a set of THING_* flags in words_after_create. One missing piece
would be to add completion for the objects themselves after IE or INE
have been entered by the user, but I would think that tweaking the
checks on words_after_create[i] would be doable as well. And that
would be localized.
--
Michael