| 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-05 04:42:16 |
| Message-ID: | CADkLM=ddBau8AFeQ3OHCWKZXFrB=Y-5dLkZnNdAOtbr1Qa2rGg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
>
> v5-0001 still requires a lot of changes at quick glance, as of:
> - Some diffs not required in some comment blocks.
>
Kept the s/stxdexprs/stxdexpr, as that is the correct column name, the
other changes are reverted.
> - Regression test diffs show up for cases where WARNING messages are
> changed, but should not. See for example the MCV checks.
>
reverted.
> - A couple of newly-introduced error strings use wordings that are not
> project-style, as of the "malformed expressions", there is also a
"malformed expr expression".
>
Updated all error messages. I tried to add in expr/name location via the
context stack, but those callback are only called for ERRORs and higher.
So, I did what I could with errhint()s.
> + * #include "catalog/pg_collation_d.h" if (typid == TSVECTOROID)
> stacoll =
> + * DEFAULT_COLLATION_OID;
>
We did need to do this, and it has been implemented, along with a new test
on a new statistics object which has a to_tsvector(text_column) expression
in it.
> + *
> + * The multirange step-down may also need to happen here too.
> + */
>
Multirange types in expressions generate exactly 2
stakinds: STATISTIC_KIND_RANGE_LENGTH_HISTOGRAM (6)
and STATISTIC_KIND_BOUNDS_HISTOGRAM (7), neither of which are exposed by
pg_stats_ext_exprs, so they can't be imported, so at present there is
nothing to test and we can skip the multirange step-down for now.
Inspecting the stavalues1 and stavalues2 values in pg_statistic_ext_data
gives me the impression that we don't need the step-down, as the values
appear to be regular range types, and the exiting function to get the
element type should suffice.
We should probably modify the view to include these new-ish statistic
kinds, but that's for another patch.
> Now that we have an agreement about the format of the input data and
> how it maps with the assumptions of the stats computations in ANALYZE,
> the hardest part is IMO done. However, could you clean up things?
> This patch is not really in an acceptable shape: it has a bunch of
> avoidable errors and it requires a lot of tweaks before even being
> looked at more closely.
Moving to the found[] and vals[] arrays cleans up the logic a lot, I think,
which is all the more important now that we keep building the expressions
after components of that statistic fail.
| Attachment | Content-Type | Size |
|---|---|---|
| v6-0001-Add-support-for-exprs-in-pg_restore_extended_stat.patch | text/x-patch | 79.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alexander Lakhin | 2026-02-05 05:00:01 | Re: BUG: Former primary node might stuck when started as a standby |
| Previous Message | Shinya Kato | 2026-02-05 04:39:27 | Re: Report oldest xmin source when autovacuum cannot remove tuples |