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

Commit8f29467

Browse files
committed
Change "long" numGroups fields to be Cardinality (i.e., double).
We've been nibbling away at removing uses of "long" for a long time,since its width is platform-dependent. Here's one more: change theremaining "long" fields in Plan nodes to Cardinality, since the threesurviving examples all represent group-count estimates. The upstreamplanner code was converted to Cardinality some time ago; for examplethe corresponding fields in Path nodes are type Cardinality, as arethe arguments of the make_foo_path functions. Downstream in theexecutor, it turns out that these all feed to the table-size argumentof BuildTupleHashTable. Change that to "double" as well, and fix itso that it safely clamps out-of-range values to the uint32 limit ofsimplehash.h, as was not being done before.Essentially, this is removing all the artificial datatype-dependentlimitations on these values from upstream processing, and applyingjust one clamp at the moment where we're forced to do so by thedatatype choices of simplehash.h.Also, remove BuildTupleHashTable's misguided attempt to enforcework_mem/hash_mem_limit. It doesn't have enough information(particularly not the expected tuple width) to do that accurately,and it has no real business second-guessing the caller's choice.For all these plan types, it's really the planner's responsibilityto not choose a hashed implementation if the hashtable is expectedto exceed hash_mem_limit. The previous patch improved theaccuracy of those estimates, and even if BuildTupleHashTable hadmore information it should arrive at the same conclusions.Reported-by: Jeff Janes <jeff.janes@gmail.com>Author: Tom Lane <tgl@sss.pgh.pa.us>Reviewed-by: David Rowley <dgrowleyml@gmail.com>Discussion:https://postgr.es/m/CAMkU=1zia0JfW_QR8L5xA2vpa0oqVuiapm78h=WpNsHH13_9uw@mail.gmail.com
1 parent1ea5bdb commit8f29467

File tree

11 files changed

+55
-95
lines changed

11 files changed

+55
-95
lines changed

‎src/backend/executor/execGrouping.c‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
*/
1515
#include"postgres.h"
1616

