shihao zhong <[email protected]> writes:
> The original mail said returning them right away would need a big
> rewrite [1]. The patch gives such a row a dummy hash value of 0 and
> lets it probe as usual. That value always maps to the current batch,
> and the row cannot match, so it comes out null-extended right away,
> like in 18.
I do not think I believe this patch. In particular, it assumes that
hashvalue 0 will map to the current batch, which is only accidentally
true for ExecHashGetBucketAndBatch (but okay, we could add a provision
to its API that hashvalue 0 must map to batch 0) and more importantly
can fail entirely for ExecHashGetSkewBucket. There's no reason to
assume there couldn't be a real tuple with hashvalue 0, and if there
were one already in the skew table then the wrong things would happen.
Also, trading off storing to a tuplestore for scanning a hash bucket
is not obviously a win.
> Another way is to return such a row with a flag and jump straight to
> HJ_FILL_OUTER_TUPLE, skipping the probe.
I like this idea substantially better. More or less as attached?
regards, tom lane
diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c
index 202dd866251..5df351ac5b6 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -195,10 +195,12 @@
static TupleTableSlot *ExecHashJoinOuterGetTuple(PlanState *outerNode,
HashJoinState *hjstate,
- uint32 *hashvalue);
+ uint32 *hashvalue,
+ bool *is_null);
static TupleTableSlot *ExecParallelHashJoinOuterGetTuple(PlanState *outerNode,
HashJoinState *hjstate,
- uint32 *hashvalue);
+ uint32 *hashvalue,
+ bool *is_null);
static TupleTableSlot *ExecHashJoinGetSavedTuple(HashJoinState *hjstate,
BufFile *file,
uint32 *hashvalue,
@@ -233,6 +235,7 @@ ExecHashJoinImpl(PlanState *pstate, bool parallel)
HashJoinTable hashtable;
TupleTableSlot *outerTupleSlot;
uint32 hashvalue;
+ bool is_null;
int batchno;
ParallelHashJoinState *parallel_state;
@@ -440,10 +443,13 @@ ExecHashJoinImpl(PlanState *pstate, bool parallel)
if (parallel)
outerTupleSlot =
ExecParallelHashJoinOuterGetTuple(outerNode, node,
- &hashvalue);
+ &hashvalue,
+ &is_null);
else
outerTupleSlot =
- ExecHashJoinOuterGetTuple(outerNode, node, &hashvalue);
+ ExecHashJoinOuterGetTuple(outerNode, node,
+ &hashvalue,
+ &is_null);
if (TupIsNull(outerTupleSlot))
{
@@ -489,6 +495,16 @@ ExecHashJoinImpl(PlanState *pstate, bool parallel)
econtext->ecxt_outertuple = outerTupleSlot;
node->hj_MatchedOuter = false;
+ /*
+ * If the tuple can't match anything, skip all the tedium and
+ * go directly to emitting it in null-extended form.
+ */
+ if (is_null)
+ {
+ node->hj_JoinState = HJ_FILL_OUTER_TUPLE;
+ continue;
+ }
+
/*
* Find the corresponding bucket for this tuple in the main
* hash table or skew hash table.
@@ -1104,12 +1120,15 @@ ExecEndHashJoin(HashJoinState *node)
* Returns a null slot if no more outer tuples (within the current batch).
*
* On success, the tuple's hash value is stored at *hashvalue --- this is
- * either originally computed, or re-read from the temp file.
+ * either originally computed, or re-read from the temp file. Also, we
+ * set *is_null to true if the tuple cannot have any matches due to
+ * containing a null join key (in which case the hashvalue is meaningless).
*/
static TupleTableSlot *
ExecHashJoinOuterGetTuple(PlanState *outerNode,
HashJoinState *hjstate,
- uint32 *hashvalue)
+ uint32 *hashvalue,
+ bool *is_null)
{
HashJoinTable hashtable = hjstate->hj_HashTable;
int curbatch = hashtable->curbatch;
@@ -1129,8 +1148,6 @@ ExecHashJoinOuterGetTuple(PlanState *outerNode,
while (!TupIsNull(slot))
{
- bool isnull;
-
/*
* We have to compute the tuple's hash value.
*/
@@ -1142,9 +1159,9 @@ ExecHashJoinOuterGetTuple(PlanState *outerNode,
*hashvalue = DatumGetUInt32(ExecEvalExprSwitchContext(hjstate->hj_OuterHash,
econtext,
- &isnull));
+ is_null));
- if (!isnull)
+ if (!*is_null)
{
/* normal case with a non-null join key */
/* remember outer relation is not empty for possible rescan */
@@ -1154,10 +1171,8 @@ ExecHashJoinOuterGetTuple(PlanState *outerNode,
}
else if (hjstate->hj_KeepNullTuples)
{
- /* null join key, but we must save tuple to be emitted later */
- if (hjstate->hj_NullOuterTupleStore == NULL)
- hjstate->hj_NullOuterTupleStore = ExecHashBuildNullTupleStore(hashtable);
- tuplestore_puttupleslot(hjstate->hj_NullOuterTupleStore, slot);
+ /* null join key, but we must return tuple to be emitted */
+ return slot;
}
/*
@@ -1183,7 +1198,10 @@ ExecHashJoinOuterGetTuple(PlanState *outerNode,
hashvalue,
hjstate->hj_OuterTupleSlot);
if (!TupIsNull(slot))
+ {
+ *is_null = false; /* else it'd not be in the batch file */
return slot;
+ }
}
/* End of this batch */
@@ -1196,7 +1214,8 @@ ExecHashJoinOuterGetTuple(PlanState *outerNode,
static TupleTableSlot *
ExecParallelHashJoinOuterGetTuple(PlanState *outerNode,
HashJoinState *hjstate,
- uint32 *hashvalue)
+ uint32 *hashvalue,
+ bool *is_null)
{
HashJoinTable hashtable = hjstate->hj_HashTable;
int curbatch = hashtable->curbatch;
@@ -1213,8 +1232,6 @@ ExecParallelHashJoinOuterGetTuple(PlanState *outerNode,
while (!TupIsNull(slot))
{
- bool isnull;
-
ExprContext *econtext = hjstate->js.ps.ps_ExprContext;
econtext->ecxt_outertuple = slot;
@@ -1223,19 +1240,17 @@ ExecParallelHashJoinOuterGetTuple(PlanState *outerNode,
*hashvalue = DatumGetUInt32(ExecEvalExprSwitchContext(hjstate->hj_OuterHash,
econtext,
- &isnull));
+ is_null));
- if (!isnull)
+ if (!*is_null)
{
/* normal case with a non-null join key */
return slot;
}
else if (hjstate->hj_KeepNullTuples)
{
- /* null join key, but we must save tuple to be emitted later */
- if (hjstate->hj_NullOuterTupleStore == NULL)
- hjstate->hj_NullOuterTupleStore = ExecHashBuildNullTupleStore(hashtable);
- tuplestore_puttupleslot(hjstate->hj_NullOuterTupleStore, slot);
+ /* null join key, but we must return tuple to be emitted */
+ return slot;
}
/*
@@ -1256,6 +1271,7 @@ ExecParallelHashJoinOuterGetTuple(PlanState *outerNode,
ExecForceStoreMinimalTuple(tuple,
hjstate->hj_OuterTupleSlot,
false);
+ *is_null = false; /* else it'd not be in the batch file */
slot = hjstate->hj_OuterTupleSlot;
return slot;
}
diff --git a/src/include/executor/hashjoin.h b/src/include/executor/hashjoin.h
index 4d342174b9a..e77971ed0d1 100644
--- a/src/include/executor/hashjoin.h
+++ b/src/include/executor/hashjoin.h
@@ -74,11 +74,12 @@
* If an input tuple has a null join key, then it cannot match anything from
* the other side of the join. Normally we can just discard such a tuple
* immediately, but if it comes from the outer side of an outer join then we
- * must emit it with null-extension of the other side. For various reasons
- * it's not convenient to do that immediately on seeing the tuple, so we dump
- * the tuple into a tuplestore and emit it later. (In the unlikely but
- * supported case of a non-strict join operator, we treat null keys as normal
- * data.)
+ * must emit it with null-extension of the other side. For an outer tuple
+ * it's possible to emit it immediately and then discard it. But for an inner
+ * tuple, or an outer tuple of a multi-batch parallel join, that is not
+ * convenient, so we dump the tuple into a tuplestore and emit it later.
+ * (In the unlikely but supported case of a non-strict join operator, we treat
+ * null keys as normal data.)
* ----------------------------------------------------------------
*/
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index e3cea52e7df..fe8681ba642 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -2203,7 +2203,8 @@ typedef struct MergeJoinState
* outer joins
* hj_NullInnerTupleSlot prepared null tuple for left/full outer joins
* hj_NullOuterTupleStore tuplestore holding outer tuples that have
- * null join keys (but must be emitted anyway)
+ * null join keys (but must be emitted anyway);
+ * used only in multi-batch parallel joins
* hj_FirstOuterTupleSlot first tuple retrieved from outer plan
* hj_JoinState current state of ExecHashJoin state machine
* hj_KeepNullTuples true to keep outer tuples with null join keys
