Re: [PATCH] add CLUSTER table USING index (take 2)

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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

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