Re: Extended Statistics set/restore/clear functions. - Mailing list pgsql-hackers
| From | Chao Li |
|---|---|
| Subject | Re: Extended Statistics set/restore/clear functions. |
| Date | |
| Msg-id | EFE9F959-01C6-45D3-9EEE-F06C766EF3CA@gmail.com Whole thread Raw |
| In response to | Re: Extended Statistics set/restore/clear functions. (Michael Paquier <michael@paquier.xyz>) |
| Responses |
Re: Extended Statistics set/restore/clear functions.
|
| List | pgsql-hackers |
> On Nov 25, 2025, at 15:28, Michael Paquier <michael@paquier.xyz> wrote:
>
> 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
>
<v18-0001-Add-working-input-function-for-pg_ndistinct.patch><v18-0002-Add-working-input-function-for-pg_dependencies.patch>
I don’t see any of my comments are addressed in v18.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
pgsql-hackers by date: