Feedback-ID: i0fe9450f:Fastmail
Date: Mon, 18 May 2026 13:19:02 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Chao Li <li.evan.chao@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>,
	Michael Paquier <michael.paquier@gmail.com>,
	Corey Huinker <corey.huinker@gmail.com>
Subject: Re: Fix small issues of pg_restore_extended_stats()
Message-ID: <agqTNkgZqim5J9gV@paquier.xyz>
References: <A7C11B83-7534-4A09-9071-FBD09175CFC8@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="d5c8Yikloi3XRsMn"
Content-Disposition: inline
In-Reply-To: <A7C11B83-7534-4A09-9071-FBD09175CFC8@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/agqTNkgZqim5J9gV%40paquier.xyz>
Precedence: bulk


--d5c8Yikloi3XRsMn
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, May 18, 2026 at 10:25:16AM +0800, Chao Li wrote:
> 1. Inconsistent expression key verification behavior
>=20
> This function is actually doing a prefix comparison using the input
> key length, so as long as an input key matches a prefix of a valid
> key name, the function returns true.
>=20
> At runtime, it does not lead to an incorrect value being imported,
> because the invalid key will still be filtered out later. But one
> bad scenario I can imagine is that a user is trying to set
> =E2=80=9Ccorrelation=E2=80=9D, and accidentally omits the last =E2=80=9Cn=
=E2=80=9D. In that case,
> the key is silently discarded and the user is not aware of it, which
> can lead to a surprising result.

It's a silent data loss, with data getting ignored.  The call should
report a failure because the key does not match with a name we'd
expect.  pg_dump would not generate that, but stats injection is a
supported use-case so that would be confusing.

Nice investigation. =20

> 2. Wrong number in a warning message
>=20
> The warning message says =E2=80=9C3 required=E2=80=9D, but it should be =
=E2=80=9C1
> required=E2=80=9D.

Harmless, still wrong.

> 3. Inconsistent heap_freetuple()
>=20
> In pg_clear_extended_stats(), heap_freetuple(tup) is only called
> before the final return. However, in two earlier paths, the function
> only emits warning messages and returns. It seems those paths should
> also call heap_freetuple(tup).

In the !IsValid() case, there is nothing to free, but you are right
about the second case where stxrelid does not match.  It's minor, but
I don't have an argument against doing that either, and the free at
the bottom is a trace that we want to be consistent.

I'll go fix all that.  Thanks for the report!
--
Michael

--d5c8Yikloi3XRsMn
Content-Type: application/pgp-signature; name=signature.asc

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmoKkzYACgkQnvQgOdby
QH2DCw//UPz9sQ6Pq8xvFaAE3eejarGIRXYFtCCw0HcLoUANWaB+Hc5LsyvB3fLV
optbn7dTLHCC9BvsI5Zcs/l9QUf5BPX4Q9yFHQmkp85jB2B7tXICnxEmTHSzH7Zp
TKU98HE6VcrCD/UaNiTpErbzFnTQQFLtCbGsRI2UkM+/P0Xpt2E0y15+zHtt9a/J
h/umSf4k/CTwFXju66T7Jgf3zXFj3MkBCiSl0IDAIWieLLHUdU5XY4q7iXd+E1Eo
z1XWZAENCWQOZQobgzw22mCXmJJ3zvMXRLFRAm1nOionMSMx+xSU3s2d9OsZfTNl
ElBXQmMajofYEelcmF223zhIRMjDbEKpntSrx2E1x9hakutFqkC6sM4P2MKdjqu1
LtNn0FZeO5/2MSckLrUEm0AQZ8FXYr/zAFSUMzhHs8AzFgWUFyGQ8WpG6UImrF4N
3uEGdoty1UoNpxCxe0pTW/Ac4N2MHSpqnUAL31EGSFUj4bUbyv4CUHi5qJ3xM0jN
kh0LVx4DamI9+CNt84Py2KnsnYPonfhKwb8Si/EG8ovOr49HjiyLIH/kkKmWb7Pf
4UJ3Jez6nbUI0nOOfs08QiVlLbN8VHa+DIbUYIJF4yMldrZBNbZJeMbozi47sjJF
0Y8brAl9YWSqWXf+M+5qiC0f32Ki5toTLodByAvy2Ki9NesDh/Y=
=zR/l
-----END PGP SIGNATURE-----

--d5c8Yikloi3XRsMn--