* Robert Haas (robertmh...@gmail.com) wrote: > On Sun, Mar 1, 2015 at 6:58 AM, Michael Paquier > <michael.paqu...@gmail.com> wrote: > > Hm, why not. That would remove all inconsistencies between the parser > > and the autovacuum code path. Perhaps something like the attached > > makes sense then? > > I really don't see this patch, or any of the previous ones, as solving > any actual problem. There's no bug here, and no reason to suspect > that future code changes would be particularly like to introduce one. > Assertions are a great way to help developers catch coding mistakes, > but it's a real stretch to think that a developer is going to add a > new syntax for ANALYZE that involves setting options proper to VACUUM > and not notice it.
Yeah, I haven't been terribly excited about it for the same reasons.
Had Michael's latest patch meant that we didn't need to pass VacuumStmt
down into the other functions then I might have been a bit more behind
it, but as is we seem to be simply duplicating everything except the
actual Node entry in the struct, which kind of missed the point.
> This thread started out because Michael read an assertion in the code
> and misunderstood what that assertion was trying to guard against.
> I'm not sure there's any code change needed here at all, but if there
> is, I suggest we confine it to adding a one-line comment above that
> assertion clarifying its purpose, like /* check that parser didn't
> produce ANALYZE FULL or ANALYZE FREEZE */.
I'd be fine with that.
Thanks!
Stephen
signature.asc
Description: Digital signature
