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

On 9/1/17, 12:11 AM, "Michael Paquier" <michael(dot)paquier(at)gmail(dot)com> wrote:
> Hm. Here is the diff between v11 and v12:
> static void
> autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy)
> {
> - RangeVar rangevar;
> - VacuumRelation *rel;
> -
> - /* Set up command parameters --- use local variables instead of palloc */
> - MemSet(&rangevar, 0, sizeof(rangevar));
> -
> - rangevar.schemaname = tab->at_nspname;
> - rangevar.relname = tab->at_relname;
> - rangevar.location = -1;
> + RangeVar *rangevar;
> + VacuumRelation *rel;
>
> /* Let pgstat know what we're doing */
> autovac_report_activity(tab);
>
> - rel = makeVacuumRelation(&rangevar, NIL, tab->at_relid);
> + rangevar = makeRangeVar(tab->at_nspname, tab->at_relname, -1);
> + rel = makeVacuumRelation(rangevar, NIL, tab->at_relid);
> But there is this commit in vacuum.c:
> * It is the caller's responsibility that all parameters are allocated
> in a
> * memory context that will not disappear at transaction commit.
> And I don't think we want to break that promise as newNode() uses
> palloc0fast() which allocates data in the current memory context (see
> 4873c96f). I think that you had better just use NodeSetTag here and be
> done with it. Also, it seems to me that this could be fixed as a
> separate patch. It is definitely an incorrect pattern...

Don't we have a similar problem with makeVacuumRelation() and list_make1()?

I went ahead and moved the RangeVar, VacuumRelation, and List into local
variables for now, but I agree that this could be improved in a separate
patch. Perhaps these could be allocated in AutovacMemCxt. I see from
4873c96f that autovacuum_do_vac_analyze() used to allocate the list of OIDs
in that "long-lived" memory context.

> - $$ = (Node *)n;
> + $$ = (Node *) n;
> Spurious noise. And the coding pattern in gram.y is to not add a space
> (make new code look like its surroundings as the documentation says).

I've fixed this in v13.

Nathan