Thread

Commits

  1. Avoid collation lookup failure when considering a "char" column.

  2. Remove lc_collate_is_c().

  1. [PATCH] Avoid collation lookup for "char" statistics

    Feng Wu <wufengwufengwufeng@gmail.com> — 2026-06-27T02:08:59Z

    
        
  2. Re: [PATCH] Avoid collation lookup for "char" statistics

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-27T02:44:07Z

    Feng Wu <wufengwufengwufeng@gmail.com> writes:
    > The internal "char" type is not collatable, and its btree operators
    > order the byte value directly.  Return the one-byte string for CHAROID
    > without consulting collation state, avoiding a lookup of InvalidOid as
    > collation 0.
    
    Can you demonstrate that there is a problem worth worrying about here?
    We've had no field reports of failures in this code, and I don't see
    a problem with, e.g.
    
     explain select * from pg_type a join pg_type b using (typtype);
    
    			regards, tom lane
    
    
    
    
  3. Re: [PATCH] Avoid collation lookup for "char" statistics

    Feng Wu <wufengwufengwufeng@gmail.com> — 2026-06-28T13:59:49Z

    Hi Tom,
    
    Thanks for looking at it.  I should have included the exact reproducer in
    the first email.
    
    The failure needs histogram statistics on a "char" column, so simple
    catalog joins such as the pg_type example do not necessarily reach the
    problematic path.  For example, I just rechecked that your query plans as
    a hash join for me:
    
      explain select * from pg_type a join pg_type b using (typtype);
    
    To reach the failing path, this reproduces it for me on current master:
    
      create temp table char_stats_1 (c "char");
      create temp table char_stats_2 (c "char");
    
      insert into char_stats_1
      select v::"char"
      from unnest(array['I','S','c','i','m','p','r','t','v']) as v,
           generate_series(1,
                           case when v in ('i','v','r','t') then 50 else 1 end);
    
      insert into char_stats_2
      select v::"char"
      from unnest(array['a','e','i']) as v,
           generate_series(1, case when v = 'i' then 50 else 5 end);
    
      analyze char_stats_1;
      analyze char_stats_2;
    
      set enable_hashjoin = off;
      set enable_nestloop = off;
    
      explain (costs off)
      select count(*)
      from char_stats_1 s1
      join char_stats_2 s2 on s1.c = s2.c;
    
    After ANALYZE, the first column has histogram statistics, e.g.:
    
      tablename    | attname | n_distinct | most_common_vals | histogram_bounds
      -------------+---------+------------+------------------+------------------
      char_stats_1 | c       |          9 | {i,r,t,v}        | {I,S,c,m,p}
      char_stats_2 | c       |          3 | {i,a,e}          |
    
    Without the patch, planning fails with:
    
      ERROR:  XX000: cache lookup failed for collation 0
      LOCATION:  pg_newlocale_from_collation, pg_locale.c:1211
    
    The reason is that mergejoinscansel() calls scalarineqsel() while costing
    the merge join.  That can reach convert_to_scalar() for histogram bounds.
    For CHAROID, convert_string_datum() builds a one-byte string, but then
    still calls pg_newlocale_from_collation() with the clause input collation,
    which is InvalidOid for the non-collatable "char" type.
    
    The patch only skips that collation lookup for CHAROID.  For text,
    varchar, bpchar, and name, the existing locale handling is unchanged.
    
    Regards,
    Feng
    
    
    
    
  4. Re: [PATCH] Avoid collation lookup for "char" statistics

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-28T15:39:00Z

    Feng Wu <wufengwufengwufeng@gmail.com> writes:
    > Thanks for looking at it.  I should have included the exact reproducer in
    > the first email.
    > The failure needs histogram statistics on a "char" column, so simple
    > catalog joins such as the pg_type example do not necessarily reach the
    > problematic path.
    
    Ah, now I understand.  The failing code is only reached if we have
    a histogram on a "char" column.  For almost all the catalogs with
    "char" columns, there is no histogram because there are very few
    distinct values and so the MCV list accounts for all of them.
    Even where there is a histogram, it'd be unlikely for someone to
    issue a query that would hit this code, so the lack of prior
    reports isn't as surprising as I thought.
    
    Your example could be simplified though.  The failure is within
    scalarineqsel, so we don't need a join at all, just an inequality
    qual:
    
    regression=# explain verbose select * from char_stats_1 where c < 'c';
    ERROR:  cache lookup failed for collation 0
    
    I agree that just skipping the collation correction is the right
    fix.  We could make the patch a bit shorter by simply returning
    out of the CHAROID case in the preceding switch.  Will see to it.
    
    			regards, tom lane
    
    
    
    
  5. Re: [PATCH] Avoid collation lookup for "char" statistics

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-28T16:41:21Z

    I wrote:
    > Even where there is a histogram, it'd be unlikely for someone to
    > issue a query that would hit this code, so the lack of prior
    > reports isn't as surprising as I thought.
    
    I had supposed that this was an old bug, but on attempting to
    back-patch I found it wasn't broken before v18.  So that's another
    big reason for lack of prior reports.  Commit 06421b084 replaced a
    call to lc_collate_is_c(), which tolerated InvalidOid, with
    pg_newlocale_from_collation() which doesn't.
    
    Seeing this, I thought we'd be best off to put the test where
    you had it but make it test for !OidIsValid(collid) rather than
    hard-wiring typid == CHAROID.  There probably aren't other cases
    where we reach here with collid 0, but if there are, we want the
    code to not fail, as it did not before.
    
    Pushed with a test based on an inequality comparison (in HEAD
    only, because there didn't seem to be a suitable test file in 18).
    
    			regards, tom lane
    
    
    
    
  6. Re: [PATCH] Avoid collation lookup for "char" statistics

    Feng Wu <wufengwufengwufeng@gmail.com> — 2026-06-28T16:53:04Z

    Thanks Tom, the generalized InvalidOid check makes sense to me.
    
    Regards,
    Feng