Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd96d1d5

Browse files
committed
Fix incorrect slot type in BuildTupleHashTableExt
0f57382 adjusted the execGrouping.c code so it made use of ExprStates togenerate hash values. That commit made a wrong assumption that the slottype to pass to ExecBuildHash32FromAttrs() is always &TTSOpsMinimalTuple.That's not the case as the slot type depends on the slot type passed toLookupTupleHashEntry(), which for nodeRecursiveunion.c, could be any ofthe current slot types.Here we fix this by adding a new parameter to BuildTupleHashTableExt()to allow the slot type to be passed in. In the case of nodeSubplan.cand nodeAgg.c the slot type is always &TTSOpsVirtual, so for both ofthose cases, it's beneficial to pass the known slot type as that allowsExecBuildHash32FromAttrs() to skip adding the tuple deform step to theresulting ExprState. Another possible fix would have been to haveExecBuildHash32FromAttrs() set "fetch.kind" to NULL so thatExecComputeSlotInfo() always determines the EEOP_INNER_FETCHSOME isrequired, however, that option isn't favorable as slows down aggregationand hashed subplan evaluation due to the extra (needless) deform step.Thanks to Nathan Bossart for bisecting to find the offending commitbased on Paul's report.Reported-by: Paul Ramsey <pramsey@cleverelephant.ca>Discussion:https://postgr.es/m/99F064C1-B3EB-4BE7-97D2-D2A0AA487A71@cleverelephant.ca
1 parent84f1b0b commitd96d1d5

File tree

8 files changed

+50
-1
lines changed

8 files changed

+50
-1
lines changed

