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: "Shinoda, Noriyoshi (SXD Japan FSIP)" <noriyoshi.shinoda@hpe.com>, jian he <jian.universality@gmail.com>, 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" <alvherre@alvh.no-ip.org>
Date: 2024-10-18T00:54:37Z
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, Oct 16, 2024 at 7:20 PM Corey Huinker <corey.huinker@gmail.com>
wrote:

> Code fix with comment on why nobody expects a relpages -1. Test case to
>> demonstrate that relpages -1 can happen, and updated doc to reflect the new
>> lower bound.
>>
>
> Additional fixes, now in a patch-set:
>
> 1. Allow relpages to be set to -1 (partitioned tables with partitions have
> this value after ANALYZE).
> 2. Turn off autovacuum on tables (where possible) if they are going to be
> the target of pg_set_relation_stats().
> 3. Allow pg_set_relation_stats to continue past an out-of-range detection
> on one attribute, rather than immediately returning false.
>
>

There is some uncertainty on what, if anything, should be returned by
pg_set_relation_stats() and pg_set_attribute_stats().

Initially the function (and there was just one) returned void, but it had a
bool return added around the time it split into relation/attribute stats.

Returning a boolean seems like good instrumentation and a helper for
allowing other tooling to use the functions. However, it's rather limited
in what it can convey.

Currently, a return of true means "a record was written", and false means
that a record was not written. Cases where a record was not written for
pg_set_relation_stats amount to the following:

1. No attributes were specified, therefore there is nothing to change.
2. The attributes were set to the values that the record already has, thus
no change was necessary.

#2 can be confusing, because false may look like a failure, but it means
"the pg_class values were already set to what you wanted".

An alternate use of boolean, suggested by Jeff was the following:

1. Return true if all of the fields specified were applied to the record.
2. Return false if any field that was specified was NOT set, even if the
other ones were.

#2 is also confusing in that the user has received a false value, but the
operation did modify the record, just not as fully as the caller had hoped.

These show that a boolean isn't really up to conveying the nuances of
potential outcomes. Complex return types have met with considerable
resistance, enumerations are similarly undesirable, no other scalar value
seems up to the task, and while an INFO or LOG message could convey
considerable complexity, it wouldn't be readily handled programmatically.
This re-raises the question of whether the pg_set_*_stats functions should
return anything at all.

Any feedback on what users would expect from these functions in terms of
return value is appreciated. Bear in mind that these functions will NOT be
integrated into pg_upgrade/pg_dump, as that functionality will be handled
by functions that are less user friendly but more flexible and forgiving of
bad data. We're talking purely about functions meant for tweaking stats to
look for changes in planner behavior.