Re: Statistics Import and Export

Bruce Momjian <bruce@momjian.us>

From: Bruce Momjian <bruce@momjian.us>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Corey Huinker <corey.huinker@gmail.com>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2023-12-27T15:29:01Z
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 Wed, Dec 27, 2023 at 01:08:47PM +0100, Tomas Vondra wrote:
> On 12/26/23 20:19, Tom Lane wrote:
> > Bruce Momjian <bruce@momjian.us> writes:
> >> I think we need a robust API to handle two cases:
> > 
> >> *  changes in how we store statistics
> >> *  changes in how how data type values are represented in the statistics
> > 
> >> We have had such changes in the past, and I think these two issues are
> >> what have prevented import/export of statistics up to this point.
> >> Developing an API that doesn't cleanly handle these will cause long-term
> >> pain.
> > 
> > Agreed.
> > 
> 
> I agree the format is important - we don't want to end up with a format
> that's cumbersome or inconvenient to use. But I don't think the proposed
> format is somewhat bad in those respects - it mostly reflects how we
> store statistics and if I was designing a format for humans, it might
> look a bit differently. But that's not the goal here, IMHO.
> 
> I don't quite understand the two cases above. Why should this affect how
> we store statistics? Surely, making the statistics easy to use for the
> optimizer is much more important than occasional export/import.

The two items above were to focus on getting a solution that can easily
handle future statistics storage changes.  I figured we would want to
manipulate the data as a table internally so I am confused why we would
export JSON instead of a COPY format.  I didn't think we were changing
how we internall store or use the statistics.

> >> In summary, I think we need an SQL-level command for this.
> > 
> > I think a SQL command is an actively bad idea.  It'll just add development
> > and maintenance overhead that we don't need.  When I worked on this topic
> > years ago at Salesforce, I had things set up with simple functions, which
> > pg_dump would invoke by writing more or less
> > 
> > 	SELECT pg_catalog.load_statistics(....);
> > 
> > This has a number of advantages, not least of which is that an extension
> > could plausibly add compatible functions to older versions.  The trick,
> > as you say, is to figure out what the argument lists ought to be.
> > Unfortunately I recall few details of what I wrote for Salesforce,
> > but I think I had it broken down in a way where there was a separate
> > function call occurring for each pg_statistic "slot", thus roughly
> > 
> > load_statistics(table regclass, attname text, stakind int, stavalue ...);
> > 
> > I might have had a separate load_statistics_xxx function for each
> > stakind, which would ease the issue of deciding what the datatype
> > of "stavalue" is.  As mentioned already, we'd also need some sort of
> > version identifier, and we'd expect the load_statistics() functions
> > to be able to transform the data if the old version used a different
> > representation.  I agree with the idea that an explicit representation
> > of the source table attribute's type would be wise, too.
> > 
> 
> Yeah, this is pretty much what I meant by "functional" interface. But if
> I said maybe the format implemented by the patch is maybe too close to
> how we store the statistics, then this has exactly the same issue. And
> it has other issues too, I think - it breaks down the stats into
> multiple function calls, so ensuring the sanity/correctness of whole
> sets of statistics gets much harder, I think.

I was suggesting an SQL command because this feature is going to need a
lot of options and do a lot of different things, I am afraid, and a
single function might be too complex to manage.

> I'm not sure about the extension idea. Yes, we could have an extension
> providing such functions, but do we have any precedent of making
> pg_upgrade dependent on an external extension? I'd much rather have
> something built-in that just works, especially if we intend to make it
> the default behavior (which I think should be our aim here).

Uh, an extension seems nice to allow people in back branches to install
it, but not for normal usage.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.