‎src/backend/executor/execGrouping.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ execTuplesHashPrepare(int numCols,
135135
/*
136136
* Construct an empty TupleHashTable
137137
*
138+
* inputOps: slot ops for input hash values, or NULL if unknown or not fixed
138139
*numCols, keyColIdx: identify the tuple fields to use as lookup key
139140
*eqfunctions: equality comparison functions to use
140141
*hashfunctions: datatype-specific hashing functions to use
@@ -154,6 +155,7 @@ execTuplesHashPrepare(int numCols,
154155
TupleHashTable
155156
BuildTupleHashTableExt(PlanState*parent,
156157
TupleDescinputDesc,
158+
constTupleTableSlotOps*inputOps,
157159
intnumCols,AttrNumber*keyColIdx,
158160
constOid*eqfuncoids,
159161
FmgrInfo*hashfunctions,
@@ -225,7 +227,7 @@ BuildTupleHashTableExt(PlanState *parent,
225227

226228
/* build hash ExprState for all columns */
227229
hashtable->tab_hash_expr=ExecBuildHash32FromAttrs(inputDesc,
228-
&TTSOpsMinimalTuple,
230+
inputOps,
229231
hashfunctions,
230232
collations,
231233
numCols,
@@ -274,6 +276,7 @@ BuildTupleHashTable(PlanState *parent,
274276
{
275277
returnBuildTupleHashTableExt(parent,
276278
inputDesc,
279+
NULL,
277280
numCols,keyColIdx,
278281
eqfuncoids,
279282
hashfunctions,

‎src/backend/executor/nodeAgg.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,6 +1520,7 @@ build_hash_table(AggState *aggstate, int setno, long nbuckets)
15201520

15211521
perhash->hashtable=BuildTupleHashTableExt(&aggstate->ss.ps,
15221522
perhash->hashslot->tts_tupleDescriptor,
1523+
perhash->hashslot->tts_ops,
15231524
perhash->numCols,
15241525
perhash->hashGrpColIdxHash,
15251526
perhash->eqfuncoids,

‎src/backend/executor/nodeRecursiveunion.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,10 @@ build_hash_table(RecursiveUnionState *rustate)
3737
Assert(node->numCols>0);
3838
Assert(node->numGroups>0);
3939

40+
/* XXX is it worth working a bit harder to determine the inputOps here? */
4041
rustate->hashtable=BuildTupleHashTableExt(&rustate->ps,
4142
desc,
43+
NULL,
4244
node->numCols,
4345
node->dupColIdx,
4446
rustate->eqfuncoids,

‎src/backend/executor/nodeSetOp.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ build_hash_table(SetOpState *setopstate)
128128

129129
setopstate->hashtable=BuildTupleHashTableExt(&setopstate->ps,
130130
desc,
131+
NULL,
131132
node->numCols,
132133
node->dupColIdx,
133134
setopstate->eqfuncoids,

‎src/backend/executor/nodeSubplan.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,11 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
519519
*
520520
* If it's not necessary to distinguish FALSE and UNKNOWN, then we don't
521521
* need to store subplan output rows that contain NULL.
522+
*
523+
* Because the input slot for each hash table is always the slot resulting
524+
* from an ExecProject(), we can use TTSOpsVirtual for the input ops. This
525+
* saves a needless fetch inner op step for the hashing ExprState created
526+
* in BuildTupleHashTableExt().
522527
*/
523528
MemoryContextReset(node->hashtablecxt);
524529
node->havehashrows= false;
@@ -533,6 +538,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
533538
else
534539
node->hashtable=BuildTupleHashTableExt(node->parent,
535540
node->descRight,
541+
&TTSOpsVirtual,
536542
ncols,
537543
node->keyColIdx,
538544
node->tab_eq_funcoids,
@@ -561,6 +567,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
561567
else
562568
node->hashnulls=BuildTupleHashTableExt(node->parent,
563569
node->descRight,
570+
&TTSOpsVirtual,
564571
ncols,
565572
node->keyColIdx,
566573
node->tab_eq_funcoids,

‎src/include/executor/executor.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ extern TupleHashTable BuildTupleHashTable(PlanState *parent,
140140
MemoryContexttempcxt,booluse_variable_hash_iv);
141141
externTupleHashTableBuildTupleHashTableExt(PlanState*parent,
142142
TupleDescinputDesc,
143+
constTupleTableSlotOps*inputOps,
143144
intnumCols,AttrNumber*keyColIdx,
144145
constOid*eqfuncoids,
145146
FmgrInfo*hashfunctions,

‎src/test/regress/expected/with.out

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,26 @@ SELECT * FROM subdepartment ORDER BY name;
329329
1 | 0 | A
330330
(1 row)
331331

332+
-- exercise the deduplication code of a UNION with mixed input slot types
333+
WITH RECURSIVE subdepartment AS
334+
(
335+
-- select all columns to prevent projection
336+
SELECT id, parent_department, name FROM department WHERE name = 'A'
337+
UNION
338+
-- joins do projection
339+
SELECT d.id, d.parent_department, d.name FROM department AS d
340+
INNER JOIN subdepartment AS sd ON d.parent_department = sd.id
341+
)
342+
SELECT * FROM subdepartment ORDER BY name;
343+
id | parent_department | name
344+
----+-------------------+------
345+
1 | 0 | A
346+
2 | 1 | B
347+
3 | 2 | C
348+
4 | 2 | D
349+
6 | 4 | F
350+
(5 rows)
351+
332352
-- inside subqueries
333353
SELECT count(*) FROM (
334354
WITH RECURSIVE t(n) AS (

‎src/test/regress/sql/with.sql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,20 @@ WITH RECURSIVE subdepartment AS
216216
)
217217
SELECT*FROM subdepartmentORDER BY name;
218218

219+
-- exercise the deduplication code of a UNION with mixed input slot types
220+
WITH RECURSIVE subdepartmentAS
221+
(
222+
-- select all columns to prevent projection
223+
SELECT id, parent_department, nameFROM departmentWHERE name='A'
224+
225+
UNION
226+
227+
-- joins do projection
228+
SELECTd.id,d.parent_department,d.nameFROM departmentAS d
229+
INNER JOIN subdepartmentAS sdONd.parent_department=sd.id
230+
)
231+
SELECT*FROM subdepartmentORDER BY name;
232+
219233
-- inside subqueries
220234
SELECTcount(*)FROM (
221235
WITH RECURSIVE t(n)AS (

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp