Re: ANALYZE patch for review

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Mark Cave-Ayland <m(dot)cave-ayland(at)webbased(dot)co(dot)uk>
Cc: "'Tom Lane'" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-patches(at)postgresql(dot)org
Subject: Re: ANALYZE patch for review
Date: 2004-02-13 04:25:58
Message-ID: 200402130425.i1D4Pwt26136@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches


Applied by Tom. Thanks.

---------------------------------------------------------------------------

Mark Cave-Ayland wrote:
> Hi Tom,
>
> > -----Original Message-----
> > From: Tom Lane [mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us]
> > Sent: 02 February 2004 16:54
> > 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:
> > > So I'd like to propose a slightly different solution. I think that
> > > examine_attribute() should return a pointer to a custom structure
> > > containing any information that needs to be passed to the datatype
> > > specific routine (not the entire VacAttrStats structure),
> > or NULL if
> > > the column should not be analyzed.
> >
> > Just a void* you mean? Sure, we could do that, although it
> > might result in some duplicated effort. Another possibility
> > is that analyze.c goes ahead and creates a VacAttrStats
> > struct (including a void* extension pointer that it
> > initializes to NULL) and then passes the struct to
> > examine_attribute, which returns a bool and optionally
> > modifies fields in the VacAttrStats struct --- in particular
> > the requested-row-count and extension pointer. If false is
> > returned then analyze.c just throws away the VacAttrStats
> > struct instead of including it in its to-do list.
>
> Yup indeed. Please find enclosed the latest version of the analyze patch
> taking into account all the things we have discussed in the thread. It
> feels a lot cleaner than the other - the advantage of having
> analyze_rel() create the VacAttrStats structure is that you can put some
> vaguely sensible defaults in the user-defined function so it shouldn't
> bomb too badly if someone gets it wrong, plus the attribute and type
> information can be fed in automagically.
>
> This rewrite has the benefit that it gets rid of the duplication of
> routines having to get their own copies of the attribute and type rows
> based on attnum, and also the (void *) pointer gives a clear abstraction
> of the data required for the pg_statistic table entry and those that are
> used by the current default routine. 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), and also as discussed the internal
> stats routine is called directly without requiring an entry in the
> catalog.
>
> > >> If you suppose that the "major" field is the upper bits of
> > >> the statistics ID value, then this is just a slightly
> > >> different way of thinking about the range-based allocation
> > >> method I suggested before.
> >
> > > I was thinking perhaps in terms of an extra staowner int2 field in
> > > pg_statistic where the IDs are allocated by the PGDG.
> >
> > I do not really want to add a field to pg_statistic. That
> > complicates matters more than we have a justification to do.
> > Nor do we have any reason at this point to think that we need
> > a 2^32 namespace for statistics kinds. (If 2^16 ever starts
> > to look cramped, we can just widen the fields to int4 without
> > introducing any backwards compatibility issues --- existing
> > code assignments will remain valid. But I find it hard to
> > foresee that happening.)
>
> OK, I've left this as it is at the moment.
>
>
> Cheers,
>
> 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.

[ Attachment, skipping... ]

[ Attachment, skipping... ]

[ Attachment, skipping... ]

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

In response to

Browse pgsql-patches by date

  From Date Subject
Next Message Bruce Momjian 2004-02-13 04:53:22 Re: [PATCHES] log session end - again
Previous Message Christopher Browne 2004-02-13 04:20:53 Re: Vacuum Delay feature