Re: [PATCH] add CLUSTER table USING index (take 2) - Mailing list pgsql-patches

FYI, this is a great example of valuable patch review.

---------------------------------------------------------------------------

Heikki Linnakangas wrote:
> Holger Schurig wrote:
> > Index: src/doc/src/sgml/ref/cluster.sgml
> > ===================================================================
> > *** src.orig/doc/src/sgml/ref/cluster.sgml    2007-03-28 23:02:12.000000000 +0200
> > --- src/doc/src/sgml/ref/cluster.sgml    2007-03-28 23:03:14.000000000 +0200
> > ***************
> > *** 20,27 ****
> >
> >    <refsynopsisdiv>
> >   <synopsis>
> > ! CLUSTER <replaceable class="PARAMETER">indexname</replaceable> ON <replaceable
class="PARAMETER">tablename</replaceable>
> > ! CLUSTER <replaceable class="PARAMETER">tablename</replaceable>
> >   CLUSTER
> >   </synopsis>
> >    </refsynopsisdiv>
> > --- 20,26 ----
> >
> >    <refsynopsisdiv>
> >   <synopsis>
> > ! CLUSTER <replaceable class="PARAMETER">tablename</replaceable> [ USING <replaceable
class="PARAMETER">indexname</replaceable>] 
> >   CLUSTER
> >   </synopsis>
> >    </refsynopsisdiv>
>
> We still need to document the old syntax, especially if we don't change
> the example as well.
>
> > Index: src/src/backend/parser/gram.y
> > ===================================================================
> > *** src.orig/src/backend/parser/gram.y    2007-03-28 22:58:48.000000000 +0200
> > --- src/src/backend/parser/gram.y    2007-03-28 22:59:15.000000000 +0200
> > ***************
> > *** 209,215 ****
> >
> >   %type <str>        relation_name copy_file_name
> >                   database_name access_method_clause access_method attr_name
> > !                 index_name name file_name
> >
> >   %type <list>    func_name handler_name qual_Op qual_all_Op subquery_Op
> >                   opt_class opt_validator
> > --- 209,215 ----
> >
> >   %type <str>        relation_name copy_file_name
> >                   database_name access_method_clause access_method attr_name
> > !                 index_name name file_name opt_cluster_using
> >
> >   %type <list>    func_name handler_name qual_Op qual_all_Op subquery_Op
> >                   opt_class opt_validator
>
> Is the placement of opt_cluster_using completely arbitrary? I'm not very
> familiar with the parser, it really looks like those type-definitions
> are in random order.
>
> --
>    Heikki Linnakangas
>    EnterpriseDB   http://www.enterprisedb.com
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +