Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER - Mailing list pgsql-hackers

David Fetter <david@fetter.org> writes:

> On Wed, Oct 24, 2018 at 08:43:05AM +0900, Michael Paquier wrote:
>> On Tue, Oct 23, 2018 at 12:26:35PM +0100, Dagfinn Ilmari Mannsåker wrote:
>> > The last-minute change for CREATE (EVENT) TRIGGER to accept EXECUTE
>> > FUNCTION as well as EXECUTE PROCEDURE did not update the tab completion
>> > in psql to match.  Please find attached two patches that do this for
>> > CREATE TRIGGER and CREATE EVENT TRIGGER, respectively.
>> 
>> Yes, it would be nice to fix that.
>> 
>> > To keep the duplication minimal, I've changed it from completing EXECUTE
>> > PROCEDURE as a single thing to just EXECUTE, and then completing
>> > FUNCTION or FUNCTION and PROCEDURE after that depending on the server
>> > version.
>> 
>> +       else if (HeadMatches("CREATE", "EVENT", "TRIGGER") && TailMatches("EXECUTE"))
>> +               if (pset.sversion >= 110000)
>> +                       COMPLETE_WITH("FUNCTION", "PROCEDURE");
>> +               else
>> +                       COMPLETE_WITH("PROCEDURE");
>> 
>> PROCEDURE is documented as deprecated as of v11 for CREATE TRIGGER
>> and CREATE EVENT TRIGGER.  Wouldn't it be better to just complete
>> with FUNCTION for a v11 or newer server?  I think that we want to
>> encourage users to use EXECUTE FUNCTION if possible.
>
> +1 for not completing with syntax we've just deprecated.

Fair point. I was unsure about whether to complete every supported
variant or just the new one.  Updated patches attached.

- ilmari
-- 
"I use RMS as a guide in the same way that a boat captain would use
 a lighthouse.  It's good to know where it is, but you generally
 don't want to find yourself in the same spot." - Tollef Fog Heen

From 3fc4e87a1818cbc1386b0cf07275a2af0ee2e27b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Fri, 19 Oct 2018 17:13:05 +0100
Subject: [PATCH 1/2] Tab complete EXECUTE FUNCTION for CREATE TRIGGER
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The change to accept EXECUTE FUNCTION as well as EXECUTE PROCEDURE in
CREATE TRIGGER (commit 0a63f996e018ac508c858e87fa39cc254a5db49f)
didn't tell psql's tab completion system about this.

Change the completion from EXECUTE PROCEDURE to just EXECUTE, then
complete any CREATE TRIGGER … EXECUTE with FUNCTION instead of
PROCEDURE if we're connected to a version 11 or newer server.

In passing, add tab completion of EXECUTE FUNCTION/PROCEDURE after a
complete WHEN ( … ) clause.

The FUNCTION alternative for completing function names isn't strictly
necessary, because words_after_create makes it complete function names
after FUNCTION, but having all the completion logic for a single
command in one place seems neater to me.
---
 src/bin/psql/tab-complete.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 299600652f..b665d1f5e4 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2474,10 +2474,10 @@ psql_completion(const char *text, int start, int end)
         COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_views, NULL);
     else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("ON", MatchAny))
         COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
-                      "REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE");
+                      "REFERENCING", "FOR", "WHEN (", "EXECUTE");
     else if (HeadMatches("CREATE", "TRIGGER") &&
              (TailMatches("DEFERRABLE") || TailMatches("INITIALLY", "IMMEDIATE|DEFERRED")))
-        COMPLETE_WITH("REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE");
+        COMPLETE_WITH("REFERENCING", "FOR", "WHEN (", "EXECUTE");
     else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("REFERENCING"))
         COMPLETE_WITH("OLD TABLE", "NEW TABLE");
     else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("OLD|NEW", "TABLE"))
@@ -2485,17 +2485,17 @@ psql_completion(const char *text, int start, int end)
     else if (HeadMatches("CREATE", "TRIGGER") &&
              (TailMatches("REFERENCING", "OLD", "TABLE", "AS", MatchAny) ||
               TailMatches("REFERENCING", "OLD", "TABLE", MatchAny)))
-        COMPLETE_WITH("NEW TABLE", "FOR", "WHEN (", "EXECUTE PROCEDURE");
+        COMPLETE_WITH("NEW TABLE", "FOR", "WHEN (", "EXECUTE");
     else if (HeadMatches("CREATE", "TRIGGER") &&
              (TailMatches("REFERENCING", "NEW", "TABLE", "AS", MatchAny) ||
               TailMatches("REFERENCING", "NEW", "TABLE", MatchAny)))
