Re: Fix some newly modified tab-complete changes

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Peter Smith <smithpb2250(at)gmail(dot)com>
Cc: "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Fix some newly modified tab-complete changes
Date: 2022-11-10 04:53:58
Message-ID: Y2yD5mYKpMOgC+dt@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Oct 18, 2022 at 05:17:32PM +1100, Peter Smith wrote:
> I re-tested and confirm that the patch does indeed fix the quirk I'd
> previously reported.
>
> But, looking at the patch code, I don't know if it is the best way to
> fix the problem or not. Someone with more experience of the
> tab-complete module can judge that.

It seems to me that the patch as proposed has more problems than
that. I have spotted a few issues at quick glance, there may be
more.

For example, take this one:
+ else if (TailMatches("GRANT") ||
+ TailMatches("REVOKE", "GRANT", "OPTION", "FOR"))
COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_roles,

"REVOKE GRANT OPTION FOR" completes with a list of role names, which
is incorrect.

FWIW, I am not much a fan of the approach taken by the patch to
duplicate the full list of keywords to append after REVOKE or GRANT,
at the only difference of "GRANT OPTION FOR". This may be readable if
unified with a single list, with extra items appended for GRANT and
REVOKE?

Note that REVOKE has a "ADMIN OPTION FOR" clause, which is not
completed to.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2022-11-10 04:56:45 Re: [PATCH] Allow usage of archive .backup files as backup_label
Previous Message Yugo NAGATA 2022-11-10 04:49:19 Re: BUG #17434: CREATE/DROP DATABASE can be executed in the same transaction with other commands