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.
