Re: Extended Statistics set/restore/clear functions.

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