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
signature.asc
Description: PGP signature
