Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: Defects with invalid stats data for expressions in extended stats
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <aaEctAQb8yDhqwaE@paquier.xyz>
Date: Fri, 27 Feb 2026 14:51:40 +0800
Cc: Corey Huinker <corey.huinker@gmail.com>,
 Postgres hackers <pgsql-hackers@lists.postgresql.org>,
 Tomas Vondra <tomas@vondra.me>
Content-Transfer-Encoding: quoted-printable
Message-Id: <92BA3F52-47FB-49F2-A156-551D7BFC326E@gmail.com>
References: <aaDrJsE1I5mrE-QF@paquier.xyz>
 <CADkLM=c_3mPHr0JGP_72csQkpAt-v2fRN830RO8or1_R1qX2Wg@mail.gmail.com>
 <aaEctAQb8yDhqwaE@paquier.xyz>
To: Michael Paquier <michael@paquier.xyz>
Archived-At: <https://www.postgresql.org/message-id/92BA3F52-47FB-49F2-A156-551D7BFC326E%40gmail.com>
Precedence: bulk


> On Feb 27, 2026, at 12:25, Michael Paquier <michael@paquier.xyz> =
wrote:
>=20
> 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:
>=20
> 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.
>=20
>> 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.
>=20
> 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] =3D examine_attribute(expr);
=20
+		/*
+		 * 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] =3D=3D 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.
```

=E2=80=9CThis function returns sets =E2=80=A6=E2=80=9D, is =E2=80=9Creturn=
s=E2=80=9D 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 =E2=80=9Cthis function does not use a function body=E2=80=
=9D, but the function has a body. This appears in two places.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/