Hi,
On 2017-03-16 11:15:16 -0400, Tom Lane wrote: > The thing that actually made the ExecEvalCase code into a bug was that > we were using ExprContext-level fields to store the current caseValue, > allowing aliasing to occur across nested CASEs. I think that in > this implementation, it ought to be possible to get rid of > ExprContext.caseValue_datum et al altogether, in favor of some storage > location that's private to each CASE expression. I'm a bit disappointed > that that doesn't seem to have happened. The patch actually does so - during ExecInitExprRec the "relevant" case/domain testval is stored in ExprState->innermost_*, those pointers are then stored directly in the relevant steps for CaseTest/CoerceToDomainValue evaluation. Unfortunately CaseTest/CoerceToDomainValue are reused outside of domain / case expressions in a bunch of places (plpgsql uses CaseTest for casts evaluation, validateDomainConstraint/domain_check_input evaluate domain constraints without a CoerceToDomain node). I.e ExprContext.caseValue_datum etc. aren't used for normal expressions anymore, just for the ad-hoc hackery in a bunch of places. I'd like to get rid of those usages, but that'd recurse into rewriting plpgsql casts and other random pieces of code into a different approach - something I'd like to avoid doing at the same as this already large patch. I've been pondering if we can't entirely get rid of CaseTest etc, the amount of hackery required seems not like a good thing. One way I'd prototyped was to replace them with PARAM_EXEC nodes - then the whole issue of them potentially having different values at different parts of an expression vanishes because the aliasing is removed. > Eventually, I would also like to find a way to remove the restriction > imposed by the other part of f0c7b789a, ie that we can't inline a SQL > function when that would result in intermixing two levels of CASE > expression. An implementation along the lines of what I've sketched > above could handle that easily enough, as long as we could identify > which nested level of CASE a particular CaseTestExpr belongs to. > I don't know how to do that offhand, but it seems like it ought to be > soluble if we put a bit of thought into it. I haven't thought overly much about this, but I agree, it looks like it should be doable. That seems to suggest my PARAM_EXEC idea isn't necessarily perfect - inlining would cause aliasing again, but it'd also not hard to fix that up. - Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
