Re: ANALYZE patch for review

Hi Tom,

> -----Original Message-----
> From: Tom Lane [mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us]
> Sent: 12 February 2004 23:52
> To: Mark Cave-Ayland
> Cc: pgsql-patches(at)postgresql(dot)org
> Subject: Re: [PATCHES] ANALYZE patch for review
>
>
> "Mark Cave-Ayland" <m(dot)cave-ayland(at)webbased(dot)co(dot)uk> writes:
> > Yup indeed. Please find enclosed the latest version of the analyze
> > patch taking into account all the things we have discussed in the
> > thread.
>
> I've reviewed and applied this with some small changes. You
> did a good job --- the only things you missed AFAICS were
> pg_dump support and documentation.

Ahhh pg_dump. Knew I forgot something :) I was going to ask about
patches for the documentation but it looks like you beat me to it.

> I changed the API slightly for the typanalyze function and
> the compute_stats subroutine. I don't think it's necessary
> or appropriate to pass in the Relation pointer to typanalyze;
> that simplifies its signature to a single INTERNAL parameter.
> (If we did want to pass the Relation, I'd be inclined to
> make it a field in VacAttrStats.) Also I tweaked
> compute_stats so that the attribute number of the column is
> passed explicitly. I foresee needing this for
> functional-index stats gathering --- it's likely that the
> accumulated sample rows will be built on the fly and will
> have a tupdesc that includes both the main table rows and the
> functional index columns. So compute_stats shouldn't assume
> that the nominal column number is the right thing to use to
> extract the column it wants.
>
> > I also altered the examine_attribute() to allow the user-defined
> > function to check attstattarget for the column manually (in
> case they
> > want to do something special with minus or zero values),
>
> As I've committed it, the convention that zero means "no
> stats" is enforced by examine_attribute(), but the typanalyze
> function is responsible for deciding what a negative value
> means. Seem reasonable?

I've had a quick browse of CVS on developer.postgresql.org and the work
you've done looks great. The only reason I kept the Relation parameter
was because I wasn't sure if there was a historical reason why someone
would need the relation information as well as the attribute
information. No problem with the zero value enforcing no stats. I also
noticed your later commit with the added fetch_func parameter which
brings stats on functional indexes even closer :)

> Also, I put the following documentation about "kind" values
> into pg_statistic.h.
>
> /*
> * The present allocation of "kind" codes is:
> *
> * 1-99: reserved for assignment by the core
> PostgreSQL project
> * (values in this range will be documented
> in this file)
> * 100-199: reserved for assignment by the PostGIS project
> * (values to be documented in PostGIS documentation)
> * 200-9999: reserved for future public assignments
> *
> * For private use you may choose a "kind" code at random in the range
> * 10000-30000. However, for code that is to be widely
> disseminated it is
> * better to obtain a publicly defined "kind" code by request from the
> * PostgreSQL Global Development Group.
> */

Fantastic stuff. I'll email Dave and Paul at Refractions and let them
know the changes for analyzing custom types have been made - not sure
whether it will be them or myself that get to implement the changes in
PostGIS but it will be a good test that everything works as it should :)

Thanks again for all your work on this,

Mark.

---

Mark Cave-Ayland
Webbased Ltd.
Tamar Science Park
Derriford
Plymouth
PL6 8BX
England

Tel: +44 (0)1752 764445
Fax: +44 (0)1752 764446

This email and any attachments are confidential to the intended
recipient and may also be privileged. If you are not the intended
recipient please delete it from your system and notify the sender. You
should not copy it or use it for any purpose nor disclose or distribute
its contents to any other person.