Re: default attstattarget

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: nconway(at)klamath(dot)dyndns(dot)org (Neil Conway)
Cc: PostgreSQL Patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: default attstattarget
Date: 2002-07-20 05:18:22
Message-ID: 18257.1027142302@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

nconway(at)klamath(dot)dyndns(dot)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

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Bruce Momjian 2002-07-20 05:43:19 Re: [HACKERS] [PATCH] Win32 native fixes after SSL updates (+more)
Previous Message Bruce Momjian 2002-07-20 05:13:34 Re: Optional Oid