Amit Langote <amitlangote09@gmail.com> writes: > On Wed, Oct 16, 2024 at 11:26 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Would it be better for parse analysis to explicitly NULL out this >> field once it's done looking at it? Carrying unmaintained pieces >> of an expression tree around seems both inefficient and prone to >> future failures of this same ilk. The further the raw_expr gets >> out of step with current reality, the worse the hazards.
> It seems we can't do that in this case, because get_rule_expr() wants > to read it.
I think you just increased my level of concern. Aren't you saying that get_rule_expr is likely to deliver utter garbage, because it will be working from an expression that has not tracked transformations made to other parts of the tree?
Also, I see that ruleutils is applying get_rule_expr to raw_expr, which AFAICT means it's *not* raw-parser output, else that wouldn't work at all. Which means the field is badly named and incorrectly documented. But in particular it means you cannot just make the tree-walking routines ignore it --- you *must* update it during transformations such as subquery pullup, or you will have garbage that will cause EXPLAIN to crash or at least produce wrong results.
When I first see the "raw_expr", I think it is output after syntax parsing.
But it is not. The "raw_expr" is transformed in transformJsonValueExpr().
And the formated_expr is different from raw_expr when the targettype is not
same with the expr's type.
I do some codes hack that make the raw_expr store the output of syntax parsing not
transformed result. As you said above, explain will report error.
In other words, the proposed patch is dangerously wrong. I suspect at this point that the true bug might be the opposite: somebody feeling that they could dispense with updating raw_expr when they shouldn't. In particular, I think "... is not evaluated by eval_const_expressions_mutator()" is already a huge red flag. There are transformations that eval_const_expressions does that are not optional.
Can we make the raw_expr just save the raw_parser output, and formated_expr
store the transformed output, and we only touch the formated_expr when need to
