Re: Columns correlation and adaptive query optimization - Mailing list pgsql-hackers

From Konstantin Knizhnik
Subject Re: Columns correlation and adaptive query optimization
Date
Msg-id aa10f759-e591-63e3-52ec-ac1db14beabf@postgrespro.ru
Whole thread Raw
In response to Re: Columns correlation and adaptive query optimization  (Zhihong Yu <zyu@yugabyte.com>)
Responses Re: Columns correlation and adaptive query optimization  (Yugo NAGATA <nagata@sraoss.co.jp>)
List pgsql-hackers


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.knizhnik@postgrespro.ru> wrote:


On 19.03.2021 12:17, Yugo NAGATA wrote:
> On Wed, 10 Mar 2021 03:00:25 +0100
> Tomas Vondra <tomas.vondra@enterprisedb.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


pgsql-hackers by date:

Previous
From: Tomas Vondra
Date:
Subject: Re: [HACKERS] Custom compression methods
Next
From: Dilip Kumar
Date:
Subject: Re: [HACKERS] Custom compression methods