Обсуждение: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

Поиск
Список
Период
Сортировка

[PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
Hi hackers,

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.

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.

- ilmari
-- 
- Twitter seems more influential [than blogs] in the 'gets reported in
  the mainstream press' sense at least.               - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
  to a mainstream media article.                      - Calle Dybedahl

From 61d6fcc4f979f31b40fb54d3a7481e27d62eb772 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 as well as
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..70aa629a3b 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", "PROCEDURE");
+        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 1fa81580cc399cff78e4a3d0914145ca824494e0 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 TRIGGER/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 70aa629a3b..3253e51ebe 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", "PROCEDURE");
+        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


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes:

> Hi hackers,
>
> 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.

Added to the current commitfest: https://commitfest.postgresql.org/20/1837/

- ilmari
-- 
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
 the consequences of."                              -- Skud's Meta-Law


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Michael Paquier
Дата:
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.
--
Michael

Вложения

Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
David Fetter
Дата:
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.

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
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


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Michael Paquier
Дата:
On Wed, Oct 24, 2018 at 10:36:41AM +0100, Dagfinn Ilmari Mannsåker wrote:
> Fair point. I was unsure about whether to complete every supported
> variant or just the new one.  Updated patches attached.

One problem with this approach is that a user needs to use twice tab.
The first time is to show "EXECUTE", and the second time is to show
automatically "PROCEDURE" or "FUNCTION" automatically.  The patch as it
stands is less complicated, but I can imagine as well that what's
propose could confuse people into thinking that they need to type
something after EXECUTE has showed up on the prompt.  I vote for
simplicity and there are other code paths where completion for one
element only is done.  Any objections?

+   else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("WHEN", "(*)"))
+       COMPLETE_WITH("EXECUTE");

It seems to me that this should be removed, it would fail at parsing if
completed.
--
Michael

Вложения

Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Tom Lane
Дата:
Michael Paquier <michael@paquier.xyz> writes:
> On Wed, Oct 24, 2018 at 10:36:41AM +0100, Dagfinn Ilmari Mannsåker wrote:
>> Fair point. I was unsure about whether to complete every supported
>> variant or just the new one.  Updated patches attached.

> One problem with this approach is that a user needs to use twice tab.
> The first time is to show "EXECUTE", and the second time is to show
> automatically "PROCEDURE" or "FUNCTION" automatically.

Yeah.  Why don't we keep the existing behavior of completing both
words at once, but make it server-version-dependent which completion
you get?

            regards, tom lane


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
Tom Lane <tgl@sss.pgh.pa.us> writes:

> Michael Paquier <michael@paquier.xyz> writes:
>> On Wed, Oct 24, 2018 at 10:36:41AM +0100, Dagfinn Ilmari Mannsåker wrote:
>>> Fair point. I was unsure about whether to complete every supported
>>> variant or just the new one.  Updated patches attached.
>
>> One problem with this approach is that a user needs to use twice tab.
>> The first time is to show "EXECUTE", and the second time is to show
>> automatically "PROCEDURE" or "FUNCTION" automatically.
>
> Yeah.  Why don't we keep the existing behavior of completing both
> words at once, but make it server-version-dependent which completion
> you get?

I did that initially, but because COMPLETE_WITH() requres constant
arguments, I had to repeat the whole list with just changing PROCEDURE
to FUNCTION, which I thought was undesirably repetitive.  If there's a
more concise alternative to the below, or the consensus is that saving
one TAB press is worth it, I'll change it.

    else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("ON", MatchAny))
        if (pset.sversion >= 110000)
            COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
                          "REFERENCING", "FOR", "WHEN (", "EXECUTE FUNCTION");
        else
            COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
                          "REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE");

- ilmari
-- 
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
 the consequences of."                              -- Skud's Meta-Law


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Tom Lane
Дата:
ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> Yeah.  Why don't we keep the existing behavior of completing both
>> words at once, but make it server-version-dependent which completion
>> you get?

