Re: Tab completion for CREATE SCHEMAAUTHORIZATION

On Mon, Aug 09, 2021 at 07:00:02PM +0100, Dagfinn Ilmari Mannsåker wrote:
> Thanks for the review.  Updated patch attached, with the CURRENT/SESSION
> ROLE/USER changes for other commands separated out.

+#define Query_for_list_of_owner_roles \
+Query_for_list_of_roles \
 " UNION ALL SELECT 'CURRENT_ROLE'"\
 " UNION ALL SELECT 'CURRENT_USER'"\
 " UNION ALL SELECT 'SESSION_USER'"
I don't object to the refactoring you are doing here with three
Query_for_list_of_*_roles each one depending on the other for clarity.
Neither do I really object to not using COMPLETE_WITH_QUERY() with
some extra UNION ALL hardcoded in each code path as there 6 cases for
_owner_, 6 for _grant_ and 6 for _roles if my count is right.  Still,
if I may ask, wouldn't it be better to document a bit what's the
expectation behind each one of them?  Perhaps the names of the queries
are too generic for the purposes where they are used (say _grant_ for
CREATE USER MAPPING looks confusing)?

+       else if (Matches("CREATE", "SCHEMA", "AUTHORIZATION"))
+               COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles);
+       else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION"))
+               COMPLETE_WITH_QUERY(Query_for_list_of_owner_roles);
+       else if (Matches("CREATE", "SCHEMA", MatchAny, "AUTHORIZATION", MatchAny))
+               COMPLETE_WITH("CREATE", "GRANT");
+       else if (Matches("CREATE", "SCHEMA", MatchAny))
+               COMPLETE_WITH("AUTHORIZATION", "CREATE", "GRANT");
Looks like you forgot the case "CREATE SCHEMA AUTHORIZATION MatchAny"
that should be completed by GRANT and CREATE.
--
Michael