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

On Fri, Sep 1, 2017 at 12:25 AM, Bossart, Nathan <bossartn@amazon.com> wrote:
> On 8/31/17, 2:24 AM, "Masahiko Sawada" <sawada.mshk@gmail.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