> I did that initially, but because COMPLETE_WITHc() requres constant
> arguments, I had to repeat the whole list with just changing PROCEDURE
> to FUNCTION, which I thought was undesirably repetitive.  If there's a
> more concise alternative to the below, or the consensus is that saving
> one TAB press is worth it, I'll change it.

>     else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("ON", MatchAny))
>         if (pset.sversion >= 110000)
>             COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
>                           "REFERENCING", "FOR", "WHEN (", "EXECUTE FUNCTION");
>         else
>             COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
>                           "REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE");

Well, that's not beautiful, but there aren't so many alternatives
that it's really unmaintainable.  I think it's probably better than
requiring an additional TAB-press.

            regards, tom lane


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
Michael Paquier <michael@paquier.xyz> writes:

> +   else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("WHEN", "(*)"))
> +       COMPLETE_WITH("EXECUTE");
>
> It seems to me that this should be removed, it would fail at parsing if
> completed. 

The * is a wildcard, so it completes EXECUTE after CREATE TRIGGER … WHEN
(<condition>), which is perfectly valid.  Using * in the middle of an
alternative is a fairly new feature, added by Tom a month ago in commit
121213d9d8527f880f153e4a032ee1a4cd43833f.

- 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


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Michael Paquier
Дата:
On Thu, Oct 25, 2018 at 12:25:33PM +0100, Dagfinn Ilmari Mannsåker wrote:
> I did that initially, but because COMPLETE_WITH() requres constant
> arguments, I had to repeat the whole list with just changing PROCEDURE
> to FUNCTION, which I thought was undesirably repetitive.  If there's a
> more concise alternative to the below, or the consensus is that saving
> one TAB press is worth it, I'll change it.
>
>     else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("ON", MatchAny))
>         if (pset.sversion >= 110000)
>             COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
>                           "REFERENCING", "FOR", "WHEN (", "EXECUTE FUNCTION");
>         else
>             COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
>                           "REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE");

[thinking]

To keep the code simple, you could do something like that, by checking
the head keywords for a match with CREATE TRIGGER, and then move all the
existing conditions within it:
else if (HeadMatches("CREATE", "TRIGGER", MatchAny))
{
    char *execute_keyword;

    if (pset.sversion >= 110000)
        execute_keyword = "EXECUTE FUNCTION";
    else
        execute_keyword = "EXECUTE PROCEDURE";

    if (TailMatches("CREATE", "TRIGGER", MatchAny))
        COMPLETE_WITH("BEFORE", "AFTER", "INSTEAD OF");
    [...]
    else if (the other existing conditions)
        blah and use execute_keyword in the lists;
}

If we do the automatic completion of both words at the same time, let's
put only in a single place the version-based switch.  This method costs
an extra match check on the header keywords when CREATE TRIGGER matches,
but it allows all the other checks to skip steps, which is actually a
win for the rest.
--
Michael

Вложения

Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Tom Lane
Дата:
Michael Paquier <michael@paquier.xyz> writes:
> To keep the code simple, you could do something like that, by checking
> the head keywords for a match with CREATE TRIGGER, and then move all the
> existing conditions within it:
> ...
>     char *execute_keyword;
> ...
>         blah and use execute_keyword in the lists;

Don't think that works unless we remove some of the "const" annotation
in COMPLETE_WITH; which I'd rather not.

            regards, tom lane


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
Tom Lane <tgl@sss.pgh.pa.us> writes:

> ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
>> Tom Lane <tgl@sss.pgh.pa.us> writes:
>>> Yeah.  Why don't we keep the existing behavior of completing both
>>> words at once, but make it server-version-dependent which completion
>>> you get?
>
>> I did that initially, but because COMPLETE_WITHc() requres constant
>> arguments, I had to repeat the whole list with just changing PROCEDURE
>> to FUNCTION, which I thought was undesirably repetitive.  If there's a
>> more concise alternative to the below, or the consensus is that saving
>> one TAB press is worth it, I'll change it.
>
>>     else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("ON", MatchAny))
>>         if (pset.sversion >= 110000)
>>             COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
>>                           "REFERENCING", "FOR", "WHEN (", "EXECUTE FUNCTION");
>>         else
>>             COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
>>                           "REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE");
>
> Well, that's not beautiful, but there aren't so many alternatives
> that it's really unmaintainable.  I think it's probably better than
> requiring an additional TAB-press.

