On Fri, Mar 8, 2019 at 12:22 AM Robert Haas <robertmh...@gmail.com> wrote: > > On Wed, Mar 6, 2019 at 10:58 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > Why make it a Node? I mean I think a struct makes sense, but what's > > > the point of giving it a NodeTag? > > > > Well, the main point is consistency with other nodes and keep the code > > clean. > > It looks to me like if we made it a plain struct rather than a node, > and embedded that struct (not a pointer) in VacuumStmt, then what > would happen is that _copyVacuumStmt and _equalVacuumStmt would have > clauses for each vacuum option individually, with a dot, like > COPY_SCALAR_FIELD(options.flags). > > Also, the grammar production for VacuumStmt would need to be jiggered > around a bit; the way that options consolidation is done there would > have to be changed. > > Neither of those things sound terribly hard or terribly messy, but on > the other hand I guess there's nothing really wrong with the way you > did it, either ... anybody else have an opinion? >
I don't have a strong opinion but the using a Node would be more suitable in the future when we add more options to vacuum. And it seems to me that it's unlikely to change a Node to a plain struct. So there is an idea of doing it now anyway if we might need to do it someday. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center