Re: Add support for (Var op Var) clause in extended MCV statistics

On 12.8.24 14:53, Tomas Vondra wrote:

> I agree, and I'm grateful someone picked up the original patch. I'll try
> to help to keep it moving forward. If the thread gets stuck, feel free
> to ping me to take a look.
Good. Thank you!
>> I started reviewing it and want to suggest some changes to better code:
>> I think we should consider the case where the expression is not neither
>> an OpExpr and VarOpVar expression.
>>
> Do you have some specific type of clauses in mind? Most of the extended
> statistics only really handles this type of clauses, so I'm not sure
> it's feasible to extend that - at least not in this patch.

I agree with Alena that we need to consider the following clauses: (Expr 
op Var), (Var op Expr) and (Expr op Expr). And we need to return false 
in these cases because we did it before my patch in

         /* Check if the expression has the right shape */
         if (!examine_opclause_args(expr->args, &clause_expr, NULL, NULL))
             return false;

In is_opclause_var_op_var() function it is really useless local Node 
*expr_left, *expr_right variables. However, we can't assign them NULL at 
the begin because if I passed not-null pointers I have to return the 
values. Otherwise remain them NULL.

Nevertheless, thank you for review, Alena.

>> Have you tested this code with any benchmarks?
>>
> FWIW I think we need to test two things - that it (a) improves the
> estimates and (b) does not have significant overhead.
Yes, but only TPC-B. And the performance did not drop. In general, it'd 
be better to do more tests and those listed by Tomas with new attached 
patch.