From f2ec57dee93b29e2e27235ec8cc0683d0bc7d775 Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Wed, 15 Feb 2017 16:40:05 -0500 Subject: [PATCH] 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 --- src/backend/executor/nodeHash.c | 6 ++++++ src/backend/executor/nodeHashjoin.c | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c index af5934d2bcf..e695d8834b5 100644 --- a/src/backend/executor/nodeHash.c +++ b/src/backend/executor/nodeHash.c @@ -720,6 +720,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 */ @@ -1599,6 +1602,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 8a04294b402..c50d93f43db 100644 --- a/src/backend/executor/nodeHashjoin.c +++ b/src/backend/executor/nodeHashjoin.c @@ -856,6 +856,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 -- GitLab