From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | More smarts about CoerceViaIO, and less stupidity about ArrayCoerceExpr |
Date: | 2019-02-19 23:11:21 |
Message-ID: | 27571.1550617881@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I poked into a recent complaint[1] about PG not being terribly smart
about whether an IS NOT NULL index predicate is implied by a WHERE
clause, and determined that there are a couple of places where we
are being less bright than we could be about CoerceViaIO semantics.
CoerceViaIO is strict independently of whether the I/O functions it
calls are (and they might not be --- in particular, domain_in isn't).
However, not everyplace knows that:
* clauses.c's contain_nonstrict_functions_walker() uses default logic
that will examine the referenced I/O functions to see if they're strict.
That's expensive, requiring several syscache lookups, and it might not
even give the right answer --- though fortunately it'd err in the
conservative direction.
* predtest.c's clause_is_strict_for() doesn't know anything about
CoerceViaIO, so it fails to make the proof requested in [1].
I started to fix this, and was in the midst of copying-and-pasting
contain_nonstrict_functions_walker's handling of ArrayCoerceExpr,
when I realized that that code is actually wrong:
return expression_tree_walker((Node *) ((ArrayCoerceExpr *) node)->arg,
contain_nonstrict_functions_walker,
context);
It should be recursing to itself, not to expression_tree_walker.
As coded, the strictness check doesn't get applied to the immediate
child node of the ArrayCoerceExpr, so that if that node is non-strict
we may arrive at the wrong conclusion.
contain_nonstrict_functions() isn't used in very many places, fortunately,
and ArrayCoerceExpr isn't that easy to produce either, which may explain
the lack of field reports. I was able to cons up this example though,
which demonstrates an incorrect conclusion about whether it's safe to
inline a function declared STRICT:
regression=# create table t1 (f1 int);
CREATE TABLE
regression=# insert into t1 values(1),(null);
INSERT 0 2
regression=# create or replace function sfunc(int) returns int[] language sql
as 'select array[0, $1]::bigint[]::int[]' strict;
CREATE FUNCTION
regression=# select sfunc(f1) from t1;
sfunc
----------
{0,1}
{0,NULL}
(2 rows)
Of course, since sfunc is strict, that last output should be NULL not
an array containing a NULL.
The attached patch fixes both of these things. At least the second
hunk needs to be back-patched. I'm less sure about whether the
CoerceViaIO changes merit back-patching; they're not fixing wrong
answers, but they are responding to a field complaint. Thoughts?
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
fix-planner-coercion-issues-1.patch | text/x-diff | 1.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2019-02-19 23:28:14 | Re: WAL insert delay settings |
Previous Message | Paul Ramsey | 2019-02-19 23:09:39 | Re: Compressed TOAST Slicing |