Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cKGXR-0007xR-Dk for pgsql-hackers@arkaria.postgresql.org; Fri, 23 Dec 2016 03:28:57 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1cKGXQ-0001XI-Qp for pgsql-hackers@arkaria.postgresql.org; Fri, 23 Dec 2016 03:28:56 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1cKGXO-0001Vh-Sk for pgsql-hackers@postgresql.org; Fri, 23 Dec 2016 03:28:54 +0000 Received: from tamriel.snowman.net ([72.66.115.51]) by magus.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cKGXC-0003ck-5z for pgsql-hackers@postgresql.org; Fri, 23 Dec 2016 03:28:52 +0000 Received: by tamriel.snowman.net (Postfix, from userid 1000) id E04B65F794; Thu, 22 Dec 2016 22:27:40 -0500 (EST) Date: Thu, 22 Dec 2016 22:27:40 -0500 From: Stephen Frost To: Gilles Darold Cc: pgsql-hackers@postgresql.org Subject: Re: [patch] psql tab completion for ALTER DEFAULT PRIVILEGES Message-ID: <20161223032740.GV18360@tamriel.snowman.net> References: <1614593c-e356-5b27-6dba-66320a9bc68b@dalibo.com> <9905474c-ab29-bb66-408f-ca776fb3d69c@dalibo.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="qy9NDBoEzxL7V3MP" Content-Disposition: inline In-Reply-To: <9905474c-ab29-bb66-408f-ca776fb3d69c@dalibo.com> X-Editor: Vim http://www.vim.org/ X-Info: http://www.snowman.net X-Operating-System: Linux/3.13.0-91-generic (x86_64) X-Uptime: 22:23:15 up 167 days, 4:47, 36 users, load average: 0.52, 0.50, 0.50 User-Agent: Mutt/1.5.21 (2010-09-15) X-Pg-Spam-Score: -5.0 (-----) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgsql-hackers Precedence: bulk Sender: pgsql-hackers-owner@postgresql.org --qy9NDBoEzxL7V3MP Content-Type: multipart/mixed; boundary="aNCdKw9zdX/rqzEJ" Content-Disposition: inline --aNCdKw9zdX/rqzEJ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Gilles, * Gilles Darold (gilles.darold@dalibo.com) wrote: > Added to next commitfest. To explain more this patch, the completion of > SQL command: >=20 > ALTER DEFAULT PRIVILEGES FOR ROLE xxx [tab] Here is a cleaned up patch for master and 9.6. Tomorrow I'll look into what we can do for 9.5 and earlier, which are also wrong, but the code is quite a bit different. Note that beyond just changing the comments, I removed the alternative spelling of 'role' when doing tab completion- there's no different between 'role' and 'user', so there's no point in making the user have to pick one when they're tab-completing. Of course, we still accept both and if the user chooses to write out 'for user', we will handle that correctly and continue the tab completion beyond that. Thanks! Stephen --aNCdKw9zdX/rqzEJ Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="patch-completion-alter-default-privileges_v2_master.diff" Content-Transfer-Encoding: quoted-printable =46rom 1f7eb8473d40497b67cc30b40aefe2e1529317c0 Mon Sep 17 00:00:00 2001 =46rom: Stephen Frost Date: Thu, 22 Dec 2016 22:00:55 -0500 Subject: [PATCH] Fix tab completion in psql for ALTER DEFAULT PRIVILEGES When providing tab completion for ALTER DEFAULT PRIVILEGES, we are including the list of roles as possible options for completion after the GRANT or REVOKE. Further, we accept FOR ROLE/IN SCHEMA at the same time and in either order, but the tab completion was only working for one or the other. Lastly, we weren't using the actual list of allowed kinds of objects for default privileges for completion after the 'GRANT X ON' but instead were completeing to what 'GRANT X ON' supports, which isn't the ssame at all. Address these issues by improving the forward tab-completion for ALTER DEFAULT PRIVILEGES and then constrain and correct how the tail completion is done when it is for ALTER DEFAULT PRIVILEGES. Author: Gilles Darold, cleaned up and comments added by me. Discussion: https://www.postgresql.org/message-id/1614593c-e356-5b27-6dba-6= 6320a9bc68b@dalibo.com Back-patch to 9.2. --- src/bin/psql/tab-complete.c | 57 ++++++++++++++++++++++++++++++++++++++---= ---- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index cd64c39..02c8d60 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1570,13 +1570,31 @@ psql_completion(const char *text, int start, int en= d) COMPLETE_WITH_CONST("PASSWORD"); /* ALTER DEFAULT PRIVILEGES */ else if (Matches3("ALTER", "DEFAULT", "PRIVILEGES")) - COMPLETE_WITH_LIST3("FOR ROLE", "FOR USER", "IN SCHEMA"); + COMPLETE_WITH_LIST2("FOR ROLE", "IN SCHEMA"); /* ALTER DEFAULT PRIVILEGES FOR */ else if (Matches4("ALTER", "DEFAULT", "PRIVILEGES", "FOR")) - COMPLETE_WITH_LIST2("ROLE", "USER"); - /* ALTER DEFAULT PRIVILEGES { FOR ROLE ... | IN SCHEMA ... } */ - else if (Matches6("ALTER", "DEFAULT", "PRIVILEGES", "FOR", "ROLE|USER", M= atchAny) || - Matches6("ALTER", "DEFAULT", "PRIVILEGES", "IN", "SCHEMA", MatchAny)) + COMPLETE_WITH_CONST("ROLE"); + /* ALTER DEFAULT PRIVILEGES IN */ + else if (Matches4("ALTER", "DEFAULT", "PRIVILEGES", "IN")) + COMPLETE_WITH_CONST("SCHEMA"); + /* ALTER DEFAULT PRIVILEGES FOR ROLE|USER ... */ + else if (Matches6("ALTER", "DEFAULT", "PRIVILEGES", "FOR", "ROLE|USER", + MatchAny)) + COMPLETE_WITH_LIST3("GRANT", "REVOKE", "IN SCHEMA"); + /* ALTER DEFAULT PRIVILEGES IN SCHEMA ... */ + else if (Matches6("ALTER", "DEFAULT", "PRIVILEGES", "IN", "SCHEMA", + MatchAny)) + COMPLETE_WITH_LIST3("GRANT", "REVOKE", "FOR ROLE"); + /* ALTER DEFAULT PRIVILEGES IN SCHEMA ... FOR */ + else if (Matches7("ALTER", "DEFAULT", "PRIVILEGES", "IN", "SCHEMA", + MatchAny, "FOR")) + COMPLETE_WITH_CONST("ROLE"); + /* ALTER DEFAULT PRIVILEGES FOR ROLE|USER ... IN SCHEMA ... */ + /* ALTER DEFAULT PRIVILEGES IN SCHEMA ... FOR ROLE|USER ... */ + else if (Matches9("ALTER", "DEFAULT", "PRIVILEGES", "FOR", "ROLE|USER", + MatchAny, "IN", "SCHEMA", MatchAny) || + Matches9("ALTER", "DEFAULT", "PRIVILEGES", "IN", "SCHEMA", + MatchAny, "FOR", "ROLE|USER", MatchAny)) COMPLETE_WITH_LIST2("GRANT", "REVOKE"); /* ALTER DOMAIN */ else if (Matches3("ALTER", "DOMAIN", MatchAny)) @@ -2566,10 +2584,22 @@ psql_completion(const char *text, int start, int en= d) else if (TailMatches2("FOREIGN", "SERVER")) COMPLETE_WITH_QUERY(Query_for_list_of_servers); =20 -/* GRANT && REVOKE --- is allowed inside CREATE SCHEMA, so use TailMatches= */ +/* + * GRANT and REVOKE are allowed inside CREATE SCHEMA and + * ALTER DEFAULT PRIVILEGES, so use TailMatches + */ /* Complete GRANT/REVOKE with a list of roles and privileges */ else if (TailMatches1("GRANT|REVOKE")) - COMPLETE_WITH_QUERY(Query_for_list_of_roles + /* + * With ALTER DEFAULT PRIVILEGES, restrict completion + * to grantable privileges (can't grant roles) + */ + if (HeadMatches3("ALTER","DEFAULT","PRIVILEGES")) + COMPLETE_WITH_LIST10("SELECT", "INSERT", "UPDATE", + "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER", + "EXECUTE", "USAGE", "ALL"); + else + COMPLETE_WITH_QUERY(Query_for_list_of_roles " UNION SELECT 'SELECT'" " UNION SELECT 'INSERT'" " UNION SELECT 'UPDATE'" @@ -2610,7 +2640,14 @@ psql_completion(const char *text, int start, int end) * privilege. */ else if (TailMatches3("GRANT|REVOKE", MatchAny, "ON")) - COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tsvmf, + /* + * With ALTER DEFAULT PRIVILEGES, restrict completion + * to the kinds of objects supported. + */ + if (HeadMatches3("ALTER","DEFAULT","PRIVILEGES")) + COMPLETE_WITH_LIST4("TABLES", "SEQUENCES", "FUNCTIONS", "TYPES"); + else + COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tsvmf, " UNION SELECT 'ALL FUNCTIONS IN SCHEMA'" " UNION SELECT 'ALL SEQUENCES IN SCHEMA'" " UNION SELECT 'ALL TABLES IN SCHEMA'" @@ -2673,7 +2710,9 @@ psql_completion(const char *text, int start, int end) else if ((HeadMatches1("GRANT") && TailMatches1("TO")) || (HeadMatches1("REVOKE") && TailMatches1("FROM"))) COMPLETE_WITH_QUERY(Query_for_list_of_grant_roles); - + /* Complete "ALTER DEFAULT PRIVILEGES ... GRANT/REVOKE ... TO/FROM */ + else if (HeadMatches3("ALTER","DEFAULT", "PRIVILEGES") && TailMatches1("T= O|FROM")) + COMPLETE_WITH_QUERY(Query_for_list_of_grant_roles); /* Complete "GRANT/REVOKE ... ON * *" with TO/FROM */ else if (HeadMatches1("GRANT") && TailMatches3("ON", MatchAny, MatchAny)) COMPLETE_WITH_CONST("TO"); --=20 2.7.4 --aNCdKw9zdX/rqzEJ-- --qy9NDBoEzxL7V3MP Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJYXJmsAAoJEO1sijiDR2RVp2kP/319w5xpBKxpK6nTyGstmS2/ z4p/E+48EltRsCQ+nmmzv2Gnc5GxrqjsGBOZVMsSV9z8lf488+cq+SJYrzZbZIS8 Pky0Whl3CZhirZyCiwFBvqJnFikum7CUdbOFjmzTHY1R0c5FkbhKHoyHJ/N7L14C yBLZYwj5FBqTzWKMPRhRlMBcFugdyib0FAU6HqNQD14YiBoufRH73ybMEGqp8kIV p6XayFUrnwlO3dIbBEyxogwMNlSOhyo8fwktYDKP8H6H9aBwvilOVUzGGWr5ZBN+ QiwqWJMZ7gSRSSXjyyJpaM3DlC5IFIUB/1uuKrCcN9sFjw9WWgH8uT6ekpBf8s0L 8b2m4/BGBsiwF815KlxO/0v1apCkuyu2Ad0WaGccuRUJ4Yiu3yfm7isTRgvcWUGp 0xX0XRV2gLWeR3Dw07qctvoZwu0wjlvWqADa5lnmwFHDo5sVAtL+mOEKCfuQulyQ 8LjV3BqSB1CszoH46vlE7ZjKIBxxoTFHNTwzLHqpfRZmfRpTxqJiII2Ixu59gFaS BMjhKgF9+JmmiH+kUsnqa6z7c37yeQObhLDU57nO8eCsJpgID8jRYND6/7q09Cuj gUCTvNp8b/J+fSCAr0dge03blZAojrxiSklBgZNRRH1R17Z5RAy4Qe4gNSsFsROx U4itLvvQu1TKaByiw9Gx =/TvN -----END PGP SIGNATURE----- --qy9NDBoEzxL7V3MP--