17+
#include<math.h>
18+
1719
#include"access/htup_details.h"
1820
#include"access/parallel.h"
1921
#include"common/hashfn.h"
@@ -144,7 +146,7 @@ execTuplesHashPrepare(int numCols,
144146
*eqfuncoids: OIDs of equality comparison functions to use
145147
*hashfunctions: FmgrInfos of datatype-specific hashing functions to use
146148
*collations: collations to use in comparisons
147-
*nbuckets: initial estimate of hashtable size
149+
*nelements: initial estimate of hashtable size
148150
*additionalsize: size of data that may be stored along with the hash entry
149151
*metacxt: memory context for long-lived data and the simplehash table
150152
*tuplescxt: memory context in which to store the hashed tuples themselves
@@ -187,39 +189,39 @@ BuildTupleHashTable(PlanState *parent,
187189
constOid*eqfuncoids,
188190
FmgrInfo*hashfunctions,
189191
Oid*collations,
190-
longnbuckets,
192+
doublenelements,
191193
Sizeadditionalsize,
192194
MemoryContextmetacxt,
193195
MemoryContexttuplescxt,
194196
MemoryContexttempcxt,
195197
booluse_variable_hash_iv)
196198
{
197199
TupleHashTablehashtable;
198-
Sizeentrysize;
199-
Sizehash_mem_limit;
200+
uint32nbuckets;
200201
MemoryContextoldcontext;
201202
uint32hash_iv=0;
202203

203-
Assert(nbuckets>0);
204+
/*
205+
* tuplehash_create requires a uint32 element count, so we had better
206+
* clamp the given nelements to fit in that. As long as we have to do
207+
* that, we might as well protect against completely insane input like
208+
* zero or NaN. But it is not our job here to enforce issues like staying
209+
* within hash_mem: the caller should have done that, and we don't have
210+
* enough info to second-guess.
211+
*/
212+
if (isnan(nelements)||nelements <=0)
213+
nbuckets=1;
214+
elseif (nelements >=PG_UINT32_MAX)
215+
nbuckets=PG_UINT32_MAX;
216+
else
217+
nbuckets= (uint32)nelements;
204218

205219
/* tuplescxt must be separate, else ResetTupleHashTable breaks things */
206220
Assert(metacxt!=tuplescxt);
207221

208222
/* ensure additionalsize is maxalign'ed */
209223
additionalsize=MAXALIGN(additionalsize);
210224

211-
/*
212-
* Limit initial table size request to not more than hash_mem.
213-
*
214-
* XXX this calculation seems pretty misguided, as it counts only overhead
215-
* and not the tuples themselves. But we have no knowledge of the
216-
* expected tuple width here.
217-
*/
218-
entrysize=sizeof(TupleHashEntryData)+additionalsize;
219-
hash_mem_limit=get_hash_memory_limit() /entrysize;
220-
if (nbuckets>hash_mem_limit)
221-
nbuckets=hash_mem_limit;
222-
223225
oldcontext=MemoryContextSwitchTo(metacxt);
224226

225227
hashtable= (TupleHashTable)palloc(sizeof(TupleHashTableData));

‎src/backend/executor/nodeAgg.c‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -402,12 +402,12 @@ static void find_cols(AggState *aggstate, Bitmapset **aggregated,
402402
Bitmapset**unaggregated);
403403
staticboolfind_cols_walker(Node*node,FindColsContext*context);
404404
staticvoidbuild_hash_tables(AggState*aggstate);
405-
staticvoidbuild_hash_table(AggState*aggstate,intsetno,longnbuckets);
405+
staticvoidbuild_hash_table(AggState*aggstate,intsetno,doublenbuckets);
406406
staticvoidhashagg_recompile_expressions(AggState*aggstate,boolminslot,
407407
boolnullcheck);
408408
staticvoidhash_create_memory(AggState*aggstate);
409-
staticlonghash_choose_num_buckets(doublehashentrysize,
410-
longngroups,Sizememory);
409+
staticdoublehash_choose_num_buckets(doublehashentrysize,
410+
doublengroups,Sizememory);
411411
staticinthash_choose_num_partitions(doubleinput_groups,
412412
doublehashentrysize,
413413
intused_bits,
@@ -1469,7 +1469,7 @@ build_hash_tables(AggState *aggstate)
14691469
for (setno=0;setno<aggstate->num_hashes;++setno)
14701470
{
14711471
AggStatePerHashperhash=&aggstate->perhash[setno];
1472-
longnbuckets;
1472+
doublenbuckets;
14731473
Sizememory;
14741474

14751475
if (perhash->hashtable!=NULL)
@@ -1478,8 +1478,6 @@ build_hash_tables(AggState *aggstate)
14781478
continue;
14791479
}
14801480

1481-
Assert(perhash->aggnode->numGroups>0);
1482-
14831481
memory=aggstate->hash_mem_limit /aggstate->num_hashes;
14841482

14851483
/* choose reasonable number of buckets per hashtable */
@@ -1505,7 +1503,7 @@ build_hash_tables(AggState *aggstate)
15051503
* Build a single hashtable for this grouping set.
15061504
*/
15071505
staticvoid
1508-
build_hash_table(AggState*aggstate,intsetno,longnbuckets)
1506+
build_hash_table(AggState*aggstate,intsetno,doublenbuckets)
15091507
{
15101508
AggStatePerHashperhash=&aggstate->perhash[setno];
15111509
MemoryContextmetacxt=aggstate->hash_metacxt;
@@ -2053,24 +2051,28 @@ hash_create_memory(AggState *aggstate)
20532051
/*
20542052
* Choose a reasonable number of buckets for the initial hash table size.
20552053
*/
2056-
staticlong
2057-
hash_choose_num_buckets(doublehashentrysize,longngroups,Sizememory)
2054+
staticdouble
2055+
hash_choose_num_buckets(doublehashentrysize,doublengroups,Sizememory)
20582056
{
2059-
longmax_nbuckets;
2060-
longnbuckets=ngroups;
2057+
doublemax_nbuckets;
2058+
doublenbuckets=ngroups;
20612059

20622060
max_nbuckets=memory /hashentrysize;
20632061

20642062
/*
20652063
* Underestimating is better than overestimating. Too many buckets crowd
20662064
* out space for group keys and transition state values.
20672065
*/
2068-
max_nbuckets>>=1;
2066+
max_nbuckets/=2;
20692067

20702068
if (nbuckets>max_nbuckets)
20712069
nbuckets=max_nbuckets;
20722070

2073-
returnMax(nbuckets,1);
2071+
/*
2072+
* BuildTupleHashTable will clamp any obviously-insane result, so we don't
2073+
* need to be too careful here.
2074+
*/
2075+
returnnbuckets;
20742076
}
20752077

20762078
/*
@@ -3686,7 +3688,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
36863688
if (use_hashing)
36873689
{
36883690
Plan*outerplan=outerPlan(node);
3689-
uint64totalGroups=0;
3691+
doubletotalGroups=0;
36903692

36913693
aggstate->hash_spill_rslot=ExecInitExtraTupleSlot(estate,scanDesc,
36923694
&TTSOpsMinimalTuple);

‎src/backend/executor/nodeRecursiveunion.c‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ build_hash_table(RecursiveUnionState *rustate)
3535
TupleDescdesc=ExecGetResultType(outerPlanState(rustate));
3636

3737
Assert(node->numCols>0);
38-
Assert(node->numGroups>0);
3938

4039
/*
4140
* If both child plans deliver the same fixed tuple slot type, we can tell

‎src/backend/executor/nodeSetOp.c‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ build_hash_table(SetOpState *setopstate)
8888
TupleDescdesc=ExecGetResultType(outerPlanState(setopstate));
8989

9090
Assert(node->strategy==SETOP_HASHED);
91-
Assert(node->numGroups>0);
9291

9392
/*
9493
* If both child plans deliver the same fixed tuple slot type, we can tell

‎src/backend/executor/nodeSubplan.c‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
#include"miscadmin.h"
3535
#include"nodes/makefuncs.h"
3636
#include"nodes/nodeFuncs.h"
37-
#include"optimizer/optimizer.h"
3837
#include"utils/array.h"
3938
#include"utils/lsyscache.h"
4039
#include"utils/memutils.h"
@@ -481,7 +480,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
481480
intncols=node->numCols;
482481
ExprContext*innerecontext=node->innerecontext;
483482
MemoryContextoldcontext;
484-
longnbuckets;
483+
doublenentries;
485484
TupleTableSlot*slot;
486485

487486
Assert(subplan->subLinkType==ANY_SUBLINK);
@@ -509,9 +508,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
509508
node->havehashrows= false;
510509
node->havenullrows= false;
511510

512-
nbuckets=clamp_cardinality_to_long(planstate->plan->plan_rows);
513-
if (nbuckets<1)
514-
nbuckets=1;
511+
nentries=planstate->plan->plan_rows;
515512

516513
if (node->hashtable)
517514
ResetTupleHashTable(node->hashtable);
@@ -524,7 +521,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
524521
node->tab_eq_funcoids,
525522
node->tab_hash_funcs,
526523
node->tab_collations,
527-
nbuckets,
524+
nentries,
528525
0,/* no additional data */
529526
node->planstate->state->es_query_cxt,
530527
node->tuplesContext,
@@ -534,12 +531,12 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
534531
if (!subplan->unknownEqFalse)
535532
{
536533
if (ncols==1)
537-
nbuckets=1;/* there can only be one entry */
534+
nentries=1;/* there can only be one entry */
538535
else
539536
{
540-
nbuckets /=16;
541-
if (nbuckets<1)
542-
nbuckets=1;
537+
nentries /=16;
538+
if (nentries<1)
539+
nentries=1;
543540
}
544541

545542
if (node->hashnulls)
@@ -553,7 +550,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
553550
node->tab_eq_funcoids,
554551
node->tab_hash_funcs,
555552
node->tab_collations,
556-
nbuckets,
553+
nentries,
557554
0,/* no additional data */
558555
node->planstate->state->es_query_cxt,
559556
node->tuplesContext,

‎src/backend/optimizer/path/costsize.c‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -257,32 +257,6 @@ clamp_width_est(int64 tuple_width)
257257
return (int32)tuple_width;
258258
}
259259

260-
/*
261-
* clamp_cardinality_to_long
262-
*Cast a Cardinality value to a sane long value.
263-
*/
264-
long
265-
clamp_cardinality_to_long(Cardinalityx)
266-
{
267-
/*
268-
* Just for paranoia's sake, ensure we do something sane with negative or
269-
* NaN values.
270-
*/
271-
if (isnan(x))
272-
returnLONG_MAX;
273-
if (x <=0)
274-
return0;
275-
276-
/*
277-
* If "long" is 64 bits, then LONG_MAX cannot be represented exactly as a
278-
* double. Casting it to double and back may well result in overflow due
279-
* to rounding, so avoid doing that. We trust that any double value that
280-
* compares strictly less than "(double) LONG_MAX" will cast to a
281-
* representable "long" value.
282-
*/
283-
return (x< (double)LONG_MAX) ? (long)x :LONG_MAX;
284-
}
285-
286260

287261
/*
288262
* cost_seqscan

‎src/backend/optimizer/plan/createplan.c‎

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ static RecursiveUnion *make_recursive_union(List *tlist,
226226
Plan*righttree,
227227
intwtParam,
228228
List*distinctList,
229-
longnumGroups);
229+
CardinalitynumGroups);
230230
staticBitmapAnd*make_bitmap_and(List*bitmapplans);
231231
staticBitmapOr*make_bitmap_or(List*bitmapplans);
232232
staticNestLoop*make_nestloop(List*tlist,
@@ -301,7 +301,7 @@ static Gather *make_gather(List *qptlist, List *qpqual,
301301
intnworkers,intrescan_param,boolsingle_copy,Plan*subplan);
302302
staticSetOp*make_setop(SetOpCmdcmd,SetOpStrategystrategy,
303303
List*tlist,Plan*lefttree,Plan*righttree,
304-
List*groupList,longnumGroups);
304+
List*groupList,CardinalitynumGroups);
305305
staticLockRows*make_lockrows(Plan*lefttree,List*rowMarks,intepqParam);
306306
staticResult*make_gating_result(List*tlist,Node*resconstantqual,
307307
Plan*subplan);
@@ -2564,7 +2564,6 @@ create_setop_plan(PlannerInfo *root, SetOpPath *best_path, int flags)
25642564
List*tlist=build_path_tlist(root,&best_path->path);
25652565
Plan*leftplan;
25662566
Plan*rightplan;
2567-
longnumGroups;
25682567

25692568
/*
25702569
* SetOp doesn't project, so tlist requirements pass through; moreover we
@@ -2575,16 +2574,13 @@ create_setop_plan(PlannerInfo *root, SetOpPath *best_path, int flags)
25752574
rightplan=create_plan_recurse(root,best_path->rightpath,
25762575
flags |CP_LABEL_TLIST);
25772576

2578-
/* Convert numGroups to long int --- but 'ware overflow! */
2579-
numGroups=clamp_cardinality_to_long(best_path->numGroups);
2580-
25812577
plan=make_setop(best_path->cmd,
25822578
best_path->strategy,
25832579
tlist,
25842580
leftplan,
25852581
rightplan,
25862582
best_path->groupList,
2587-
numGroups);
2583+
best_path->numGroups);
25882584

25892585
copy_generic_path_info(&plan->plan, (Path*)best_path);
25902586

@@ -2604,23 +2600,19 @@ create_recursiveunion_plan(PlannerInfo *root, RecursiveUnionPath *best_path)
26042600
Plan*leftplan;
26052601
Plan*rightplan;
26062602
List*tlist;
2607-
longnumGroups;
26082603

26092604
/* Need both children to produce same tlist, so force it */
26102605
leftplan=create_plan_recurse(root,best_path->leftpath,CP_EXACT_TLIST);
26112606
rightplan=create_plan_recurse(root,best_path->rightpath,CP_EXACT_TLIST);
26122607

26132608
tlist=build_path_tlist(root,&best_path->path);
26142609

2615-
/* Convert numGroups to long int --- but 'ware overflow! */
2616-
numGroups=clamp_cardinality_to_long(best_path->numGroups);
2617-
26182610
plan=make_recursive_union(tlist,
26192611
leftplan,
26202612
rightplan,
26212613
best_path->wtParam,
26222614
best_path->distinctList,
2623-
numGroups);
2615+
best_path->numGroups);
26242616

26252617
copy_generic_path_info(&plan->plan, (Path*)best_path);
26262618

@@ -5845,7 +5837,7 @@ make_recursive_union(List *tlist,
58455837
Plan*righttree,
58465838
intwtParam,
58475839
List*distinctList,
5848-
longnumGroups)
5840+
CardinalitynumGroups)
58495841
{
58505842
RecursiveUnion*node=makeNode(RecursiveUnion);
58515843
Plan*plan=&node->plan;
@@ -6582,15 +6574,11 @@ Agg *
65826574
make_agg(List*tlist,List*qual,
65836575
AggStrategyaggstrategy,AggSplitaggsplit,
65846576
intnumGroupCols,AttrNumber*grpColIdx,Oid*grpOperators,Oid*grpCollations,
6585-
List*groupingSets,List*chain,doubledNumGroups,
6577+
List*groupingSets,List*chain,CardinalitynumGroups,
65866578
SizetransitionSpace,Plan*lefttree)
65876579
{
65886580
Agg*node=makeNode(Agg);
65896581
Plan*plan=&node->plan;
6590-
longnumGroups;
6591-
6592-
/* Reduce to long, but 'ware overflow! */
6593-
numGroups=clamp_cardinality_to_long(dNumGroups);
65946582

65956583
node->aggstrategy=aggstrategy;
65966584
node->aggsplit=aggsplit;
@@ -6822,7 +6810,7 @@ make_gather(List *qptlist,
68226810
staticSetOp*
68236811
make_setop(SetOpCmdcmd,SetOpStrategystrategy,
68246812
List*tlist,Plan*lefttree,Plan*righttree,
6825-
List*groupList,longnumGroups)
6813+
List*groupList,CardinalitynumGroups)
68266814
{
68276815
SetOp*node=makeNode(SetOp);
68286816
Plan*plan=&node->plan;

‎src/include/executor/executor.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ extern TupleHashTable BuildTupleHashTable(PlanState *parent,
138138
constOid*eqfuncoids,
139139
FmgrInfo*hashfunctions,
140140
Oid*collations,
141-
longnbuckets,
141+
doublenelements,
142142
Sizeadditionalsize,
143143
MemoryContextmetacxt,
144144
MemoryContexttuplescxt,

0 commit comments

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