The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, failed
Documentation: tested, passed
Hi Christoph,
The patch have a potential, although I have to agree with Jim Jones, it obviously have a problem with "alter view
<name>set<tab>" handling.
First of all user can notice, that SET and RESET alternatives are proposed in an absolutely equivalent way:
postgres=# alter view VVV <tab>
ALTER COLUMN OWNER TO RENAME RESET ( SET ( SET SCHEMA
But completion of a parentheses differs fore these alternatives:
postgres=# alter view VVV reset<tab> -> completes with "<space>(" -> <tab>
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
postgres=# alter view VVV set<tab> -> completes with a single spase -> <tab>
Display all 188 possibilities? (y or n)
(and these 188 possibilities do not contain "(")
The probmen is obviously in the newly added second line of the following clause:
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
"SET SCHEMA", "SET (", "RESET (");
"set schema" and "set (" alternatives are competing, while completion of the common part "set<space>" leads to a string
compositionwhich does not have the check branch (Matches("ALTER", "VIEW", MatchAny, "SET")).
I think it may worth looking at "alter materialized view" completion tree and making "alter view" the same way.
The new status of this patch is: Waiting on Author