[PATCH] psql: Add tab-complete for optional view parameters
Hi all!
This adds tab-complete for optional parameters (as can be specified
using WITH) for views, similarly to how it works for storage parameters
of tables.
Thanks,
Christoph Heiss
Attachments:
0001-psql-Add-tab-complete-for-optional-view-parameters.patchtext/x-patch; charset=UTF-8; name=0001-psql-Add-tab-complete-for-optional-view-parameters.patchDownload
From bd12c08a80b5973fdcac59def213216d06f150b0 Mon Sep 17 00:00:00 2001
From: Christoph Heiss <contact@christoph-heiss.at>
Date: Wed, 7 Dec 2022 19:15:48 +0100
Subject: [PATCH] psql: Add tab-complete for optional view parameters
This adds them in the same matter as it works for storage parameters of
tables.
Signed-off-by: Christoph Heiss <contact@christoph-heiss.at>
---
src/bin/psql/tab-complete.c | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 89e7317c23..999a0e5aa1 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1310,6 +1310,13 @@ static const char *const table_storage_parameters[] = {
NULL
};
+/* Optional parameters for CREATE VIEW and ALTER VIEW */
+static const char *const view_optional_parameters[] = {
+ "check_option",
+ "security_barrier",
+ "security_invoker",
+ NULL
+};
/* Forward declaration of functions */
static char **psql_completion(const char *text, int start, int end);
@@ -2139,7 +2146,7 @@ psql_completion(const char *text, int start, int end)
/* ALTER VIEW <name> */
else if (Matches("ALTER", "VIEW", MatchAny))
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
- "SET SCHEMA");
+ "SET SCHEMA", "SET (", "RESET (");
/* ALTER VIEW xxx RENAME */
else if (Matches("ALTER", "VIEW", MatchAny, "RENAME"))
COMPLETE_WITH_ATTR_PLUS(prev2_wd, "COLUMN", "TO");
@@ -2155,6 +2162,12 @@ psql_completion(const char *text, int start, int end)
/* ALTER VIEW xxx RENAME COLUMN yyy */
else if (Matches("ALTER", "VIEW", MatchAny, "RENAME", "COLUMN", MatchAnyExcept("TO")))
COMPLETE_WITH("TO");
+ /* ALTER VIEW xxx SET ( yyy [= zzz] ) */
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "("))
+ COMPLETE_WITH_LIST(view_optional_parameters);
+ /* ALTER VIEW xxx RESET ( yyy , ... ) */
+ else if (Matches("ALTER", "VIEW", MatchAny, "RESET", "("))
+ COMPLETE_WITH_LIST(view_optional_parameters);
/* ALTER MATERIALIZED VIEW <name> */
else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny))
@@ -3426,13 +3439,23 @@ psql_completion(const char *text, int start, int end)
}
/* CREATE VIEW --- is allowed inside CREATE SCHEMA, so use TailMatches */
- /* Complete CREATE [ OR REPLACE ] VIEW <name> with AS */
+ /* Complete CREATE [ OR REPLACE ] VIEW <name> with AS or WITH ( */
else if (TailMatches("CREATE", "VIEW", MatchAny) ||
TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny))
+ COMPLETE_WITH("AS", "WITH (");
+ /* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( with supported options */
+ else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "("))
+ COMPLETE_WITH_LIST(view_optional_parameters);
+ /* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( ... ) with "AS" */
+ else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)"))
COMPLETE_WITH("AS");
- /* Complete "CREATE [ OR REPLACE ] VIEW <sth> AS with "SELECT" */
+ /* Complete "CREATE [ OR REPLACE ] VIEW <sth> [ WITH ( ... ) ] AS with "SELECT" */
else if (TailMatches("CREATE", "VIEW", MatchAny, "AS") ||
- TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS"))
+ TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)", "AS") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)", "AS"))
COMPLETE_WITH("SELECT");
/* CREATE MATERIALIZED VIEW */
--
2.38.1
Hi Christoph,
I just took a quick look at your patch.
Some suggestions:
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "("))
+ COMPLETE_WITH_LIST(view_optional_parameters); + /* ALTER VIEW xxx RESET ( yyy , ... ) */ + else if (Matches("ALTER", "VIEW", MatchAny, "RESET", "(")) + COMPLETE_WITH_LIST(view_optional_parameters);
What about combining these two cases into one like Matches("ALTER", "VIEW",
MatchAny, "SET|RESET", "(") ?
/* ALTER VIEW <name> */
else if (Matches("ALTER", "VIEW", MatchAny))
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
"SET SCHEMA");
Also seems like SET and RESET don't get auto-completed for "ALTER VIEW
<name>".
I think it would be nice to include those missing words.
Thanks,
--
Melih Mutlu
Microsoft
Thanks for the review!
On 12/8/22 12:19, Melih Mutlu wrote:
Hi Christoph,
I just took a quick look at your patch.
Some suggestions:+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(")) + COMPLETE_WITH_LIST(view_optional_parameters); + /* ALTER VIEW xxx RESET ( yyy , ... ) */ + else if (Matches("ALTER", "VIEW", MatchAny, "RESET", "(")) + COMPLETE_WITH_LIST(view_optional_parameters);What about combining these two cases into one like Matches("ALTER",
"VIEW", MatchAny, "SET|RESET", "(") ?
Good thinking, incorporated that into v2.
While at it, I also added completion for the values of the SET
parameters, since that is useful as well.
/* ALTER VIEW <name> */
else if (Matches("ALTER", "VIEW", MatchAny))
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
"SET SCHEMA");Also seems like SET and RESET don't get auto-completed for "ALTER VIEW
<name>".
I think it would be nice to include those missing words.
That is already contained in the patch:
@@ -2139,7 +2146,7 @@ psql_completion(const char *text, int start, int end)
/* ALTER VIEW <name> */
else if (Matches("ALTER", "VIEW", MatchAny))
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
- "SET SCHEMA");
+ "SET SCHEMA", "SET (", "RESET (");Thanks,
Christoph
Attachments:
v2-0001-psql-Add-tab-complete-for-optional-view-parameter.patchtext/x-patch; charset=UTF-8; name=v2-0001-psql-Add-tab-complete-for-optional-view-parameter.patchDownload
From a53f73da6c3f6d6ace59ec9d8d9db5efef323f6c Mon Sep 17 00:00:00 2001
From: Christoph Heiss <contact@christoph-heiss.at>
Date: Fri, 9 Dec 2022 11:22:38 +0100
Subject: [PATCH v2] psql: Add tab-complete for optional view parameters
This adds them in the same matter as it works for storage parameters of
tables.
Signed-off-by: Christoph Heiss <contact@christoph-heiss.at>
---
src/bin/psql/tab-complete.c | 32 ++++++++++++++++++++++++++++----
1 file changed, 28 insertions(+), 4 deletions(-)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 89e7317c23..7d30ec6d5a 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1310,6 +1310,13 @@ static const char *const table_storage_parameters[] = {
NULL
};
+/* Optional parameters for CREATE VIEW and ALTER VIEW */
+static const char *const view_optional_parameters[] = {
+ "check_option",
+ "security_barrier",
+ "security_invoker",
+ NULL
+};
/* Forward declaration of functions */
static char **psql_completion(const char *text, int start, int end);
@@ -2139,7 +2146,7 @@ psql_completion(const char *text, int start, int end)
/* ALTER VIEW <name> */
else if (Matches("ALTER", "VIEW", MatchAny))
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
- "SET SCHEMA");
+ "SET SCHEMA", "SET (", "RESET (");
/* ALTER VIEW xxx RENAME */
else if (Matches("ALTER", "VIEW", MatchAny, "RENAME"))
COMPLETE_WITH_ATTR_PLUS(prev2_wd, "COLUMN", "TO");
@@ -2155,6 +2162,13 @@ psql_completion(const char *text, int start, int end)
/* ALTER VIEW xxx RENAME COLUMN yyy */
else if (Matches("ALTER", "VIEW", MatchAny, "RENAME", "COLUMN", MatchAnyExcept("TO")))
COMPLETE_WITH("TO");
+ /* ALTER VIEW xxx SET|RESET ( yyy [= zzz] ) */
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET|RESET", "("))
+ COMPLETE_WITH_LIST(view_optional_parameters);
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "check_option", "="))
+ COMPLETE_WITH("local", "cascaded");
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "security_barrier|security_invoker", "="))
+ COMPLETE_WITH("true", "false");
/* ALTER MATERIALIZED VIEW <name> */
else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny))
@@ -3426,13 +3440,23 @@ psql_completion(const char *text, int start, int end)
}
/* CREATE VIEW --- is allowed inside CREATE SCHEMA, so use TailMatches */
- /* Complete CREATE [ OR REPLACE ] VIEW <name> with AS */
+ /* Complete CREATE [ OR REPLACE ] VIEW <name> with AS or WITH ( */
else if (TailMatches("CREATE", "VIEW", MatchAny) ||
TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny))
+ COMPLETE_WITH("AS", "WITH (");
+ /* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( with supported options */
+ else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "("))
+ COMPLETE_WITH_LIST(view_optional_parameters);
+ /* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( ... ) with "AS" */
+ else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)"))
COMPLETE_WITH("AS");
- /* Complete "CREATE [ OR REPLACE ] VIEW <sth> AS with "SELECT" */
+ /* Complete "CREATE [ OR REPLACE ] VIEW <sth> [ WITH ( ... ) ] AS with "SELECT" */
else if (TailMatches("CREATE", "VIEW", MatchAny, "AS") ||
- TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS"))
+ TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)", "AS") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)", "AS"))
COMPLETE_WITH("SELECT");
/* CREATE MATERIALIZED VIEW */
--
2.38.1
On Fri, 9 Dec 2022 at 16:01, Christoph Heiss <christoph@c8h4.io> wrote:
Thanks for the review!
On 12/8/22 12:19, Melih Mutlu wrote:
Hi Christoph,
I just took a quick look at your patch.
Some suggestions:+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(")) + COMPLETE_WITH_LIST(view_optional_parameters); + /* ALTER VIEW xxx RESET ( yyy , ... ) */ + else if (Matches("ALTER", "VIEW", MatchAny, "RESET", "(")) + COMPLETE_WITH_LIST(view_optional_parameters);What about combining these two cases into one like Matches("ALTER",
"VIEW", MatchAny, "SET|RESET", "(") ?Good thinking, incorporated that into v2.
While at it, I also added completion for the values of the SET
parameters, since that is useful as well./* ALTER VIEW <name> */
else if (Matches("ALTER", "VIEW", MatchAny))
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
"SET SCHEMA");Also seems like SET and RESET don't get auto-completed for "ALTER VIEW
<name>".
I think it would be nice to include those missing words.That is already contained in the patch:
@@ -2139,7 +2146,7 @@ psql_completion(const char *text, int start, int end) /* ALTER VIEW <name> */ else if (Matches("ALTER", "VIEW", MatchAny)) COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME", - "SET SCHEMA"); + "SET SCHEMA", "SET (", "RESET (");
One suggestion:
Tab completion for "alter view v1 set (check_option =" is handled to
complete with CASCADED and LOCAL but the same is not handled for
create view: "create view viewname with ( check_option ="
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(",
"check_option", "="))
+ COMPLETE_WITH("local", "cascaded");I felt we should keep the handling consistent for both create view and
alter view.
Regards,
Vignesh
On Fri, 6 Jan 2023 at 11:52, vignesh C <vignesh21@gmail.com> wrote:
One suggestion: Tab completion for "alter view v1 set (check_option =" is handled to complete with CASCADED and LOCAL but the same is not handled for create view: "create view viewname with ( check_option =" + else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "check_option", "=")) + COMPLETE_WITH("local", "cascaded");I felt we should keep the handling consistent for both create view and
alter view.
Hmm, I don't think we should be offering "check_option" as a tab
completion for CREATE VIEW at all, since that would encourage users to
use non-SQL-standard syntax, rather than CREATE VIEW ... WITH
[CASCADED|LOCAL] CHECK OPTION.
Regards,
Dean
Hi Christoph,
Thanks for the patch! I just tested it and I could reproduce the
expected behaviour in these cases:
postgres=# CREATE VIEW w
AS WITH (
postgres=# CREATE OR REPLACE VIEW w
AS WITH (
postgres=# CREATE VIEW w WITH (
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
postgres=# CREATE OR REPLACE VIEW w WITH (
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
postgres=# ALTER VIEW w
ALTER COLUMN OWNER TO RENAME RESET ( SET (
SET SCHEMA
postgres=# ALTER VIEW w RESET (
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
postgres=# ALTER VIEW w SET (check_option =
CASCADED LOCAL
However, an "ALTER TABLE <name> S<tab>" does not complete the open
parenthesis "(" from "SET (", as suggested in "ALTER VIEW <name> <tab>".
postgres=# ALTER VIEW w SET
Display all 187 possibilities? (y or n)
Is it intended to behave like this? If so, an "ALTER VIEW <name>
RES<tab>" does complete the open parenthesis -> "RESET (".
Best,
Jim
Show quoted text
On 09.12.22 11:31, Christoph Heiss wrote:
Thanks for the review!
On 12/8/22 12:19, Melih Mutlu wrote:
Hi Christoph,
I just took a quick look at your patch.
Some suggestions:+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "("))
+ COMPLETE_WITH_LIST(view_optional_parameters);
+ /* ALTER VIEW xxx RESET ( yyy , ... ) */
+ else if (Matches("ALTER", "VIEW", MatchAny, "RESET", "("))
+ COMPLETE_WITH_LIST(view_optional_parameters);What about combining these two cases into one like Matches("ALTER",
"VIEW", MatchAny, "SET|RESET", "(") ?Good thinking, incorporated that into v2.
While at it, I also added completion for the values of the SET
parameters, since that is useful as well./* ALTER VIEW <name> */
else if (Matches("ALTER", "VIEW", MatchAny))
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
"SET SCHEMA");Also seems like SET and RESET don't get auto-completed for "ALTER
VIEW <name>".
I think it would be nice to include those missing words.That is already contained in the patch:
@@ -2139,7 +2146,7 @@ psql_completion(const char *text, int start, int end) /* ALTER VIEW <name> */ else if (Matches("ALTER", "VIEW", MatchAny)) COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME", - "SET SCHEMA"); + "SET SCHEMA", "SET (", "RESET (");Thanks,
Christoph
Attachments:
On Fri, 9 Dec 2022 at 16:01, Christoph Heiss <christoph@c8h4.io> wrote:
Thanks for the review!
On 12/8/22 12:19, Melih Mutlu wrote:
Hi Christoph,
I just took a quick look at your patch.
Some suggestions:+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(")) + COMPLETE_WITH_LIST(view_optional_parameters); + /* ALTER VIEW xxx RESET ( yyy , ... ) */ + else if (Matches("ALTER", "VIEW", MatchAny, "RESET", "(")) + COMPLETE_WITH_LIST(view_optional_parameters);What about combining these two cases into one like Matches("ALTER",
"VIEW", MatchAny, "SET|RESET", "(") ?Good thinking, incorporated that into v2.
While at it, I also added completion for the values of the SET
parameters, since that is useful as well./* ALTER VIEW <name> */
else if (Matches("ALTER", "VIEW", MatchAny))
COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
"SET SCHEMA");Also seems like SET and RESET don't get auto-completed for "ALTER VIEW
<name>".
I think it would be nice to include those missing words.That is already contained in the patch:
@@ -2139,7 +2146,7 @@ psql_completion(const char *text, int start, int end) /* ALTER VIEW <name> */ else if (Matches("ALTER", "VIEW", MatchAny)) COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME", - "SET SCHEMA"); + "SET SCHEMA", "SET (", "RESET (");
For some reason CFBot is not able to apply the patch, please have a
look and post an updated version if required, also check and handle
Dean Rasheed and Jim Jones comment if applicable:
=== Applying patches on top of PostgreSQL commit ID
5f6401f81cb24bd3930e0dc589fc4aa8b5424cdc ===
=== applying patch
./v2-0001-psql-Add-tab-complete-for-optional-view-parameter.patch
gpatch: **** Only garbage was found in the patch input.
[1]: http://cfbot.cputube.org/patch_41_4053.log
Regards,
Vignesh
On Thu, Jan 12, 2023 at 5:50 AM vignesh C <vignesh21@gmail.com> wrote:
For some reason CFBot is not able to apply the patch, please have a
look and post an updated version if required, also check and handle
Dean Rasheed and Jim Jones comment if applicable:
=== Applying patches on top of PostgreSQL commit ID
5f6401f81cb24bd3930e0dc589fc4aa8b5424cdc ===
=== applying patch
./v2-0001-psql-Add-tab-complete-for-optional-view-parameter.patch
gpatch: **** Only garbage was found in the patch input.
Melanie pointed me at this issue. This particular entry is now fixed,
and I think I know what happened: cfbot wasn't checking the HTTP
status when downloading patches from the web archives, because I had
incorrectly assumed Python's requests.get() would raise an exception
if the web server sent an error status, but it turns out you have to
ask for that. I've now fixed that. So I think it was probably trying
to apply one of those "guru meditation" error message the web archives
occasionally spit out.
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 composition which 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
On Sun, 29 Jan 2023 at 05:20, Mikhail Gribkov <youzhick@gmail.com> wrote:
The problem 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 composition which 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
I think this patch received real feedback and it looks like it's clear
where there's more work needed. I'll move this to the next commitfest.
If you plan to work on it this month we can always move it back.
--
Gregory Stark
As Commitfest Manager
On 29 Jan 2023, at 11:19, Mikhail Gribkov <youzhick@gmail.com> wrote:
The new status of this patch is: Waiting on Author
This patch has been Waiting on Author since January with the thread being
stale, so I am marking this as Returned with Feedback for now. Please feel
free to resubmit to a future CF when there is renewed interest in working on
this.
--
Daniel Gustafsson
Hi all,
sorry for the long delay.
On Mon, Jan 09, 2023 at 04:32:09PM +0100, Jim Jones wrote:
However, an "ALTER TABLE <name> S<tab>" does not complete the open
parenthesis "(" from "SET (", as suggested in "ALTER VIEW <name> <tab>".postgres=# ALTER VIEW w SET
Display all 187 possibilities? (y or n)Is it intended to behave like this? If so, an "ALTER VIEW <name>
RES<tab>" does complete the open parenthesis -> "RESET (".
On Sun, Jan 29, 2023 at 10:19:12AM +0000, Mikhail Gribkov wrote:
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.
[..]
I think it may worth looking at "alter materialized view" completion
tree and making "alter view" the same way.
Thank you both for reviewing/testing and the suggestions. Yeah,
definitively, sounds very sensible.
I've attached a new revision, rebased and addressing the above by
aligning it with how "ALTER MATERIALIZED VIEW" works, such that "SET ("
and "SET SCHEMA" won't compete anymore. So that should now work more
like expected.
postgres=# ALTER MATERIALIZED VIEW m
ALTER COLUMN CLUSTER ON DEPENDS ON EXTENSION
NO DEPENDS ON EXTENSION OWNER TO RENAME
RESET ( SET
postgres=# ALTER MATERIALIZED VIEW m SET
( ACCESS METHOD SCHEMA TABLESPACE
WITHOUT CLUSTER
postgres=# ALTER VIEW v
ALTER COLUMN OWNER TO RENAME RESET ( SET
postgres=# ALTER VIEW v SET
( SCHEMA
postgres=# ALTER VIEW v SET (
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
On Fri, Jan 06, 2023 at 12:18:44PM +0000, Dean Rasheed wrote:
Hmm, I don't think we should be offering "check_option" as a tab
completion for CREATE VIEW at all, since that would encourage users to
use non-SQL-standard syntax, rather than CREATE VIEW ... WITH
[CASCADED|LOCAL] CHECK OPTION.
Left that part in for now. I would argue that it is a well-documented
combination and as such users would expect it to turn up in the
tab-complete as well. OTOH not against removing it either, if there are
others voicing the same opinion ..
Thanks,
Christoph
Attachments:
v3-0001-psql-Add-tab-complete-for-optional-view-parameter.patchtext/plain; charset=us-asciiDownload
From 3663eb0b5008d632972d4b66a105fc08cfff13fb Mon Sep 17 00:00:00 2001
From: Christoph Heiss <christoph@c8h4.io>
Date: Mon, 7 Aug 2023 20:37:19 +0200
Subject: [PATCH v3] psql: Add tab-complete for optional view parameters
This adds them in the same matter as it works for storage parameters of
tables.
Signed-off-by: Christoph Heiss <christoph@c8h4.io>
---
src/bin/psql/tab-complete.c | 36 +++++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 779fdc90cb..83ec1508bb 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1329,6 +1329,13 @@ static const char *const table_storage_parameters[] = {
NULL
};
+/* Optional parameters for CREATE VIEW and ALTER VIEW */
+static const char *const view_optional_parameters[] = {
+ "check_option",
+ "security_barrier",
+ "security_invoker",
+ NULL
+};
/* Forward declaration of functions */
static char **psql_completion(const char *text, int start, int end);
@@ -2216,8 +2223,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("TO");
/* ALTER VIEW <name> */
else if (Matches("ALTER", "VIEW", MatchAny))
- COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
- "SET SCHEMA");
+ COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME", "RESET (", "SET");
/* ALTER VIEW xxx RENAME */
else if (Matches("ALTER", "VIEW", MatchAny, "RENAME"))
COMPLETE_WITH_ATTR_PLUS(prev2_wd, "COLUMN", "TO");
@@ -2233,6 +2239,16 @@ psql_completion(const char *text, int start, int end)
/* ALTER VIEW xxx RENAME COLUMN yyy */
else if (Matches("ALTER", "VIEW", MatchAny, "RENAME", "COLUMN", MatchAnyExcept("TO")))
COMPLETE_WITH("TO");
+ /* ALTER VIEW xxx SET ( yyy [= zzz] ) */
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET"))
+ COMPLETE_WITH("(", "SCHEMA");
+ /* ALTER VIEW xxx SET|RESET ( yyy [= zzz] ) */
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET|RESET", "("))
+ COMPLETE_WITH_LIST(view_optional_parameters);
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "check_option", "="))
+ COMPLETE_WITH("local", "cascaded");
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "security_barrier|security_invoker", "="))
+ COMPLETE_WITH("true", "false");
/* ALTER MATERIALIZED VIEW <name> */
else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny))
@@ -3525,13 +3541,23 @@ psql_completion(const char *text, int start, int end)
}
/* CREATE VIEW --- is allowed inside CREATE SCHEMA, so use TailMatches */
- /* Complete CREATE [ OR REPLACE ] VIEW <name> with AS */
+ /* Complete CREATE [ OR REPLACE ] VIEW <name> with AS or WITH ( */
else if (TailMatches("CREATE", "VIEW", MatchAny) ||
TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny))
+ COMPLETE_WITH("AS", "WITH (");
+ /* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( with supported options */
+ else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "("))
+ COMPLETE_WITH_LIST(view_optional_parameters);
+ /* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( ... ) with "AS" */
+ else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)"))
COMPLETE_WITH("AS");
- /* Complete "CREATE [ OR REPLACE ] VIEW <sth> AS with "SELECT" */
+ /* Complete "CREATE [ OR REPLACE ] VIEW <sth> [ WITH ( ... ) ] AS with "SELECT" */
else if (TailMatches("CREATE", "VIEW", MatchAny, "AS") ||
- TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS"))
+ TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)", "AS") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)", "AS"))
COMPLETE_WITH("SELECT");
/* CREATE MATERIALIZED VIEW */
--
2.41.0
On Mon, 7 Aug 2023 at 19:49, Christoph Heiss <christoph@c8h4.io> wrote:
On Fri, Jan 06, 2023 at 12:18:44PM +0000, Dean Rasheed wrote:
Hmm, I don't think we should be offering "check_option" as a tab
completion for CREATE VIEW at all, since that would encourage users to
use non-SQL-standard syntax, rather than CREATE VIEW ... WITH
[CASCADED|LOCAL] CHECK OPTION.Left that part in for now. I would argue that it is a well-documented
combination and as such users would expect it to turn up in the
tab-complete as well. OTOH not against removing it either, if there are
others voicing the same opinion ..
On reflection, I think that's probably OK. I mean, I still don't like
the fact that it's offering to complete with non-SQL-standard syntax,
but that seems less bad than using an incomplete list of options, and
I don't really have any other better ideas.
Regards,
Dean
On Tue, Aug 08, 2023 at 09:17:51AM +0100, Dean Rasheed wrote:
On Mon, 7 Aug 2023 at 19:49, Christoph Heiss <christoph@c8h4.io> wrote:
On Fri, Jan 06, 2023 at 12:18:44PM +0000, Dean Rasheed wrote:
Hmm, I don't think we should be offering "check_option" as a tab
completion for CREATE VIEW at all, since that would encourage users to
use non-SQL-standard syntax, rather than CREATE VIEW ... WITH
[CASCADED|LOCAL] CHECK OPTION.Left that part in for now. I would argue that it is a well-documented
combination and as such users would expect it to turn up in the
tab-complete as well. OTOH not against removing it either, if there are
others voicing the same opinion ..On reflection, I think that's probably OK. I mean, I still don't like
the fact that it's offering to complete with non-SQL-standard syntax,
but that seems less bad than using an incomplete list of options, and
I don't really have any other better ideas.
My thought pretty much as well. While obviously far from ideal as you
say, it's the less bad case of these both. I would also guess that it is
not the only instance of non-SQL-standard syntax completion in psql ..
Thanks for weighing in once again.
Cheers,
Christoph
Applied v3 patch to master and verified it with below commands,
#Alter view
postgres=# alter view v <tab>
ALTER COLUMN OWNER TO RENAME RESET ( SET
postgres=# alter view v set <tab>
( SCHEMA
postgres=# alter view v set ( <tab>
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
postgres=# alter view v reset ( <tab>
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
postgres=# alter view v set ( check_option = <tab>
CASCADED LOCAL
postgres=# alter view v set ( security_barrier = <tab>
FALSE TRUE
postgres=# alter view v set ( security_invoker = <tab>
FALSE TRUE
#Create view
postgres=# create view v
AS WITH (
postgres=# create or replace view v
AS WITH (
postgres=# create view v with (
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
postgres=# create or replace view v with (
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
postgres=# create view v with (*)<tab>AS
postgres=# create or replace view v with (*)<tab>AS
postgres=# create view v as <tab>SELECT
postgres=# create or replace view v as <tab>SELECT
For below changes,
else if (TailMatches("CREATE", "VIEW", MatchAny, "AS") ||
- TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny,
"AS"))
+ TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)",
"AS") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny,
"AS") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny,
"WITH", "(*)", "AS"))it would be great to switch the order of the 3rd and the 4th line to
make a better match for "CREATE" and "CREATE OR REPLACE" .
Since it supports <tab> in the middle for below case,
postgres=# alter view v set ( security_<tab>
security_barrier security_invoker
and during view reset it can also provide all the options list,
postgres=# alter view v reset (
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKER
but not sure if it is a good idea or possible to autocomplete the reset
options after seeing one of the options showing up with "," for example,
postgres=# alter view v reset ( CHECK_OPTION, <tab>
SECURITY_BARRIER SECURITY_INVOKER
Thank you,
David
On Fri, Aug 11, 2023 at 12:48:17PM -0700, David Zhang wrote:
Applied v3 patch to master and verified it with below commands,
Thanks for testing!
[..]
For below changes,
���� else if (TailMatches("CREATE", "VIEW", MatchAny, "AS") || -��� ��� ��� �TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS")) +��� ��� ��� �TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)", "AS") || +��� ��� ��� �TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS") || +��� ��� ��� �TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)", "AS"))it would be great to switch the order of the 3rd and the 4th line to make a
better match for "CREATE" and "CREATE OR REPLACE" .
I don't see how it would effect matching in any way - or am I
overlooking something here?
postgres=# CREATE VIEW v <tab>
AS WITH (
postgres=# CREATE VIEW v AS <tab>
.. autocompletes with "SELECT"
postgres=# CREATE VIEW v WITH ( security_invoker = true ) <tab>
.. autocompletes with "AS" and so on
And the same for CREATE OR REPLACE VIEW.
Can you provide an example case that would benefit from that?
Since it supports <tab> in the middle for below case,
postgres=# alter view v set ( security_<tab>
security_barrier� security_invokerand during view reset it can also provide all the options list,
postgres=# alter view v reset (
CHECK_OPTION����� SECURITY_BARRIER� SECURITY_INVOKERbut not sure if it is a good idea or possible to autocomplete the reset
options after seeing one of the options showing up with "," for example,
postgres=# alter view v reset ( CHECK_OPTION, <tab>
SECURITY_BARRIER� SECURITY_INVOKER
I'd rather not add this and leave it as-is. It doesn't add any real
value - how often does this case really come up, especially with ALTER
VIEW only having three options?
Thanks,
Christoph
[..]
For below changes,
else if (TailMatches("CREATE", "VIEW", MatchAny, "AS") || - TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS")) + TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)", "AS") || + TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS") || + TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)", "AS"))it would be great to switch the order of the 3rd and the 4th line to make a
better match for "CREATE" and "CREATE OR REPLACE" .I don't see how it would effect matching in any way - or am I
overlooking something here?
It won't affect the SQL matching. What I was trying to say is that using
'CREATE OR REPLACE ...' after 'CREATE ...' can enhance code structure,
making it more readable. For instance,
/* Complete CREATE [ OR REPLACE ] VIEW <name> WITH ( ... ) with "AS" */
else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)") ||
TailMatches("CREATE", "OR", "REPLACE", "VIEW",
MatchAny, "WITH", "(*)"))
COMPLETE_WITH("AS");
"CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)" follows
"CREATE", "VIEW", MatchAny, "WITH", "(*)") immediately.
best regards,
David
I noticed that on this commitfest entry
(https://commitfest.postgresql.org/44/4491/), the reviewers were
assigned by the patch author (presumably because they had previously
contributed to this thread). Unless these individuals know about that,
this is unlikely to work out. It's better to remove the reviewer
entries and let people sign up on their own. (You can nudge potential
candidates, of course.)
Show quoted text
On 07.08.23 20:49, Christoph Heiss wrote:
Hi all,
sorry for the long delay.On Mon, Jan 09, 2023 at 04:32:09PM +0100, Jim Jones wrote:
However, an "ALTER TABLE <name> S<tab>" does not complete the open
parenthesis "(" from "SET (", as suggested in "ALTER VIEW <name> <tab>".postgres=# ALTER VIEW w SET
Display all 187 possibilities? (y or n)Is it intended to behave like this? If so, an "ALTER VIEW <name>
RES<tab>" does complete the open parenthesis -> "RESET (".On Sun, Jan 29, 2023 at 10:19:12AM +0000, Mikhail Gribkov wrote:
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.
[..]
I think it may worth looking at "alter materialized view" completion
tree and making "alter view" the same way.Thank you both for reviewing/testing and the suggestions. Yeah,
definitively, sounds very sensible.I've attached a new revision, rebased and addressing the above by
aligning it with how "ALTER MATERIALIZED VIEW" works, such that "SET ("
and "SET SCHEMA" won't compete anymore. So that should now work more
like expected.postgres=# ALTER MATERIALIZED VIEW m
ALTER COLUMN CLUSTER ON DEPENDS ON EXTENSION
NO DEPENDS ON EXTENSION OWNER TO RENAME
RESET ( SETpostgres=# ALTER MATERIALIZED VIEW m SET
( ACCESS METHOD SCHEMA TABLESPACE
WITHOUT CLUSTERpostgres=# ALTER VIEW v
ALTER COLUMN OWNER TO RENAME RESET ( SETpostgres=# ALTER VIEW v SET
( SCHEMApostgres=# ALTER VIEW v SET (
CHECK_OPTION SECURITY_BARRIER SECURITY_INVOKEROn Fri, Jan 06, 2023 at 12:18:44PM +0000, Dean Rasheed wrote:
Hmm, I don't think we should be offering "check_option" as a tab
completion for CREATE VIEW at all, since that would encourage users to
use non-SQL-standard syntax, rather than CREATE VIEW ... WITH
[CASCADED|LOCAL] CHECK OPTION.Left that part in for now. I would argue that it is a well-documented
combination and as such users would expect it to turn up in the
tab-complete as well. OTOH not against removing it either, if there are
others voicing the same opinion ..Thanks,
Christoph
On Mon, 14 Aug 2023 at 18:34, David Zhang <david.zhang@highgo.ca> wrote:
it would be great to switch the order of the 3rd and the 4th line to make a
better match for "CREATE" and "CREATE OR REPLACE" .
I took a look at this, and I think it's probably neater to keep the
"AS SELECT" completion for CREATE [OR REPLACE] VIEW xxx WITH (*)
separate from the already existing support for "AS SELECT" without
WITH.
A couple of other points:
1. It looks slightly neater, and works better, to complete one word at
a time -- e.g., "WITH" then "(", instead of "WITH (", since the latter
doesn't work if the user has already typed "WITH".
2. It should also complete with "=" after the option, where appropriate.
3. CREATE VIEW should offer "local" and "cascaded" after
"check_option" (though there's no point in doing likewise for the
boolean options, since they default to true, if present, and false
otherwise).
Attached is an updated patch, incorporating those comments.
Barring any further comments, I think this is ready for commit.
Regards,
Dean
Attachments:
v4-0001-psql-Add-tab-completion-for-view-options.patchtext/x-patch; charset=US-ASCII; name=v4-0001-psql-Add-tab-completion-for-view-options.patchDownload
From 2f143cbfe185c723419a8fffcf5cbcd921f08ea7 Mon Sep 17 00:00:00 2001
From: Christoph Heiss <christoph@c8h4.io>
Date: Mon, 7 Aug 2023 20:37:19 +0200
Subject: [PATCH v4] psql: Add tab completion for view options.
Add support for tab completion of WITH (...) options to CREATE VIEW,
and the corresponding SET/RESET (...) support to ALTER VIEW.
Christoph Heiss, reviewed by Melih Mutlu, Vignesh C, Jim Jones,
Mikhail Gribkov, David Zhang, and me.
Discussion: https://postgr.es/m/a2075c5a-66f9-a564-f038-9ac044b03117@c8h4.io
---
src/bin/psql/tab-complete.c | 50 ++++++++++++++++++++++++++++++++++---
1 file changed, 46 insertions(+), 4 deletions(-)
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 006e10f5d2..049801186c 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1329,6 +1329,13 @@ static const char *const table_storage_parameters[] = {
NULL
};
+/* Optional parameters for CREATE VIEW and ALTER VIEW */
+static const char *const view_optional_parameters[] = {
+ "check_option",
+ "security_barrier",
+ "security_invoker",
+ NULL
+};
/* Forward declaration of functions */
static char **psql_completion(const char *text, int start, int end);
@@ -2216,8 +2223,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("TO");
/* ALTER VIEW <name> */
else if (Matches("ALTER", "VIEW", MatchAny))
- COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME",
- "SET SCHEMA");
+ COMPLETE_WITH("ALTER COLUMN", "OWNER TO", "RENAME", "RESET", "SET");
/* ALTER VIEW xxx RENAME */
else if (Matches("ALTER", "VIEW", MatchAny, "RENAME"))
COMPLETE_WITH_ATTR_PLUS(prev2_wd, "COLUMN", "TO");
@@ -2233,6 +2239,21 @@ psql_completion(const char *text, int start, int end)
/* ALTER VIEW xxx RENAME COLUMN yyy */
else if (Matches("ALTER", "VIEW", MatchAny, "RENAME", "COLUMN", MatchAnyExcept("TO")))
COMPLETE_WITH("TO");
+ /* ALTER VIEW xxx RESET ( */
+ else if (Matches("ALTER", "VIEW", MatchAny, "RESET"))
+ COMPLETE_WITH("(");
+ /* Complete ALTER VIEW xxx SET with "(" or "SCHEMA" */
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET"))
+ COMPLETE_WITH("(", "SCHEMA");
+ /* ALTER VIEW xxx SET|RESET ( yyy [= zzz] ) */
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET|RESET", "("))
+ COMPLETE_WITH_LIST(view_optional_parameters);
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", MatchAny))
+ COMPLETE_WITH("=");
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "check_option", "="))
+ COMPLETE_WITH("local", "cascaded");
+ else if (Matches("ALTER", "VIEW", MatchAny, "SET", "(", "security_barrier|security_invoker", "="))
+ COMPLETE_WITH("true", "false");
/* ALTER MATERIALIZED VIEW <name> */
else if (Matches("ALTER", "MATERIALIZED", "VIEW", MatchAny))
@@ -3531,14 +3552,35 @@ psql_completion(const char *text, int start, int end)
}
/* CREATE VIEW --- is allowed inside CREATE SCHEMA, so use TailMatches */
- /* Complete CREATE [ OR REPLACE ] VIEW <name> with AS */
+ /* Complete CREATE [ OR REPLACE ] VIEW <name> with AS or WITH */
else if (TailMatches("CREATE", "VIEW", MatchAny) ||
TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny))
- COMPLETE_WITH("AS");
+ COMPLETE_WITH("AS", "WITH");
/* Complete "CREATE [ OR REPLACE ] VIEW <sth> AS with "SELECT" */
else if (TailMatches("CREATE", "VIEW", MatchAny, "AS") ||
TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "AS"))
COMPLETE_WITH("SELECT");
+ /* CREATE [ OR REPLACE ] VIEW <name> WITH ( yyy [= zzz] ) */
+ else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH"))
+ COMPLETE_WITH("(");
+ else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "("))
+ COMPLETE_WITH_LIST(view_optional_parameters);
+ else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(", "check_option") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(", "check_option"))
+ COMPLETE_WITH("=");
+ else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(", "check_option", "=") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(", "check_option", "="))
+ COMPLETE_WITH("local", "cascaded");
+ /* CREATE [ OR REPLACE ] VIEW <name> WITH ( ... ) AS */
+ else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)"))
+ COMPLETE_WITH("AS");
+ /* CREATE [ OR REPLACE ] VIEW <name> WITH ( ... ) AS SELECT */
+ else if (TailMatches("CREATE", "VIEW", MatchAny, "WITH", "(*)", "AS") ||
+ TailMatches("CREATE", "OR", "REPLACE", "VIEW", MatchAny, "WITH", "(*)", "AS"))
+ COMPLETE_WITH("SELECT");
/* CREATE MATERIALIZED VIEW */
else if (Matches("CREATE", "MATERIALIZED"))
--
2.35.3
On Thu, Nov 23, 2023 at 4:37 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
On Mon, 14 Aug 2023 at 18:34, David Zhang <david.zhang@highgo.ca> wrote:
it would be great to switch the order of the 3rd and the 4th line to make a
better match for "CREATE" and "CREATE OR REPLACE" .I took a look at this, and I think it's probably neater to keep the
"AS SELECT" completion for CREATE [OR REPLACE] VIEW xxx WITH (*)
separate from the already existing support for "AS SELECT" without
WITH.A couple of other points:
1. It looks slightly neater, and works better, to complete one word at
a time -- e.g., "WITH" then "(", instead of "WITH (", since the latter
doesn't work if the user has already typed "WITH".2. It should also complete with "=" after the option, where appropriate.
3. CREATE VIEW should offer "local" and "cascaded" after
"check_option" (though there's no point in doing likewise for the
boolean options, since they default to true, if present, and false
otherwise).Attached is an updated patch, incorporating those comments.
Barring any further comments, I think this is ready for commit.
I reviewed the given Patch and it is working fine.
Thanks and Regards,
Shubham Khanna.