public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Chao Li <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Corey Huinker <[email protected]>
Subject: Re: Fix small issues of pg_restore_extended_stats()
Date: Mon, 18 May 2026 13:19:02 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

On Mon, May 18, 2026 at 10:25:16AM +0800, Chao Li wrote:
> 1. Inconsistent expression key verification behavior
> 
> 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.
> 
> 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
> “correlation”, and accidentally omits the last “n”. 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.  

> 2. Wrong number in a warning message
> 
> The warning message says “3 required”, but it should be “1
> required”.

Harmless, still wrong.

> 3. Inconsistent heap_freetuple()
> 
> 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


Attachments:

  [application/pgp-signature] signature.asc (833B, 2-signature.asc)
  download

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Fix small issues of pg_restore_extended_stats()
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox