Tab complete for CREATE OR REPLACE TRIGGER statement
Hi, Hackers.
Yesterday, OR REPLACE clause was provided to the CREATE TRIGGER statement, so I wrote a patch for tab completion for the psql command.
TRIGGER adds tab completion to the CREATE OR REPLACE statement, and the CREATE TRIGGER and CREATE OR REPLACE TRIGGER statements are completed in the same way.
I referred to the tab completion code of the CREATE RULE statement.
Regards,
Noriyoshi Shinoda
Attachments:
psql_create_or_replace_trigger_tab.diffapplication/octet-stream; name=psql_create_or_replace_trigger_tab.diffDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 5238a96..557826e 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1580,7 +1580,7 @@ psql_completion(const char *text, int start, int end)
/* complete with something you can create or replace */
else if (TailMatches("CREATE", "OR", "REPLACE"))
COMPLETE_WITH("FUNCTION", "PROCEDURE", "LANGUAGE", "RULE", "VIEW",
- "AGGREGATE", "TRANSFORM");
+ "AGGREGATE", "TRANSFORM", "TRIGGER");
/* DROP, but not DROP embedded in other commands */
/* complete with something you can drop */
@@ -2712,31 +2712,39 @@ psql_completion(const char *text, int start, int end)
"slot_name", "synchronous_commit");
/* CREATE TRIGGER --- is allowed inside CREATE SCHEMA, so use TailMatches */
- /* complete CREATE TRIGGER <name> with BEFORE,AFTER,INSTEAD OF */
- else if (TailMatches("CREATE", "TRIGGER", MatchAny))
+ /* complete CREATE [ OR REPLACE ] TRIGGER <name> with BEFORE,AFTER,INSTEAD OF */
+ else if (TailMatches("CREATE", "TRIGGER", MatchAny) ||
+ TailMatches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAny))
COMPLETE_WITH("BEFORE", "AFTER", "INSTEAD OF");
- /* complete CREATE TRIGGER <name> BEFORE,AFTER with an event */
- else if (TailMatches("CREATE", "TRIGGER", MatchAny, "BEFORE|AFTER"))
+ /* complete CREATE [ OR REPLACE ] TRIGGER <name> BEFORE,AFTER with an event */
+ else if (TailMatches("CREATE", "TRIGGER", MatchAny, "BEFORE|AFTER") ||
+ TailMatches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAny, "BEFORE|AFTER"))
COMPLETE_WITH("INSERT", "DELETE", "UPDATE", "TRUNCATE");
- /* complete CREATE TRIGGER <name> INSTEAD OF with an event */
- else if (TailMatches("CREATE", "TRIGGER", MatchAny, "INSTEAD", "OF"))
+ /* complete CREATE [ OR REPLACE ] TRIGGER <name> INSTEAD OF with an event */
+ else if (TailMatches("CREATE", "TRIGGER", MatchAny, "INSTEAD", "OF") ||
+ TailMatches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAny, "INSTEAD", "OF"))
COMPLETE_WITH("INSERT", "DELETE", "UPDATE");
- /* complete CREATE TRIGGER <name> BEFORE,AFTER sth with OR,ON */
+ /* complete CREATE [ OR REPLACE ] TRIGGER <name> BEFORE,AFTER sth with OR,ON */
else if (TailMatches("CREATE", "TRIGGER", MatchAny, "BEFORE|AFTER", MatchAny) ||
- TailMatches("CREATE", "TRIGGER", MatchAny, "INSTEAD", "OF", MatchAny))
+ TailMatches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAny, "BEFORE|AFTER", MatchAny) ||
+ TailMatches("CREATE", "TRIGGER", MatchAny, "INSTEAD", "OF", MatchAny) ||
+ TailMatches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAny, "INSTEAD", "OF", MatchAny))
COMPLETE_WITH("ON", "OR");
/*
- * complete CREATE TRIGGER <name> BEFORE,AFTER event ON with a list of
+ * complete CREATE [ OR REPLACE ] TRIGGER <name> BEFORE,AFTER event ON with a list of
* tables. EXECUTE FUNCTION is the recommended grammar instead of EXECUTE
* PROCEDURE in version 11 and upwards.
*/
- else if (TailMatches("CREATE", "TRIGGER", MatchAny, "BEFORE|AFTER", MatchAny, "ON"))
+ else if (TailMatches("CREATE", "TRIGGER", MatchAny, "BEFORE|AFTER", MatchAny, "ON") ||
+ TailMatches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAny, "BEFORE|AFTER", MatchAny, "ON"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
- /* complete CREATE TRIGGER ... INSTEAD OF event ON with a list of views */
- else if (TailMatches("CREATE", "TRIGGER", MatchAny, "INSTEAD", "OF", MatchAny, "ON"))
+ /* complete CREATE [ OR REPLACE ] TRIGGER ... INSTEAD OF event ON with a list of views */
+ else if (TailMatches("CREATE", "TRIGGER", MatchAny, "INSTEAD", "OF", MatchAny, "ON") ||
+ TailMatches("CREATE", "OR", "REPLACE", "TRIGGER", MatchAny, "INSTEAD", "OF", MatchAny, "ON"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_views, NULL);
- else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("ON", MatchAny))
+ else if ((HeadMatches("CREATE", "TRIGGER") || HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) &&
+ TailMatches("ON", MatchAny))
{
if (pset.sversion >= 110000)
COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
@@ -2745,7 +2753,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("NOT DEFERRABLE", "DEFERRABLE", "INITIALLY",
"REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE");
}
- else if (HeadMatches("CREATE", "TRIGGER") &&
+ else if ((HeadMatches("CREATE", "TRIGGER") || HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) &&
(TailMatches("DEFERRABLE") || TailMatches("INITIALLY", "IMMEDIATE|DEFERRED")))
{
if (pset.sversion >= 110000)
@@ -2753,11 +2761,13 @@ psql_completion(const char *text, int start, int end)
else
COMPLETE_WITH("REFERENCING", "FOR", "WHEN (", "EXECUTE PROCEDURE");
}
- else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("REFERENCING"))
+ else if ((HeadMatches("CREATE", "TRIGGER") || HeadMatches("CREATE", "OR", "REPLACE","TRIGGER")) &&
+ TailMatches("REFERENCING"))
COMPLETE_WITH("OLD TABLE", "NEW TABLE");
- else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("OLD|NEW", "TABLE"))
+ else if ((HeadMatches("CREATE", "TRIGGER") || HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) &&
+ TailMatches("OLD|NEW", "TABLE"))
COMPLETE_WITH("AS");
- else if (HeadMatches("CREATE", "TRIGGER") &&
+ else if ((HeadMatches("CREATE", "TRIGGER") || HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) &&
(TailMatches("REFERENCING", "OLD", "TABLE", "AS", MatchAny) ||
TailMatches("REFERENCING", "OLD", "TABLE", MatchAny)))
{
@@ -2766,7 +2776,7 @@ psql_completion(const char *text, int start, int end)
else
COMPLETE_WITH("NEW TABLE", "FOR", "WHEN (", "EXECUTE PROCEDURE");
}
- else if (HeadMatches("CREATE", "TRIGGER") &&
+ else if ((HeadMatches("CREATE", "TRIGGER") || HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) &&
(TailMatches("REFERENCING", "NEW", "TABLE", "AS", MatchAny) ||
TailMatches("REFERENCING", "NEW", "TABLE", MatchAny)))
{
@@ -2775,7 +2785,7 @@ psql_completion(const char *text, int start, int end)
else
COMPLETE_WITH("OLD TABLE", "FOR", "WHEN (", "EXECUTE PROCEDURE");
}
- else if (HeadMatches("CREATE", "TRIGGER") &&
+ else if ((HeadMatches("CREATE", "TRIGGER") || HeadMatches("CREATE", "OR", "REPLACE", "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) ||
@@ -2786,11 +2796,11 @@ psql_completion(const char *text, int start, int end)
else
COMPLETE_WITH("FOR", "WHEN (", "EXECUTE PROCEDURE");
}
- else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("FOR"))
+ else if ((HeadMatches("CREATE", "TRIGGER") || HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && TailMatches("FOR"))
COMPLETE_WITH("EACH", "ROW", "STATEMENT");
- else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("FOR", "EACH"))
+ else if ((HeadMatches("CREATE", "TRIGGER") || HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && TailMatches("FOR", "EACH"))
COMPLETE_WITH("ROW", "STATEMENT");
- else if (HeadMatches("CREATE", "TRIGGER") &&
+ else if ((HeadMatches("CREATE", "TRIGGER") || HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) &&
(TailMatches("FOR", "EACH", "ROW|STATEMENT") ||
TailMatches("FOR", "ROW|STATEMENT")))
{
@@ -2799,22 +2809,22 @@ psql_completion(const char *text, int start, int end)
else
COMPLETE_WITH("WHEN (", "EXECUTE PROCEDURE");
}
- else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("WHEN", "(*)"))
+ else if ((HeadMatches("CREATE", "TRIGGER") || HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && TailMatches("WHEN", "(*)"))
{
if (pset.sversion >= 110000)
COMPLETE_WITH("EXECUTE FUNCTION");
else
COMPLETE_WITH("EXECUTE PROCEDURE");
}
- /* complete CREATE TRIGGER ... EXECUTE with PROCEDURE|FUNCTION */
- else if (HeadMatches("CREATE", "TRIGGER") && TailMatches("EXECUTE"))
+ /* complete CREATE [ OR REPLACE ] TRIGGER ... EXECUTE with PROCEDURE|FUNCTION */
+ else if ((HeadMatches("CREATE", "TRIGGER") || HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) && TailMatches("EXECUTE"))
{
if (pset.sversion >= 110000)
COMPLETE_WITH("FUNCTION");
else
COMPLETE_WITH("PROCEDURE");
}
- else if (HeadMatches("CREATE", "TRIGGER") &&
+ else if ((HeadMatches("CREATE", "TRIGGER") || HeadMatches("CREATE", "OR", "REPLACE", "TRIGGER")) &&
TailMatches("EXECUTE", "FUNCTION|PROCEDURE"))
COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_functions, NULL);
On Mon, Nov 16, 2020 at 08:12:05AM +0000, Shinoda, Noriyoshi (PN Japan FSI) wrote:
Yesterday, OR REPLACE clause was provided to the CREATE TRIGGER
statement, so I wrote a patch for tab completion for the psql
command.
Thanks, the logic looks fine. I'll apply if there are no objections.
Please note that git diff --check and that the indentation does not
seem quite right, but that's easy enough to fix.
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
On Mon, Nov 16, 2020 at 08:12:05AM +0000, Shinoda, Noriyoshi (PN Japan FSI) wrote:
Yesterday, OR REPLACE clause was provided to the CREATE TRIGGER
statement, so I wrote a patch for tab completion for the psql
command.
Thanks, the logic looks fine. I'll apply if there are no objections.
Please note that git diff --check and that the indentation does not
seem quite right, but that's easy enough to fix.
It's kind of depressing how repetitive the patch is. There's
probably not much to be done about that in the short run, but
it seems to point up the need to start thinking about how to
refactor tab-complete.c more thoroughly.
regards, tom lane
On Mon, Nov 16, 2020 at 09:14:51PM -0500, Tom Lane wrote:
It's kind of depressing how repetitive the patch is. There's
probably not much to be done about that in the short run, but
it seems to point up the need to start thinking about how to
refactor tab-complete.c more thoroughly.
Agreed. One thing that I'd think could help is a new wild card to
make some of the words conditional in the list of items. But that may
be tricky once you consider the case of a group of words.
I don't think that this is a requirement for this thread, though.
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
I don't think that this is a requirement for this thread, though.
Agreed, I'm not trying to block this patch. Just wishing
there were a better way.
regards, tom lane
On Mon, Nov 16, 2020 at 10:14:10PM -0500, Tom Lane wrote:
Michael Paquier <michael@paquier.xyz> writes:
I don't think that this is a requirement for this thread, though.
Agreed, I'm not trying to block this patch. Just wishing
there were a better way.
Okay. I have tweaked a couple of things in the patch and applied it.
I am wondering if libreadline gives the possibility to implement an
optional grouping of words to complete, but diving into its
documentation I have not found something that we could use.
--
Michael
Okay. I have tweaked a couple of things in the patch and applied it.
Thank you so much.
The next time I post a patch, be careful with git --diff check and indentation.
Regards,
Noriyoshi Shinoda
-----Original Message-----
From: Michael Paquier [mailto:michael@paquier.xyz]
Sent: Wednesday, November 18, 2020 2:07 PM
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Shinoda, Noriyoshi (PN Japan FSI) <noriyoshi.shinoda@hpe.com>; pgsql-hackers@lists.postgresql.org
Subject: Re: Tab complete for CREATE OR REPLACE TRIGGER statement
On Mon, Nov 16, 2020 at 10:14:10PM -0500, Tom Lane wrote:
Michael Paquier <michael@paquier.xyz> writes:
I don't think that this is a requirement for this thread, though.
Agreed, I'm not trying to block this patch. Just wishing there were a
better way.
Okay. I have tweaked a couple of things in the patch and applied it.
I am wondering if libreadline gives the possibility to implement an optional grouping of words to complete, but diving into its documentation I have not found something that we could use.
--
Michael
On 2020-11-18 06:06, Michael Paquier wrote:
On Mon, Nov 16, 2020 at 10:14:10PM -0500, Tom Lane wrote:
Michael Paquier <michael@paquier.xyz> writes:
I don't think that this is a requirement for this thread, though.
Agreed, I'm not trying to block this patch. Just wishing
there were a better way.Okay. I have tweaked a couple of things in the patch and applied it.
I am wondering if libreadline gives the possibility to implement an
optional grouping of words to complete, but diving into its
documentation I have not found something that we could use.
To me the code looks like a prime candidate for "data-driven"
refactoring.
It should be redone as generic code that reads a table of rules with
params and then checks and applies each. Thus the repetitive code would
be replaced by a bit more generic code and a lot of code-free data.
--
Best regards,
Tels
Tels <nospam-pg-abuse@bloodgate.com> writes:
On 2020-11-18 06:06, Michael Paquier wrote:
On Mon, Nov 16, 2020 at 10:14:10PM -0500, Tom Lane wrote:
Agreed, I'm not trying to block this patch. Just wishing
there were a better way.
To me the code looks like a prime candidate for "data-driven"
refactoring.
It should be redone as generic code that reads a table of rules with
params and then checks and applies each. Thus the repetitive code would
be replaced by a bit more generic code and a lot of code-free data.
In the past I've looked into whether the rules could be autogenerated
from the backend's grammar. It did not look very promising though.
The grammar isn't really factorized appropriately -- for instance,
tab-complete has a lot of knowledge about which kinds of objects can
be named in particular places, while the Bison productions only know
that's a "name". Still, the precedent of ECPG suggests it might be
possible to process the grammar rules somehow to get to a useful
result.
regards, tom lane
Hello Tom,
On 2020-11-18 16:49, Tom Lane wrote:
Tels <nospam-pg-abuse@bloodgate.com> writes:
On 2020-11-18 06:06, Michael Paquier wrote:
On Mon, Nov 16, 2020 at 10:14:10PM -0500, Tom Lane wrote:
Agreed, I'm not trying to block this patch. Just wishing
there were a better way.To me the code looks like a prime candidate for "data-driven"
refactoring.
It should be redone as generic code that reads a table of rules with
params and then checks and applies each. Thus the repetitive code
would
be replaced by a bit more generic code and a lot of code-free data.In the past I've looked into whether the rules could be autogenerated
from the backend's grammar. It did not look very promising though.
The grammar isn't really factorized appropriately -- for instance,
tab-complete has a lot of knowledge about which kinds of objects can
be named in particular places, while the Bison productions only know
that's a "name". Still, the precedent of ECPG suggests it might be
possible to process the grammar rules somehow to get to a useful
result.
Hm, that would be even better, for now I was just thinking that
code like this:
IF RULE_A_MATCHES THEN
DO STUFF A
ELSE IF RULE_B_MATCHES THEN
DO_STUFF_B
ELSE IF RULE_C_MATCHES THEN
DO_STUFF_C
...
should be replaced by
RULE_A MATCH STUFF
RULE_B MATCH STUFF
RULE_C MATCH STUFF
...
FOREACH RULE DO
IF RULE.MATCH THEN
DO RULE.STUFF
END FOREACH
Even if the rules would be manually created (converted from the current
code), that would be more compact and probably less error-prone.
Creating the rule automatically turns this into a completely different
story.
--
Best regards,
Tels