On Thu, 7 Nov 2024 at 22:33, Karina Litskevich <litskevichkar...@gmail.com> wrote: > > I looked through the new set of patches. > > On Thu, Nov 7, 2024 at 2:42 PM Kirill Reshke <reshkekir...@gmail.com> wrote: > > v3-0002 patch actually mixes two types of completion. First one, which > > adds a data type completion for ADD ATTRIBUTE, is pretty useful. > > similar to existing tab completion for `ALTER TABLE xxx ADD [IF NOT > > EXISTS] [COLUMN] yyy `. > > As I understand, it's v4-0001. Looks fine, I agree that it's a useful > completion and I can't see any arguments against that. > > > The second one (CASCADE/RESTRICT) is dubious, and I'm also not sure if > > we really need it, so perhaps we should wait for some other views.. > > For this reason, I split v3-0002 into two separate patches. > > And this is v4-0002. Agreed that we should wait for another opinion to > see if it's needed. Anyway, I have a few remarks here: > 1) "ALTER TYPE xxx RENAME VALUE yyy TO zzz" can't be followed by > CASCADE/RESTRICT at least according to the docs [1].
Yes, my bad. I removed "VALUE".
> 2) I still can't see why you don't complete "ALTER TYPE ... ALTER
> ATTRIBUTE ... TYPE ..." with CASCADE/RESTRICT.
My bad, fixed this in v5. Thank you.
> By the way, I suggest adding a completion of "ALTER TYPE ... ALTER
> ATTRIBUTE ... TYPE" with the list of types while we are here. It should
> probably go together with v4-0001.
Surprisingly this is already supported in an interesting manner. psql
will tab-complete everything that end with token "TYPE" with list of
datatypes:
```
reshke=# FOO BAR BAZ
<tab> *nothing*
reshke=# FOO BAR BAZ TYPE
<tab>
Display all 119 possibilities? (y or n)
```
> Next, v4-0003 is the same as v3-0003, and we already agreed on this.
> (Just a note for myself.)
>
> > I also want to propose an Access method completion for create M.V.
> > <foo> using. This is a patch I forgot to attach in my last email.
> > Please, see v4-0004
>
> In v4-0004 I like the idea to add this completion, but I have some
> remarks to implementation:
> 1) Try to keep comments identical, at least in one piece of code. Right
> now you have "CREATE MATERIALIZED VIEW <name>" and "CREATE MATERIALIZED
> VIEW <sth>" within three consecutive lines. I can see there was the
> same problem before your changes, so it's not exactly your fault. Let's
> correct it, though.
Ok, sure. I did the correction.
> 2) The comment
> /* Complete CREATE MATERIALIZED VIEW <name> with AS */
> is no longer correct since you've changed
> - COMPLETE_WITH("AS");
> to
> + COMPLETE_WITH("AS", "USING");
> Please fix it.
Nice catch! Fixed.
> I hope to look more thoroughly into tab-complete.in.c tomorrow or on
> Monday to see if there are any other problems I can't see at first
> glance. I'll send another mail when I get to do this.
Your effort is much appreciated.
>
> [1] https://www.postgresql.org/docs/current/sql-altertype.html
>
>
> Best regards,
> Karina Litskevich
> Postgres Professional: http://postgrespro.com/
PFA v5.
--
Best regards,
Kirill Reshke
v5-0002-Add-CASCADE-RESRTICT-to-ALTER-TYPE-patterns.patch
Description: Binary data
v5-0004-Suggest-table-access-method-for-create-maateriali.patch
Description: Binary data
v5-0003-Add-missing-tab-completion-for-CREATE-TEMP-TABLE-.patch
Description: Binary data
v5-0001-Enhance-tab-completion-to-ALTER-TYPE-ADD-ATTRIBUT.patch
Description: Binary data
