Re: default attstattarget

nconway@klamath.dyndns.org (Neil Conway) writes:
>       /* Don't analyze column if user has specified not to */
> !     if (attr->attstattarget <= 0)
>           return NULL;

>       /* If column has no "=" operator, we can't do much of anything */
> --- 384,390 ----
>       VacAttrStats *stats;

>       /* Don't analyze column if user has specified not to */
> !     if (attr->attstattarget == 0)
>           return NULL;

followed by

> +     /* If the attstattarget column is set to "-1", use the default value */
> +     if (stats->attr->attstattarget == -1)
> +         stats->attr->attstattarget = DEFAULT_ATTSTATTARGET;
> +
>       /* Is there a "<" operator with suitable semantics? */

As written, this code will crash (or at least behave very undesirably)
if attstattarget < -1 --- a situation easily created with manual update
of attstattarget, regardless of what you may try to enforce in ALTER
TABLE.  I suggest making the second test be

+     /* If the attstattarget column is negative, use the default value */
+     if (stats->attr->attstattarget < 0)
+         stats->attr->attstattarget = DEFAULT_ATTSTATTARGET;

which is bulletproof at the same or less cost as not being bulletproof.
The same sort of change should be made in pg_dump (ignore any negative
attstattarget, not only -1).

The modified pg_dump will fail on pre-7.2 databases, because you
neglected to add a dummy attstattarget result for the pg_attribute
select commands used in pre-7.2 cases.  You need a "-1 as attstattarget"
in there.

I'm unexcited about the proposed gram.y changes, especially since you
didn't document them.  I do not think we need a SET DEFAULT variant;
anyone bright enough to be toying with attstattarget at all can figure
out how to revert it to -1 if they want.

Otherwise it looks reasonable ...

            regards, tom lane