Horton

Re: Extended Statistics set/restore/clear functions.

Michael Paquier <michael@paquier.xyz>

View thread
From: Michael Paquier <michael@paquier.xyz>
To: Corey Huinker <corey.huinker@gmail.com>
Cc: jian he <jian.universality@gmail.com>, Tomas Vondra <tomas@vondra.me>, pgsql-hackers@lists.postgresql.org, tgl@sss.pgh.pa.us
Date: 2025-11-25T07:28:51Z
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

Attachments

On Sat, Nov 22, 2025 at 03:26:19AM -0500, Corey Huinker wrote:
> I added a comment debating the feasibility of testing for subsets of
> attribute sets in pg_dependencies. Basically, I think we can't have the
> test at all, but I haven't removed it just yet pending consensus.

+     * Verify that all attnum sets are a proper subset of the first longest
+     * attnum set.
+     *
+     * TODO:
+     *
+     * I'm fairly certain that because statisticsally insignificant dependency
+     * combinations are not stored, there is a chance that the longest dependency
+     * does not exist, and therefore this test cannot be done. I have left the
+     * test in place for the time being until the issue can be definitively
+     * settled.

As you have already quoted upthread, statext_dependencies_build()
settles the issue on this one, I think.  It is entirely possible that
any group returned by DependencyGenerator generates a degree value
that would prevent a given group to be stored, and this could as well
be the largest possible group there could be in the set.  So we cannot
do any of that for dependencies, unfortunately.  We can always rely on
the list of attributes when assigning the json blob to the stats
object, at least, cross-checking that each attribute list matches with
the numbers of the stats object.  At least we can check for
duplicates, which is better than nothing at all.

Regarding the suggested check where we'd want to enforce all the
groups of attributes to be listed depending on the longest set we have
found, at the end estimate_multivariate_ndistinct() checks the items
listed one-by-one, giving up if we cannot find something in the list
of items.  I think that I am going to be content with the patch as it
is, without this piece.  Let's add an extra SQL test to treat that as
valid input, though.  So I am feeling OK with the input for ndistinct
at this stage.  I have noticed a couple of issues in passing,
adjusting them.  We are reaching more than 90% of coverage with the
tests, and I am not sure that we can actually reach the rest except if
one of the previous steps failed.

So That's one.  Now into the second patch for the input of the
dependencies.

+SELECT '[{"attributes" : [2], "dependency" : 4, "degree": "NaN"}]'::pg_dependencies;
+SELECT '[{"attributes" : [2], "dependency" : 4, "degree": "-inf"}]'::pg_dependencies;
+SELECT '[{"attributes" : [2], "dependency" : 4, "degree": "inf"}]'::pg_dependencies;
+SELECT '[{"attributes" : [2], "dependency" : 4, "degree": "-inf"}]'::pg_dependencies::text::pg_dependencies;

Okay, I have to admit that these ones are fun.  I doubt that anybody
would actually do that, and these do not produce valid json objects,
which is what the last case shows.  Hmm, it makes sense to keep these,
and I'm still siding that we should not care too much about applying
checks on the values and complicate the input function more than that,
so fine by me.

There were a couple of things in the tests, missing quite a few soft
errors.  Many typos, grammar mistakes in the whole.  Also, please do
not split the error strings into multiple lines to make these
greppable.  There is also no need for a break after a return.  In some
cases, a return was used where a break made more sense as the default
path returned a failure.. 

The TODO in build_mvdependencies() could be an elog(), but I have left
it untouched for the errdetail().

We're reaching 91% of coverage here, not bad.  The rest does not seem
reachable, as far as I can see.

With that said, a v18 for the first two patches with the input
functions.  Comments and/or opinions?
--
Michael