On Fri, 2009-11-13 at 10:47 +0900, Itagaki Takahiro wrote:
> Alvaro Herrera <alvherre@commandprompt.com> wrote:
>
> > BTW I think the vacstmt.options change, which seems a reasonable idea to
> > me, could be extracted from the patch and applied separately. That'd
> > reduce the size of the patch somewhat.
>
> It's a good idea. I separated the part into the attached patch.
> It replaces VacuumStmt's "vacuum", "full", "analyze", and "verbose"
> fields into one "options" flag field.
I added a separate commitfest item for this patch to track it separately
from the rewrite version of VACUUM:
https://commitfest.postgresql.org/action/patch_view?id=222
> The only user-visible change is to support "VACUUM (options)" syntax:
> VACUUM ( FULL, FREEZE, VERBOSE, ANALYZE ) table (columns)
> We don't bother with the order of options in this form :)
I looked at this patch. You left INPLACE in the patch, which looks like
an oversight when you were separating the two. Please remove that from
this part.
> There is a debatable issue that we can use "VACUUM (VERBOSE) table (col)"
> in the abobe syntax. Columns are specified but no ANALYZE option there.
> An ANALYZE option is added automatically in the current implementation,
> but we should have thrown an syntax error in such case.
Sounds fine, but worth a mention in the documentation. Just add to the
"columns" part of the VACUUM page something like: "If specified, implies
ANALYZE".
Other than these two minor issues, I don't see any problems with the
patch. Please post an updated version to the new commitfest entry.
Regards,Jeff Davis
