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

On Sat, Apr 22, 2023 at 10:38:58AM -0400, Melanie Plageman wrote:
> 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).

I think this can wait for v17, but if there's a strong argument for doing
some of this sooner, we can reevaluate.

> 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?

Yes.  This appears to go pretty far back.  I traced it to 8bc717c (2002).
It looks like the VACUUM syntaxes have been consolidated since 37d2f76
(1998).  So AFAICT this small inconsistency has been around for a while.

>> 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.

The existing uses in the docs all omit the hypen, but I see it both ways in
some web searches.  Other than keeping the Postgres docs consistent, I
don't have a terribly ѕtrong opinion here.

>> +            | 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.

Good idea.

>>              | 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.

It might be possible to extract the name and index part to a separate
optional rule.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com