Re: [Proposal] Allow users to specify multiple tables in VACUUM commands

On 9/9/17, 7:28 AM, "Michael Paquier" <michael(dot)paquier(at)gmail(dot)com> wrote:
> In the duplicate patch, it seems to me that you can save one lookup at
> the list of VacuumRelation items by checking for column duplicates
> after checking that all the columns are defined. If you put the
> duplicate check before closing the relation you can also use the
> schema name associated with the Relation.

I've made these changes in v4 of the duplicate patch.

> + if (i == InvalidAttrNumber)
> + ereport(ERROR,
> + (errcode(ERRCODE_UNDEFINED_COLUMN),
> + errmsg("column \"%s\" of relation \"%s\" does not exist",
> + col, RelationGetRelationName(rel))));
> This could use the schema name unconditionally as you hold a Relation
> here, using RelationGetNamespace().

This is added in v16 of the main patch.

> So if the relation is analyzed but skipped, we would have no idea that
> it actually got skipped because there are no reports about it. That's
> not really user-friendly. I am wondering if we should not instead have
> analyze_rel also enforce the presence of a RangeVar, and adding an
> assert at the beginning of the function to undertline that, and also
> do the same for vacuum(). It would make things also consistent with
> vacuum() which now implies on HEAD that a RangeVar *must* be
> specified.

I've made these changes in v16 of the main patch.

Nathan