Re: IF (NOT) EXISTS in psql-completion

2016-11-25 2:24 GMT+01:00 Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp
>:

> Hello,
>
> Thank you for looking this long-and-bothersome patch.
>
>
> At Wed, 23 Nov 2016 07:12:00 +0100, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote in <CAFj8pRBxgUrg-6CKbVOy4VqwSFkrf--uCzj3q-
> vd9FgpSGV+qQ(at)mail(dot)gmail(dot)com>
> > Hi
> >
> > 2016-11-15 12:26 GMT+01:00 Kyotaro HORIGUCHI <
> horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)
> > jp>:
> >
> > > Hello, I rebased this patch on the current master.
> > >
> > > At Mon, 31 Oct 2016 10:15:48 +0900 (Tokyo Standard Time), Kyotaro
> > > HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote in <
> > > 20161031(dot)101548(dot)162143279(dot)horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
> > > > Anyway, I fixed space issues and addressed the
> > > > COMPLETE_WITH_QUERY()'s almost unused parameter problem. And
> > > > tried to write a README files.
> > >
> > > tab-complete.c have gotten some improvements after this time.
> > >
> > > 577f0bdd2b8904cbdfde6c98f4bda6fd93a05ffc psql: Tab completion for
> > > renaming enum values.
> > > 927d7bb6b120a2ca09a164898f887eb850b7a329 Improve tab completion for
> > > CREATE TRIGGER.
> > > 1d15d0db50a5f39ab69c1fe60f2d5dcc7e2ddb9c psql: Tab-complete LOCK
> [TABLE]
> > > ... IN {ACCESS|ROW|SHARE}.
> > >
> > > The attached patchset is rebsaed on the master including these
> > > patches.
> > >
> >
> > I checked patches 0001, 0002, 0003 patches
> >
> > There are no any problems with patching, compiling, it is working as
> > expected
> >
> > These patches can be committed separately - they are long, but the code
> is
> > almost mechanical
>
> Thanks.
>
> You're right. I haven't consider about relations among them.
>

I am sure about benefit of all patches - but it is lot of changes in one
moment, and it is not necessary in this moment.

patches 0004 and 0005 does some bigger mental changes, and the work can be
separated.

Regards

Pavel

>
>
> 0001 (if-else refactoring) does not anyting functionally. It is
> required by 0004(word-shift-and-removal) and 0005(if-not-exists).
>
> 0002 (keywords case improvement) is almost independent from all
> other patches in this patch set. And it brings an obvious
> improvement.
>
> 0003 (addition to 0002) is move embedded keywords out of defined
> queries. Functionally can be united to 0002 but separated for
> understandability
>
> 0004 (word-shift-and-removal) is quite arguable one. This
> introduces an ability to modify (or destroy) previous_words
> array. This reduces almost redundant matching predicates such as,
>
> > if (TailMatches3("CREATE|UNIQUE", "INDEX", MatchAny) ||
> > TailMatches4("CREATE|UNIQUE", "INDEX", "CONCURRENTLY", MatchAny))
>
> into
>
> > if (Matches3("CREATE", "INDEX", MatchAnyExcept("ON")))
>
> by removing "CONCURRENTLY". This obviously simplifies the
> predicates literally but it the code implies history of
> modification. The implied history might be worse than the
> previous shape, especially for the simple cases like this. For a
> complex case of CREATE TRIGGER, it seems worse than the original
> shape... I'll consider this a bit more. Maybe match-and-collapse
> template should be written a predicate.
>
> 0005 (if-not-exists). I have admit that this is arguable
> feature...
>
> 0006 is the terder point:( but.
>
> > The README is perfect
>
> Thank you, I'm relieved by hearing that.
>
> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
>
>