Re: BUG #17985: Inconsistent results of SELECT comparing two CASE WHEN clause - Mailing list pgsql-bugs

PG Bug reporting form <noreply@postgresql.org> writes:
> My fuzzer finds a correctness bug in Postgres, which makes Postgres return
> inconsistent results.

Huh.  This is a very ancient bug, dating seemingly to commit d24d75ff1.
When rescanning a hash join, we'll skip rebuilding the hash table if
the inner subplan contains no updated Param values.  However, the
inner hash key expressions can themselves contain Param references,
which evidently are not caught by that test.  A change in the value
of such a Param necessitates rebuilding the hash table, and this
example shows that we're not doing that.

One way to fix it is as attached.  I wonder though if this isn't
telling us that there's a bug in the planner's assignment of
allParams bits for Hash nodes.  The dangerous Param is present
in the Hash node's hashkeys field, so why isn't the existing
chgParam test adequate?

            regards, tom lane

diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c
index 980746128b..41217d189d 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -169,6 +169,7 @@
 #include "executor/nodeHash.h"
 #include "executor/nodeHashjoin.h"
 #include "miscadmin.h"
+#include "optimizer/clauses.h"
 #include "pgstat.h"
 #include "utils/memutils.h"
 #include "utils/sharedtuplestore.h"
@@ -1413,13 +1414,14 @@ ExecReScanHashJoin(HashJoinState *node)
      * In a multi-batch join, we currently have to do rescans the hard way,
      * primarily because batch temp files may have already been released. But
      * if it's a single-batch join, and there is no parameter change for the
-     * inner subnode, then we can just re-use the existing hash table without
-     * rebuilding it.
+     * inner subnode nor any PARAM_EXEC Params in the inner hash keys, then we
+     * can just re-use the existing hash table without rebuilding it.
      */
     if (node->hj_HashTable != NULL)
     {
         if (node->hj_HashTable->nbatch == 1 &&
-            innerPlan->chgParam == NULL)
+            innerPlan->chgParam == NULL &&
+            !contain_any_exec_param((Node *) ((Hash *) innerPlan->plan)->hashkeys))
         {
             /*
              * Okay to reuse the hash table; needn't rescan inner, either.
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 7f453b04f8..bee69014d4 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -103,6 +103,7 @@ static bool contain_volatile_functions_not_nextval_walker(Node *node, void *cont
 static bool max_parallel_hazard_walker(Node *node,
                                        max_parallel_hazard_context *context);
 static bool contain_nonstrict_functions_walker(Node *node, void *context);
+static bool contain_any_exec_param_walker(Node *node, void *arg);
 static bool contain_exec_param_walker(Node *node, List *param_ids);
 static bool contain_context_dependent_node(Node *clause);
 static bool contain_context_dependent_node_walker(Node *node, int *flags);
@@ -1043,9 +1044,37 @@ contain_nonstrict_functions_walker(Node *node, void *context)
  *****************************************************************************/

 /*
- * contain_exec_param
+ * contain_any_exec_param
  *      Recursively search for PARAM_EXEC Params within a clause.
  *
+ * Returns true if the clause contains any PARAM_EXEC Param.
+ * Does not descend into subqueries!
+ */
+bool
+contain_any_exec_param(Node *clause)
+{
+    return contain_any_exec_param_walker(clause, NULL);
+}
+
+static bool
+contain_any_exec_param_walker(Node *node, void *arg)
+{
+    if (node == NULL)
+        return false;
+    if (IsA(node, Param))
+    {
+        Param       *p = (Param *) node;
+
+        if (p->paramkind == PARAM_EXEC)
+            return true;
+    }
+    return expression_tree_walker(node, contain_any_exec_param_walker, arg);
+}
+
+/*
+ * contain_exec_param
+ *      Recursively search for specific PARAM_EXEC Params within a clause.
+ *
  * Returns true if the clause contains any PARAM_EXEC Param with a paramid
  * appearing in the given list of Param IDs.  Does not descend into
  * subqueries!
diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h
index cbe0607e85..058f9c0de9 100644
--- a/src/include/optimizer/clauses.h
+++ b/src/include/optimizer/clauses.h
@@ -35,6 +35,7 @@ extern bool contain_subplans(Node *clause);
 extern char max_parallel_hazard(Query *parse);
 extern bool is_parallel_safe(PlannerInfo *root, Node *node);
 extern bool contain_nonstrict_functions(Node *clause);
+extern bool contain_any_exec_param(Node *clause);
 extern bool contain_exec_param(Node *clause, List *param_ids);
 extern bool contain_leaked_vars(Node *clause);

diff --git a/src/test/regress/expected/join_hash.out b/src/test/regress/expected/join_hash.out
index 4faf010f8c..262fa71ed8 100644
--- a/src/test/regress/expected/join_hash.out
+++ b/src/test/regress/expected/join_hash.out
@@ -1128,3 +1128,39 @@ WHERE
 (1 row)

 ROLLBACK;
+-- Verify that we behave sanely when the inner hash keys contain parameters
+-- (that is, outer or lateral references).  This situation has to defeat
+-- re-use of the inner hash table across rescans.
+begin;
+set local enable_hashjoin = on;
+explain (costs off)
+select i8.q2, ss.* from
+int8_tbl i8,
+lateral (select t1.fivethous, i4.f1 from tenk1 t1 join int4_tbl i4
+         on t1.fivethous = i4.f1+i8.q2 order by 1,2) ss;
+                        QUERY PLAN
+-----------------------------------------------------------
+ Nested Loop
+   ->  Seq Scan on int8_tbl i8
+   ->  Sort
+         Sort Key: t1.fivethous, i4.f1
+         ->  Hash Join
+               Hash Cond: (t1.fivethous = (i4.f1 + i8.q2))
+               ->  Seq Scan on tenk1 t1
+               ->  Hash
+                     ->  Seq Scan on int4_tbl i4
+(9 rows)
+
+select i8.q2, ss.* from
+int8_tbl i8,
+lateral (select t1.fivethous, i4.f1 from tenk1 t1 join int4_tbl i4
+         on t1.fivethous = i4.f1+i8.q2 order by 1,2) ss;
+ q2  | fivethous | f1
+-----+-----------+----
+ 456 |       456 |  0
+ 456 |       456 |  0
+ 123 |       123 |  0
+ 123 |       123 |  0
+(4 rows)
+
+rollback;
diff --git a/src/test/regress/sql/join_hash.sql b/src/test/regress/sql/join_hash.sql
index e73f645e9e..6b0688ab0a 100644
--- a/src/test/regress/sql/join_hash.sql
+++ b/src/test/regress/sql/join_hash.sql
@@ -604,3 +604,22 @@ WHERE
     AND hjtest_1.a <> hjtest_2.b;

 ROLLBACK;
+
+-- Verify that we behave sanely when the inner hash keys contain parameters
+-- (that is, outer or lateral references).  This situation has to defeat
+-- re-use of the inner hash table across rescans.
+begin;
+set local enable_hashjoin = on;
+
+explain (costs off)
+select i8.q2, ss.* from
+int8_tbl i8,
+lateral (select t1.fivethous, i4.f1 from tenk1 t1 join int4_tbl i4
+         on t1.fivethous = i4.f1+i8.q2 order by 1,2) ss;
+
+select i8.q2, ss.* from
+int8_tbl i8,
+lateral (select t1.fivethous, i4.f1 from tenk1 t1 join int4_tbl i4
+         on t1.fivethous = i4.f1+i8.q2 order by 1,2) ss;
+
+rollback;