Re: Strange assertion using VACOPT_FREEZE in vacuum.c

On Sun, Mar 1, 2015 at 6:58 AM, Michael Paquier
<michael.paquier@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.

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

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company