Andrey Rachitskiy <pl0h0yp1@gmail.com> writes: > I think you're right. Fixed it in v3.
I spent some time looking at this, and felt that what we really ought to do is update clauses.c's handling of XmlExpr across the board. In particular, contain_mutable_functions really shouldn't allow XmlExpr in cases where we know it's not immutable. So more or less as attached, where I also spent more effort on the comments.
I agree with handling XmlExpr more consistently. Using the same xmlexpr_is_immutable check for both constant folding and contain_mutable_functions looks right, including the estimate path.
LGTM
I've got mixed emotions about back-patching this. It's not really a bug fix, because we already disclaim the safety of relying on CASE for this purpose, cf [1]. I think it might still be reasonable to sneak into v19, but I'm unsure that putting it into 18 is a good idea.
On back-patching: I share the hesitation about 18. At the same time, SQL-language functions with this pattern did work on earlier branches and broke for some users after the plan-cache change in 18. I would be fine with putting it into 19 and master. If we back-patch to 18, I would treat it as a narrow regression fix rather than a semantic promise about CASE. Either way works for me.
Thank you for your time and for all the work you've put into this.
