Обсуждение: pgsql: Make decompilation of optimized CASE constructs more robust.
Make decompilation of optimized CASE constructs more robust. We had some hacks in ruleutils.c to cope with various odd transformations that the optimizer could do on a CASE foo WHEN "CaseTestExpr = RHS" clause. However, the fundamental impossibility of covering all cases was exposed by Heikki, who pointed out that the "=" operator could get replaced by an inlined SQL function, which could contain nearly anything at all. So give up on the hacks and just print the expression as-is if we fail to recognize it as "CaseTestExpr = RHS". (We must cover that case so that decompiled rules print correctly; but we are not under any obligation to make EXPLAIN output be 100% valid SQL in all cases, and already could not do so in some other cases.) This approach requires that we have some printable representation of the CaseTestExpr node type; I used "CASE_TEST_EXPR". Back-patch to all supported branches, since the problem case fails in all. Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/3987e9e62046bd800d8d08566ed49fee1ae6cb86 Modified Files -------------- src/backend/utils/adt/ruleutils.c | 67 ++++++++++++++++++------------------- 1 files changed, 33 insertions(+), 34 deletions(-)
On 27.05.2011 02:44, Tom Lane wrote:
> Make decompilation of optimized CASE constructs more robust.
>
> We had some hacks in ruleutils.c to cope with various odd transformations
> that the optimizer could do on a CASE foo WHEN "CaseTestExpr = RHS" clause.
> However, the fundamental impossibility of covering all cases was exposed
> by Heikki, who pointed out that the "=" operator could get replaced by an
> inlined SQL function, which could contain nearly anything at all. So give
> up on the hacks and just print the expression as-is if we fail to recognize
> it as "CaseTestExpr = RHS". (We must cover that case so that decompiled
> rules print correctly; but we are not under any obligation to make EXPLAIN
> output be 100% valid SQL in all cases, and already could not do so in some
> other cases.) This approach requires that we have some printable
> representation of the CaseTestExpr node type; I used "CASE_TEST_EXPR".
I'm afraid this is still wrong. The "=" OpExpr might also get replaced
with another OpExpr as part of inlining. For example:
-- Equality operator between int4 and bool, where 0 is false and
anything else is true.
postgres=# CREATE FUNCTION int4_bool_eq (int4, bool) returns boolean AS
$$ SELECT ($1 <> 0 AND $2) OR ($1 = 0 AND NOT $2) $$ language sql;
CREATE FUNCTION
postgres=# CREATE OPERATOR = (procedure = int4_bool_eq, leftarg = int4,
rightarg = bool);
CREATE OPERATOR
postgres=# explain verbose SELECT CASE i WHEN true THEN 'yes' ELSE 'no'
END FROM foo;
QUERY PLAN
--------------------------------------------------------------
Seq Scan on public.foo (cost=0.00..40.00 rows=2400 width=4)
Output: CASE i WHEN 0 THEN 'yes'::text ELSE 'no'::text END
(2 rows)
The deparsed CASE statement is backwards. The WHEN expression got
inlined into "CaseTestExpr <> 0", and ruleutils.c incorrectly recognizes
that as the special OpExpr form, and shortens it into "CASE i WHEN 0
...". Seems that we need to also check that the OpExpr uses "=".
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> On 27.05.2011 02:44, Tom Lane wrote:
>> Make decompilation of optimized CASE constructs more robust.
> I'm afraid this is still wrong. The "=" OpExpr might also get replaced
> with another OpExpr as part of inlining.
Yeah, but looking at operator names is not going to be sufficient to
prove whether that has happened. The new OpExpr could also use an
operator that's named "=".
Even if we did check for this, the output would *still* be ambiguous and
hence confusing to anybody who didn't understand what was going on,
because it's not obvious that the literally-printed WHEN expression
is going to be used as a boolean rather than compared to the CASE's test
expression.
In the end, I think that decompilation of post-planner query structures
is something we can only do on a "best effort" basis; it's never going
to be perfect, because sometimes we produce things that simply don't
directly match any standard SQL construct. As long as it produces the
right answer for constructs as they appear in views, I'm not going to
get overly excited about it.
regards, tom lane