On Thu, Nov 10, 2022 12:54 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> 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.
Thanks a lot for looking into this patch.
I have fixed the problems you saw, and improved the patch as you suggested.
Besides, I noticed that the tab completion for "ALTER DEFAULT PRIVILEGES ...
GRANT/REVOKE ..." missed "CREATE". Fix it in 0001 patch.
And commit e3ce2de09 supported GRANT ... WITH INHERIT ..., but there's no tab
completion for it. Add this in 0002 patch.
Please see the attached patches.
Regards,
Shi yu