Making CASE error handling less surprising

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Making CASE error handling less surprising
Date: 2020-07-23 16:57:34
Message-ID: 265964.1595523454@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Every so often we get a complaint like [1] about how a CASE should have
prevented a run-time error and didn't, because constant-folding tried
to evaluate a subexpression that would not have been entered at run-time.

It struck me that it would not be hard to improve this situation a great
deal. If, within a CASE subexpression that isn't certain to be executed
at runtime, we refuse to pre-evaluate *any* function (essentially, treat
them all as volatile), then we should largely get the semantics that
users expect. There's some potential for query slowdown if a CASE
contains a constant subexpression that we formerly reduced at plan time
and now do not, but that doesn't seem to me to be a very big deal.

Attached is a draft patch that handles CASE and COALESCE this way.

This is not a complete fix, because if you write a sub-SELECT the
contents of the sub-SELECT are not processed by the outer query's
eval_const_expressions pass; instead, we look at it within the
sub-SELECT itself, and in that context there's no apparent reason
to avoid const-folding. So
CASE WHEN x < 0 THEN (SELECT 1/0) END
fails even if x is never less than zero. I don't see any great way
to avoid that, and I'm not particularly concerned about it anyhow;
usually the point of a sub-SELECT like this is to be decoupled from
outer query evaluation, so that the behavior should not be that
surprising.

One interesting point is that the join regression test contains a
number of uses of "coalesce(int8-variable, int4-constant)" which is
treated a little differently than before: we no longer constant-fold
the int4 constant to int8. That causes the run-time cost of the
expression to be estimated slightly higher, which changes plans in
a couple of these tests; and in any case the EXPLAIN output looks
different since it shows the runtime coercion explicitly. To avoid
those changes I made all these examples quote the constants, so that
the parser resolves them as int8 out of the gate. (Perhaps it'd be
okay to just accept the changes, but I didn't feel like trying to
analyze in detail what each test case had been meant to prove.)

Also, I didn't touch the docs yet. Sections 4.2.14 and 9.18.1
contain some weasel wording that could be backed off, but in light
of the sub-SELECT exception we can't just remove the issue
altogether I think. Not quite sure how to word it.

Thoughts?

regards, tom lane

[1] https://www.postgresql.org/message-id/16549-4991fbf36fcec234%40postgresql.org

Attachment Content-Type Size
safer-CASE-handling-1.patch text/x-diff 22.4 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Janes 2020-07-23 17:05:04 Re: Loaded footgun open_datasync on Windows
Previous Message Andres Freund 2020-07-23 16:19:48 'with' regression tests fails rarely (and spuriously)