Re: Statistics Import and Export

Corey Huinker <corey.huinker@gmail.com>

From: Corey Huinker <corey.huinker@gmail.com>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>, Bruce Momjian <bruce@momjian.us>, Tom Lane <tgl@sss.pgh.pa.us>, Nathan Bossart <nathandbossart@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: 2024-09-05T17:29:44Z
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.

>
> I have attached version 28j as one giant patch covering what was
> previously 0001-0003. It's a bit rough (tests in particular need some
> work), but it implelements the logic to replace only those values
> specified rather than the whole tuple.
>

I like what you did restoring the parameter enums, especially now that they
can be leveraged for the expected type oids data structure.


> At least for the interactive "set" variants of the functions, I think
> it's an improvement. It feels more natural to just change one stat
> without wiping out all the others. I realize a lot of the statistics
> depend on each other, but the point is not to replace ANALYZE, the
> point is to experiment with planner scenarios. What do others think?
>

When I first heard that was what you wanted to do, I was very uneasy about
it. The way you implemented it (one function to wipe out/reset all existing
stats, and then the _set_ function works as an upsert) puts my mind at
ease. The things I really wanted to avoid were gaps in the stakind array
(which can't happen as you wrote it) and getting the stakinds out of order
(admittedly that's more a tidiness issue, but pg_statistic before/after
fidelity is kept, so I'm happy).

For the "restore" variants, I'm not sure it matters a lot because the
> stats will already be empty. If it does matter, we could pretty easily
> define the "restore" variants to wipe out existing stats when loading
> the table, though I'm not sure if that's a good thing or not.
>

I agree, and I'm leaning towards doing the clear, because "restore" to me
implies that what resides there exactly matches what was in the function
call, regardless of what might have been there before. But you're also
right, "restore" is expected to be used on default/missing stats, and the
restore_* call generated is supposed to be comprehensive of all stats that
were there at time of dump/upgrade, so impact would be minimal.


> I also made more use of FunctionCallInfo structures to communicate
> between functions rather than huge parameter lists. I believe that
> reduced the line count substantially, and made it easier to transform
> the argument pairs in the "restore" variants into the positional
> arguments for the "set" variants.
>

You certainly did, and I see where it pays off given that _set_ / _restore_
functions are just different ways of ordering the shared internal function
call.

Observation: there is currently no way to delete a stakind, keeping the
rest of the record. It's certainly possible to compose a SQL query that
gets the current values, invokes pg_clear_* and then pg_set_* using the
values that are meant to be kept, and in fact that pattern is how I
imagined the pg_set_* functions would be used when they overwrote
everything in the tuple. So I am fine with going forward with this paradigm.

The code mentions that more explanation should be given for the special
cases (tsvector, etc) and that explanation is basically "this code follows
what the corresponding custom typanalyze() function does". In the future,
it may make sense to have custom typimport() functions for datatypes that
have a custom typanalzye(), which would solve the issue of handling custom
stakinds.

I'll continue to work on this.

p.s. dropping invalid email address from the thread