Re: Move un-parenthesized syntax docs to "compatibility" for few SQL commands - Mailing list pgsql-hackers

On Fri, Apr 21, 2023 at 09:44:51PM -0700, Nathan Bossart wrote:
> I've attached two patches.  0001 adds a parenthesized CLUSTER syntax that
> doesn't require a table name.  0002 is your patch with a couple of small
> adjustments.
> 
> On Fri, Apr 21, 2023 at 07:29:59PM -0400, Melanie Plageman wrote:
> > Attached v2 includes changes for CLUSTER syntax docs. I wasn't quite
> > sure that we can move down CLUSTER [VERBOSE] syntax to the compatibility
> > section since CLUSTER (VERBOSE) doesn't work. This seems like it should
> > work (VACUUM and ANALYZE do). I put extra "[ ]" around the main CLUSTER
> > syntax but I don't know that this is actually the same as documenting
> > that CLUSTER VERBOSE does work but CLUSTER (VERBOSE) doesn't.
> 
> I'm not sure about moving CLUSTER [VERBOSE] to the compatibility section,
> either.  At the very least, I don't think what I have in 0002 is accurate.
> It claims that syntax was used before v14, but it's still the only way to
> do a "verbose" CLUSTER without a table name in v15 and v16, too.  Perhaps
> we should break it apart, or maybe we can just say it was used before v17.
> WDYT?

If you are planning to wait and commit the change to support CLUSTER
(VERBOSE) until July, then you can consolidate the two and say before
v17. If you plan to commit it before then (making it available in v16),
I would move CLUSTER [VERBOSE] down to Compatibility and say that both
of the un-parenthesized syntaxes were used before v16. Since all of the
syntaxes still work, I think it is probably more confusing to split them
apart so granularly. The parenthesized syntax was effectively not
"feature complete" without your patch to support CLUSTER (VERBOSE).

Also, isn't this:
  CLUSTER [VERBOSE] [ <qualified_name> [ USING <index_name> ] ]
inclusive of this:
  CLUSTER [VERBOSE]
So, it would have been correct for them to be consolidated in the
existing documentation?

> From 48fff177a8f0096c99c77b4e1368cc73f7e86585 Mon Sep 17 00:00:00 2001
> From: Nathan Bossart <nbossart@postgresql.org>
> Date: Fri, 21 Apr 2023 20:16:25 -0700
> Subject: [PATCH v3 1/2] Support parenthesized syntax for CLUSTER without a
>  table name.
> 
> b5913f6 added a parenthesized syntax for CLUSTER, but it requires
> specifying a table name.  This is unlike commands such as VACUUM
> and ANALYZE, which do not require specifying a table in the
> parenthesized syntax.  This change resolves this inconsistency.
> 
> The documentation for the CLUSTER syntax has also been consolidated
> in anticipation of a follow-up change that will move the
> unparenthesized syntax to the "Compatibility" section.

I suppose we should decide if unparenthesized is a word or if we are
sticking with the hyphen.

>  doc/src/sgml/ref/cluster.sgml |  5 ++---
>  src/backend/parser/gram.y     | 21 ++++++++++++++-------
>  2 files changed, 16 insertions(+), 10 deletions(-)
> 
> --- a/src/backend/parser/gram.y
> +++ b/src/backend/parser/gram.y
...
> @@ -11593,7 +11592,17 @@ ClusterStmt:
>                          n->params = lappend(n->params, makeDefElem("verbose", NULL, @2));
>                      $$ = (Node *) n;
>                  }
> +            | CLUSTER opt_verbose
> +                {
> +                    ClusterStmt *n = makeNode(ClusterStmt);
>  
> +                    n->relation = NULL;
> +                    n->indexname = NULL;
> +                    n->params = NIL;
> +                    if ($2)
> +                        n->params = lappend(n->params, makeDefElem("verbose", NULL, @2));
> +                    $$ = (Node *) n;
> +                }

Maybe it is worth moving the un-parenthesized options all to the end and
specifying what version they were needed for.


>              | CLUSTER '(' utility_option_list ')' qualified_name cluster_index_specification
>                  {
>                      ClusterStmt *n = makeNode(ClusterStmt);
> @@ -11603,15 +11612,13 @@ ClusterStmt:
>                      n->params = $3;
>                      $$ = (Node *) n;
>                  }
> -            | CLUSTER opt_verbose
> +            | CLUSTER '(' utility_option_list ')'

It is too bad we can't do this the way VACUUM and ANALYZE do -- but
since qualified_name is required if USING is included, I suppose we
can't.

> diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
> index 20c6f9939f..ea42ec30bd 100644
> --- a/doc/src/sgml/ref/analyze.sgml
> +++ b/doc/src/sgml/ref/analyze.sgml
> @@ -22,7 +22,6 @@ PostgreSQL documentation
>   <refsynopsisdiv>
>  <synopsis>
>  ANALYZE [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] [ <replaceable
class="parameter">table_and_columns</replaceable>[, ...] ]
 
> -ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replaceable> [, ...] ]
...
> @@ -346,6 +338,14 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
>    <para>
>     There is no <command>ANALYZE</command> statement in the SQL standard.
>    </para>
> +
> +  <para>
> +   The following syntax was used before <productname>PostgreSQL</productname>
> +   version 11 and is still supported:

Good call on specifying that the order matters.

> +<synopsis>
> +ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replaceable> [, ...] ]
> +</synopsis>
> +  </para>

- Melanie