Make sure that hash join's bulk-tuple-transfer loops are interruptible.

The loops in ExecHashJoinNewBatch(), ExecHashIncreaseNumBatches(), and
ExecHashRemoveNextSkewBucket() are all capable of iterating over many
tuples without ever doing a CHECK_FOR_INTERRUPTS, so that the backend
might fail to respond to SIGINT or SIGTERM for an unreasonably long time.
Fix that.  In the case of ExecHashJoinNewBatch(), it seems useful to put
the added CHECK_FOR_INTERRUPTS into ExecHashJoinGetSavedTuple() rather
than directly in the loop, because that will also ensure that both
principal code paths through ExecHashJoinOuterGetTuple() will do a
CHECK_FOR_INTERRUPTS, which seems like a good idea to avoid surprises.

Back-patch to all supported branches.

Tom Lane and Thomas Munro

Discussion: https://postgr.es/m/6044.1487121720@sss.pgh.pa.us

diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c
index fbad0f3f13fec5b598fe01b29feb17340b348420..a192fa6a8ad844bd67e1e84d18aaf755315da962 100644 (file)
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -667,6 +667,9 @@ ExecHashIncreaseNumBatches(HashJoinTable hashtable)
            }
 
            tuple = nexttuple;
+
+           /* allow this loop to be cancellable */
+           CHECK_FOR_INTERRUPTS();
        }
    }
 
@@ -1438,6 +1441,9 @@ ExecHashRemoveNextSkewBucket(HashJoinTable hashtable)
        }
 
        hashTuple = nextHashTuple;
+
+       /* allow this loop to be cancellable */
+       CHECK_FOR_INTERRUPTS();
    }
 
    /*

diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c
index ff03f169bf62e8e042f7c7cf1e60cf9e9954752f..6c12eb5e42870a8105e903f8ce9df947b2950e49 100644 (file)
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -911,6 +911,13 @@ ExecHashJoinGetSavedTuple(HashJoinState *hjstate,
    size_t      nread;
    MinimalTuple tuple;
 
+   /*
+    * We check for interrupts here because this is typically taken as an
+    * alternative code path to an ExecProcNode() call, which would include
+    * such a check.
+    */
+   CHECK_FOR_INTERRUPTS();
+
    /*
     * Since both the hash value and the MinimalTuple length word are uint32,
     * we can read them both in one BufFileRead() call without any type