Okay, revised patches attached.  I also tweaked the CREATE EVENT TRIGGER
completion to accept multple <filter_varaible> IN (<filter_value>)
conditions seprated by AND in the WHEN clause (but not to suggest that,
since we only actually support one <filter_variable>, TAG).

- ilmari
-- 
"A disappointingly low fraction of the human race is,
 at any given time, on fire." - Stig Sandbeck Mathisen

From ba00a7ebc81008665b65bda5a3836b2b343dd804 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 v2 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.

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 | 43 +++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 299600652f..97825c9d54 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2473,11 +2473,18 @@ psql_completion(const char *text, int start, int end)
     else if (TailMatches("CREATE", "TRIGGER", MatchAny, "INSTEAD", "OF", MatchAny, "ON"))
         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");
+        if (pset.sversion >= 110000)
+            COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
+                          "REFERENCING", "FOR", "WHEN (", "EXECUTE FUNCTION");
+        else
+            COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
+                          "REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE");
     else if (HeadMatches("CREATE", "TRIGGER") &&
              (TailMatches("DEFERRABLE") || TailMatches("INITIALLY", "IMMEDIATE|DEFERRED")))
-        COMPLETE_WITH("REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE");
+        if (pset.sversion >= 110000)
+            COMPLETE_WITH("REFERENCING", "FOR", "WHEN (", "EXECUTE FUNCTION");
+        else
+            COMPLETE_WITH("REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE");
     else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("REFERENCING"))
         COMPLETE_WITH("OLD TABLE", "NEW TABLE");
     else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("OLD|NEW", "TABLE"))
@@ -2485,17 +2492,26 @@ 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");
+        if (pset.sversion >= 110000)
+            COMPLETE_WITH("NEW TABLE", "FOR", "WHEN (", "EXECUTE FUNCTION");
+        else
+            COMPLETE_WITH("NEW TABLE", "FOR", "WHEN (", "EXECUTE PROCEDURE");
     else if (HeadMatches("CREATE", "TRIGGER") &&
              (TailMatches("REFERENCING", "NEW", "TABLE", "AS", MatchAny) ||
               TailMatches("REFERENCING", "NEW", "TABLE", MatchAny)))
-        COMPLETE_WITH("OLD TABLE", "FOR", "WHEN (", "EXECUTE PROCEDURE");
+        if (pset.sversion >= 110000)
+            COMPLETE_WITH("OLD TABLE", "FOR", "WHEN (", "EXECUTE FUNCTION");
+        else
+            COMPLETE_WITH("OLD TABLE", "FOR", "WHEN (", "EXECUTE PROCEDURE");
     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");
+        if (pset.sversion >= 110000)
+            COMPLETE_WITH("FOR", "WHEN (", "EXECUTE FUNCTION");
+        else
+            COMPLETE_WITH("FOR", "WHEN (", "EXECUTE PROCEDURE");
     else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("FOR"))
         COMPLETE_WITH("EACH", "ROW", "STATEMENT");
     else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("FOR", "EACH"))
