Thread

Commits

  1. Track the maximum possible frequency of non-MCE array elements.

  2. Collect and use element-frequency statistics for arrays.

  1. Poor row estimates from planner, stat `most_common_elems` sometimes missing for a text[] column

    Mark Frost <frostmar@uk.ibm.com> — 2025-06-05T15:42:00Z

    Hello all,
    
    We’re seeing intermittently very poor performance of a query, when occasionally a poor query plan is chosen. We’re using Postgres 16.9.
    One suspicious factor when looking at the EXPLAIN ANALYZE output, is a very wrong estimated number of rows to be returned from a text[] column queried with ‘&&’.
    
    After playing around with a simple recreate (details below), it seems ANALYZE of the table is affected by the number of rows in the table. Statistic `most_common_elems` is [null] when there’s over 15,873 rows in the table when analyzed. With fewer rows it’s analyzed correctly.
    
    Is there any good explanation for this behaviour? Preferably we’d like some way for proper `most_common_elems` statistics to be collected in our production database, in the hope that influences a good query plan to always be selected.
    
    In our production system there’s ~150,000 rows in a table including a `text[]` column, where each row has an array containing a single 19ish char string, unique within the table. The full query joins against a couple more tables, and has a GIN index on the text[] column. If necessary, I can get into details of the real system, but hope the simple recreate will be sufficient to understand the problem:
    
    
    
    CREATE TABLE IF NOT EXISTS public.test(
        id SERIAL PRIMARY KEY,
        tags text[]
    )
    
    INSERT INTO public.test (tags)
            SELECT ARRAY[TO_CHAR(n,'fm00000000')] FROM ( SELECT generate_series(1,15_873) AS n );
    
    ANALYZE public.test;
    
    SELECT * FROM pg_stat_user_tables WHERE relname = 'test';
    
    EXPLAIN (ANALYZE,BUFFERS,VERBOSE)
            SELECT * FROM test WHERE tags && ARRAY['00000002']
    
    
    
    
    Results
    -------
    table with 15_000 rows has most_common_elems after ANALYZE (most_common_elem_freqs : 6.666667e-05)
    table with 15_872 rows has most_common_elems after ANALYZE (most_common_elem_freqs : 6.300403e-05)
    table with 15_873 rows has [null] most_common_elems after ANALYZE
    
    table with 100_000 rows has [null] most_common_elems after ANALYZE
    
    
    
    Query plans show an estimated 1 row is predicted when statistics has `most_common_elems` available, or the hardcoded default 1/200 of the estimated table size when most_common_elems is null.
    Here 79 rows are estimated, when the table contained 15,873 rows and stats weren’t available.
    
    Query plan
    -----------
    Seq Scan on public.test  (cost=0.00..463.41 rows=79 width=37) (actual time=9.934..17.190 rows=1 loops=1)
      Output: id, tags
      Filter: (test.tags && '{00000002}'::text[])
     Rows Removed by Filter: 15872
      Buffers: shared hit=268
    Planning:
      Buffers: shared hit=75
    Planning Time: 2.060 ms
    Execution Time: 17.205 ms
    
    
    Full version
    ------------
    "PostgreSQL 16.9 (Debian 16.9-1.pgdg120+1) on aarch64-unknown-linux-gnu, compiled by gcc (Debian 12.2.0-14) 12.2.0, 64-bit"
    
    
    Regards,
    Mark Frost
    IBM
    
    Unless otherwise stated above:
    
    IBM United Kingdom Limited
    Registered in England and Wales with number 741598
    Registered office: Building C, IBM Hursley Office, Hursley Park Road, Winchester, Hampshire SO21 2JN
    
  2. Re: Poor row estimates from planner, stat `most_common_elems` sometimes missing for a text[] column

    Frédéric Yhuel <frederic.yhuel@dalibo.com> — 2025-06-05T17:09:06Z

    
    On 6/5/25 17:42, Mark Frost wrote:
    > Is there any good explanation for this behaviour? Preferably we’d like 
    > some way for proper `most_common_elems` statistics to be collected in 
    > our production database, in the hope that influences a good query plan 
    > to always be selected.
    
    
    most_common_elems has a limited size, and if all the elements have the 
    same freq, there's nothing we can do.
    
    You could do: alter table test alter column tags set statistics X;
    
    However, X is capped at 10000, which means that the size of 
    most_common_elems will be less than 100k, and it would probably be 
    stupid to go beyond that anyway.
    
    It seems that postgres lacks some kind of "n_distinct_elems" for that 
    kind of case, but let's wait and see what the statistics gurus think.
    
    
    
    
  3. Re: Poor row estimates from planner, stat `most_common_elems` sometimes missing for a text[] column

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-06-05T18:57:10Z

    Mark Frost <FROSTMAR@uk.ibm.com> writes:
    > We're seeing intermittently very poor performance of a query, when occasionally a poor query plan is chosen. We're using Postgres 16.9.
    > One suspicious factor when looking at the EXPLAIN ANALYZE output, is a very wrong estimated number of rows to be returned from a text[] column queried with '&&'.
    > After playing around with a simple recreate (details below), it seems ANALYZE of the table is affected by the number of rows in the table. Statistic `most_common_elems` is [null] when there's over 15,873 rows in the table when analyzed. With fewer rows it's analyzed correctly.
    
    Thanks for the report.  Poking through the code, it seems like
    there are two distinct problems here:
    
    1. ANALYZE uses a "lossy counting" algorithm (dating to commit
    0e5e167aa) to estimate the frequencies of array element values.
    The part of that that seems to be going off the rails is
    this selection of a cutoff frequency below which element values
    will be dropped:
    
            /*
             * Construct an array of the interesting hashtable items, that is,
             * those meeting the cutoff frequency (s - epsilon)*N.  Also identify
             * the minimum and maximum frequencies among these items.
             *
             * Since epsilon = s/10 and bucket_width = 1/epsilon, the cutoff
             * frequency is 9*N / bucket_width.
             */
            cutoff_freq = 9 * element_no / bucket_width;
    
    The first thing I find suspicious here is that the calculation is
    based on element_no (the total number of array elements processed)
    and not nonnull_cnt (the maximum possible frequency).  Is that
    really right?  It wouldn't change the results in your reproducer
    with just one element per array, but it seems bogus.
    
    More relevant to your immediate problem, this creates a behavioral
    cliff at the point where cutoff_freq rises from 0 to 1, which with the
    default attstattarget turns out to be, you guessed it, 15873 elements.
    In your example, all the element values have frequency 1, so that
    switches us from being willing to record all the values to being
    willing to record none of them.  That doesn't feel right either.
    By analogy to our treatment of regular MCVs, it seems like we should
    never be willing to store values that we didn't see at least twice.
    Of course, doing that would make this example worse, because then
    we'd not store any "most common" elements at smaller rowcounts either.
    Which brings us to the other major problem this reveals:
    
    2. In array_selfuncs.c, we fall back to default selectivity
    estimates if there's no MCELEM statistics field.  What we
    should be doing, if there are other stats for the column but
    not MCELEM, is realizing that compute_array_stats did not find
    any elements that are common enough to be worth recording.
    Then we'd use some much lower-than-default estimate for the
    selectivity.
    
    I don't have any immediate patch to offer, but clearly this
    area could use another look.
    
    compute_array_stats seems to have borrowed the lossy-counting
    algorithm from ts_typanalyze, so we'd better take a look at
    that too.
    
    			regards, tom lane
    
    
    
    
  4. Re: Poor row estimates from planner, stat `most_common_elems` sometimes missing for a text[] column

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-06-05T21:52:53Z

    I wrote:
    > The part of that that seems to be going off the rails is
    > this selection of a cutoff frequency below which element values
    > will be dropped:
    
    >         cutoff_freq = 9 * element_no / bucket_width;
    
    > The first thing I find suspicious here is that the calculation is
    > based on element_no (the total number of array elements processed)
    > and not nonnull_cnt (the maximum possible frequency).  Is that
    > really right?
    
    I did some more digging and found that that calculation was introduced
    (in the older tsvector code) in bc0f08092, which traces to this
    discussion:
    
    https://www.postgresql.org/message-id/flat/4BF4357E.6000505%40krogh.cc
    
    So the use of element_no is correct, because what we need to consider
    here is the total number of values fed to the LC algorithm.
    
    Also, my thought that maybe we should reject entries with f < 2
    is bogus, because at the end of the algorithm f is not necessarily
    the true count of occurrences of the value: some early occurrences
    could have been forgotten via pruning.  The "behavioral cliff" is
    annoying but I'm not sure there is much to be done about it: having
    a single (still-remembered) occurrence gets less and less significant
    as the total input size increases, so sooner or later you are going
    to hit a point where such values should be thrown away.
    
    So at this point I'm thinking that there is nothing wrong with
    ANALYZE's algorithm, although I now see that there are some relevant
    comments in ts_typanalyze.c that probably ought to be transposed into
    array_typanalyze.c.
    
    The idea of treating lack of MCELEM differently from complete
    lack of stats still seems to have merit, though.
    
    			regards, tom lane
    
    
    
    
  5. Re: Poor row estimates from planner, stat `most_common_elems` sometimes missing for a text[] column

    Frédéric Yhuel <frederic.yhuel@dalibo.com> — 2025-06-06T08:07:38Z

    
    On 6/5/25 23:52, Tom Lane wrote:
    > The idea of treating lack of MCELEM differently from complete
    > lack of stats still seems to have merit, though.
    
    
    Couldn't we count / estimate the number of distinct two-by-two elements, 
    and use that instead of the default selectivity estimate?
    
    
    
    
  6. RE: Poor row estimates from planner, stat `most_common_elems` sometimes missing for a text[] column

    Mark Frost <frostmar@uk.ibm.com> — 2025-06-06T09:21:34Z

    > On 6/5/25 17:42, Mark Frost wrote:
    > > Is there any good explanation for this behaviour? Preferably we’d like
    > > some way for proper `most_common_elems` statistics to be collected in
    > > our production database, in the hope that influences a good query plan
    > > to always be selected.
    
    
    > most_common_elems has a limited size, and if all the elements have the
    > same freq, there's nothing we can do.
    
    >  You could do: alter table test alter column tags set statistics X;
    >  However, X is capped at 10000…
    
    Actually *any* most_common_elems stats would be fine, because the reasoning is:
    
      *   If the searched element is in most_common_elems we know it’s frequency
      *   If it’s not, it’s less frequent than the least most_common_elems
    
    So in our case when every row is unique, we’d only actually need stats to record a single most_common_elems (if only it would record one)
    
    Unless otherwise stated above:
    
    IBM United Kingdom Limited
    Registered in England and Wales with number 741598
    Registered office: Building C, IBM Hursley Office, Hursley Park Road, Winchester, Hampshire SO21 2JN
    
  7. Re: Poor row estimates from planner, stat `most_common_elems` sometimes missing for a text[] column

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-06-07T01:29:54Z

    Mark Frost <FROSTMAR@uk.ibm.com> writes:
    > Actually *any* most_common_elems stats would be fine, because the reasoning is:
    >   *   If the searched element is in most_common_elems we know it's frequency
    >   *   If it's not, it's less frequent than the least most_common_elems
    > So in our case when every row is unique, we'd only actually need stats to record a single most_common_elems (if only it would record one)
    
    Well, we don't have a most common element in this scenario --- the
    whole point is that the occurrence counts resulting from the lossy
    counting algorithm are too low to be trustworthy.  However, what we
    do have is the cutoff frequency, and it seems to me that we could use
    that as the estimate of the maximum frequency of the non-MCEs.
    
    Attached is an extremely crude prototype patch for that.  What it
    does is to create an MCELEM stats entry with zero MCEs, but containing
    min and max frequencies equal to the cutoff frequency (plus the nulls
    frequency, which we know accurately in any case).  Mark, this fixes
    your example case, but I wonder if it fixes your original problem ---
    are you in a position to test it?
    
    Assuming we like this direction, the main thing that makes this a hack
    not a polished patch is that I had to strongarm the code into storing
    a zero-length values array.  What update_attstats would normally do
    is leave the values column of the MCELEM stats slot NULL, which then
    causes get_attstatsslot to throw a that-shouldn't-be-null error.
    An alternative answer is to change get_attstatsslot to allow a null,
    but I'm not sure that that's any cleaner.  Either way it seems like
    there's a hazard of breaking some code that isn't expecting the case.
    
    An alternative that feels cleaner but a good bit more invasive
    is to get rid of the convention of storing the min/max/nulls
    frequencies as extra entries in the MCELEM numbers entry ---
    which surely is a hack too --- and put them into some new slot
    type.  I'm not sure if that's enough nicer to be worth the
    conversion pain.
    
    Thoughts?
    
    			regards, tom lane
    
    
  8. Re: Poor row estimates from planner, stat `most_common_elems` sometimes missing for a text[] column

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-07-18T21:54:05Z

    I wrote:
    > Well, we don't have a most common element in this scenario --- the
    > whole point is that the occurrence counts resulting from the lossy
    > counting algorithm are too low to be trustworthy.  However, what we
    > do have is the cutoff frequency, and it seems to me that we could use
    > that as the estimate of the maximum frequency of the non-MCEs.
    
    Here's a less crude patch for that.  The array_typanalyze.c changes
    are the same as before, but I reconsidered what to do about this
    stumbling block:
    
    > Assuming we like this direction, the main thing that makes this a hack
    > not a polished patch is that I had to strongarm the code into storing
    > a zero-length values array.  What update_attstats would normally do
    > is leave the values column of the MCELEM stats slot NULL, which then
    > causes get_attstatsslot to throw a that-shouldn't-be-null error.
    > An alternative answer is to change get_attstatsslot to allow a null,
    > but I'm not sure that that's any cleaner.  Either way it seems like
    > there's a hazard of breaking some code that isn't expecting the case.
    
    I concluded that letting get_attstatsslot accept nulls is too risky;
    there is probably code that assumes that get_attstatsslot would've
    rejected that.  Instead, this version changes update_attstats' rule
    for when to store an array rather than a null.  Now it will do so
    if the passed-in stavalues pointer isn't null, even if numvalues
    is zero.  I think that this doesn't change the outcome for any
    existing analyze routines because they wouldn't pass a data pointer
    if they have no values; and even if it does, storing an empty array
    not a null seems like it should be pretty harmless.
    
    > An alternative that feels cleaner but a good bit more invasive
    > is to get rid of the convention of storing the min/max/nulls
    > frequencies as extra entries in the MCELEM numbers entry ---
    > which surely is a hack too --- and put them into some new slot
    > type.  I'm not sure if that's enough nicer to be worth the
    > conversion pain.
    
    A year ago I would have seriously considered doing it that way.
    But now that we have code to dump-n-restore stats, that code would
    have to be adjusted to convert the old representation.  It's not
    worth it for this case.
    
    Hence, v1 attached, now with a commit message.
    
    			regards, tom lane
    
    
  9. Re: Poor row estimates from planner, stat `most_common_elems` sometimes missing for a text[] column

    Matt Long <matt@mattlong.org> — 2025-09-08T19:13:41Z

    Not to let perfect be the enemy of better, but we're facing a variant of
    this issue that would not be addressed by the proposed patch. If you're
    interested, the real world use case is described in
    https://github.com/medplum/medplum/issues/7310. Rows have an array
    consisting of one common element present in every row and one unique
    element. With a statistics target at the default 100, this pushes the
    threshold for vastly overestimated row counts for the unique elements down
    to 7,937 rows.
    
    In this case, the effects of the proposed patch are not applied since the
    most_common_elems array is not empty. I'm not a statistician, so maybe this
    wouldn't be valid, but it seems like using the highest frequency of an
    element that did not qualify for the mce list instead of the 0.5% default
    frequency could be an elegant, but more invasive solution.
    
    Simple repro of the problem:
    
    CREATE TABLE public.test(
        id integer,
        tags text[]
    );
    
    INSERT INTO public.test (id, tags)
            SELECT n, ARRAY['common', TO_CHAR(n,'fm00000000')] FROM ( SELECT
    generate_series(1,7_937) AS n );
    
    ANALYZE public.test;
    
    SELECT most_common_elems, most_common_elem_freqs FROM pg_stats WHERE
    schemaname = 'public' AND tablename = 'test' AND attname = 'tags';
    
    EXPLAIN (ANALYZE,BUFFERS,VERBOSE)
            SELECT * FROM test WHERE tags && ARRAY['common'];
    
    EXPLAIN (ANALYZE,BUFFERS,VERBOSE)
            SELECT * FROM test WHERE tags && ARRAY['00000001'];
    
    With 7,936 rows:
        most_common_elems has 1,000 entries and looks like
    {00000001,00000003,00000009,...,common}
        most_common_elem_freqs has 1,003 entries and looks like
    {0.00012600806,0.00012600806,0.00012600806,...,1,0.00012600806,1,0}
    
    With 7,937+ rows:
        most_common_elems is {common}
        most_common_elem_freqs is {1,1,1,0}
    
    On Mon, Sep 8, 2025 at 11:27 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    
    > I wrote:
    > > Well, we don't have a most common element in this scenario --- the
    > > whole point is that the occurrence counts resulting from the lossy
    > > counting algorithm are too low to be trustworthy.  However, what we
    > > do have is the cutoff frequency, and it seems to me that we could use
    > > that as the estimate of the maximum frequency of the non-MCEs.
    >
    > Here's a less crude patch for that.  The array_typanalyze.c changes
    > are the same as before, but I reconsidered what to do about this
    > stumbling block:
    >
    > > Assuming we like this direction, the main thing that makes this a hack
    > > not a polished patch is that I had to strongarm the code into storing
    > > a zero-length values array.  What update_attstats would normally do
    > > is leave the values column of the MCELEM stats slot NULL, which then
    > > causes get_attstatsslot to throw a that-shouldn't-be-null error.
    > > An alternative answer is to change get_attstatsslot to allow a null,
    > > but I'm not sure that that's any cleaner.  Either way it seems like
    > > there's a hazard of breaking some code that isn't expecting the case.
    >
    > I concluded that letting get_attstatsslot accept nulls is too risky;
    > there is probably code that assumes that get_attstatsslot would've
    > rejected that.  Instead, this version changes update_attstats' rule
    > for when to store an array rather than a null.  Now it will do so
    > if the passed-in stavalues pointer isn't null, even if numvalues
    > is zero.  I think that this doesn't change the outcome for any
    > existing analyze routines because they wouldn't pass a data pointer
    > if they have no values; and even if it does, storing an empty array
    > not a null seems like it should be pretty harmless.
    >
    > > An alternative that feels cleaner but a good bit more invasive
    > > is to get rid of the convention of storing the min/max/nulls
    > > frequencies as extra entries in the MCELEM numbers entry ---
    > > which surely is a hack too --- and put them into some new slot
    > > type.  I'm not sure if that's enough nicer to be worth the
    > > conversion pain.
    >
    > A year ago I would have seriously considered doing it that way.
    > But now that we have code to dump-n-restore stats, that code would
    > have to be adjusted to convert the old representation.  It's not
    > worth it for this case.
    >
    > Hence, v1 attached, now with a commit message.
    >
    >                         regards, tom lane
    >
    >
    
  10. Re: Poor row estimates from planner, stat `most_common_elems` sometimes missing for a text[] column

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-08T23:37:01Z

    Matt Long <matt@mattlong.org> writes:
    > Not to let perfect be the enemy of better, but we're facing a variant of
    > this issue that would not be addressed by the proposed patch.
    > ...
    > In this case, the effects of the proposed patch are not applied since the
    > most_common_elems array is not empty. I'm not a statistician, so maybe this
    > wouldn't be valid, but it seems like using the highest frequency of an
    > element that did not qualify for the mce list instead of the 0.5% default
    > frequency could be an elegant, but more invasive solution.
    
    Yeah, I think you are quite right: we can apply this idea not only
    when the MCE list is empty, but whenever we didn't have to truncate
    the MCE list.  In that case we know there are no additional element
    values that exceed the cutoff frequency, and that's what the
    selectivity functions want to know.
    
    Nosing around in the code that uses STATISTIC_KIND_MCELEM entries,
    I spotted two additional issues that the attached v2 patch addresses:
    
    * ts_typanalyze/ts_selfuncs have code essentially identical to the
    array case, and should receive the same treatment.
    
    * The selectivity functions believe that the upper bound on the
    frequency of non-MCEs is minfreq / 2, not the stored minfreq.
    This seems like complete brain fade: there could easily be
    elements with frequency just less than minfreq, and probably are
    if the data distribution follows Zipf's law.  I did not dig into
    the git history, but I wonder if the divide-by-two business
    predates the introduction of the lossy-counting algorithm, and
    if so whether it was less insane with the original collection
    algorithm.  In any case, this patch removes the divisions by 2,
    and makes some nearby cosmetic improvements.
    
    Many thanks for the suggestion!
    
    			regards, tom lane
    
    
  11. Re: Poor row estimates from planner, stat `most_common_elems` sometimes missing for a text[] column

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-09T19:19:46Z

    I wrote:
    > * The selectivity functions believe that the upper bound on the
    > frequency of non-MCEs is minfreq / 2, not the stored minfreq.
    > This seems like complete brain fade: there could easily be
    > elements with frequency just less than minfreq, and probably are
    > if the data distribution follows Zipf's law.  I did not dig into
    > the git history, but I wonder if the divide-by-two business
    > predates the introduction of the lossy-counting algorithm, and
    > if so whether it was less insane with the original collection
    > algorithm.  In any case, this patch removes the divisions by 2,
    > and makes some nearby cosmetic improvements.
    
    In the light of morning, I started to have second thoughts about
    this aspect of the patch.  I checked the git history this time,
    and found that the oldest instance of "minfreq / 2" dates to
    4e57668da which originated from this discussion:
    
    https://www.postgresql.org/message-id/flat/488DAEB8.3000402%40students.mimuw.edu.pl
    
    It's already coded like that in Jan's initial patch, and there
    was no discussion in the thread, so not a lot to be gleaned:
    
    +            /*
    +             * The element is not in MCELEM. Punt, but  assure that the
    +             * selectivity cannot be more than minfreq / 2.
    +             */
    +            return (Selectivity) Min(DEFAULT_TS_SEL, minfreq / 2);
    
    But looking at this, I think the problem is really that the comment
    fails to describe his thought process.  We know that the frequency
    of this not-in-the-MCE-list value cannot be more than minfreq, but
    we have no idea how much less it is.  I think the idea of the
    division might have been to assume that an "average" non-MCE value
    would have a frequency about half that of the lowest MCE.  It does
    seem reasonable to use some number lower than the cutoff, although
    I dunno if 0.5 is exactly the right factor.
    
    So now I'm wondering if we should leave the divisions alone and
    instead fix the comments to explain why they are there.  Bolstering
    this is that on the two test cases you guys submitted, the patch
    with the divisions gets a spot-on estimate (1 row) while removing
    the divisions yields an estimate of 2 rows.  I don't put a lot of
    weight on that, since these are toy examples.  But I wonder if you
    guys could test the patch on some of your real-world cases and
    see if it looks like we should keep the divisions.
    
    			regards, tom lane
    
    
    
    
  12. Re: Poor row estimates from planner, stat `most_common_elems` sometimes missing for a text[] column

    Matt Long <matt@mattlong.org> — 2025-09-19T18:26:59Z

    I finally got around to testing your patch on a realistic data set. In
    short, the patch worked beautifully even with the division by 2 removed. In
    case it's helpful, the full write up of my investigation can be found at
    https://gist.github.com/mattlong/0617bec6e1cf5bc6b70c6c2951901df7
    
    Your reasoning about the division by 2 being a way to approximate the
    "average" non-MCE value seems logical. The patch as tested above without a
    division yielded a row estimate of 28 for a non-MCE element on a table with
    ~2.1 million rows where all non-MCE elements occur exactly once. For our
    scenario/distribution of the data, the most conservative option, i.e. no
    division, was good enough to produce optimal query plans, but this is of
    course a very specific data distribution.
    
    Thinking more generally, there are two cases: When the MCE list is full and
    when the MCE list is NOT full.
    
    When the MCE list is NOT full, the case that this patch addresses, the data
    is definitionally heavily skewed; minfreq will already be quite low.
    Whether or not it gets divided by 2 (or some other number) probably won't
    make much of a difference either way.
    
    When the MCE list is full, the data is less skewed; there will generally be
    a longer tail of element frequencies. Dividing by 2 will be more impactful
    here in an attempt to approximate the "average" non-MCE element frequency.
    
    Putting that together, it seems like keeping the division by 2 would be
    preferable?
    
    On Tue, Sep 9, 2025 at 12:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >
    > I wrote:
    > > * The selectivity functions believe that the upper bound on the
    > > frequency of non-MCEs is minfreq / 2, not the stored minfreq.
    > > This seems like complete brain fade: there could easily be
    > > elements with frequency just less than minfreq, and probably are
    > > if the data distribution follows Zipf's law.  I did not dig into
    > > the git history, but I wonder if the divide-by-two business
    > > predates the introduction of the lossy-counting algorithm, and
    > > if so whether it was less insane with the original collection
    > > algorithm.  In any case, this patch removes the divisions by 2,
    > > and makes some nearby cosmetic improvements.
    >
    > In the light of morning, I started to have second thoughts about
    > this aspect of the patch.  I checked the git history this time,
    > and found that the oldest instance of "minfreq / 2" dates to
    > 4e57668da which originated from this discussion:
    >
    >
    https://www.postgresql.org/message-id/flat/488DAEB8.3000402%40students.mimuw.edu.pl
    >
    > It's already coded like that in Jan's initial patch, and there
    > was no discussion in the thread, so not a lot to be gleaned:
    >
    > +            /*
    > +             * The element is not in MCELEM. Punt, but  assure that the
    > +             * selectivity cannot be more than minfreq / 2.
    > +             */
    > +            return (Selectivity) Min(DEFAULT_TS_SEL, minfreq / 2);
    >
    > But looking at this, I think the problem is really that the comment
    > fails to describe his thought process.  We know that the frequency
    > of this not-in-the-MCE-list value cannot be more than minfreq, but
    > we have no idea how much less it is.  I think the idea of the
    > division might have been to assume that an "average" non-MCE value
    > would have a frequency about half that of the lowest MCE.  It does
    > seem reasonable to use some number lower than the cutoff, although
    > I dunno if 0.5 is exactly the right factor.
    >
    > So now I'm wondering if we should leave the divisions alone and
    > instead fix the comments to explain why they are there.  Bolstering
    > this is that on the two test cases you guys submitted, the patch
    > with the divisions gets a spot-on estimate (1 row) while removing
    > the divisions yields an estimate of 2 rows.  I don't put a lot of
    > weight on that, since these are toy examples.  But I wonder if you
    > guys could test the patch on some of your real-world cases and
    > see if it looks like we should keep the divisions.
    >
    >                         regards, tom lane
    
  13. Re: Poor row estimates from planner, stat `most_common_elems` sometimes missing for a text[] column

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-19T19:23:22Z

    Matt Long <matt@mattlong.org> writes:
    > I finally got around to testing your patch on a realistic data set. In
    > short, the patch worked beautifully even with the division by 2 removed. In
    > case it's helpful, the full write up of my investigation can be found at
    > https://gist.github.com/mattlong/0617bec6e1cf5bc6b70c6c2951901df7
    
    Thank you!  I'm now inclined to keep the divisions by 2 (with comment
    fixes).
    
    			regards, tom lane