Re: Columns correlation and adaptive query optimization

From: Zhihong Yu <zyu(at)yugabyte(dot)com>
To: Konstantin Knizhnik <k(dot)knizhnik(at)postgrespro(dot)ru>
Cc: Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>, David Steele <david(at)pgmasters(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Columns correlation and adaptive query optimization
Date: 2021-03-19 17:32:34
Message-ID: CALNJ-vRh-53giL4cfz4cCEHnqMKdevOws7trdwHJigS1918q-A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

Cheers

On Fri, Mar 19, 2021 at 9:58 AM Konstantin Knizhnik <
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> 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
>
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2021-03-19 17:44:26 Re: [HACKERS] Custom compression methods
Previous Message Tom Lane 2021-03-19 17:27:42 Bringing some sanity to RestoreGUCState()