Обсуждение: adding tab completions
Find attached tab completion for the following: "... Also, recursively perform VACUUM and ANALYZE on partitions when the command is applied to a partitioned table." 3c3bb99330aa9b4c2f6258bfa0265d806bf365c3 Add parenthesized options syntax for ANALYZE. 854dd8cff523bc17972d34772b0e39ad3d6d46a4 Add VACUUM (DISABLE_PAGE_SKIPPING) for emergencies. ede62e56fbe809baa1a7bc3873d82f12ffe7540b Allow multiple tables to be specified in one VACUUM or ANALYZE command. 11d8d72c27a64ea4e30adce11cf6c4f3dd3e60db
Вложения
Hello, On Mon, May 28, 2018 at 07:06:23PM -0500, Justin Pryzby wrote: > Find attached tab completion for the following: The patch works well. I have a couple notes. The patch replaces the variable Query_for_list_of_tmf by Query_for_list_of_tpmf. So I think Query_for_list_of_tmf isn't needed anymore and can be removed. I created partitioned table "measurement" and tried tab completion. VACUUM (ANALYZE) measurement (<tab> gives me only a comma. If I try to input table column: VACUUM (ANALYZE) measurement (city_id<tab> replaces "city_id" column by a comma: "VACUUM (ANALYZE) measurement (,". The following with whitespace after column works well: VACUUM (ANALYZE) measurement (city_id <tab> and gives: "VACUUM (ANALYZE) measurement (city_id ,". Also I think it could be good to list column names after parentheses, but I'm not sure if it easy to implement. -- Arthur Zakirov Postgres Professional: http://www.postgrespro.com Russian Postgres Company
On 29 May 2018 at 12:06, Justin Pryzby <pryzby@telsasoft.com> wrote:
> Find attached tab completion for the following:
>
> "... Also, recursively perform VACUUM and ANALYZE on partitions when the
> command is applied to a partitioned table."
> 3c3bb99330aa9b4c2f6258bfa0265d806bf365c3
>
> Add parenthesized options syntax for ANALYZE.
> 854dd8cff523bc17972d34772b0e39ad3d6d46a4
>
> Add VACUUM (DISABLE_PAGE_SKIPPING) for emergencies.
> ede62e56fbe809baa1a7bc3873d82f12ffe7540b
>
> Allow multiple tables to be specified in one VACUUM or ANALYZE command.
> 11d8d72c27a64ea4e30adce11cf6c4f3dd3e60db
Hi Justin,
I don't believe it's meaningful to match on words with spaces in them,
for instance, in
else if (Matches3("VACUUM", "FULL|FREEZE|FULL FREEZE", "ANALYZE")) {
as there will never be a word called "FULL FREEZE" (the tab completion
logic splits using spaces, though it will keep things in quotes and
parentheses together).
I don't know what the best approach is for cases like VACUUM, where
there are multiple optional words. Maybe something like the
following? It's pretty ugly, but then, it is part of the tab
completion logic; a good sense of compromise is needed there.
else if (Matches1("VACUUM"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpmf,
" UNION SELECT 'FULL'"
" UNION SELECT 'FREEZE'"
" UNION SELECT 'VERBOSE'"
" UNION SELECT 'ANALYZE'"
" UNION SELECT '('");
else if (HeadMatches1("VACUUM") && TailMatches1("FULL"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpmf,
" UNION SELECT 'FREEZE'"
" UNION SELECT 'VERBOSE'"
" UNION SELECT 'ANALYZE'");
else if (HeadMatches1("VACUUM") && TailMatches1("FREEZE"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpmf,
" UNION SELECT 'VERBOSE'"
" UNION SELECT 'ANALYZE'");
else if (HeadMatches1("VACUUM") && TailMatches1("VERBOSE"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpmf,
" UNION SELECT 'ANALYZE'");
(Not a patch file, so that you don't have to merge it with the rest of
your patch. ;-) )
Cheers,
Edmund
Is it just me that finds shocking the notion of vacuuming a foreign table? -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, May 29, 2018 at 10:12:27AM -0400, Alvaro Herrera wrote: > Is it just me that finds shocking the notion of vacuuming a foreign > table? I guess it would be nice to have the ability to tab complete that, assuming it would actually do something at least potentially useful, but until then, you're not alone. 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
I finally got back to this; thanks everyone for reviews;
I also added completion for parenthesized explain (...)
d4382c4ae7ea1e272f4fee388aac8ff99421471a
and for ALTER TABLE SET (toast_tuple_target).
c2513365a0a85e77d3c21adb92fe12cfbe0d1897
BTW..should that be toast.tuple_target ??
Note that this also tries to comply with 921059bd66c7fb1230c705d3b1a65940800c4cbb
This is okay in v10 but rejected in v11b1:
postgres=# VACUUM ANALYZE VERBOSE t;
ERROR: syntax error at or near "VERBOSE"
On Tue, May 29, 2018 at 10:12:27AM -0400, Alvaro Herrera wrote:
> Is it just me that finds shocking the notion of vacuuming a foreign
> table?
Fixed, thanks.
On Tue, May 29, 2018 at 12:27:27PM +0300, Arthur Zakirov wrote:
> The patch replaces the variable Query_for_list_of_tmf by
> Query_for_list_of_tpmf. So I think Query_for_list_of_tmf isn't needed
> anymore and can be removed.
Fixed by adding relkind='p' to the existing list rather than making a new
query entry.
> Also I think it could be good to list column names after parentheses,
> but I'm not sure if it easy to implement.
I tried this and nearly gave up, but see attached.
The complication is that table names for which to pull the attributes isn't at a
constant offset; for example:
vacuum analyze a(TAB => needs prev2_wd
but
vacuum analyze a(c, TAB => needs prev3_wd
Maybe that's too much effort to include in tab completion. If so, the
"prev_parens" stanza can be removed from VACUUM and ANALYZE. If it's desired
to keep it, I guess I should avoid the duplicated lines.
On Wed, May 30, 2018 at 12:45:30AM +1200, Edmund Horner wrote:
> I don't believe it's meaningful to match on words with spaces in them,
Excellent point...fixed.
Find attached v6 (there were some "unpublished" versions).
Justin
Вложения
Find attached an update which also supports column completion using the legacy non-parenthesized syntax.
Вложения
On Sun, Jun 03, 2018 at 10:39:22PM -0500, Justin Pryzby wrote:
> Find attached an update which also supports column completion using the legacy
> non-parenthesized syntax.
Thank you!
> BTW..should that be toast.tuple_target ??
I think shouldn't. From the documentation "This parameter cannot be set for
TOAST tables".
> > Also I think it could be good to list column names after parentheses,
> > but I'm not sure if it easy to implement.
> I tried this and nearly gave up, but see attached.
After some thought now I think that this is not so good idea. The code
doesn't look good too. It doesn't worth it and sorry for the distraction.
Moreover there is no such completion for example for the command (it shows
only first column):
CREATE INDEX ON test (
> - "SERVER", "INDEX", "LANGUAGE", "POLICY", "PUBLICATION", "RULE",
> + "SERVER", "INDEX", "LANGUAGE", "POLICY", "PUBLICATION",
Is this a typo? I think still there is a possibility to comment rules.
> else if (HeadMatches1("EXPLAIN") && previous_words_count==2 && prev_wd[0]=='(' && ends_with(prev_wd, ')'))
I think this condition can be replaced by:
else if (TailMatches2("EXPLAIN", MatchAny) && ends_with(prev_wd, ')'))
It looks better to me. Such condition is used for other commands and
works the same way.
The last point I've noticed, there is no VERBOSE entry after VACUUM FULL
ANALYZE command anymore.
I'm not sure how this patch should be commited. Can it be commited
outside the commitfest? Otherwise add it to the next commitfest please
in order not to forget it.
Thoughts?
--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
Thanks for review and comment.
On Tue, Jun 05, 2018 at 05:29:42PM +0300, Arthur Zakirov wrote:
> On Sun, Jun 03, 2018 at 10:39:22PM -0500, Justin Pryzby wrote:
> > > Also I think it could be good to list column names after parentheses,
> > > but I'm not sure if it easy to implement.
> > I tried this and nearly gave up, but see attached.
>
> After some thought now I think that this is not so good idea. The code
> doesn't look good too. It doesn't worth it and sorry for the distraction.
No problem.
> Moreover there is no such completion for example for the command (it shows
> only first column):
>
> CREATE INDEX ON test (
Noted (I misunderstood at first: you just mean there's precedent that column
names aren't completed, except maybe the first).
I can revise patch to not complete attributes in analyze; but, I think that
means that this will have to complete to table names:
postgres=# ANALYZE tt (a ,
alu_enodeb_201601 information_schema.
alu_enodeb_201602 jrn_pg_buffercache
...
.. since, without checking for matching parens, it has no idea whether to
complete with rels or atts. WDYT?
Note that HeadMatch functions have special behavior with matching parenthesis:
if previous char is an right parenthesis, then the "previous word" is taken to
be the entire parenthesized value. Maybe there's more that I don't know, but I
can't see how that can be easily applied here (by easily I mean without doing
something like making a temp copy of the buffer and inserting an "exploratory"
right parenthesis to see if TailMatches(prev_wd, ')') && prev_wd[0]=='(' or
similar).
An alternative is that only the first table is completed for vacuum/analyze.
CREATE INDEX should complete columns, too. It has the "ON" keyword (which
makes trying to parse less fragile).
> > - "SERVER", "INDEX", "LANGUAGE", "POLICY", "PUBLICATION", "RULE",
> > + "SERVER", "INDEX", "LANGUAGE", "POLICY", "PUBLICATION",
>
> Is this a typo? I think still there is a possibility to comment rules.
Not in PG11(b1) (note, that's a custom table)
postgres=# COMMENT ON RULE pg_settings_u IS 'asdf';
ERROR: syntax error at or near "IS"
I see I didn't include that description in my first mail.
Here's a description and possible commit message for the (still WIP) patch:
Update psql tab completion for commits:
Table completion for ANALYZE partitioned_table
3c3bb99330aa9b4c2f6258bfa0265d806bf365c3
Add parenthesized options syntax for ANALYZE.
854dd8cff523bc17972d34772b0e39ad3d6d46a4
Add VACUUM (DISABLE_PAGE_SKIPPING) for emergencies.
ede62e56fbe809baa1a7bc3873d82f12ffe7540b
Allow multiple tables to be specified in one VACUUM or ANALYZE command.
11d8d72c27a64ea4e30adce11cf6c4f3dd3e60db
Remove deprecated COMMENT ON RULE syntax
e8d016d81940e75c126aa52971b7903b7301002e
Add hash partitioning.
1aba8e651ac3e37e1d2d875842de1e0ed22a651e
Parameter toast_tuple_target
c2513365a0a85e77d3c21adb92fe12cfbe0d1897
Parenthesized explain (...)
d4382c4ae7ea1e272f4fee388aac8ff99421471a
Parameter toast_tuple_target controls TOAST for new rows
c2513365a0a85e77d3c21adb92fe12cfbe0d1897
no longer accepts VACUUM ANALYZE VERBOSE
921059bd66c7fb1230c705d3b1a65940800c4cbb
> > else if (HeadMatches1("EXPLAIN") && previous_words_count==2 && prev_wd[0]=='(' && ends_with(prev_wd, ')'))
>
> I think this condition can be replaced by:
>
> else if (TailMatches2("EXPLAIN", MatchAny) && ends_with(prev_wd, ')'))
>
> It looks better to me. Such condition is used for other commands and
> works the same way.
>
> The last point I've noticed, there is no VERBOSE entry after VACUUM FULL
> ANALYZE command anymore.
See commit 921059bd6, above (it's not 100% clear to me that's intended to
reject VACUUM ANALYZE VERBOSE and not just reject VACUUM VERBOSE ANALYZE
VERBOSE, but I tentatively assume it's intentional).
> I'm not sure how this patch should be commited. Can it be commited
> outside the commitfest? Otherwise add it to the next commitfest please
> in order not to forget it.
I've done https://commitfest.postgresql.org/18/1661/
Justin
On Sat, Jun 09, 2018 at 06:42:12PM -0500, Justin Pryzby wrote:
> > Moreover there is no such completion for example for the command (it shows
> > only first column):
> >
> > CREATE INDEX ON test (
>
> Noted (I misunderstood at first: you just mean there's precedent that column
> names aren't completed, except maybe the first).
Yes, exactly. It was about the precedent.
> I can revise patch to not complete attributes in analyze; but, I think that
> means that this will have to complete to table names:
>
> postgres=# ANALYZE tt (a ,
> alu_enodeb_201601 information_schema.
> alu_enodeb_201602 jrn_pg_buffercache
> ...
>
> .. since, without checking for matching parens, it has no idea whether to
> complete with rels or atts. WDYT?
IMHO, I'd leave the code as simple as possible. It is up to you of
course. But it is easy to add completion for a first attribute, by
adding the condition (and leave other attributes without completion):
else if (HeadMatches1("VACUUM") && TailMatches1("("))
COMPLETE_WITH_ATTR(prev2_wd, "");
> > > - "SERVER", "INDEX", "LANGUAGE", "POLICY", "PUBLICATION", "RULE",
> > > + "SERVER", "INDEX", "LANGUAGE", "POLICY", "PUBLICATION",
> >
> > Is this a typo? I think still there is a possibility to comment rules.
>
> Not in PG11(b1) (note, that's a custom table)
> postgres=# COMMENT ON RULE pg_settings_u IS 'asdf';
> ERROR: syntax error at or near "IS"
> ...
> Remove deprecated COMMENT ON RULE syntax
> e8d016d81940e75c126aa52971b7903b7301002e
Oh, I understood what it is it here. Those commit removed the syntax:
COMMENT ON RULE rule_name
But still there is the syntax:
COMMENT ON RULE rule_name ON table_name
I can run the command:
COMMENT ON RULE rtest ON test IS 'rtest';
> > The last point I've noticed, there is no VERBOSE entry after VACUUM FULL
> > ANALYZE command anymore.
>
> See commit 921059bd6, above (it's not 100% clear to me that's intended to
> reject VACUUM ANALYZE VERBOSE and not just reject VACUUM VERBOSE ANALYZE
> VERBOSE, but I tentatively assume it's intentional).
Right. Understood.
> > I'm not sure how this patch should be commited. Can it be commited
> > outside the commitfest? Otherwise add it to the next commitfest please
> > in order not to forget it.
>
> I've done https://commitfest.postgresql.org/18/1661/
Thank you!
--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
On Mon, Jun 11, 2018 at 11:35:51PM +0300, Arthur Zakirov wrote:
> IMHO, I'd leave the code as simple as possible. It is up to you of
> course. But it is easy to add completion for a first attribute, by
> adding the condition (and leave other attributes without completion):
>
> else if (HeadMatches1("VACUUM") && TailMatches1("("))
> COMPLETE_WITH_ATTR(prev2_wd, "");
Thanks - I've done this in the attached. It works well for having minimal
logic.
On Tue, Jun 05, 2018 at 05:29:42PM +0300, Arthur Zakirov wrote:
> On Sun, Jun 03, 2018 at 10:39:22PM -0500, Justin Pryzby wrote:
> > else if (HeadMatches1("EXPLAIN") && previous_words_count==2 && prev_wd[0]=='(' && ends_with(prev_wd, ')'))
>
> I think this condition can be replaced by:
>
> else if (TailMatches2("EXPLAIN", MatchAny) && ends_with(prev_wd, ')'))
I used:
else if (HeadMatches2("EXPLAIN", MatchAny) && ends_with(prev_wd, ')'))
> > I've done https://commitfest.postgresql.org/18/1661/
>
> Thank you!
Thanks for your repeated reviews ; if this looks+works fine, please set to
R-F-C.
Actually..another thought: since toast tables may be VACUUM-ed, should I
introduce Query_for_list_of_tpmt ?
Update psql tab completion for commits:
Table completion for ANALYZE partitioned_table
3c3bb99330aa9b4c2f6258bfa0265d806bf365c3
Add parenthesized options syntax for ANALYZE.
854dd8cff523bc17972d34772b0e39ad3d6d46a4
Add VACUUM (DISABLE_PAGE_SKIPPING) for emergencies.
ede62e56fbe809baa1a7bc3873d82f12ffe7540b
Allow multiple tables to be specified in one VACUUM or ANALYZE command.
11d8d72c27a64ea4e30adce11cf6c4f3dd3e60db
Add hash partitioning.
1aba8e651ac3e37e1d2d875842de1e0ed22a651e
Parameter toast_tuple_target
c2513365a0a85e77d3c21adb92fe12cfbe0d1897
Parenthesized explain (...)
d4382c4ae7ea1e272f4fee388aac8ff99421471a
Parameter toast_tuple_target controls TOAST for new rows
c2513365a0a85e77d3c21adb92fe12cfbe0d1897
no longer accepts VACUUM ANALYZE VERBOSE
921059bd66c7fb1230c705d3b1a65940800c4cbb
Justin
Вложения
Thank you for the new version. On Wed, Jun 27, 2018 at 03:10:51PM -0500, Justin Pryzby wrote: > Thanks - I've done this in the attached. It works well for having minimal > logic. I think everthing is OK. But I didn't notice in previous version of the patch that there is no braces in EXPLAIN and VACUUM conditions. I attached diff file to show it. Also I included TEXT, XML, JSON, YAML items for FORMAT option in the diff file. The patch shows ",", ")", "ON", "OFF" for all options, but FORMAT requires format type to be specified. Please consider this change only as a suggestion. > Thanks for your repeated reviews ; if this looks+works fine, please set to > R-F-C. > > Actually..another thought: since toast tables may be VACUUM-ed, should I > introduce Query_for_list_of_tpmt ? Unfortunately, I'm not sure about toast tables and I'm not aware about policy of completion toast tables. -- Arthur Zakirov Postgres Professional: http://www.postgrespro.com Russian Postgres Company
Вложения
Sorry for the delay..this got lost while catching up after being out of town.. On Thu, Jun 28, 2018 at 02:20:39PM +0300, Arthur Zakirov wrote: > Thank you for the new version. > > On Wed, Jun 27, 2018 at 03:10:51PM -0500, Justin Pryzby wrote: > > Thanks - I've done this in the attached. It works well for having minimal > > logic. > > I think everthing is OK. But I didn't notice in previous version of the > patch that there is no braces in EXPLAIN and VACUUM conditions. I attached > diff file to show it. > > Also I included TEXT, XML, JSON, YAML items for FORMAT option in the > diff file. The patch shows ",", ")", "ON", "OFF" for all options, but > FORMAT requires format type to be specified. Please consider this change > only as a suggestion. Your suggestion is good, so attached updated patch. > > Actually..another thought: since toast tables may be VACUUM-ed, should I > > introduce Query_for_list_of_tpmt ? I didn't include this one yet though. Feel free to bump to next CF. Justin
Вложения
On Sun, Jul 29, 2018 at 07:42:43PM -0500, Justin Pryzby wrote: > Your suggestion is good, so attached updated patch. The patch is in good shape. It compiles without errors. The patch doesn't need in documentation. I marked the patch as "Ready for Commiter". > > > Actually..another thought: since toast tables may be VACUUM-ed, should I > > > introduce Query_for_list_of_tpmt ? > > I didn't include this one yet though. > > Feel free to bump to next CF. I think it could be done by a separate patch. -- Arthur Zakirov Postgres Professional: http://www.postgrespro.com Russian Postgres Company
Arthur Zakirov <a.zakirov@postgrespro.ru> writes:
> On Sun, Jul 29, 2018 at 07:42:43PM -0500, Justin Pryzby wrote:
>>> Actually..another thought: since toast tables may be VACUUM-ed, should I
>>> introduce Query_for_list_of_tpmt ?
>> I didn't include this one yet though.
> I think it could be done by a separate patch.
I don't actually think that's a good idea. It's more likely to clutter
peoples' completion lists than offer anything they want. Even if someone
actually does want to vacuum a toast table, they are not likely to be
entering its name via tab completion; they're going to have identified
which table they want via some query, and then they'll be doing something
like copy-and-paste out of a query result.
I pushed the first three hunks of the current patch, since those seem
like pretty uncontroversial bug fixes for v11 issues. Attached is a
rebased patch for the remainder, with some very minor adjustments.
The main thing that is bothering me about the remainder is its desire
to offer single-punctuation-character completions such as "(". I do
not see the point of that. You can't select a completion without
typing at least one character, so what does it accomplish to offer
those options, except clutter?
BTW, the cfbot has been claiming that this CF item fails patch
application, but that seems to be because it's not actually testing
the most recent patch. I speculate that that's because you did not
name the attachment "something.patch" or "something.diff". Please
use a more conventional filename for future attachments.
regards, tom lane
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 7549b40..69e6632 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(const char *text, int st
*** 2802,2815 ****
else if (Matches1("EXECUTE"))
COMPLETE_WITH_QUERY(Query_for_list_of_prepared_statements);
! /* EXPLAIN */
!
! /*
! * Complete EXPLAIN [ANALYZE] [VERBOSE] with list of EXPLAIN-able commands
! */
else if (Matches1("EXPLAIN"))
! COMPLETE_WITH_LIST7("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE",
! "ANALYZE", "VERBOSE");
else if (Matches2("EXPLAIN", "ANALYZE"))
COMPLETE_WITH_LIST6("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE",
"VERBOSE");
--- 2802,2834 ----
else if (Matches1("EXECUTE"))
COMPLETE_WITH_QUERY(Query_for_list_of_prepared_statements);
! /*
! * EXPLAIN [ ( option [, ...] ) ] statement
! * EXPLAIN [ ANALYZE ] [ VERBOSE ] statement
! */
else if (Matches1("EXPLAIN"))
! COMPLETE_WITH_LIST8("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE",
! "ANALYZE", "VERBOSE", "(");
! else if (HeadMatches2("EXPLAIN", "("))
! {
! if (ends_with(prev_wd, '(') || ends_with(prev_wd, ','))
! COMPLETE_WITH_LIST7("ANALYZE", "VERBOSE", "COSTS", "BUFFERS",
! "TIMING", "SUMMARY", "FORMAT");
! else if (TailMatches1("FORMAT"))
! COMPLETE_WITH_LIST4("TEXT", "XML", "JSON", "YAML");
! else if (TailMatches1("ANALYZE|VERBOSE|COSTS|BUFFERS|TIMING|SUMMARY"))
! COMPLETE_WITH_LIST4(",", ")", "ON", "OFF");
! else
! COMPLETE_WITH_LIST2(",", ")");
! }
! else if (Matches2("EXPLAIN", MatchAny) && ends_with(prev_wd, ')'))
! {
! /*
! * get_previous_words treats a parenthesized option list as one word,
! * so the above test is correct.
! */
! COMPLETE_WITH_LIST5("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE");
! }
else if (Matches2("EXPLAIN", "ANALYZE"))
COMPLETE_WITH_LIST6("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE",
"VERBOSE");
*************** psql_completion(const char *text, int st
*** 3369,3401 ****
COMPLETE_WITH_CONST("OPTIONS");
/*
! * VACUUM [ FULL | FREEZE ] [ VERBOSE ] [ table ]
! * VACUUM [ FULL | FREEZE ] [ VERBOSE ] ANALYZE [ table [ (column [, ...] ) ] ]
*/
else if (Matches1("VACUUM"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
" UNION SELECT 'FULL'"
" UNION SELECT 'FREEZE'"
" UNION SELECT 'ANALYZE'"
" UNION SELECT 'VERBOSE'");
! else if (Matches2("VACUUM", "FULL|FREEZE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
" UNION SELECT 'ANALYZE'"
" UNION SELECT 'VERBOSE'");
! else if (Matches3("VACUUM", "FULL|FREEZE", "ANALYZE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
! " UNION SELECT 'VERBOSE'");
! else if (Matches3("VACUUM", "FULL|FREEZE", "VERBOSE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
" UNION SELECT 'ANALYZE'");
! else if (Matches2("VACUUM", "VERBOSE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
" UNION SELECT 'ANALYZE'");
! else if (Matches2("VACUUM", "ANALYZE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
! " UNION SELECT 'VERBOSE'");
else if (HeadMatches1("VACUUM"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm, NULL);
/* WITH [RECURSIVE] */
--- 3388,3435 ----
COMPLETE_WITH_CONST("OPTIONS");
/*
! * VACUUM [ ( option [, ...] ) ] [ table_and_columns [, ...] ]
! * VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ table_and_columns [, ...] ]
*/
else if (Matches1("VACUUM"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpm,
" UNION SELECT 'FULL'"
" UNION SELECT 'FREEZE'"
" UNION SELECT 'ANALYZE'"
" UNION SELECT 'VERBOSE'");
! else if (Matches2("VACUUM", "FULL"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpm,
! " UNION SELECT 'FREEZE'"
" UNION SELECT 'ANALYZE'"
" UNION SELECT 'VERBOSE'");
! else if (Matches2("VACUUM", "FULL|FREEZE") ||
! Matches3("VACUUM", "FULL", "FREEZE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpm,
! " UNION SELECT 'VERBOSE'"
" UNION SELECT 'ANALYZE'");
! else if (Matches2("VACUUM", "VERBOSE") ||
! Matches3("VACUUM", "FULL|FREEZE", "VERBOSE") ||
! Matches4("VACUUM", "FULL", "FREEZE", "VERBOSE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpm,
" UNION SELECT 'ANALYZE'");
! else if (HeadMatches2("VACUUM", "("))
! {
! if (ends_with(prev_wd, ',') || ends_with(prev_wd, '('))
! COMPLETE_WITH_LIST5("FULL", "FREEZE", "ANALYZE", "VERBOSE", "DISABLE_PAGE_SKIPPING");
! else
! COMPLETE_WITH_LIST2(",", ")");
! }
! else if (HeadMatches1("VACUUM") && TailMatches1("("))
! /* "VACUUM (" should be caught above */
! COMPLETE_WITH_ATTR(prev2_wd, "");
! else if (HeadMatches2("VACUUM", MatchAny) && !ends_with(prev_wd, ',') &&
! !TailMatches1("ANALYZE") &&
! !(previous_words_count == 2 && prev_wd[0] == '(' && ends_with(prev_wd, ')')))
! /* Comma to support vacuuming multiple tables */
! /* Parens to support analyzing a partial column list */
! COMPLETE_WITH_LIST3(",", "(", ")");
else if (HeadMatches1("VACUUM"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpm, "");
/* WITH [RECURSIVE] */
*************** psql_completion(const char *text, int st
*** 3406,3414 ****
else if (Matches1("WITH"))
COMPLETE_WITH_CONST("RECURSIVE");
! /* ANALYZE */
! /* Complete with list of tables */
else if (Matches1("ANALYZE"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tmf, NULL);
/* WHERE */
--- 3440,3467 ----
else if (Matches1("WITH"))
COMPLETE_WITH_CONST("RECURSIVE");
! /*
! * ANALYZE [ ( option [, ...] ) ] [ table_and_columns [, ...] ]
! * ANALYZE [ VERBOSE ] [ table_and_columns [, ...] ]
! */
! else if (Matches2("ANALYZE", "("))
! COMPLETE_WITH_CONST("VERBOSE)");
else if (Matches1("ANALYZE"))
+ COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tmf,
+ " UNION SELECT 'VERBOSE'"
+ " UNION SELECT '('"
+ );
+ else if (HeadMatches1("ANALYZE") && TailMatches1("("))
+ /* "ANALYZE (" should be caught above */
+ COMPLETE_WITH_ATTR(prev2_wd, "");
+ else if (HeadMatches2("ANALYZE", MatchAny) &&
+ !ends_with(prev_wd, ',') &&
+ !(previous_words_count == 2 && prev_wd[0] == '(' && ends_with(prev_wd, ')')) &&
+ !TailMatches1("VERBOSE"))
+ /* Support analyze of multiple tables */
+ /* or analyze table(column1, column2) */
+ COMPLETE_WITH_LIST3(",", "(", ")");
+ else if (HeadMatches1("ANALYZE"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tmf, NULL);
/* WHERE */
I wrote:
> The main thing that is bothering me about the remainder is its desire
> to offer single-punctuation-character completions such as "(". I do
> not see the point of that. You can't select a completion without
> typing at least one character, so what does it accomplish to offer
> those options, except clutter?
Actually, after poking at it for awhile, I noticed there was a much bigger
problem: tests like
else if (HeadMatches2("VACUUM", "("))
would continue to fire even if the option list was complete, so that
after typing say
vacuum ( verbose )
you would not get offered any table names, only option names.
I experimented with various fixes for that, but the only one that
worked required extending word_matches() to allow a wildcard in the
middle of a pattern. (Before it only allowed one at the end; but
it takes just a couple more lines of code to improve that.) With
that, we can do
else if (HeadMatches2("VACUUM", "(*") &&
!HeadMatches2("VACUUM", "(*)"))
and this test will not trigger if the option list is complete.
I've gone ahead and pushed it with those changes.
regards, tom lane