-        COMPLETE_WITH("OLD TABLE", "FOR", "WHEN (", "EXECUTE PROCEDURE");
+        COMPLETE_WITH("OLD TABLE", "FOR", "WHEN (", "EXECUTE");
     else if (HeadMatches("CREATE", "TRIGGER") &&
              (TailMatches("REFERENCING", "OLD|NEW", "TABLE", "AS", MatchAny, "OLD|NEW", "TABLE", "AS", MatchAny) ||
               TailMatches("REFERENCING", "OLD|NEW", "TABLE", MatchAny, "OLD|NEW", "TABLE", "AS", MatchAny) ||
               TailMatches("REFERENCING", "OLD|NEW", "TABLE", "AS", MatchAny, "OLD|NEW", "TABLE", MatchAny) ||
               TailMatches("REFERENCING", "OLD|NEW", "TABLE", MatchAny, "OLD|NEW", "TABLE", MatchAny)))
-        COMPLETE_WITH("FOR", "WHEN (", "EXECUTE PROCEDURE");
+        COMPLETE_WITH("FOR", "WHEN (", "EXECUTE");
     else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("FOR"))
         COMPLETE_WITH("EACH", "ROW", "STATEMENT");
     else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("FOR", "EACH"))
@@ -2503,11 +2503,15 @@ psql_completion(const char *text, int start, int end)
     else if (HeadMatches("CREATE", "TRIGGER") &&
              (TailMatches("FOR", "EACH", "ROW|STATEMENT") ||
               TailMatches("FOR", "ROW|STATEMENT")))
-        COMPLETE_WITH("WHEN (", "EXECUTE PROCEDURE");
-    /* complete CREATE TRIGGER ... EXECUTE with PROCEDURE */
+        COMPLETE_WITH("WHEN (", "EXECUTE");
+    else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("WHEN", "(*)"))
+        COMPLETE_WITH("EXECUTE");
     else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("EXECUTE"))
-        COMPLETE_WITH("PROCEDURE");
-    else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("EXECUTE", "PROCEDURE"))
+        if (pset.sversion >= 110000)
+            COMPLETE_WITH("FUNCTION");
+        else
+            COMPLETE_WITH("PROCEDURE");
+    else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("EXECUTE", "FUNCTION|PROCEDURE"))
         COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_functions, NULL);
 
 /* CREATE ROLE,USER,GROUP <name> */
-- 
2.19.1

From a9904514b57b067997a7761fa5b32e796806d47f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 22 Oct 2018 18:06:20 +0100
Subject: [PATCH 2/2] Improve CREATE EVENT TRIGGER tab completion

This adds tab completion of the WHEN and EXECUTE FUNCTION/PROCEDURE
clauses to CREATE EVENT TRIGGER, similar to CREATE TRIGGER in the
previous commit.
---
 src/bin/psql/tab-complete.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b665d1f5e4..b9adf1182b 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2567,6 +2567,19 @@ psql_completion(const char *text, int start, int end)
     /* Complete CREATE EVENT TRIGGER <name> ON with event_type */
     else if (Matches("CREATE", "EVENT", "TRIGGER", MatchAny, "ON"))
         COMPLETE_WITH("ddl_command_start", "ddl_command_end", "sql_drop");
+    else if (Matches("CREATE", "EVENT", "TRIGGER", MatchAny, "ON", MatchAny))
+        COMPLETE_WITH("WHEN TAG IN (", "EXECUTE");
+    else if (Matches("CREATE", "EVENT", "TRIGGER", MatchAny, "ON", MatchAny, "WHEN"))
+        COMPLETE_WITH("TAG IN (");
+    else if (Matches("CREATE", "EVENT", "TRIGGER", MatchAny, "ON", MatchAny, "WHEN", "TAG", "IN", "(*)"))
+        COMPLETE_WITH("EXECUTE");
+    else if (HeadMatches("CREATE", "EVENT", "TRIGGER") && TailMatches("EXECUTE"))
+        if (pset.sversion >= 110000)
+            COMPLETE_WITH("FUNCTION");
+        else
+            COMPLETE_WITH("PROCEDURE");
+    else if (HeadMatches("CREATE", "EVENT", "TRIGGER") && TailMatches("EXECUTE", "FUNCTION|PROCEDURE"))
+        COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_functions, NULL);
 
 /* DEALLOCATE */
     else if (Matches("DEALLOCATE"))
-- 
2.19.1