BUG #19715: pg_restore_attribute_stats() rejects range statistics for a domain over int4multirange
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253899psql -h localhost -U postgresBuilt from patchset v10 (message #10), September 25, 2026 at 11:56 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253899_10 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253899_10 && git checkout t253899_10Patchset v10 (message #10) is on t253899_10
The following bug has been logged on the website:
Bug reference: 19715
Logged by: Qifan Liu
Email address: imchifan@163.com
PostgreSQL version: 18.6
Operating system: Linux/amd64
Description:
pg_restore_attribute_stats() rejects range length and bounds histograms for
a column whose type is a domain over int4multirange. It returns false and
warns that the column is not a range type. However, ANALYZE generates both
range-specific statistic kinds 6 and 7 for a column of the same domain type.
As a result, statistics exported for a domain over a multirange type cannot
be faithfully restored to an equivalent column.
Steps to reproduce
------------------
Run the following input with psql -X:
\set ON_ERROR_STOP on
CREATE DOMAIN restore_stats_mr AS int4multirange;
CREATE TABLE restore_stats_src (v restore_stats_mr);
CREATE TABLE restore_stats_dst (v restore_stats_mr);
INSERT INTO restore_stats_src VALUES
('{[1,3)}'), ('{[5,9)}'), ('{[11,15)}');
ANALYZE restore_stats_src;
SELECT array_agg(k ORDER BY k) AS analyze_range_kinds
FROM (
SELECT unnest(ARRAY[stakind1, stakind2, stakind3, stakind4, stakind5]) AS
k
FROM pg_statistic
WHERE starelid = 'restore_stats_src'::regclass
AND staattnum = 1
) s
WHERE k IN (6, 7);
SELECT pg_catalog.pg_restore_attribute_stats(
'schemaname', 'public',
'relname', 'restore_stats_dst',
'attname', 'v',
'inherited', false,
'range_length_histogram', '{2,4,4}'::text,
'range_empty_frac', 0::real,
'range_bounds_histogram', ARRAY['[1,3)', '[5,9)', '[11,15)']::text
) AS restore_ok;
SELECT count(*) = 2 AS restored_both_range_kinds
FROM (
SELECT unnest(ARRAY[stakind1, stakind2, stakind3, stakind4, stakind5]) AS
k
FROM pg_statistic
WHERE starelid = 'restore_stats_dst'::regclass
AND staattnum = 1
) s
WHERE k IN (6, 7);
Actual result
-------------
analyze_range_kinds
---------------------
{6,7}
WARNING: column "v" is not a range type
DETAIL: Cannot set STATISTIC_KIND_RANGE_LENGTH_HISTOGRAM or
STATISTIC_KIND_BOUNDS_HISTOGRAM.
restore_ok
------------
f
restored_both_range_kinds
---------------------------
f
Expected result
---------------
pg_restore_attribute_stats() should return true and restore statistic kinds
6 and 7. ANALYZE produces those range statistics for the same domain type,
so the restoration path should not reject them as belonging to a non-range
column.
Additional information
----------------------
The issue was reproduced on PostgreSQL 20devel and PostgreSQL 18.6.
PostgreSQL 17.11 does not provide pg_restore_attribute_stats().
I'm looking into this.
On Wed, Sep 23, 2026 at 03:02:30PM -0400, Corey Huinker wrote:
I'm looking into this.
I have begun looking at this before you had sent this reply, and we
are handling the base type of a domain in an incorrect way, assuming
that for attribute and extended stats we should just always check for
TYPTYPE_[MULTI]RANGE, but domains don't map with that at all. I think
that we are missing an extra getBaseType(), like
[multi]range_typanalyze(), where we use a [multi]range_get_typcache()
to cope with domains (getBaseTypeAndTypmod() does the job in the
typcache). That's also mentioned in the code.
And the same can be said for expressions in extended stats where a
domain that has a [multi]range type is involved. We would be better
getting rid of these hardcoded TYPTYPE values, IMO.
Spoiler: the tests are boring, still required. And fortunately, the
only damage is stats data not restored but skipped. Annoying, but not
as annoying as in the class of problems labelled like "I corrupt the
catalogs".
What do you think?
--
Michael
On Thu, Sep 24, 2026 at 12:22 AM Michael Paquier <michael@paquier.xyz>
wrote:
On Wed, Sep 23, 2026 at 03:02:30PM -0400, Corey Huinker wrote:
I'm looking into this.
I have begun looking at this before you had sent this reply, and we
are handling the base type of a domain in an incorrect way, assuming
that for attribute and extended stats we should just always check for
TYPTYPE_[MULTI]RANGE, but domains don't map with that at all. I think
that we are missing an extra getBaseType(), like
[multi]range_typanalyze(), where we use a [multi]range_get_typcache()
to cope with domains (getBaseTypeAndTypmod() does the job in the
typcache). That's also mentioned in the code.And the same can be said for expressions in extended stats where a
domain that has a [multi]range type is involved. We would be better
getting rid of these hardcoded TYPTYPE values, IMO.Spoiler: the tests are boring, still required. And fortunately, the
only damage is stats data not restored but skipped. Annoying, but not
as annoying as in the class of problems labelled like "I corrupt the
catalogs".What do you think?
--
Michael
Here's what I was just about to post to the list, only to see that you
already posted something.
Will begin reviewing yours immediately.
On Thu, Sep 24, 2026 at 1:56 AM Corey Huinker <corey.huinker@gmail.com>
wrote:
On Thu, Sep 24, 2026 at 12:22 AM Michael Paquier <michael@paquier.xyz>
wrote:On Wed, Sep 23, 2026 at 03:02:30PM -0400, Corey Huinker wrote:
I'm looking into this.
I have begun looking at this before you had sent this reply, and we
are handling the base type of a domain in an incorrect way, assuming
that for attribute and extended stats we should just always check for
TYPTYPE_[MULTI]RANGE, but domains don't map with that at all. I think
that we are missing an extra getBaseType(), like
[multi]range_typanalyze(), where we use a [multi]range_get_typcache()
to cope with domains (getBaseTypeAndTypmod() does the job in the
typcache). That's also mentioned in the code.And the same can be said for expressions in extended stats where a
domain that has a [multi]range type is involved. We would be better
getting rid of these hardcoded TYPTYPE values, IMO.Spoiler: the tests are boring, still required. And fortunately, the
only damage is stats data not restored but skipped. Annoying, but not
as annoying as in the class of problems labelled like "I corrupt the
catalogs".What do you think?
--
MichaelHere's what I was just about to post to the list, only to see that you
already posted something.Will begin reviewing yours immediately.
So it seems we came to the very similar conclusions.
I had a plan to add something like statatt_get_range_type() as a follow-up
patch, but wanted to get the fix working first.
As for test cases, mine are based on the existing "are foo_clone stats a
faithful copy of foo stats" tests, rather than hardcoding stats. My way
feels more future proof, but the future in which the hardcoded way breaks
is a long way off.
Similarly, after adding those foo->foo_clone tests, it seemed like the
regression test could stand to have helper functions for restore stats from
object a into object a_clone, but that too was for a follow-up patch as I
didn't want to distract from the fix itself.
The additional test cases for trying to cram range stats into non-range
attributes appear correct and necessary.
Overall, it's a +1 from me.
On Thu, Sep 24, 2026 at 01:56:29AM -0400, Corey Huinker wrote:
Here's what I was just about to post to the list, only to see that you
already posted something.
As far as I can see, we are doing exactly the same thing, except that
my patch results in less lines of C and does not duplicate the same
pattern across attribute and extended stats.
Will begin reviewing yours immediately.
Thanks.
--
Michael
As for test cases, mine are based on the existing "are foo_clone stats a
faithful copy of foo stats" tests, rather than hardcoding stats. My way
feels more future proof, but the future in which the hardcoded way breaks
is a long way off.
Nevermind, I see that you did the clone tests as well.
On Thu, Sep 24, 2026 at 1:56 PM Corey Huinker <corey.huinker@gmail.com> wrote:
Here's what I was just about to post to the list, only to see that you already posted something.
Hi.
I only took a brief look.
maybe related:
https://git.postgresql.org/cgit/postgresql.git/commit/?id=4edd6036d69ce42ac1af236f659f20daed65c8d4
src/backend/statistics/attribute_stats.c
I wonder in statatt_get_type, can we do:
typcache = lookup_type_cache(*atttypid, TYPECACHE_LT_OPR |
TYPECACHE_EQ_OPR | TYPECACHE_DOMAIN_BASE_INFO);
if (OidIsValid(typcache->domainBaseType))
*atttyptype = get_typtype(typcache->domainBaseType);
else
*atttyptype = typcache->typtype;
Similar in import_pg_statistic.
change to
typcache = lookup_type_cache(typid, TYPECACHE_LT_OPR |
TYPECACHE_EQ_OPR | TYPECACHE_DOMAIN_BASE_INFO);
Disclaimer: Since I saw both of you actively working on this issue, I
haven't tried this myself.
On Thu, Sep 24, 2026 at 02:44:19PM +0800, jian he wrote:
Similar in import_pg_statistic.
change to
typcache = lookup_type_cache(typid, TYPECACHE_LT_OPR |
TYPECACHE_EQ_OPR | TYPECACHE_DOMAIN_BASE_INFO);Disclaimer: Since I saw both of you actively working on this issue, I
haven't tried this myself.
Yes, I was wondering about that a bit, feeding a single typcache entry
across the board. And while looking at the code, I was reminded about
the following exceptions:
extended_stats_funcs.c: if (typid == TSVECTOROID)
stat_utils.c: if (*atttypid == TSVECTOROID)
stat_utils.c: if (atttypid == TSVECTOROID)
I think that the existing code is also broken when defining a domain
over tsvector if we don't feed a domain base type in these three
spots.
--
Michael
On Fri, Sep 25, 2026 at 07:52:07AM +0900, Michael Paquier wrote:
Yes, I was wondering about that a bit, feeding a single typcache entry
across the board. And while looking at the code, I was reminded about
the following exceptions:
extended_stats_funcs.c: if (typid == TSVECTOROID)
stat_utils.c: if (*atttypid == TSVECTOROID)
stat_utils.c: if (atttypid == TSVECTOROID)I think that the existing code is also broken when defining a domain
over tsvector if we don't feed a domain base type in these three
spots.
With all that in mind, I am getting down to the point that we should
accept that the early stages of attribute and extended stats restore
should try to fetch the typcache data of the base types, then roll it
around. This leads to the attached, taking care of the range,
multirange and tsvector cases.
I'd certainly welcome more eyes here. Please note that this applies
on HEAD cleanly, and should mostly apply cleanly on v19. The v18
flavor would be much more localized, of course.
--
Michael
Attachments:
t253899_10v2-0001-Fix-import-of-statistics-for-domains-over-multi-r.patchtext/plain; charset=us-asciiDownload+658-47
Hi,
Michael Paquier <michael@paquier.xyz> wrote:
I'd certainly welcome more eyes here. Please note that this applies
on HEAD cleanly, and should mostly apply cleanly on v19.
It applies with git am on both master (2c10c2ce4d7) and REL_19_STABLE
(2c5cd772b90), and make check passes on both. With only the test
changes of v2 and not the C ones, stats_import fails on master with
the expected warnings, so the new tests catch the bug.
I also checked the path users hit, pg_dump --statistics-only into a
database with the same schema, and compared every field of pg_stats
and pg_stats_ext_exprs before and after. The table has 17 columns and
the statistics object 8 expressions, the cases of the report plus a
few beyond the ones in the tests:
- a domain over a domain, for range, multirange and tsvector
- a domain over int4range with a CHECK constraint
- a domain over a range type with its own collation (text, "C")
- expressions of those types, next to one over a domain over int4[]
Results:
- master: 15 warnings. Range stats are lost for 6 domain columns,
including the nested, CHECK and collated ones, and MCELEM for the 2
tsvector domains. All 8 expressions lose all their stats.
- master + v2: no warnings, all 25 identical after the restore.
- REL_19_STABLE + v2: the same.
- REL_18_STABLE (66d1de70c84): the same 8 columns fail as on master.
I can run the same script on the v18 flavor when you post it.
A domain over int4[] and an array of a domain over int4 already come
through intact without the patch, in case that question comes up.
One thing outside this bug, about why all 8 expressions were lost.
import_expressions() stores a failed expression as NULL and keeps the
others, per its comments, but extended_statistics_update() then drops
the whole stxdexpr array when exprs_is_perfect is false. With one
statistics object on the d_arr expression plus a domain-over-range
one, master restores nothing for either; with the d_arr expression
next to a plain one, it is restored. Is dropping all of them
intended? I may be missing the reason. v2 removes the cause here,
so this only matters for other rejections.
The scripts and outputs are in the attachment.
Regards,
Manu
Attachments:
nocfbot-19715-v2-roundtrip.txttext/plainDownload+241-40
One thing outside this bug, about why all 8 expressions were lost.
import_expressions() stores a failed expression as NULL and keeps the
others, per its comments, but extended_statistics_update() then drops
the whole stxdexpr array when exprs_is_perfect is false. With one
statistics object on the d_arr expression plus a domain-over-range
one, master restores nothing for either; with the d_arr expression
next to a plain one, it is restored. Is dropping all of them
intended? I may be missing the reason. v2 removes the cause here,
so this only matters for other rejections.
It's an unfortunate consequence of how expression stats are stored.
The pg_statistic_ext_data.stxdexpr array must be the same length as the
number of negative elements in pg_statistic_ext.stxkeys... so a stxkeys of
[2,4,-1,-3,-4] correlates stats from the attnum=2, attnum=4, and the first,
third, and fourth expressions defined for this very complicated and
hypothetical extended statistic.
In addition to being difficult to unpack (you can't just skip to the Nth
element, you have to count the expression and then stop on the Nth one)
[1]: It should be noted that other work on Join Statistics in v20 is proposing to change this format, which will make regular stats import marginally simpler.
could not store a NULL value for an element in the stxdexprs array or we
could not then reliably re-extract that data [2]a good starting point in thread [4]: /messages/by-id/CADkLM=cPB_V+oV5d+5bfZGA_1Loa7iMmqXWj+SM-YGzxoUYrcQ@mail.gmail.com, or both, and in either
case the stats were incomplete, thus necessitating that they be rebuilt
post-upgrade anyway, reducing the benefit of making them
slightly-less-incomplete [3]the whole thread: /messages/by-id/CADkLM=cPB_V+oV5d+5bfZGA_1Loa7iMmqXWj+SM-YGzxoUYrcQ@mail.gmail.com.
[1]: It should be noted that other work on Join Statistics in v20 is proposing to change this format, which will make regular stats import marginally simpler.
proposing to change this format, which will make regular stats import
marginally simpler.
[2]: a good starting point in thread [4]: /messages/by-id/CADkLM=cPB_V+oV5d+5bfZGA_1Loa7iMmqXWj+SM-YGzxoUYrcQ@mail.gmail.com
/messages/by-id/CADkLM=cPB_V+oV5d+5bfZGA_1Loa7iMmqXWj+SM-YGzxoUYrcQ@mail.gmail.com
[3]: the whole thread: /messages/by-id/CADkLM=cPB_V+oV5d+5bfZGA_1Loa7iMmqXWj+SM-YGzxoUYrcQ@mail.gmail.com
/messages/by-id/CADkLM=cPB_V+oV5d+5bfZGA_1Loa7iMmqXWj+SM-YGzxoUYrcQ@mail.gmail.com
With all that in mind, I am getting down to the point that we should
accept that the early stages of attribute and extended stats restore
should try to fetch the typcache data of the base types, then roll it
around. This leads to the attached, taking care of the range,
multirange and tsvector cases.
This is a good move, as it further opens the door to making stats import
work with custom stakinds, but a lot of things need to happen [1]pg_stats needs a way to expose non-builtin stakinds[2]pg_type needs to add an oid for typimport(), or the custom basel/elem oid and collation stuff would have to be calculated in the existing custom FOO_typanalyze()[3]pg_dump needs to be made aware of this
before we can even attempt that.
I'd certainly welcome more eyes here. Please note that this applies
on HEAD cleanly, and should mostly apply cleanly on v19. The v18
flavor would be much more localized, of course.
It does apply cleanly to 19, and yes, the v18 is a bigger change, and will
look much more like my patch unless we decide to backport
statatt_get_type()...which probably isn't worth it.
[1]: pg_stats needs a way to expose non-builtin stakinds
[2]: pg_type needs to add an oid for typimport(), or the custom basel/elem oid and collation stuff would have to be calculated in the existing custom FOO_typanalyze()
oid and collation stuff would have to be calculated in the existing custom
FOO_typanalyze()
[3]: pg_dump needs to be made aware of this
On Fri, Sep 25, 2026 at 03:45:10PM -0400, Corey Huinker wrote:
This is a good move, as it further opens the door to making stats import
work with custom stakinds, but a lot of things need to happen [1][2][3]
before we can even attempt that.
I don't know much about these parts, only about the problem at hand,
which is enough for me for the moment.. :)
It does apply cleanly to 19, and yes, the v18 is a bigger change, and will
look much more like my patch unless we decide to backport
statatt_get_type()...which probably isn't worth it.
The backpatch of v18 is not that bad, isolated within
attribute_stats.c, so the ABI argument does not apply.
--
Michael