Message-ID: <55562FBB.7020103@2ndquadrant.com>
Date: Fri, 15 May 2015 19:41:15 +0200
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:31.0) Gecko/20100101 Thunderbird/31.5.0
MIME-Version: 1.0
To: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
CC: jeff.janes@gmail.com, sfrost@snowman.net,
 pgsql-hackers@postgresql.org
Subject: Re: multivariate statistics / patch v6
References: <554A73A6.2060603@2ndquadrant.com>
	<20150513.173159.102421446.horiguchi.kyotaro@lab.ntt.co.jp>
	<55547A86.8020400@2ndquadrant.com>
 <20150515.152936.83796179.horiguchi.kyotaro@lab.ntt.co.jp>
In-Reply-To: <20150515.152936.83796179.horiguchi.kyotaro@lab.ntt.co.jp>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

Hello,

On 05/15/15 08:29, Kyotaro HORIGUCHI wrote:
> Hello,
>
> At Thu, 14 May 2015 12:35:50 +0200, Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote in <55547A86.8020400@2ndquadrant.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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers