Re: Add expressions to pg_restore_extended_stats()

From: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Tender Wang <tndrwang(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tomas Vondra <tomas(at)vondra(dot)me>
Subject: Re: Add expressions to pg_restore_extended_stats()
Date: 2026-02-06 07:55:47
Message-ID: CADkLM=dq1kC+aFWgj-2USfpj5Xmgw4uuDJmjgtQ210PF4Frw+Q@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>
> I am not sure to get the point about some keys removed in the future.
> First, that sounds very unlikely. Even if it were the case, isn't
> pg_dump going to filter them out anyway? Forcing a stricter check in
> the restore function to force a set of keys to exist still sounds like
> a better option, as we have only backward-compatibility requirements
> in pg_dump, not in the backend restore function at a fixed version.
>

There's something that feels incongruous about holding these expressions
pg_statistic rows to a higher level of correctness than we do for regular
attribute stats, but it's hard to articulate and I'm not sure it actually
matters.

> > We still could get a situation where all
> > the exprs are empty (i.e. [null,null,null]). There is no simple test to
> map
> > that to just a plain null, but even if there were the SQL is already
> > drifting into "clever" territory and I know that pg_dump likes to keep
> > things very simple.
>
> With a set of three expressions [null,null,null] is IMO a valid thing,
> we have no valid data.

Yeah, I'm content to let that one be as-is.

> Another case is when we have a non-NULL
> element with all the keys present, but all their values are NULL.

To be clear, pg_dump would filter that out to a non-object NULL element,
but nothing stops somebody from creating an object like that, and if they
did they'd get the the unmodified statatt_get_empty_stat_tuple.

> If
> my test of the patch is right, the restore function accepts this case,
> and decides to set the following fields:
> "avg_width": "0",
> "null_frac": "0",
> "n_distinct": "0"
>

statatt_get_empty_stat_tuple is just assigning the defaults.

> jbv_string_get_cstr() is IMO incorrect in deciding that, no? It
> sounds to me that some specific NULL checks would be in order?
>

Any explicit null parameters would be handled by the loop that fills out
the found[] and val[] arrays, and explicit jbvNull is treated the same as
if the parameter wasn't specified in the first place. So I think the null
checks you seek are already there, but the existence of defaults in the
pg_statistic tuple (which has no nullable columns, btw) is throwing you off.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2026-02-06 08:29:51 Re: [PING] fallocate() causes btrfs to never compress postgresql files
Previous Message zengman 2026-02-06 07:54:22 Re: Small fixes for incorrect error messages