Hi Álvaro,
Thanks for the patch, the gap is real (SET LOCAL <TAB> currently falls
through to the generic Matches("SET", MatchAny) rule and unhelpfully
suggests TO).
I see a couple of issues with the current patch.
1. The new branch makes an existing rule dead code. Further down in the
same else if chain there is:
/* Complete SET SESSION with AUTHORIZATION or CHARACTERISTICS... */
else if (Matches("SET", "SESSION"))
COMPLETE_WITH("AUTHORIZATION", "CHARACTERISTICS AS TRANSACTION");
Because the new branch is inserted earlier, that rule never fires anymore,
and SET SESSION <TAB> loses its AUTHORIZATION / CHARACTERISTICS AS
TRANSACTION suggestions. That's a regression
Also LOCAL and SESSION aren't symmetric here. Looking at gram.y,
"AUTHORIZATION" and "CHARACTERISTICS AS TRANSACTION" are bound to a literal
SESSION token in set_rest_more / set_rest, not to the LOCAL/SESSION
modifier on SET.
I am thinking maybe we could split the cases as
/* Complete "SET LOCAL" */
else if (Matches("SET", "LOCAL"))
COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_set_vars,
"TIME ZONE",
"SCHEMA",
"NAMES");
/* Complete "SET SESSION" */
else if (Matches("SET", "SESSION"))
COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_set_vars,
"AUTHORIZATION",
"CHARACTERISTICS AS TRANSACTION",
"TIME ZONE",
"SCHEMA",
"NAMES");
and remove the now-redundant later Matches("SET", "SESSION") block
Worth considering whether ROLE should be in both lists too, since SET LOCAL
ROLE … and SET SESSION ROLE … are both valid.
Regards,
Surya Poondla