@@ -2503,11 +2519,16 @@ 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 */
-    else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("EXECUTE"))
-        COMPLETE_WITH("PROCEDURE");
-    else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("EXECUTE", "PROCEDURE"))
+        if (pset.sversion >= 110000)
+            COMPLETE_WITH("WHEN (", "EXECUTE FUNCTION");
+        else
+            COMPLETE_WITH("WHEN (", "EXECUTE PROCEDURE");
+    else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("WHEN", "(*)"))
+        if (pset.sversion >= 110000)
+            COMPLETE_WITH("EXECUTE FUNCTION");
+        else
+            COMPLETE_WITH("EXECUTE 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 e4aaa0f2c5c5f11c5796b726ef2e2b9ab1ac8f33 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 v2 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 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 97825c9d54..3d0d738bed 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2584,6 +2584,18 @@ 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))
+        if (pset.sversion >= 110000)
+            COMPLETE_WITH("WHEN TAG IN (", "EXECUTE FUNCTION");
+        else
+            COMPLETE_WITH("WHEN TAG IN (", "EXECUTE PROCEDURE");
+    else if (HeadMatches("CREATE", "EVENT", "TRIGGER") && TailMatches("WHEN|AND", MatchAny, "IN", "(*)"))
+        if (pset.sversion >= 110000)
+            COMPLETE_WITH("EXECUTE FUNCTION");
+        else
+            COMPLETE_WITH("EXECUTE 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


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Michael Paquier
Дата:
On Thu, Oct 25, 2018 at 12:52:01PM +0100, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> To keep the code simple, you could do something like that, by checking
>> the head keywords for a match with CREATE TRIGGER, and then move all the
>> existing conditions within it:
>> ...
>>     char *execute_keyword;
>> ...
>>         blah and use execute_keyword in the lists;
>
> Don't think that works unless we remove some of the "const" annotation
> in COMPLETE_WITH; which I'd rather not.

Indeed, gcc just complained about the thing not being a constant after
testing, so please let me discard this suggestion.
--
Michael

Вложения

Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Michael Paquier
Дата:
On Thu, Oct 25, 2018 at 05:02:32PM +0100, Dagfinn Ilmari Mannsåker wrote:
> Okay, revised patches attached.  I also tweaked the CREATE EVENT TRIGGER
> completion to accept multple <filter_varaible> IN (<filter_value>)
> conditions seprated by AND in the WHEN clause (but not to suggest that,
> since we only actually support one <filter_variable>, TAG).

Committed 0001 now which adds tab completion for CREATE TRIGGER.
Something you missed is that we should still be able to complete with
PROCEDURE or FUNCTION (depending on the backend version) if CREATE
TRIGGER .. EXECUTE is present on screen.  The priority is given to
complete with both EXECUTE PROCEDURE and EXECUTE FUNCTION at the same
time, but you also should have the intermediate state.  I also added
some brackets for clarity, and a comment about the grammar selection.
--
Michael

Вложения

Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Michael Paquier
Дата:
On Fri, Oct 26, 2018 at 09:31:48AM +0900, Michael Paquier wrote:
> Committed 0001 now which adds tab completion for CREATE TRIGGER.
> Something you missed is that we should still be able to complete with
> PROCEDURE or FUNCTION (depending on the backend version) if CREATE
> TRIGGER .. EXECUTE is present on screen.  The priority is given to
> complete with both EXECUTE PROCEDURE and EXECUTE FUNCTION at the same
> time, but you also should have the intermediate state.  I also added
> some brackets for clarity, and a comment about the grammar selection.

And 0002 is committed as well.  Thanks for the patches!
--
Michael

Вложения

Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Tom Lane
Дата:
Michael Paquier <michael@paquier.xyz> writes:
> On Fri, Oct 26, 2018 at 09:31:48AM +0900, Michael Paquier wrote:
>> Committed 0001 now which adds tab completion for CREATE TRIGGER.

> And 0002 is committed as well.  Thanks for the patches! 

The committed patches look sane to me, but should we back-patch into 11?
This isn't quite a bug fix maybe, but it's inconsistent if v11 server &
docs recommend this syntax while v11 psql doesn't produce it.

            regards, tom lane


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
Tom Lane <tgl@sss.pgh.pa.us> writes:

> Michael Paquier <michael@paquier.xyz> writes:
>> On Fri, Oct 26, 2018 at 09:31:48AM +0900, Michael Paquier wrote:
>>> Committed 0001 now which adds tab completion for CREATE TRIGGER.
>
>> And 0002 is committed as well.  Thanks for the patches! 

Thanks for reviewing and committing!

> The committed patches look sane to me, but should we back-patch into 11?
> This isn't quite a bug fix maybe, but it's inconsistent if v11 server &
> docs recommend this syntax while v11 psql doesn't produce it.

I was going to suggest backpatching it, as I consider it a bug in the
original implementation, if not critical.  Making it harder for people
to use the recommended syntax than the deprecated one is not nice.

>             regards, tom lane

- ilmari
-- 
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
 the consequences of."                              -- Skud's Meta-Law


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Michael Paquier
Дата:
On Fri, Oct 26, 2018 at 11:15:19AM +0100, Dagfinn Ilmari Mannsåker wrote:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> The committed patches look sane to me, but should we back-patch into 11?
>> This isn't quite a bug fix maybe, but it's inconsistent if v11 server &
>> docs recommend this syntax while v11 psql doesn't produce it.
>
> I was going to suggest backpatching it, as I consider it a bug in the
> original implementation, if not critical.  Making it harder for people
> to use the recommended syntax than the deprecated one is not nice.

I didn't think that this was much of a big deal as the deprecated
grammar is still supported on v11, but as both of you think in this
sense I am fine to patch REL_11_STABLE as well.  Please just wait a
bit...
--
Michael

Вложения

Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
Tom Lane <tgl@sss.pgh.pa.us> writes:

> Michael Paquier <michael@paquier.xyz> writes:
>> On Fri, Oct 26, 2018 at 09:31:48AM +0900, Michael Paquier wrote:
>>> Committed 0001 now which adds tab completion for CREATE TRIGGER.
>
>> And 0002 is committed as well.  Thanks for the patches! 
>
> The committed patches look sane to me, but should we back-patch into 11?
> This isn't quite a bug fix maybe, but it's inconsistent if v11 server &
> docs recommend this syntax while v11 psql doesn't produce it.

A related issue is whether we should change pg_get_triggerdef() to emit
the new syntax as well (in HEAD only, since it would break things that
parse the output).

- 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


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Tom Lane
Дата:
ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> A related issue is whether we should change pg_get_triggerdef() to emit
> the new syntax as well (in HEAD only, since it would break things that
> parse the output).

No, because pg_get_triggerdef() does not know what its output will be
used for.  We can probably change that five years down the road, but
right now it could be expected to break reasonable use-cases.

            regards, tom lane


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Michael Paquier
Дата:
On Fri, Oct 26, 2018 at 09:08:13PM +0900, Michael Paquier wrote:
> I didn't think that this was much of a big deal as the deprecated
> grammar is still supported on v11, but as both of you think in this
> sense I am fine to patch REL_11_STABLE as well.  Please just wait a
> bit...

Done for the part with CREATE TRIGGER.  The recent support added for tab
completion of CREATE EVENT TRIGGER .. EXECUTE is new as of v12 so this
is not gaining a backpatch.
--
Michael

Вложения

Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
Michael Paquier <michael@paquier.xyz> writes:

> On Fri, Oct 26, 2018 at 09:08:13PM +0900, Michael Paquier wrote:
>> I didn't think that this was much of a big deal as the deprecated
>> grammar is still supported on v11, but as both of you think in this
>> sense I am fine to patch REL_11_STABLE as well.  Please just wait a
>> bit...
>
> Done for the part with CREATE TRIGGER.  The recent support added for tab
> completion of CREATE EVENT TRIGGER .. EXECUTE is new as of v12 so this
> is not gaining a backpatch.

Thanks!  I've updated the commitfest entry with you as the committer and
Tom as the reviewer, and marked it as committed.

- ilmari
-- 
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
 the consequences of."                              -- Skud's Meta-Law


Re: [PATCH] Tab complete EXECUTE FUNCTION for CREATE (EVENT) TRIGGER

От
Michael Paquier
Дата:
On Fri, Oct 26, 2018 at 05:11:59PM +0100, Dagfinn Ilmari Mannsåker wrote:
> Thanks!  I've updated the commitfest entry with you as the committer and
> Tom as the reviewer, and marked it as committed.

Thanks a lot for updating the CF app.  I have been a bit sloppy here.
--
Michael

Вложения