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

On Fri, Sep 1, 2017 at 12:25 AM, Bossart, Nathan <bossartn(at)amazon(dot)com> wrote:
> On 8/31/17, 2:24 AM, "Masahiko Sawada" <sawada(dot)mshk(at)gmail(dot)com> wrote:
>> I reviewed these patches and found a issue.
>
> Thanks for reviewing.
>
>> autovacuum worker seems not to work fine. I got an error message;
>>
>> ERROR: unrecognized node type: 0
>> CONTEXT: automatic analyze of table "postgres.public.hoge"
>>
>> I think we should set T_RangeVar to rangevar.type in
>> autovacuum_do_vac_analyze function.
>
> Yes, it looks like the NodeTag is not getting set on the RangeVar.
> I went ahead and switched this to makeRangeVar(...) instead of
> keeping it manually allocated on the stack. Autovacuum seems to be
> working as usual now.

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...

- $$ = (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).
--
Michael