I looked through the new set of patches.
On Thu, Nov 7, 2024 at 2:42 PM Kirill Reshke <reshkekirill@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].
2) I still can't see why you don't complete "ALTER TYPE ... ALTER
ATTRIBUTE ... TYPE ..." with CASCADE/RESTRICT.
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.
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.
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.
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.
[1] https://www.postgresql.org/docs/current/sql-altertype.html
Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/
