Re: Defects with invalid stats data for expressions in extended stats
Chao Li <li.evan.chao@gmail.com>
From: Chao Li <li.evan.chao@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Corey Huinker <corey.huinker@gmail.com>,
Postgres hackers <pgsql-hackers@lists.postgresql.org>,
Tomas Vondra <tomas@vondra.me>
Date: 2026-02-27T06:51:40Z
Lists: pgsql-hackers
> On Feb 27, 2026, at 12:25, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Feb 26, 2026 at 10:52:48PM -0500, Corey Huinker wrote:
>> Patch applies for me, but there seems to be some user-specific stuff in the
>> test, which causes it to fail:
>
> Yep. I've noticed that in the CI a few minutes ago. I have switched
> the tests to use a query where the owner does not show up, leading to
> the same coverage without the user-dependency blip. I have checked
> that this version cools down the CI.
>
>> A nitpick about the test - it uses a plpgsql function when we've been
>> moving such trivial functions to SQL standard function bodies for a while
>> now, and they were introduced back in v14 so there's no backporting
>> concern.
>
> No, that's on purpose. Using a SQL function with a body would not
> trigger the problem with the stats loaded at the end of the SQL test
> as we would skip the fatal call of statext_expressions_load(). Based
> on your confusion, I guess that a note to document that is in order,
> at least, so as nobody comes with the idea of changing the definition
> of this function..
> --
> Michael
> <v2-0001-Fix-two-defects-with-extended-statistics-for-expr.patch><v2-0002-test_custom_types-Test-module-for-custom-data-typ.patch>
A few small comments from an eyeball review:
1 - 0001
```
stats[i] = examine_attribute(expr);
+ /*
+ * If the expression has been found as non-analyzable, give up. We
+ * will not be able to build extended stats with it.
+ */
+ if (stats[i] == NULL)
+ {
+ pfree(stats);
+ return NULL;
+ }
```
Here stats itself is destroyed, but memory pointed by stats[0]~stats[i-1] are not free-ed, those memory are returned from examine_attribute() by palloc0_object().
2 - 0002
```
/*
* int_custom_typanalyze_invalid
*
* This function returns sets some invalid stats data, letting the caller know
* that we are safe for an analyze, returning true.
```
“This function returns sets …”, is “returns” a typo and not needed?
3 - 0002
```
+-- Dummy function used for expression evaluations.
+-- Note that this function does not use a function body on purpose, so as
+-- external statistics can be loaded from it.
+CREATE OR REPLACE FUNCTION func_int_custom (p_value int_custom)
+ RETURNS int_custom LANGUAGE plpgsql AS $$
+ BEGIN
+ RETURN p_value;
+ END; $$;
```
The comment says “this function does not use a function body”, but the function has a body. This appears in two places.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Commits
-
test_custom_types: Test module with fancy custom data types
- 858c83b3e666 14.23 landed
- 8eedbc2cc4b3 15.18 landed
- c0676c8294f7 16.14 landed
- d4f33d026a6b 17.10 landed
- 017e4e395d0d 18.4 landed
- 2176520089e8 19 (unreleased) landed
-
Fix set of issues with extended statistics on expressions
- 038c7d4a3b24 14.23 landed
- f033abc6c4f2 15.18 landed
- 04745ba9c72e 16.14 landed
- 530b6b02f891 17.10 landed
- 83671c0da049 18.4 landed
- 3b7a6fa15720 19 (unreleased) landed