On disable_cost
Hi,
Postgres has a global variable `disable_cost`. It is set the value
1.0e10.
This value will be added to the cost of path if related GUC is set off.
For example,
if enable_nestloop is set off, when planner trys to add nestloop join
path, it continues
to add such path but with a huge cost `disable_cost`.
But 1.0e10 may not be large enough. I encounter this issue in
Greenplum(based on postgres).
Heikki tolds me that someone also encountered the same issue on Postgres.
So I send it here to
have a discussion.
My issue: I did some spikes and tests on TPCDS 1TB Bytes data. For
query 104, it generates
nestloop join even with enable_nestloop set off. And the final plan's
total cost is very huge (about 1e24). But If I enlarge the disable_cost to
1e30, then, planner will generate hash join.
So I guess that disable_cost is not large enough for huge amount of
data.
It is tricky to set disable_cost a huge number. Can we come up with
better solution?
The following thoughts are from Heikki:
Aside from not having a large enough disable cost, there's also the
fact that the high cost might affect the rest of the plan, if we have to
use a plan type that's disabled. For example, if a table doesn't have any
indexes, but enable_seqscan is off, we might put the unavoidable Seq Scan
on different side of a join than we we would with enable_seqscan=on,
because of the high cost estimate.
I think a more robust way to disable forbidden plan types would be to
handle the disabling in add_path(). Instead of having a high disable cost
on the Path itself, the comparison add_path() would always consider
disabled paths as more expensive than others, regardless of the cost.
Any thoughts or ideas on the problem? Thanks!
Best Regards,
Zhenghua Lyu
On Fri, Nov 1, 2019 at 7:42 PM Zhenghua Lyu <zlv@pivotal.io> wrote:
It is tricky to set disable_cost a huge number. Can we come up with better solution?
What happens if you use DBL_MAX?
Em sex, 1 de nov de 2019 às 03:42, Zhenghua Lyu <zlv@pivotal.io> escreveu:
My issue: I did some spikes and tests on TPCDS 1TB Bytes data. For query 104, it generates
nestloop join even with enable_nestloop set off. And the final plan's total cost is very huge (about 1e24). But If I enlarge the disable_cost to 1e30, then, planner will generate hash join.So I guess that disable_cost is not large enough for huge amount of data.
It is tricky to set disable_cost a huge number. Can we come up with better solution?
Isn't it a case for a GUC disable_cost? As Thomas suggested, DBL_MAX
upper limit should be sufficient.
The following thoughts are from Heikki:
Aside from not having a large enough disable cost, there's also the fact that the high cost might affect the rest of the plan, if we have to use a plan type that's disabled. For example, if a table doesn't have any indexes, but enable_seqscan is off, we might put the unavoidable Seq Scan on different side of a join than we we would with enable_seqscan=on, because of the high cost estimate.
I think a more robust way to disable forbidden plan types would be to handle the disabling in add_path(). Instead of having a high disable cost on the Path itself, the comparison add_path() would always consider disabled paths as more expensive than others, regardless of the cost.
I'm afraid it is not as cheap as using diable_cost as a node cost. Are
you proposing to add a new boolean variable in Path struct to handle
those cases in compare_path_costs_fuzzily?
--
Euler Taveira Timbira -
http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
Hi,
On 2019-11-01 19:58:04 +1300, Thomas Munro wrote:
On Fri, Nov 1, 2019 at 7:42 PM Zhenghua Lyu <zlv@pivotal.io> wrote:
It is tricky to set disable_cost a huge number. Can we come up with better solution?
What happens if you use DBL_MAX?
That seems like a bad idea - we add the cost multiple times. And we
still want to compare plans that potentially involve that cost, if
there's no other way to plan the query.
- Andres
On Fri, Nov 1, 2019 at 12:00 PM Andres Freund <andres@anarazel.de> wrote:
That seems like a bad idea - we add the cost multiple times. And we
still want to compare plans that potentially involve that cost, if
there's no other way to plan the query.
Yeah. I kind of wonder if we shouldn't instead (a) skip adding paths
that use methods which are disabled and then (b) if we don't end up
with any paths for that reloptinfo, try again, ignoring disabling
GUCs.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
re: coping with adding disable_cost more than once
Another option would be to have a 2-part Cost structure. If disable_cost is
ever added to the Cost, then you set a flag recording this. If any plans
exist that have no disable_costs added to them, then the planner chooses the
minimum cost among those, otherwise you choose the minimum cost path.
-----
Jim Finnerty, AWS, Amazon Aurora PostgreSQL
--
Sent from: https://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html
Hi,
On 2019-11-01 12:22:06 -0400, Robert Haas wrote:
On Fri, Nov 1, 2019 at 12:00 PM Andres Freund <andres@anarazel.de> wrote:
That seems like a bad idea - we add the cost multiple times. And we
still want to compare plans that potentially involve that cost, if
there's no other way to plan the query.Yeah. I kind of wonder if we shouldn't instead (a) skip adding paths
that use methods which are disabled and then (b) if we don't end up
with any paths for that reloptinfo, try again, ignoring disabling
GUCs.
Hm. That seems complicated. Is it clear that we'd always notice that we
have no plan early enough to know which paths to reconsider? I think
there's cases where that'd only happen a few levels up.
As a first step I'd be inclined to "just" adjust disable_cost up to
something like 1.0e12. Unfortunately much higher and and we're getting
into the area where the loss of precision starts to be significant
enough that I'm not sure that we're always careful enough to perform
math in the right order (e.g. 1.0e16 + 1 being 1.0e16, and 1e+20 + 1000
being 1e+20). I've seen queries with costs above 1e10 where that costing
wasn't insane.
And then, in a larger patch, go for something like Heikki's proposal
quoted by Zhenghua Lyu upthread, where we treat 'forbidden' as a
separate factor in comparisons of path costs, rather than fudging the
cost upwards. But there's some care to be taken to make sure we don't
regress performance too much due to the additional logic in
compare_path_cost et al.
I'd also be curious to see if there's some other problem with cost
calculation here - some of the quoted final costs seem high enough to be
suspicious. I'd be curious to see a plan...
Greetings,
Andres Freund
On Fri, Nov 1, 2019 at 12:43 PM Andres Freund <andres@anarazel.de> wrote:
Hm. That seems complicated. Is it clear that we'd always notice that we
have no plan early enough to know which paths to reconsider? I think
there's cases where that'd only happen a few levels up.
Yeah, there could be problems of that kind. I think if a baserel has
no paths, then we know right away that we've got a problem, but for
joinrels it might be more complicated.
As a first step I'd be inclined to "just" adjust disable_cost up to
something like 1.0e12. Unfortunately much higher and and we're getting
into the area where the loss of precision starts to be significant
enough that I'm not sure that we're always careful enough to perform
math in the right order (e.g. 1.0e16 + 1 being 1.0e16, and 1e+20 + 1000
being 1e+20). I've seen queries with costs above 1e10 where that costing
wasn't insane.
We've done that before and we can do it again. But we're going to need
to have something better eventually, I think, not just keep kicking
the can down the road.
Another point to consider here is that in some cases we could really
just skip generating certain paths altogether. We already do this for
hash joins: if we're planning a join and enable_hashjoin is disabled,
we just don't generate hash joins paths at all, except for full joins,
where there might be no other legal method. As this example shows,
this cannot be applied in all cases, but maybe we could do it more
widely than we do today. I'm not sure how beneficial that technique
would be, though, because it doesn't seem like it's quite enough to
solve this problem by itself.
Yet another approach would be to divide the cost into two parts, a
"cost" component and a "violations" component. If two paths are
compared, the one with fewer violations always wins; if it's a tie,
they compare on cost. A path's violation count is the total of its
children, plus one for itself if it does something that's disabled.
This would be more principled than the current approach, but maybe
it's too costly.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Fri, Nov 01, 2019 at 09:30:52AM -0700, Jim Finnerty wrote:
re: coping with adding disable_cost more than once
Another option would be to have a 2-part Cost structure. If disable_cost is
ever added to the Cost, then you set a flag recording this. If any plans
exist that have no disable_costs added to them, then the planner chooses the
minimum cost among those, otherwise you choose the minimum cost path.
Yeah, I agree having is_disabled flag, and treat all paths with 'true'
as more expensive than paths with 'false' (and when both paths have the
same value then actually compare the cost) is probably the way forward.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019-11-01 12:56:30 -0400, Robert Haas wrote:
On Fri, Nov 1, 2019 at 12:43 PM Andres Freund <andres@anarazel.de> wrote:
As a first step I'd be inclined to "just" adjust disable_cost up to
something like 1.0e12. Unfortunately much higher and and we're getting
into the area where the loss of precision starts to be significant
enough that I'm not sure that we're always careful enough to perform
math in the right order (e.g. 1.0e16 + 1 being 1.0e16, and 1e+20 + 1000
being 1e+20). I've seen queries with costs above 1e10 where that costing
wasn't insane.We've done that before and we can do it again. But we're going to need
to have something better eventually, I think, not just keep kicking
the can down the road.
Yea, that's why I continued on to describe what we should do afterwards
;)
Yet another approach would be to divide the cost into two parts, a
"cost" component and a "violations" component. If two paths are
compared, the one with fewer violations always wins; if it's a tie,
they compare on cost. A path's violation count is the total of its
children, plus one for itself if it does something that's disabled.
This would be more principled than the current approach, but maybe
it's too costly.
Namely go for something like this. I think we probably get away with the
additional comparison, especially if we were to store the violations as
an integer and did it like if (unlikely(path1->nviolations !=
path2->nviolations)) or such - that ought to be very well predicted in
nearly all cases.
I wonder how much we'd need to reformulate
compare_path_costs/compare_path_costs_fuzzily to allow the compiler to
auto-vectorize. Might not be worth caring...
Greetings,
Andres Freund
Zhenghua Lyu <zlv@pivotal.io> writes:
I think a more robust way to disable forbidden plan types would be to
handle the disabling in add_path(). Instead of having a high disable cost
on the Path itself, the comparison add_path() would always consider
disabled paths as more expensive than others, regardless of the cost.
Getting rid of disable_cost would be a nice thing to do, but I would
rather not do it by adding still more complexity to add_path(), not
to mention having to bloat Paths with a separate "disabled" marker.
The idea that I've been thinking about is to not generate disabled
Paths in the first place, thus not only fixing the problem but saving
some cycles. While this seems easy enough for "optional" paths,
we have to reserve the ability to generate certain path types regardless,
if there's no other way to implement the query. This is a bit of a
stumbling block :-(. At the base relation level, we could do something
like generating seqscan last, and only if no other path has been
successfully generated. But I'm not sure how to scale that up to
joins. In particular, imagine that we consider joining A to B, and
find that the only way is a nestloop, so we generate a nestloop join
despite that being nominally disabled. The next join level would
then see that as an available path, and it might decide that
((A nestjoin B) join C) is the cheapest choice, even though there
might have been a way to do, say, ((A join C) join B) with no use of
nestloops. Users would find this surprising.
Maybe the only way to do this is a separate number-of-uses-of-
disabled-plan-types cost figure in Paths, but I still don't want
to go there. The number of cases where disable_cost's shortcomings
really matter is too small to justify that, IMHO.
regards, tom lane
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
On Fri, Nov 01, 2019 at 09:30:52AM -0700, Jim Finnerty wrote:
re: coping with adding disable_cost more than once
Another option would be to have a 2-part Cost structure. If disable_cost is
ever added to the Cost, then you set a flag recording this. If any plans
exist that have no disable_costs added to them, then the planner chooses the
minimum cost among those, otherwise you choose the minimum cost path.
Yeah, I agree having is_disabled flag, and treat all paths with 'true'
as more expensive than paths with 'false' (and when both paths have the
same value then actually compare the cost) is probably the way forward.
It would have to be a count, not a boolean --- for example, you want to
prefer a path that uses one disabled SeqScan over a path that uses two.
I'm with Andres in being pretty worried about the extra burden imposed
on add_path comparisons.
regards, tom lane
As a proof of concept, I hacked around a bit today to re-purpose one of the
bits of the Cost structure to mean "is_disabled" so that we can distinguish
'disabled' from 'non-disabled' paths without making the Cost structure any
bigger. In fact, it's still a valid double. The obvious choice would have
been to re-purpose the sign bit, but I've had occasion to exploit negative
costs before so for this POC I used the high-order bit of the fractional
bits of the double. (see Wikipedia for double precision floating point for
the layout).
The idea is to set a special bit when disable_cost is added to a cost.
Dedicating multiple bits instead of just 1 would be easily done, but as it
is we can accumulate many disable_costs without overflowing, so just
comparing the cost suffices.
The patch is not fully debugged and fails on a couple of tests in the serial
test suite. It seems to fail on Cartesian products, and maybe in one other
non-CP case. I wasn't able to debug it before the day came to an end.
In one place the core code subtracts off the disable_cost. I left the
"disabled" bit set in this case, which might be wrong.
I don't see an option to attach the patch as an attachment, so here is the
patch inline (it is based on PG11). The more interesting part is in a small
number of lines in costsize.c. Other changes just add functions that assign
a disable_cost and set the bit, or that compare costs such that a
non-disabled cost always compares less than a disabled cost.
------------------
diff --git a/src/backend/optimizer/path/costsize.c
b/src/backend/optimizer/path/costsize.c
index 4e86458672..3718639330 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -123,6 +123,8 @@ double parallel_setup_cost =
DEFAULT_PARALLEL_SETUP_COST;
int effective_cache_size = DEFAULT_EFFECTIVE_CACHE_SIZE; Cost disable_cost = 1.0e10;
+uint64 disabled_mask = 0x8000000000000;
+#define IS_DISABLED(cost) (((uint64) cost) & disabled_mask)int max_parallel_workers_per_gather = 2;
@@ -205,6 +207,53 @@ clamp_row_est(double nrows)
return nrows;
}
+Cost
+add_cost(Cost cost, Cost delta_cost)
+{
+ uint64 mask = (delta_cost == disable_cost) ? disabled_mask : 0;
+ Cost max_cost = disabled_mask - disable_cost;
+
+ if (cost + delta_cost < max_cost)
+ return ((Cost) ((uint64)(cost + delta_cost) | mask));
+ else
+ return ((Cost) ((uint64)(max_cost) | mask));
+}
+
+bool
+is_lower_cost(Cost cost1, Cost cost2)
+{
+ if ((uint64)cost1 & disabled_mask && !((uint64)cost2 & disabled_mask))
+ return false;
+
+ if (!((uint64)cost1 & disabled_mask) && (uint64)cost2 & disabled_mask)
+ return true;
+
+ return (cost1 < cost2);
+}
+
+bool
+is_greater_cost(Cost cost1, Cost cost2)
+{
+ if ((uint64)cost1 & disabled_mask && !((uint64)cost2 & disabled_mask))
+ return true;
+
+ if (!((uint64)cost1 & disabled_mask) && (uint64)cost2 & disabled_mask)
+ return false;
+
+ return (cost1 > cost2);
+}
+
+bool
+is_geq_cost(Cost cost1, Cost cost2)
+{
+ if ((uint64)cost1 & disabled_mask && !((uint64)cost2 & disabled_mask))
+ return true;
+
+ if (!((uint64)cost1 & disabled_mask) && (uint64)cost2 & disabled_mask)
+ return false;
+
+ return (cost1 >= cost2);
+} /*
* cost_seqscan
@@ -235,7 +284,7 @@ cost_seqscan(Path *path, PlannerInfo *root,
path->rows = baserel->rows;
if (!enable_seqscan)
- startup_cost += disable_cost;
+ startup_cost = add_cost(startup_cost, disable_cost); /* fetch estimated page cost for tablespace containing table */
get_tablespace_page_costs(baserel->reltablespace,
@@ -424,7 +473,7 @@ cost_gather_merge(GatherMergePath *path, PlannerInfo
*root,
path->path.rows = rel->rows;
if (!enable_gathermerge)
- startup_cost += disable_cost;
+ startup_cost = add_cost(startup_cost, disable_cost); /*
* Add one to the number of workers to account for the leader. This might
@@ -538,7 +587,7 @@ cost_index(IndexPath *path, PlannerInfo *root, double
loop_count,
}
if (!enable_indexscan)
- startup_cost += disable_cost;
+ startup_cost = add_cost(startup_cost, disable_cost);
/* we don't need to check enable_indexonlyscan; indxpath.c does that */ /*
@@ -976,7 +1025,7 @@ cost_bitmap_heap_scan(Path *path, PlannerInfo *root,
RelOptInfo *baserel,
path->rows = baserel->rows;
if (!enable_bitmapscan)
- startup_cost += disable_cost;
+ startup_cost = add_cost(startup_cost, disable_cost); pages_fetched = compute_bitmap_pages(root, baserel, bitmapqual,
loop_count, &indexTotalCost,
@@ -1242,10 +1291,10 @@ cost_tidscan(Path *path, PlannerInfo *root,
if (isCurrentOf)
{
Assert(baserel->baserestrictcost.startup >= disable_cost);
- startup_cost -= disable_cost;
+ startup_cost -= disable_cost; /* but do not un-set the disabled mark */
}
else if (!enable_tidscan)
- startup_cost += disable_cost;
+ startup_cost = add_cost(startup_cost, disable_cost); /*
* The TID qual expressions will be computed once, any other baserestrict
@@ -1676,7 +1725,7 @@ cost_sort(Path *path, PlannerInfo *root,
long sort_mem_bytes = sort_mem * 1024L;
if (!enable_sort)
- startup_cost += disable_cost;
+ startup_cost = add_cost(startup_cost, disable_cost);path->rows = tuples;
@@ -2121,8 +2170,8 @@ cost_agg(Path *path, PlannerInfo *root,
total_cost = input_total_cost;
if (aggstrategy == AGG_MIXED && !enable_hashagg)
{
- startup_cost += disable_cost;
- total_cost += disable_cost;
+ startup_cost = add_cost(startup_cost, disable_cost);
+ total_cost = add_cost(total_cost, disable_cost);
}
/* calcs phrased this way to match HASHED case, see note above */
total_cost += aggcosts->transCost.startup;
@@ -2137,7 +2186,7 @@ cost_agg(Path *path, PlannerInfo *root,
/* must be AGG_HASHED */
startup_cost = input_total_cost;
if (!enable_hashagg)
- startup_cost += disable_cost;
+ startup_cost = add_cost(startup_cost, disable_cost);
startup_cost += aggcosts->transCost.startup;
startup_cost += aggcosts->transCost.per_tuple * input_tuples;
startup_cost += (cpu_operator_cost * numGroupCols) * input_tuples;
@@ -2436,7 +2485,7 @@ final_cost_nestloop(PlannerInfo *root, NestPath *path,
* disabled, which doesn't seem like the way to bet.
*/
if (!enable_nestloop)
- startup_cost += disable_cost;
+ startup_cost = add_cost(startup_cost, disable_cost);/* cost of inner-relation source data (we already dealt with outer rel) */
@@ -2882,7 +2931,7 @@ final_cost_mergejoin(PlannerInfo *root, MergePath
*path,
* disabled, which doesn't seem like the way to bet.
*/
if (!enable_mergejoin)
- startup_cost += disable_cost;
+ startup_cost = add_cost(startup_cost, disable_cost); /*
* Compute cost of the mergequals and qpquals (other restriction clauses)
@@ -3312,7 +3361,7 @@ final_cost_hashjoin(PlannerInfo *root, HashPath *path,
* disabled, which doesn't seem like the way to bet.
*/
if (!enable_hashjoin)
- startup_cost += disable_cost;
+ startup_cost = add_cost(startup_cost, disable_cost); /* mark the path with estimated # of batches */
path->num_batches = numbatches;
@@ -3410,7 +3459,7 @@ final_cost_hashjoin(PlannerInfo *root, HashPath *path,
if (relation_byte_size(clamp_row_est(inner_path_rows * innermcvfreq),
inner_path->pathtarget->width) >
(work_mem * 1024L))
- startup_cost += disable_cost;
+ startup_cost = add_cost(startup_cost, disable_cost); /*
* Compute cost of the hashquals and qpquals (other restriction clauses)
@@ -3930,7 +3979,7 @@ cost_qual_eval_walker(Node *node,
cost_qual_eval_context *context)
else if (IsA(node, CurrentOfExpr))
{
/* Report high cost to prevent selection of anything but TID scan */
- context->total.startup += disable_cost;
+ context->total.startup = add_cost(context->total.startup, disable_cost);
}
else if (IsA(node, SubLink))
{
diff --git a/src/backend/optimizer/util/pathnode.c
b/src/backend/optimizer/util/pathnode.c
index 4736d84a83..fd746a06bc 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -72,33 +72,33 @@ compare_path_costs(Path *path1, Path *path2,
CostSelector criterion)
{
if (criterion == STARTUP_COST)
{
- if (path1->startup_cost < path2->startup_cost)
+ if (is_lower_cost(path1->startup_cost, path2->startup_cost))
return -1;
- if (path1->startup_cost > path2->startup_cost)
+ if (is_greater_cost(path1->startup_cost, path2->startup_cost))
return +1; /*
* If paths have the same startup cost (not at all unlikely), order
* them by total cost.
*/
- if (path1->total_cost < path2->total_cost)
+ if (is_lower_cost(path1->total_cost, path2->total_cost))
return -1;
- if (path1->total_cost > path2->total_cost)
+ if (is_greater_cost(path1->total_cost, path2->total_cost))
return +1;
}
else
{
- if (path1->total_cost < path2->total_cost)
+ if (is_lower_cost(path1->total_cost, path2->total_cost))
return -1;
- if (path1->total_cost > path2->total_cost)
+ if (is_greater_cost(path1->total_cost, path2->total_cost))
return +1; /*
* If paths have the same total cost, order them by startup cost.
*/
- if (path1->startup_cost < path2->startup_cost)
+ if (is_lower_cost(path1->startup_cost, path2->startup_cost))
return -1;
- if (path1->startup_cost > path2->startup_cost)
+ if (is_greater_cost(path1->startup_cost, path2->startup_cost))
return +1;
}
return 0;
@@ -126,9 +126,9 @@ compare_fractional_path_costs(Path *path1, Path *path2,
fraction * (path1->total_cost - path1->startup_cost);
cost2 = path2->startup_cost +
fraction * (path2->total_cost - path2->startup_cost);
- if (cost1 < cost2)
+ if (is_lower_cost(cost1, cost2))
return -1;
- if (cost1 > cost2)
+ if (is_greater_cost(cost1, cost2))
return +1;
return 0;
}
@@ -172,11 +172,11 @@ compare_path_costs_fuzzily(Path *path1, Path *path2,
double fuzz_factor)
* Check total cost first since it's more likely to be different; many
* paths have zero startup cost.
*/
- if (path1->total_cost > path2->total_cost * fuzz_factor)
+ if (is_greater_cost(path1->total_cost, path2->total_cost * fuzz_factor))
{
/* path1 fuzzily worse on total cost */
if (CONSIDER_PATH_STARTUP_COST(path1) &&
- path2->startup_cost > path1->startup_cost * fuzz_factor)
+ is_greater_cost(path2->startup_cost, path1->startup_cost * fuzz_factor))
{
/* ... but path2 fuzzily worse on startup, so DIFFERENT */
return COSTS_DIFFERENT;
@@ -184,11 +184,11 @@ compare_path_costs_fuzzily(Path *path1, Path *path2,
double fuzz_factor)
/* else path2 dominates */
return COSTS_BETTER2;
}
- if (path2->total_cost > path1->total_cost * fuzz_factor)
+ if (is_greater_cost(path2->total_cost, path1->total_cost * fuzz_factor))
{
/* path2 fuzzily worse on total cost */
if (CONSIDER_PATH_STARTUP_COST(path2) &&
- path1->startup_cost > path2->startup_cost * fuzz_factor)
+ is_greater_cost(path1->startup_cost, path2->startup_cost * fuzz_factor))
{
/* ... but path1 fuzzily worse on startup, so DIFFERENT */
return COSTS_DIFFERENT;
@@ -197,12 +197,12 @@ compare_path_costs_fuzzily(Path *path1, Path *path2,
double fuzz_factor)
return COSTS_BETTER1;
}
/* fuzzily the same on total cost ... */
- if (path1->startup_cost > path2->startup_cost * fuzz_factor)
+ if (is_greater_cost(path1->startup_cost, path2->startup_cost *
fuzz_factor))
{
/* ... but path1 fuzzily worse on startup, so path2 wins */
return COSTS_BETTER2;
}
- if (path2->startup_cost > path1->startup_cost * fuzz_factor)
+ if (is_greater_cost(path2->startup_cost, path1->startup_cost *
fuzz_factor))
{
/* ... but path2 fuzzily worse on startup, so path1 wins */
return COSTS_BETTER1;
@@ -605,7 +605,7 @@ add_path(RelOptInfo *parent_rel, Path *new_path)
else
{
/* new belongs after this old path if it has cost >= old's */
- if (new_path->total_cost >= old_path->total_cost)
+ if (is_geq_cost(new_path->total_cost, old_path->total_cost))
insert_after = p1;
/* p1_prev advances */
p1_prev = p1;
@@ -681,7 +681,7 @@ add_path_precheck(RelOptInfo *parent_rel,
*
* Cost comparisons here should match compare_path_costs_fuzzily.
*/
- if (total_cost > old_path->total_cost * STD_FUZZ_FACTOR)
+ if (is_greater_cost(total_cost, old_path->total_cost * STD_FUZZ_FACTOR))
{
/* new path can win on startup cost only if consider_startup */
if (startup_cost > old_path->startup_cost * STD_FUZZ_FACTOR ||
@@ -796,14 +796,14 @@ add_partial_path(RelOptInfo *parent_rel, Path
*new_path)
/* Unless pathkeys are incompable, keep just one of the two paths. */
if (keyscmp != PATHKEYS_DIFFERENT)
{
- if (new_path->total_cost > old_path->total_cost * STD_FUZZ_FACTOR)
+ if (is_greater_cost(new_path->total_cost, old_path->total_cost *
STD_FUZZ_FACTOR))
{
/* New path costs more; keep it only if pathkeys are better. */
if (keyscmp != PATHKEYS_BETTER1)
accept_new = false;
}
- else if (old_path->total_cost > new_path->total_cost
- * STD_FUZZ_FACTOR)
+ else if (is_greater_cost(old_path->total_cost, new_path->total_cost
+ * STD_FUZZ_FACTOR))
{
/* Old path costs more; keep it only if pathkeys are better. */
if (keyscmp != PATHKEYS_BETTER2)
@@ -819,7 +819,7 @@ add_partial_path(RelOptInfo *parent_rel, Path *new_path)
/* Costs are about the same, old path has better pathkeys. */
accept_new = false;
}
- else if (old_path->total_cost > new_path->total_cost * 1.0000000001)
+ else if (is_greater_cost(old_path->total_cost, new_path->total_cost *
1.0000000001))
{
/* Pathkeys are the same, and the old path costs more. */
remove_old = true;
@@ -847,7 +847,7 @@ add_partial_path(RelOptInfo *parent_rel, Path *new_path)
else
{
/* new belongs after this old path if it has cost >= old's */
- if (new_path->total_cost >= old_path->total_cost)
+ if (is_geq_cost(new_path->total_cost, old_path->total_cost))
insert_after = p1;
/* p1_prev advances */
p1_prev = p1;
@@ -913,10 +913,10 @@ add_partial_path_precheck(RelOptInfo *parent_rel, Cost
total_cost,
keyscmp = compare_pathkeys(pathkeys, old_path->pathkeys);
if (keyscmp != PATHKEYS_DIFFERENT)
{
- if (total_cost > old_path->total_cost * STD_FUZZ_FACTOR &&
+ if (is_greater_cost(total_cost, old_path->total_cost * STD_FUZZ_FACTOR)
&&
keyscmp != PATHKEYS_BETTER1)
return false;
- if (old_path->total_cost > total_cost * STD_FUZZ_FACTOR &&
+ if (is_greater_cost(old_path->total_cost, total_cost * STD_FUZZ_FACTOR)
&&
keyscmp != PATHKEYS_BETTER2)
return true;
}
@@ -1697,7 +1697,7 @@ create_unique_path(PlannerInfo *root, RelOptInfo *rel,
Path *subpath, if (sjinfo->semi_can_btree && sjinfo->semi_can_hash)
{
- if (agg_path.total_cost < sort_path.total_cost)
+ if (is_lower_cost(agg_path.total_cost, sort_path.total_cost))
pathnode->umethod = UNIQUE_PATH_HASH;
else
pathnode->umethod = UNIQUE_PATH_SORT;
diff --git a/src/backend/utils/cache/relcache.c
b/src/backend/utils/cache/relcache.c
index 78f3b99a76..c261a9d790 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -5076,8 +5076,8 @@ IsProjectionFunctionalIndex(Relation index)
* when values differ because the expression is recalculated when
* inserting a new index entry for the changed value.
*/
- if ((index_expr_cost.startup + index_expr_cost.per_tuple) >
- HEURISTIC_MAX_HOT_RECHECK_EXPR_COST)
+ if (is_greater_cost((index_expr_cost.startup +
index_expr_cost.per_tuple),
+ HEURISTIC_MAX_HOT_RECHECK_EXPR_COST))
is_projection = false; tuple = SearchSysCache1(RELOID,
ObjectIdGetDatum(RelationGetRelid(index)));
diff --git a/src/include/optimizer/cost.h b/src/include/optimizer/cost.h
index 9159f2bab1..c01d08eae5 100644
--- a/src/include/optimizer/cost.h
+++ b/src/include/optimizer/cost.h
@@ -251,6 +251,12 @@ extern PathTarget
*set_pathtarget_cost_width(PlannerInfo *root, PathTarget *targ
extern double compute_bitmap_pages(PlannerInfo *root, RelOptInfo *baserel,
Path *bitmapqual, int loop_count, Cost *cost, double *tuple);+extern Cost add_cost(Cost cost, Cost delta_cost);
+extern bool is_lower_cost(Cost cost1, Cost cost2);
+extern bool is_greater_cost(Cost cost1, Cost cost2);
+extern bool is_geq_cost(Cost cost1, Cost cost2);
+
+
/*
* prototypes for clausesel.c
* routines to compute clause selectivities-----
Jim Finnerty, AWS, Amazon Aurora PostgreSQL
--
Sent from: https://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html
On Tue, 2019-12-10 at 15:50 -0700, Jim Finnerty wrote:
As a proof of concept, I hacked around a bit today to re-purpose one of the
bits of the Cost structure to mean "is_disabled" so that we can distinguish
'disabled' from 'non-disabled' paths without making the Cost structure any
bigger. In fact, it's still a valid double. The obvious choice would have
been to re-purpose the sign bit, but I've had occasion to exploit negative
costs before so for this POC I used the high-order bit of the fractional
bits of the double. (see Wikipedia for double precision floating point for
the layout).The idea is to set a special bit when disable_cost is added to a cost.
Dedicating multiple bits instead of just 1 would be easily done, but as it
is we can accumulate many disable_costs without overflowing, so just
comparing the cost suffices.
Doesn't that rely on a specific implementation of double precision (IEEE)?
I thought that we don't want to limit ourselves to platforms with IEEE floats.
Yours,
Laurenz Albe
On Wed, 11 Dec 2019 at 01:24, Laurenz Albe <laurenz.albe@cybertec.at> wrote:
On Tue, 2019-12-10 at 15:50 -0700, Jim Finnerty wrote:
As a proof of concept, I hacked around a bit today to re-purpose one of the
bits of the Cost structure to mean "is_disabled" so that we can distinguish
'disabled' from 'non-disabled' paths without making the Cost structure any
bigger. In fact, it's still a valid double. The obvious choice would have
been to re-purpose the sign bit, but I've had occasion to exploit negative
costs before so for this POC I used the high-order bit of the fractional
bits of the double. (see Wikipedia for double precision floating point for
the layout).The idea is to set a special bit when disable_cost is added to a cost.
Dedicating multiple bits instead of just 1 would be easily done, but as it
is we can accumulate many disable_costs without overflowing, so just
comparing the cost suffices.Doesn't that rely on a specific implementation of double precision (IEEE)?
I thought that we don't want to limit ourselves to platforms with IEEE floats.
We could always implement it again in another format....
However, I wouldn't have expected to be bit twiddling. I would have
expected to use standard functions like ldexp to do this. In fact I
think if you use the high bit of the exponent you could do it entirely
using ldexp and regular double comparisons (with fabs).
Ie, to set the bit you set cost = ldexp(cost, __DBL_MAX_EXP__/2). And
to check for the bit being set you compare ilogb(cost,
__DBL_MAX_EXP__/2). Hm. that doesn't handle if the cost is already < 1
in which case I guess you would have to set it to 1 first. Or reserve
the two high bits of the cost so you can represent disabled values
that had negative exponents before being disabled.
I wonder if it wouldn't be a lot cleaner and more flexible to just go
with a plain float for Cost and use the other 32 bits for counters and
bitmasks and still be ahead of the game. A double can store 2^1024 but
a float 2^128 which still feels like it should be more than enough to
store the kinds of costs plans have without the disabled costs. 2^128
milliseconds is still 10^28 years which is an awfully expensive
query....
--
greg
On Wed, Dec 11, 2019 at 7:24 PM Laurenz Albe <laurenz.albe@cybertec.at> wrote:
Doesn't that rely on a specific implementation of double precision (IEEE)?
I thought that we don't want to limit ourselves to platforms with IEEE floats.
Just by the way, you might want to read the second last paragraph of
the commit message for 02ddd499. The dream is over, we're never going
to run on Vax.
Thomas Munro <thomas.munro@gmail.com> writes:
On Wed, Dec 11, 2019 at 7:24 PM Laurenz Albe <laurenz.albe@cybertec.at> wrote:
Doesn't that rely on a specific implementation of double precision (IEEE)?
I thought that we don't want to limit ourselves to platforms with IEEE floats.
Just by the way, you might want to read the second last paragraph of
the commit message for 02ddd499. The dream is over, we're never going
to run on Vax.
Still, the proposed hack is doubling down on IEEE dependency in a way
that I quite dislike, in that (a) it doesn't just read float values
but generates new ones (and assumes that the hardware/libc will react in
a predictable way to them), (b) in a part of the code that has no damn
business having close dependencies on float format, and (c) for a gain
far smaller than what we got from the Ryu code.
We have had prior discussions about whether 02ddd499 justifies adding
more IEEE dependencies elsewhere. I don't think it does. IEEE 754
is not the last word that will ever be said on floating-point arithmetic,
any more than x86_64 is the last CPU architecture that anyone will ever
care about. We should keep our dependencies on it well circumscribed.
regards, tom lane
I think this would be ready to abstract away behind a few functions that
could always be replaced by something else later...
However on further thought I really think just using a 32-bit float and 32
bits of other bitmaps or counters would be a better approach.
On Sun., Dec. 15, 2019, 14:54 Tom Lane, <tgl@sss.pgh.pa.us> wrote:
Show quoted text
Thomas Munro <thomas.munro@gmail.com> writes:
On Wed, Dec 11, 2019 at 7:24 PM Laurenz Albe <laurenz.albe@cybertec.at>
wrote:
Doesn't that rely on a specific implementation of double precision
(IEEE)?
I thought that we don't want to limit ourselves to platforms with IEEE
floats.
Just by the way, you might want to read the second last paragraph of
the commit message for 02ddd499. The dream is over, we're never going
to run on Vax.Still, the proposed hack is doubling down on IEEE dependency in a way
that I quite dislike, in that (a) it doesn't just read float values
but generates new ones (and assumes that the hardware/libc will react in
a predictable way to them), (b) in a part of the code that has no damn
business having close dependencies on float format, and (c) for a gain
far smaller than what we got from the Ryu code.We have had prior discussions about whether 02ddd499 justifies adding
more IEEE dependencies elsewhere. I don't think it does. IEEE 754
is not the last word that will ever be said on floating-point arithmetic,
any more than x86_64 is the last CPU architecture that anyone will ever
care about. We should keep our dependencies on it well circumscribed.regards, tom lane
Hi hackers,
I have write an initial patch to retire the disable_cost GUC, which labeled a flag on the Path struct instead of adding up a big cost which is hard to estimate. Though it involved in tons of plan changes in regression tests, I have tested on some simple test cases such as eagerly generate a two-stage agg plans and it worked. Could someone help to review?
regards,
Jian
________________________________
From: Euler Taveira <euler@timbira.com.br>
Sent: Friday, November 1, 2019 22:48
To: Zhenghua Lyu <zlyu@vmware.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: On disable_cost
!! External Email
Em sex, 1 de nov de 2019 às 03:42, Zhenghua Lyu <zlv@pivotal.io> escreveu:
My issue: I did some spikes and tests on TPCDS 1TB Bytes data. For query 104, it generates
nestloop join even with enable_nestloop set off. And the final plan's total cost is very huge (about 1e24). But If I enlarge the disable_cost to 1e30, then, planner will generate hash join.So I guess that disable_cost is not large enough for huge amount of data.
It is tricky to set disable_cost a huge number. Can we come up with better solution?
Isn't it a case for a GUC disable_cost? As Thomas suggested, DBL_MAX
upper limit should be sufficient.
The following thoughts are from Heikki:
Aside from not having a large enough disable cost, there's also the fact that the high cost might affect the rest of the plan, if we have to use a plan type that's disabled. For example, if a table doesn't have any indexes, but enable_seqscan is off, we might put the unavoidable Seq Scan on different side of a join than we we would with enable_seqscan=on, because of the high cost estimate.
I think a more robust way to disable forbidden plan types would be to handle the disabling in add_path(). Instead of having a high disable cost on the Path itself, the comparison add_path() would always consider disabled paths as more expensive than others, regardless of the cost.
I'm afraid it is not as cheap as using diable_cost as a node cost. Are
you proposing to add a new boolean variable in Path struct to handle
those cases in compare_path_costs_fuzzily?
--
Euler Taveira Timbira -
https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.timbira.com.br%2F&data=05%7C01%7Cgjian%40vmware.com%7C12a30b2852dd4651667608db9401d056%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C638266507757076648%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=v54JhsW8FX4mSmjgt2yP59t7xtv1mZvC%2BBhtKrfp%2FBY%3D&reserved=0<http://www.timbira.com.br/>
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
!! External Email: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender.
Attachments:
0001-Retire-disable_cost-introduce-new-flag-is_disabled.patchtext/x-patch; name=0001-Retire-disable_cost-introduce-new-flag-is_disabled.patchDownload+73-17
On Thu, Aug 3, 2023 at 5:22 AM Jian Guo <gjian@vmware.com> wrote:
I have write an initial patch to retire the disable_cost GUC, which labeled a flag on the Path struct instead of adding up a big cost which is hard to estimate. Though it involved in tons of plan changes in regression tests, I have tested on some simple test cases such as eagerly generate a two-stage agg plans and it worked. Could someone help to review?
I took a look at this patch today. I believe that overall this may
well be an approach worth pursuing. However, more work is going to be
needed. Here are some comments:
1. You stated that it changes lots of plans in the regression tests,
but you haven't provided any sort of analysis of why those plans
changed. I'm kind of surprised that there would be "tons" of plan
changes. You (or someone) should look into why that's happening.
2. The change to compare_path_costs_fuzzily() seems incorrect to me.
When path1->is_disabled && path2->is_disabled, costs should be
compared just as they are when neither path is disabled. Instead, the
patch treats any two such paths as having equal cost. That seems
catastrophically bad. Maybe it accounts for some of those plan
changes, although that would only be true if those plans were created
while using some disabling GUC.
3. Instead of adding is_disabled at the end of the Path structure, I
suggest adding it between param_info and parallel_aware. I think if
you do that, the space used by the new field will use up padding bytes
that are currently included in the struct, instead of making it
longer.
4. A critical issue for any patch of this type is performance. This
concern was raised earlier on this thread, but your path doesn't
address it. There's no performance analysis or benchmarking included
in your email. One idea that I have is to write the cost-comparison
test like this:
if (unlikely(path1->is_disabled || path2->is_disabled))
{
if (!path1->is_disabled)
return COSTS_BETTER1;
if (!path2->is_disabled)
return COSTS_BETTER2;
/* if both disabled, fall through */
}
I'm not sure that would be enough to prevent the patch from adding
noticeably to the cost of path comparison, but maybe it would help.
5. The patch changes only compare_path_costs_fuzzily() but I wonder
whether compare_path_costs() and compare_fractional_path_costs() need
similar surgery. Whether they do or don't, there should likely be some
comments explaining the situation.
6. In fact, the patch changes no comments at all, anywhere. I'm not
sure how many comment changes a patch like this needs to make, but the
answer definitely isn't "none".
7. The patch doesn't actually remove disable_cost. I guess it should.
8. When you submit a patch, it's a good idea to also add it on
commitfest.postgresql.org. It doesn't look like that was done in this
case.
--
Robert Haas
EDB: http://www.enterprisedb.com
