agora inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
Subject: [PATCH v1] wip-fix-hash-key-computation
Date: Mon, 17 Jun 2019 23:59:38 -0700
---
src/backend/executor/nodeHash.c | 19 +++++++++++--------
src/backend/executor/nodeHashjoin.c | 18 +++---------------
src/backend/nodes/copyfuncs.c | 1 +
src/backend/nodes/outfuncs.c | 1 +
src/backend/nodes/readfuncs.c | 1 +
src/backend/optimizer/plan/createplan.c | 13 +++++++++++++
src/backend/optimizer/plan/setrefs.c | 16 ++++++++++++++++
src/include/nodes/execnodes.h | 3 ---
src/include/nodes/plannodes.h | 1 +
9 files changed, 47 insertions(+), 26 deletions(-)
diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c
index d16120b9c48..181f45a5b1c 100644
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -157,7 +157,8 @@ MultiExecPrivateHash(HashState *node)
econtext = node->ps.ps_ExprContext;
/*
- * get all inner tuples and insert into the hash table (or temp files)
+ * Get all tuples from subsidiary node and insert into the hash table (or
+ * temp files).
*/
for (;;)
{
@@ -165,7 +166,7 @@ MultiExecPrivateHash(HashState *node)
if (TupIsNull(slot))
break;
/* We have to compute the hash value */
- econtext->ecxt_innertuple = slot;
+ econtext->ecxt_outertuple = slot;
if (ExecHashGetHashValue(hashtable, econtext, hashkeys,
false, hashtable->keepNulls,
&hashvalue))
@@ -281,7 +282,7 @@ MultiExecParallelHash(HashState *node)
slot = ExecProcNode(outerNode);
if (TupIsNull(slot))
break;
- econtext->ecxt_innertuple = slot;
+ econtext->ecxt_outertuple = slot;
if (ExecHashGetHashValue(hashtable, econtext, hashkeys,
false, hashtable->keepNulls,
&hashvalue))
@@ -388,8 +389,8 @@ ExecInitHash(Hash *node, EState *estate, int eflags)
/*
* initialize child expressions
*/
- hashstate->ps.qual =
- ExecInitQual(node->plan.qual, (PlanState *) hashstate);
+ hashstate->hashkeys =
+ ExecInitExprList(node->hashkeys, (PlanState *) hashstate);
return hashstate;
}
@@ -1773,9 +1774,11 @@ ExecParallelHashTableInsertCurrentBatch(HashJoinTable hashtable,
* ExecHashGetHashValue
* Compute the hash value for a tuple
*
- * The tuple to be tested must be in either econtext->ecxt_outertuple or
- * econtext->ecxt_innertuple. Vars in the hashkeys expressions should have
- * varno either OUTER_VAR or INNER_VAR.
+ * The tuple to be tested must be in econtext->ecxt_outertuple and Vars in the
+ * hashkeys expressions should have a varno of OUTER_VAR (but note that the
+ * econtext, expression, and slot might belong to the HashJoin's outer rel, or
+ * be from the Hash, and thus refer to the join's outer/inner side
+ * respectively).
*
* A true result means the tuple's hash value has been successfully computed
* and stored at *hashvalue. A false result means the tuple cannot match
diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c
index 8484a287e73..f9262b1aa38 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -601,8 +601,6 @@ ExecInitHashJoin(HashJoin *node, EState *estate, int eflags)
Plan *outerNode;
Hash *hashNode;
List *lclauses;
- List *rclauses;
- List *rhclauses;
List *hoperators;
List *hcollations;
TupleDesc outerDesc,
@@ -731,14 +729,11 @@ ExecInitHashJoin(HashJoin *node, EState *estate, int eflags)
hjstate->hj_CurTuple = NULL;
/*
- * Deconstruct the hash clauses into outer and inner argument values, so
- * that we can evaluate those subexpressions separately. Also make a list
- * of the hash operator OIDs, in preparation for looking up the hash
- * functions to use.
+ * Deconstruct the hash clauses into outer argument values, so that we can
+ * evaluate those subexpressions separately. Also make a list of the hash
+ * operator OIDs, in preparation for looking up the hash functions to use.
*/
lclauses = NIL;
- rclauses = NIL;
- rhclauses = NIL;
hoperators = NIL;
hcollations = NIL;
foreach(l, node->hashclauses)
@@ -747,19 +742,12 @@ ExecInitHashJoin(HashJoin *node, EState *estate, int eflags)
lclauses = lappend(lclauses, ExecInitExpr(linitial(hclause->args),
(PlanState *) hjstate));
- rclauses = lappend(rclauses, ExecInitExpr(lsecond(hclause->args),
- (PlanState *) hjstate));
- rhclauses = lappend(rhclauses, ExecInitExpr(lsecond(hclause->args),
- innerPlanState(hjstate)));
hoperators = lappend_oid(hoperators, hclause->opno);
hcollations = lappend_oid(hcollations, hclause->inputcollid);
}
hjstate->hj_OuterHashKeys = lclauses;
- hjstate->hj_InnerHashKeys = rclauses;
hjstate->hj_HashOperators = hoperators;
hjstate->hj_Collations = hcollations;
- /* child Hash node needs to evaluate inner hash keys, too */
- ((HashState *) innerPlanState(hjstate))->hashkeys = rhclauses;
hjstate->hj_JoinState = HJ_BUILD_HASHTABLE;
hjstate->hj_MatchedOuter = false;
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 78deade89b4..65098f04712 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -1066,6 +1066,7 @@ _copyHash(const Hash *from)
/*
* copy remainder of node
*/
+ COPY_NODE_FIELD(hashkeys);
COPY_SCALAR_FIELD(skewTable);
COPY_SCALAR_FIELD(skewColumn);
COPY_SCALAR_FIELD(skewInherit);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 8400dd319e2..567a26fd636 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -863,6 +863,7 @@ _outHash(StringInfo str, const Hash *node)
_outPlanInfo(str, (const Plan *) node);
+ WRITE_NODE_FIELD(hashkeys);
WRITE_OID_FIELD(skewTable);
WRITE_INT_FIELD(skewColumn);
WRITE_BOOL_FIELD(skewInherit);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 6c2626ee62b..6ae136e0bb3 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -2274,6 +2274,7 @@ _readHash(void)
ReadCommonPlan(&local_node->plan);
+ READ_NODE_FIELD(hashkeys);
READ_OID_FIELD(skewTable);
READ_INT_FIELD(skewColumn);
READ_BOOL_FIELD(skewInherit);
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 608d5adfed2..f7d486ed8e1 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -225,6 +225,7 @@ static HashJoin *make_hashjoin(List *tlist,
Plan *lefttree, Plan *righttree,
JoinType jointype, bool inner_unique);
static Hash *make_hash(Plan *lefttree,
+ List *hashkeys,
Oid skewTable,
AttrNumber skewColumn,
bool skewInherit);
@@ -4381,9 +4382,11 @@ create_hashjoin_plan(PlannerInfo *root,
List *joinclauses;
List *otherclauses;
List *hashclauses;
+ List *hashkeys = NIL;
Oid skewTable = InvalidOid;
AttrNumber skewColumn = InvalidAttrNumber;
bool skewInherit = false;
+ ListCell *lc;
/*
* HashJoin can project, so we don't have to demand exact tlists from the
@@ -4475,10 +4478,18 @@ create_hashjoin_plan(PlannerInfo *root,
}
}
+ foreach(lc, hashclauses)
+ {
+ OpExpr *hclause = (OpExpr *) lfirst(lc);
+
+ hashkeys = lappend(hashkeys, lsecond(hclause->args));
+ }
+
/*
* Build the hash node and hash join node.
*/
hash_plan = make_hash(inner_plan,
+ hashkeys,
skewTable,
skewColumn,
skewInherit);
@@ -5568,6 +5579,7 @@ make_hashjoin(List *tlist,
static Hash *
make_hash(Plan *lefttree,
+ List *hashkeys,
Oid skewTable,
AttrNumber skewColumn,
bool skewInherit)
@@ -5580,6 +5592,7 @@ make_hash(Plan *lefttree,
plan->lefttree = lefttree;
plan->righttree = NULL;
+ node->hashkeys = hashkeys;
node->skewTable = skewTable;
node->skewColumn = skewColumn;
node->skewInherit = skewInherit;
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index dc11f098e0f..1b0564337f9 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -646,6 +646,22 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
break;
case T_Hash:
+ {
+ Hash *hplan = (Hash *) plan;
+ Plan *outer_plan = plan->lefttree;
+ indexed_tlist *outer_itlist;
+
+ outer_itlist = build_tlist_index(outer_plan->targetlist);
+
+ hplan->hashkeys = (List *)
+ fix_upper_expr(root,
+ (Node *) hplan->hashkeys,
+ outer_itlist,
+ OUTER_VAR,
+ rtoffset);
+ }
+ /* fall through */
+
case T_Material:
case T_Sort:
case T_Unique:
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 99b9fa414f1..4c4194617ed 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1852,7 +1852,6 @@ typedef struct MergeJoinState
*
* hashclauses original form of the hashjoin condition
* hj_OuterHashKeys the outer hash keys in the hashjoin condition
- * hj_InnerHashKeys the inner hash keys in the hashjoin condition
* hj_HashOperators the join operators in the hashjoin condition
* hj_HashTable hash table for the hashjoin
* (NULL if table not built yet)
@@ -1883,7 +1882,6 @@ typedef struct HashJoinState
JoinState js; /* its first field is NodeTag */
ExprState *hashclauses;
List *hj_OuterHashKeys; /* list of ExprState nodes */
- List *hj_InnerHashKeys; /* list of ExprState nodes */
List *hj_HashOperators; /* list of operator OIDs */
List *hj_Collations;
HashJoinTable hj_HashTable;
@@ -2222,7 +2220,6 @@ typedef struct HashState
PlanState ps; /* its first field is NodeTag */
HashJoinTable hashtable; /* hash table for the hashjoin */
List *hashkeys; /* list of ExprState nodes */
- /* hashkeys is same as parent's hj_InnerHashKeys */
SharedHashInfo *shared_info; /* one entry per worker */
HashInstrumentation *hinstrument; /* this worker's entry */
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 70f8b8e22b7..f1bce9eb66c 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -904,6 +904,7 @@ typedef struct Hash
bool skewInherit; /* is outer join rel an inheritance tree? */
/* all other info is in the parent HashJoin node */
double rows_total; /* estimate total rows if parallel_aware */
+ List *hashkeys; /* hash keys from the hashjoin condition */
} Hash;
/* ----------------
--
2.22.0.dirty
--rzixpp34lrjluasu--
view thread (2+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH v1] wip-fix-hash-key-computation
In-Reply-To: <no-message-id-1883061@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox