More smarts about CoerceViaIO, and less stupidity about ArrayCoerceExpr

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

[1] https://www.postgresql.org/message-id/CAHkN8V9Rfh6uAjQLURJfnHsQfC_MYiFUSWEVcwVSiPdokmkniw%40mail.gmail.com

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 3737166..501b0e9 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -1172,6 +1172,16 @@ contain_nonstrict_functions_walker(Node *node, void *context)
         return true;
     if (IsA(node, FieldStore))
         return true;
+    if (IsA(node, CoerceViaIO))
+    {
+        /*
+         * CoerceViaIO is strict regardless of whether the I/O functions are,
+         * so just go look at its argument; asking check_functions_in_node is
+         * useless expense and could deliver the wrong answer.
+         */
+        return contain_nonstrict_functions_walker((Node *) ((CoerceViaIO *) node)->arg,
+                                                  context);
+    }
     if (IsA(node, ArrayCoerceExpr))
     {
         /*
@@ -1179,9 +1189,8 @@ contain_nonstrict_functions_walker(Node *node, void *context)
          * the per-element expression is; so we should ignore elemexpr and
          * recurse only into the arg.
          */
-        return expression_tree_walker((Node *) ((ArrayCoerceExpr *) node)->arg,
-                                      contain_nonstrict_functions_walker,
-                                      context);
+        return contain_nonstrict_functions_walker((Node *) ((ArrayCoerceExpr *) node)->arg,
+                                                  context);
     }
     if (IsA(node, CaseExpr))
         return true;
diff --git a/src/backend/optimizer/util/predtest.c b/src/backend/optimizer/util/predtest.c
index 01f64ee..179b9aa 100644
--- a/src/backend/optimizer/util/predtest.c
+++ b/src/backend/optimizer/util/predtest.c
@@ -1352,6 +1352,15 @@ clause_is_strict_for(Node *clause, Node *subexpr)
         return false;
     }

+    /*
+     * CoerceViaIO is strict (whether or not the I/O functions it calls are).
+     * This is worth checking mainly because it saves us having to explain to
+     * users why some type coercions are known strict and others aren't.
+     */
+    if (IsA(clause, CoerceViaIO))
+        return clause_is_strict_for((Node *) ((CoerceViaIO *) clause)->arg,
+                                    subexpr);
+
     return false;
 }