Re: multivariate statistics / patch v6

Hello,

On 05/15/15 08:29, Kyotaro HORIGUCHI wrote:
> Hello,
>
> At Thu, 14 May 2015 12:35:50 +0200, Tomas Vondra
<tomas(dot)vondra(at)2ndquadrant(dot)com> wrote in <55547A86(dot)8020400(at)2ndquadrant(dot)com>
...
>
>> Regarding the functional dependencies - you're right there's room for
>> improvement. For example it only works with dependencies between pairs
>> of columns, not multi-column dependencies. Is this what you mean by
>> incomplete?
>
> No, It overruns dependencies->deps because build_mv_dependencies
> stores many elements into dependencies->deps[n] although it
> really has a room for only one element. I suppose that you paused
> writing it when you noticed that the number of required elements
> is unknown before finising walk through all pairs of
> values. palloc'ing numattrs^2 is reasonable enough as POC code
> for now. Am I looking wrong version of patch?
>
> - dependencies = (MVDependencies)palloc0(sizeof(MVDependenciesData))
> + dependencies = (MVDependencies)palloc0(sizeof(MVDependenciesData) +
> + sizeof(MVDependency) * numattrs * numattrs);

Actually, looking at this a bit more, I think the current behavior is
correct. I assume the line is from build_mv_dependencies(), but the
whole block looks like this:

if (dependencies == NULL)
{
dependencies = (MVDependencies)palloc0(sizeof(MVDependenciesData));
dependencies->magic = MVSTAT_DEPS_MAGIC;
}
else
dependencies = repalloc(dependencies,
offsetof(MVDependenciesData, deps) +
sizeof(MVDependency) * (dependencies->ndeps + 1));

which allocates space for a single element initially, and then extends
that when other dependencies are added.

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services