Horton

Re:Re: Extended Statistics set/restore/clear functions.

Yu Wang <wangyu_runtime@163.com>

View thread
From: WangYu <wangyu_runtime@163.com>
To: "Corey Huinker" <corey.huinker@gmail.com>
Cc: "Chao Li" <li.evan.chao@gmail.com>, "Michael Paquier" <michael@paquier.xyz>, "jian he" <jian.universality@gmail.com>, "Tomas Vondra" <tomas@vondra.me>, pgsql-hackers@lists.postgresql.org, tgl@sss.pgh.pa.us
Date: 2025-12-04T00:52:16Z
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. Add test doing some cloning of extended statistics data

  2. Add test for pg_restore_extended_stats() with multiranges

  3. Add support for "mcv" in pg_restore_extended_stats()

  4. Include extended statistics data in pg_dump

  5. Add support for "dependencies" in pg_restore_extended_stats()

  6. Add test for MAINTAIN permission with pg_restore_extended_stats()

  7. Add pg_restore_extended_stats()

  8. Add routine to free MCVList

  9. Improve pg_clear_extended_stats() with incorrect relation/stats combination

  10. Add pg_clear_extended_stats()

  11. Introduce routines to validate and free MVNDistinct and MVDependencies

  12. Fix typo in stat_utils.c

  13. Move attribute statistics functions to stat_utils.c

  14. Improve error messages of input functions for pg_dependencies and pg_ndistinct

  15. Improve test output of extended statistics for ndistinct and dependencies

  16. Fix some compiler warnings

  17. Add input function for data type pg_dependencies

  18. Add input function for data type pg_ndistinct

  19. Rework output format of pg_dependencies

  20. Rework output format of pg_ndistinct

  21. Fix comments of output routines for pg_ndistinct and pg_dependencies

  22. Move code specific to pg_dependencies to new file

  23. Move code specific to pg_ndistinct to new file

  24. Document some structures in attribute_stats.c

  25. Fix FATAL message for invalid recovery timeline at beginning of recovery

Hi Corey
I was reviewing the recent patch v19-0003-Include-Extended-Statistics-in-pg_dump.patch and noticed a couple of small typo issues in the explanatory comments — nothing that affects the functionality.


Here are the two minor fixes I’d suggest:
1. “ndistintinct” should be “ndistinct”.
2. “depdendencies” should be “dependencies”.


Best regards,
Yu







在 2025-12-04 08:46:29，"Corey Huinker" <corey.huinker@gmail.com> 写道：





On Tue, Nov 25, 2025 at 11:14 PM Corey Huinker <corey.huinker@gmail.com> wrote:


I don’t see any of my comments are addressed in v18.




My apologies. My v17 focused entirely on the input functions, as those were receiving the vast majority of the attention. Now that those are out of the way (Thanks Michael!) I can address those issues.


Paraphrasing the "quotes" here for brevity...


> Several functions are made external visible, they are all renamed with adding a prefix “statatt_”, why text_to_stavalues is an exception?


Michael had specifically said that one didn't need to be renamed. I suppose statatt_import_stavalues() might be a good name for it. It *is* specific to attribute stats, though that definition also applies to the attribute stats nested in the stxdexprs of extended stats. I have no strong opinion on the matter.


> This MVDependency * can be const.

+1

> static void
> upsert_pg_statistic_ext_data(Datum *values, bool *nulls, bool *replaces)
> {
> ```

> This function pass values, nulls and replaces to heap_modify_tuple() and heap_form_tuple(),
> the both functions take all const pointers as parameters.
> So, here values, nulls and replaces can all be const.
I find your argument here persuasive enough to override Michael's previously stated non-excitement.


> ...the two NULL_FRAC questions


Yes, fixed.

> import_mcvlist is declared twice, looks like a copy-paste mistake.


That or a rebase/apply gone wrong. Fixed.


> PREPQUERY_DUMPEXTSTATSSTATS weird, suggest PREPQUERY_DUMPEXTSTATSDATA


Awkward names are almost inevitable when talking about the statistics associated with an object of type "statistics".


There was a debate about whether statistics were data or not, and I'd rather not restart that, so I went with PREPQUERY_DUMPEXTSTATSOBJSTATS for now.


 Incorporated these fixes, and some other lessons learned.