>>>>> "Aleksandr" == Aleksandr Parfenov <a.parfe...@postgrespro.ru> writes:

 >> From an implementation point of view your patch is obviously broken
 >> in many ways (starting with not checking varattno anywhere, and not
 >> actually checking anywhere if the expression is volatile).

 Aleksandr> The actual checking if the expression volatile or not is
 Aleksandr> done inside evaluate_function(). This is called through few
 Aleksandr> more function in eval_const_experssion(). If it's volatile,
 Aleksandr> the eval_const_expression() will return FuncExpr node, Const
 Aleksandr> otherwise. It also checks are arguments immutable or not.

You're missing a ton of other possible cases, of which by far the most
notable is function inlining: eval_const_expressions will inline even a
volatile function and return a new expression tree (which could be
almost anything depending on the function body).

 Aleksandr> I agree about varattno, it should be checked. Even in case
 Aleksandr> of SRF not replaced, it is better to be sure that Var points
 Aleksandr> to first (and the only) attribute.

It's not a matter of "better", but of basic correctness. Functions can
return composite values with columns.

-- 
Andrew (irc:RhodiumToad)

Reply via email to