Re: Columns correlation and adaptive query optimization

On 19.03.2021 20:32, Zhihong Yu wrote:
> Hi,
> In AddMultiColumnStatisticsForQual(),
>
> +   /* Loop until we considered all vars */
> +   while (vars != NULL)
> ...
> +       /* Contruct list of unique vars */
> +       foreach (cell, vars)
>
> What if some cell / node, gets into the else block:
>
> +               else
> +               {
> +                   continue;
>
> and being left in vars. Is there a chance for infinite loop ?
> It seems there should be a bool variable indicating whether any cell
> gets to the following:
>
> +           vars = foreach_delete_current(vars, cell);
>
> If no cell gets removed in the current iteration, the outer while loop
> should exit.

Each iteration of outer loop (while (vars != NULL))
process variables belonging to one relation.
We take "else continue" branch only if variable belongs to some other
relation.
At first iteration of foreach (cell, vars)
variable "cols" is NULL and we always take first branch of the if.
In other words, at each iteration of outer loop we always make some
progress in processing "vars" list and remove some elements
from this list. So infinite loop can never happen.

>
> Cheers
>
> On Fri, Mar 19, 2021 at 9:58 AM Konstantin Knizhnik
> <k(dot)knizhnik(at)postgrespro(dot)ru <mailto:k(dot)knizhnik(at)postgrespro(dot)ru>> wrote:
>
>
>
> On 19.03.2021 12:17, Yugo NAGATA wrote:
> > On Wed, 10 Mar 2021 03:00:25 +0100
> > Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com
> <mailto:tomas(dot)vondra(at)enterprisedb(dot)com>> wrote:
> >
> >> What is being proposed here - an extension suggesting which
> statistics
> >> to create (and possibly creating them automatically) is certainly
> >> useful, but I'm not sure I'd call it "adaptive query
> optimization". I
> >> think "adaptive" means the extension directly modifies the
> estimates
> >> based on past executions. So I propose calling it maybe "statistics
> >> advisor" or something like that.
> > I am also agree with the idea to implement this feature as a new
> > extension for statistics advisor.
> >
> >> BTW Why is "qual" in
> >>
> >>    static void
> >>    AddMultiColumnStatisticsForQual(void* qual, ExplainState *es)
> >>
> >> declared as "void *"? Shouldn't that be "List *"?
> > When I tested this extension using TPC-H queries, it raised
> segmentation
> > fault in this function. I think the cause would be around this
> argument.
> >
> > Regards,
> > Yugo Nagata
> >
> Attached please find new version of the patch with
> AddMultiColumnStatisticsForQual parameter type fix and one more fix
> related with handling synthetic attributes.
> I can not reproduce the crash on TPC-H queries, so if the problem
> persists, can you please send me stack trace and may be some other
> information helping to understand the reason of SIGSEGV?
>
> Thanks in advance,
> Konstantin
>