| 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.
| 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 |