Horton

Re: Statistics Import and Export

Corey Huinker <corey.huinker@gmail.com>

View thread
From: Corey Huinker <corey.huinker@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>, Jeff Davis <pgsql@j-davis.com>, Michael Paquier <michael@paquier.xyz>, jian he <jian.universality@gmail.com>, Nathan Bossart <nathandbossart@gmail.com>, Bruce Momjian <bruce@momjian.us>, Matthias van de Meent <boekewurm+postgres@gmail.com>, Magnus Hagander <magnus@hagander.net>, Stephen Frost <sfrost@snowman.net>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, Peter Smith <smithpb2250@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, alvherre@alvh.no-ip.org
Date: 2025-02-24T18:47:19Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Change pg_dump default for statistics export.

  2. pg_dump: Adjust reltuples from 0 to -1 for dumps of older versions.

  3. vacuumdb: Don't skip empty relations in --missing-stats-only mode.

  4. pg_dump: Fix query for gathering attribute stats on older versions.

  5. Prevent redeclaration of typedef TocEntry.

  6. Remove unused function parameters in pg_backup_archiver.c.

  7. pg_dump: Retrieve attribute statistics in batches.

  8. pg_dump: Reduce memory usage of dumps with statistics.

  9. Skip second WriteToc() call for custom-format dumps without data.

  10. Add relallfrozen to pg_dump statistics.

  11. Matview statistics depend on matview data.

  12. Add pg_dump --with-{schema|data|statistics} options.

  13. Stats: use schemaname/relname instead of regclass.

  14. CREATE INDEX: do update index stats if autovacuum=off.

  15. Don't convert to and from floats in pg_dump.

  16. CREATE INDEX: don't update table stats if autovacuum=off.

  17. Organize and deduplicate statistics import tests.

  18. Address stats export review comments.

  19. Address stats import review comments.

  20. Add relallfrozen to pg_class

  21. Fix pg_strtof() to not crash on NULL endptr.

  22. Use attnum to identify index columns in pg_restore_attribute_stats().

  23. pg_dump: prepare attribute stats query.

  24. Avoid unnecessary relation stats query in pg_dump.

  25. Remove redundant pg_set_*_stats() variants.

  26. Do not use in-place updates for statistics import.

  27. Fix confusion about data type of pg_class.relpages and relallvisible.

  28. Documentation fixups for dumping statistics.

  29. Trial fix for old cross-version upgrades.

  30. Transfer statistics during pg_upgrade.

  31. Lock table in ShareUpdateExclusive when importing index stats.

  32. Use in-place updates for pg_restore_relation_stats().

  33. Improve error message for replication of generated columns.

  34. pg_dump: Add dumpSchema and dumpData derivative flags.

  35. Disallow modifying statistics on system columns.

  36. Add missing CommandCounterIncrement() in stats import functions.

  37. Add functions pg_restore_relation_stats(), pg_restore_attribute_stats().

  38. Documentation fixup.

  39. Add functions pg_set_attribute_stats() and pg_clear_attribute_stats().

  40. Change pg_*_relation_stats() functions to return type to void.

  41. Disable autovacuum for tables in stats import tests.

  42. Allow pg_set_relation_stats() to set relpages to -1.

  43. Fixup for pg_set_relation_stats().

  44. Create functions pg_set_relation_stats, pg_clear_relation_stats.

  45. Add memory/disk usage for Window aggregate nodes in EXPLAIN.

  46. Improve performance of dumpSequenceData().

  47. Add INJECTION_POINT_CACHED() to run injection points directly from cache

  48. Improve performance of binary_upgrade_set_pg_class_oids().

  49. Improve assertion in mdwritev()

  50. CREATE INDEX: do not update stats during binary upgrade.

  51. Redefine pg_class.reltuples to be -1 before the first VACUUM or ANALYZE.

On Mon, Feb 24, 2025 at 12:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Andres Freund <andres@anarazel.de> writes:
> > On 2025-02-24 05:11:48 -0500, Corey Huinker wrote:
> >> * relpages/reltuples/relallvisible are now char[32] buffers in
> RelStatsInfo
> >> and nowhere else (existing relpages conversion remains, however)
>
> > I don't see the point. This will use more memory and if we can't get
> > conversions between integers and strings right we have much bigger
> > problems. The same code was used in the backend too!
>
> I don't like that either.  But there's a bigger problem with 0002:
> it's still got mostly table-driven output.  I've been working on
> fixing the problem discussed over in the -committers thread about how
> we need to identify index-expression columns by number not name [1].
>

There doesn't seem to be any way around it, but it will slightly complicate
the dump-ing side of things, in that we need to either:

a) switch to attnums for index expressions and keep attname calls for
everything else.

b) track what the attnum will be on the destination side, which will be
different when we're not doing a binary upgrade and there are any preceding
dropped columns.

The patch Tom provided opens the door for option "a", and I'm inclined to
take it.



> It's not too awful in the backend (WIP patch attached), but
> getting appendAttStatsImport to do it seems like a complete disaster,
> and this patch fails to make that any easier.  It'd be much better
> if you gave up on that table-driven business and just open-coded the
> handling of the successive output values as was discussed upthread.
>

Can do.


> I don't think the table-driven approach has anything to recommend it
> anyway.  It requires keeping att_stats_arginfo[] in sync with the
> query in getAttStatsExportQuery, an extremely nonobvious (and
> undocumented) connection.  Personally I would nuke the separate
> getAttStatsExportQuery and appendAttStatsImport functions altogether,
> and have one function that executes a query and immediately interprets
> the PGresult.
>

+1, though that comes at the cost of shutting off the possibility of a mass
fetch from pg_stats without also rendering the pg_restore_attribute_stats
calls at the same time.


>
> Also, while working on the attached, I couldn't help forming the
> opinion that we'd be better off to nuke pg_set_attribute_stats()
> from orbit and require people to use pg_restore_attribute_stats().
> pg_set_attribute_stats() would be fine if we had a way to force
> people to call it with only named-argument notation, but we don't.
> So I'm afraid that its existence will encourage people to rely
> on a specific parameter order, and then they'll whine if we
> add/remove/reorder parameters, as indeed I had to do below.
>

They've always had split goals. To recap for people just joining the show,
the "set" family had the following properties:

1. transactional, even for pg_class
2. assumes all stats given are relevant and correct for current db version
3. guaranteed to ERROR if any parameter doesn't check out
4. unstable call signature, can and will change to match the realities of
the current version
5. intended for planner experiments and fuzzing

and the "restore" family has the following properties:

1. will inplace update pg_class to avoid table bloat
2. states the version from whence the stats came, so that adjustments can
be made to suit the current db version, up to and including rejecting that
particular statistic
3. attempts to sidestep errors with WARNINGs so as not to kill a restore
4. stable but highly fluid kwargs-ish call signature
5. intended to be machine generated and used only in restore/upgrade

The attnum change certainly throws a wrench into that, and if we get rid of
the setter functions then we will need to (re)introduce parameters to
indicate our choice for properties 1 and 3. I suppose we could use the
existence or non-existence of the "version" parameter as an indicator of
which mode we want (if it exists, we want WARNINGS and inplace updates, if
not we want pure transactional and ERROR at the first problem), but I'm not
certain that proxy will hold true in the future.


> BTW, I pushed the 0003 patch with minor adjustments.
>

Thanks!