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 ab766b84dc92c752f12836bf0a7fd0a2df899c71..94441f807e66a721f27c4794eab792f1b26a5e94 100644 (file)
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -724,6 +724,9 @@ ExecHashIncreaseNumBatches(HashJoinTable hashtable)
 
                        /* next tuple in this chunk */
                        idx += MAXALIGN(hashTupleSize);
+
+                       /* allow this loop to be cancellable */
+                       CHECK_FOR_INTERRUPTS();
                }
 
                /* we're done with this chunk - free it and proceed to the next one */
@@ -1602,6 +1605,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 1d78cdf98d4b07cc68f88f6033569a1540247b33..7ca263922973646edae2c651f7539f7cd8421ae3 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