Re: On disable_cost

2024-07-03 Thread David Rowley
On Wed, 3 Jul 2024 at 09:49, Tom Lane wrote: > > Heikki Linnakangas writes: > > 3. Oh right, bitmap scan, I forgot about that one. Let's disable that too: > > Yeah, I've hit that too, although more often (for me) it's the first > choice of plan. In any case, it usually takes more than one

Re: On disable_cost

2024-07-03 Thread Robert Haas
On Tue, Jul 2, 2024 at 5:39 PM Heikki Linnakangas wrote: > I would be somewhat annoyed if we add another step to that, to also > disable index-only scans separately. It would be nice if > enable_indexscan=off would also disable bitmap scans, that would > eliminate one step from the above. Almost

Re: On disable_cost

2024-07-02 Thread Tom Lane
Heikki Linnakangas writes: > 3. Oh right, bitmap scan, I forgot about that one. Let's disable that too: Yeah, I've hit that too, although more often (for me) it's the first choice of plan. In any case, it usually takes more than one change to get to a seqscan. > It almost feels like we should

Re: On disable_cost

2024-07-02 Thread Heikki Linnakangas
On 02/07/2024 22:54, Robert Haas wrote: On Tue, Jul 2, 2024 at 3:36 PM Tom Lane wrote: One could argue for other things, of course. And maybe those other things are fine, if they're properly justified and documented. [ shrug... ] This isn't a hill that I'm prepared to die on. But I see no

Re: On disable_cost

2024-07-02 Thread Tom Lane
Robert Haas writes: > Well, I don't really know where to go from here. I mean, I think that > three committers (David, Heikki, yourself) have expressed some > concerns about changing the behavior. So maybe we shouldn't. But I > don't understand how it's reasonable to have two very similarly named

Re: On disable_cost

2024-07-02 Thread Robert Haas
On Tue, Jul 2, 2024 at 3:36 PM Tom Lane wrote: > > One could argue for other things, of course. And maybe those other > > things are fine, if they're properly justified and documented. > > [ shrug... ] This isn't a hill that I'm prepared to die on. > But I see no good reason to change the very

Re: On disable_cost

2024-07-02 Thread Tom Lane
Robert Haas writes: > What the patch does is: if you set either enable_indexscan=false or > enable_indexonlyscan=false, then the corresponding path type is not > generated, and the other is unaffected. To me, that seems like the > logical way to clean this up. > One could argue for other things,

Re: On disable_cost

2024-07-02 Thread Robert Haas
On Tue, Jul 2, 2024 at 2:37 PM Tom Lane wrote: > Robert Haas writes: > > What happens right now is: > > > - If you set enable_indexscan=false, then disable_cost is added to the > > cost of index scan paths and the cost of index-only scan paths. > > > - If you set enable_indexonlyscan=false, then

Re: On disable_cost

2024-07-02 Thread Tom Lane
Robert Haas writes: > What happens right now is: > - If you set enable_indexscan=false, then disable_cost is added to the > cost of index scan paths and the cost of index-only scan paths. > - If you set enable_indexonlyscan=false, then index-only scan paths > are not generated at all. Hm. The

Re: On disable_cost

2024-07-02 Thread Robert Haas
On Tue, Jul 2, 2024 at 1:40 PM Tom Lane wrote: > FWIW, I disagree completely. I think it's entirely natural to > consider bitmap index scans to be a subset of index scans, so that > enable_indexscan should affect both. I admit that the current set > of GUCs doesn't let you force a bitmap scan

Re: On disable_cost

2024-07-02 Thread Tom Lane
Robert Haas writes: > On Tue, Jul 2, 2024 at 10:57 AM Heikki Linnakangas wrote: >> I fear this will break people's applications, if they are currently >> forcing a sequential scan with "set enable_indexscan=off". Now they will >> need to do "set enable_indexscan=off; set

Re: On disable_cost

2024-07-02 Thread Robert Haas
Thanks for the review! On Tue, Jul 2, 2024 at 10:57 AM Heikki Linnakangas wrote: > v3-0001-Remove-grotty-use-of-disable_cost-for-TID-scan-pl.patch: > > +1, this seems ready for commit Cool. > v3-0002-Rationalize-behavior-of-enable_indexscan-and-enab.patch: > > I fear this will break people's

Re: On disable_cost

2024-07-02 Thread Heikki Linnakangas
On 28/06/2024 18:46, Robert Haas wrote: On Wed, Jun 12, 2024 at 11:35 AM Robert Haas wrote: Well, that didn't generate much discussion, but here I am trying again. Here I've got patches 0001 and 0002 from my previous posting; I've dropped 0003 and 0004 from the previous set for now so as not

Re: On disable_cost

2024-06-12 Thread Robert Haas
On Wed, Jun 12, 2024 at 2:48 PM Andres Freund wrote: > Sorry, should have been more precise. With "set" I didn't mean set to true, > but that that it's only modified within select_mergejoin_clauses(). Oh. "set" has more than one relevant meaning here. > > It starts out true, and always stays

Re: On disable_cost

2024-06-12 Thread Andres Freund
Hi, On 2024-06-12 14:33:31 -0400, Robert Haas wrote: > On Wed, Jun 12, 2024 at 2:11 PM Andres Freund wrote: > > > > > > In an extreme case i can see a tiny bit of overhead, but not enough to be > > worth worrying about. Mostly because we're so profligate in doing > > bms_overlap() that cost

Re: On disable_cost

2024-06-12 Thread Robert Haas
On Wed, Jun 12, 2024 at 2:11 PM Andres Freund wrote: > > > In an extreme case i can see a tiny bit of overhead, but not enough to be > worth worrying about. Mostly because we're so profligate in doing > bms_overlap() that cost comparisons don't end up mattering as much - I seem to > recall that

Re: On disable_cost

2024-06-12 Thread Andres Freund
Hi, On 2024-06-12 11:35:48 -0400, Robert Haas wrote: > Subject: [PATCH v2 3/4] Treat the # of disabled nodes in a path as a separate > cost metric. > > Previously, when a path type was disabled by e.g. enable_seqscan=false, > we either avoided generating that path type in the first place, or >

Re: On disable_cost

2024-05-07 Thread Robert Haas
On Mon, May 6, 2024 at 4:30 PM Peter Geoghegan wrote: > FWIW I always found those weird inconsistencies to be annoying at > best, and confusing at worst. I speak as somebody that uses > disable_cost a lot. > > I certainly wouldn't ask anybody to make it a priority for that reason > alone -- it's

Re: On disable_cost

2024-05-06 Thread Peter Geoghegan
On Mon, May 6, 2024 at 8:27 AM Robert Haas wrote: > Stepping back a bit, my current view of this area is: disable_cost is > highly imperfect both as an idea and as implemented in PostgreSQL. > Although I'm discovering that the current implementation gets more > things right than I had realized,

Re: On disable_cost

2024-05-06 Thread Tom Lane
Robert Haas writes: > I'll look into this, unless you want to do it. I have a draft patch already. Need to add a test case. > Incidentally, another thing I just noticed is that > IsCurrentOfClause()'s test for (node->cvarno == rel->relid) is > possibly dead code. At least, there are no

Re: On disable_cost

2024-05-06 Thread Robert Haas
On Mon, May 6, 2024 at 3:26 PM Tom Lane wrote: > Nah, I'm wrong: we do treat it as leakproof, and the comment about > that in contain_leaked_vars_walker shows that the interaction with > RLS quals *was* thought about. What wasn't thought about was the > possibility of RLS quals that themselves

Re: On disable_cost

2024-05-06 Thread Tom Lane
I wrote: > Robert Haas writes: >> ... Which then allowed me to >> construct the example above, where there are two possible TID quals >> and the logic in tidpath.c latches onto the wrong one. > Hmm. Without having traced through it, I'm betting that the > CurrentOfExpr qual is rejected as a

Re: On disable_cost

2024-05-06 Thread Tom Lane
Robert Haas writes: > On Mon, May 6, 2024 at 9:39 AM Robert Haas wrote: >> It's not very clear that this mechanism is actually 100% reliable, > It isn't. Here's a test case. Very interesting. > ... Which then allowed me to > construct the example above, where there are two possible TID quals

Re: On disable_cost

2024-05-06 Thread Robert Haas
On Mon, May 6, 2024 at 9:39 AM Robert Haas wrote: > It's not very clear that this mechanism is actually 100% reliable, It isn't. Here's a test case. As a non-superuser, do this: create table foo (a int, b text, primary key (a)); insert into foo values (1, 'Apple'); alter table foo enable row

Re: On disable_cost

2024-05-06 Thread Robert Haas
On Sat, May 4, 2024 at 12:57 PM Tom Lane wrote: > There is also some weirdness around needing to force use of tidscan > if we have WHERE CURRENT OF. But perhaps a different hack could be > used for that. Yeah, figuring out what to do about this was the trickiest part of the experimental patch

Re: On disable_cost

2024-05-06 Thread Robert Haas
On Sat, May 4, 2024 at 9:16 AM David Rowley wrote: > I don't think you'd need to wait longer than where we do set_cheapest > and find no paths to find out that there's going to be a problem. I'm confused by this response, because I thought that the main point of my previous email was explaining

Re: On disable_cost

2024-05-04 Thread David Rowley
On Sun, 5 May 2024 at 04:57, Tom Lane wrote: > > David Rowley writes: > > That doesn't get you the benefits of fewer CPU cycles, but where did > > that come from as a motive to change this? There's no shortage of > > other ways to make the planner faster if that's an issue. > > The concern was

Re: On disable_cost

2024-05-04 Thread Tom Lane
David Rowley writes: > I don't think you'd need to wait longer than where we do set_cheapest > and find no paths to find out that there's going to be a problem. At a base relation, yes, but that doesn't work for joins: it may be that a particular join cannot be formed, yet other join sequences

Re: On disable_cost

2024-05-04 Thread David Rowley
On Sat, 4 May 2024 at 08:34, Robert Haas wrote: > Another idea is to remove the ERROR mentioned above from > set_cheapest() and just allow planning to continue even if some > relations end up with no paths. (This would necessitate finding and > fixing any code that could be confused by a pathless

Re: On disable_cost

2024-05-03 Thread Robert Haas
On Sat, Nov 2, 2019 at 10:57 AM Tom Lane wrote: > 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

Re: On disable_cost

2024-04-04 Thread Robert Haas
On Wed, Apr 3, 2024 at 11:09 PM David Rowley wrote: > On Thu, 4 Apr 2024 at 10:15, David Rowley wrote: > > In short, I don't find it strange that disabling one node type results > > in considering another type that we'd otherwise not consider in cases > > where we assume that the disabled node

Re: On disable_cost

2024-04-03 Thread David Rowley
On Thu, 4 Apr 2024 at 10:15, David Rowley wrote: > In short, I don't find it strange that disabling one node type results > in considering another type that we'd otherwise not consider in cases > where we assume that the disabled node type is always superior and > should always be used when it is

Re: On disable_cost

2024-04-03 Thread David Rowley
On Thu, 4 Apr 2024 at 08:21, Robert Haas wrote: > I wanted to further explore the idea of just not generating plans of > types that are currently disabled. I looked into doing this for > enable_indexscan and enable_indexonlyscan. As a first step, I > investigated how those settings work now, and

Re: On disable_cost

2024-04-03 Thread Greg Sabino Mullane
On Wed, Apr 3, 2024 at 3:21 PM Robert Haas wrote: > It's also pretty clear to me that the fact that enable_indexscan > and enable_indexonlyscan work completely differently from each other > is surprising at best, wrong at worst, but here again, what this patch > does about that is not above

Re: On disable_cost

2024-04-03 Thread Robert Haas
On Tue, Apr 2, 2024 at 12:58 PM Tom Lane wrote: > Not really. But if you had, say, a join of a promoted path to a > disabled path, that would be treated as on-par with a join of two > regular paths, which seems like it'd lead to odd choices. Maybe > it'd be fine, but my gut says it'd likely not

Re: On disable_cost

2024-04-02 Thread Tom Lane
Robert Haas writes: > On Tue, Apr 2, 2024 at 11:54 AM Tom Lane wrote: >> I suspect that it'd behave poorly when there are both disabled and >> promoted sub-paths in a tree, for pretty much the same reasons you >> explained just upthread. > Hmm, can you explain further? I think essentially you'd

Re: On disable_cost

2024-04-02 Thread Robert Haas
On Tue, Apr 2, 2024 at 11:54 AM Tom Lane wrote: > > I'm pretty sure negative costs are going to create a variety of > > unpleasant planning artifacts. > > Indeed. It might be okay to have negative values for disabled-ness > if we treat disabled-ness as a "separate order of infinity", but > I

Re: On disable_cost

2024-04-02 Thread Tom Lane
Robert Haas writes: > On Tue, Apr 2, 2024 at 10:04 AM Greg Sabino Mullane > wrote: >> if (!enable_seqscan) >> startup_cost += disable_cost; >> else if (promote_seqscan) >> startup_cost -= promotion_cost; // or replace "promote" with "encourage"? > I'm pretty sure negative costs are

Re: On disable_cost

2024-04-02 Thread Robert Haas
On Tue, Apr 2, 2024 at 10:04 AM Greg Sabino Mullane wrote: > So rather than listing all the things we don't want to happen, we need a way > to force (nay, highly encourage) a particular solution. As our costing is a > based on positive numbers, what if we did something like this in costsize.c?

Re: On disable_cost

2024-04-02 Thread Greg Sabino Mullane
On Mon, Apr 1, 2024 at 7:54 PM Robert Haas wrote: > What I think we're mostly doing in the regression tests is shutting > off every relevant type of plan except one. I theorize that what we > actually want to do is tell the planner what we do want to happen, > rather than what we don't want to

Re: On disable_cost

2024-04-01 Thread Robert Haas
On Mon, Apr 1, 2024 at 5:00 PM Tom Lane wrote: > Very interesting, thanks for the summary. So the fact that > disable_cost is additive across plan nodes is actually a pretty > important property of the current setup. I think this is closely > related to one argument you made against my upthread

Re: On disable_cost

2024-04-01 Thread Tom Lane
Robert Haas writes: > One of the things I realized relatively early is that the patch does > nothing to propagate disable_cost upward through the plan tree. > ... > After straining my brain over various plan changes for a long time, > and hacking on the code somewhat, I realized that just

Re: On disable_cost

2024-04-01 Thread Robert Haas
On Tue, Mar 12, 2024 at 1:32 PM Tom Lane wrote: > > 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

Re: On disable_cost

2024-03-13 Thread Robert Haas
On Tue, Mar 12, 2024 at 4:55 PM David Rowley wrote: > The primary place I see issues with disabled_cost is caused by > STD_FUZZ_FACTOR. When you add 1.0e10 to a couple of modestly costly > paths, it makes them appear fuzzily the same in cases where one could > be significantly cheaper than the

Re: On disable_cost

2024-03-12 Thread Tom Lane
David Rowley writes: > So maybe the fix could be to set disable_cost to something like > 1.0e110 and adjust compare_path_costs_fuzzily to not apply the > fuzz_factor for paths >= disable_cost. However, I wonder if that > risks the costs going infinite after a couple of cartesian joins.

Re: On disable_cost

2024-03-12 Thread David Rowley
On Wed, 13 Mar 2024 at 08:55, Robert Haas wrote: > But in the absence of that, we need some way to privilege the > non-disabled paths over the disabled ones -- and I'd prefer to have > something more principled than disable_cost, if we can work out the > details. The primary place I see issues

Re: On disable_cost

2024-03-12 Thread Robert Haas
On Tue, Mar 12, 2024 at 3:36 PM Tom Lane wrote: > Yeah. I keep thinking that the right solution is to not generate > disabled paths in the first place if there are any other ways to > produce the same relation. That has obvious order-of-operations > problems though, and I've not been able to

Re: On disable_cost

2024-03-12 Thread Tom Lane
Robert Haas writes: > On Tue, Mar 12, 2024 at 1:32 PM Tom Lane wrote: >> BTW, having written that paragraph, I wonder if we couldn't get >> the same end result with a nearly one-line change that consists of >> making disable_cost be IEEE infinity. > I don't think so, because I think that what

Re: On disable_cost

2024-03-12 Thread Robert Haas
On Tue, Mar 12, 2024 at 1:32 PM Tom Lane wrote: > BTW, having written that paragraph, I wonder if we couldn't get > the same end result with a nearly one-line change that consists of > making disable_cost be IEEE infinity. Years ago we didn't want > to rely on IEEE float semantics in this area,

Re: On disable_cost

2024-03-12 Thread Tom Lane
Robert Haas writes: > On Thu, Aug 3, 2023 at 5:22 AM Jian Guo 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

Re: On disable_cost

2024-03-12 Thread Robert Haas
On Thu, Aug 3, 2023 at 5:22 AM Jian Guo 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

Re: On disable_cost

2023-08-03 Thread Jian Guo
generate a two-stage agg plans and it worked. Could someone help to review? regards, Jian From: Euler Taveira Sent: Friday, November 1, 2019 22:48 To: Zhenghua Lyu Cc: PostgreSQL Hackers Subject: Re: On disable_cost !! External Email Em sex, 1 de nov de 2019

Re: On disable_cost

2019-12-15 Thread Greg Stark
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

Re: On disable_cost

2019-12-15 Thread Tom Lane
Thomas Munro writes: > On Wed, Dec 11, 2019 at 7:24 PM Laurenz Albe 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

Re: On disable_cost

2019-12-12 Thread Thomas Munro
On Wed, Dec 11, 2019 at 7:24 PM Laurenz Albe 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

Re: On disable_cost

2019-12-12 Thread Greg Stark
On Wed, 11 Dec 2019 at 01:24, Laurenz Albe 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

Re: On disable_cost

2019-12-10 Thread Laurenz Albe
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.

Re: On disable_cost

2019-11-02 Thread Tom Lane
Tomas Vondra 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

Re: On disable_cost

2019-11-02 Thread Tom Lane
Zhenghua Lyu 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,

Re: On disable_cost

2019-11-01 Thread Andres Freund
On 2019-11-01 12:56:30 -0400, Robert Haas wrote: > On Fri, Nov 1, 2019 at 12:43 PM Andres Freund 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

Re: On disable_cost

2019-11-01 Thread Tomas Vondra
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

Re: On disable_cost

2019-11-01 Thread Robert Haas
On Fri, Nov 1, 2019 at 12:43 PM Andres Freund 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

Re: On disable_cost

2019-11-01 Thread Andres Freund
Hi, On 2019-11-01 12:22:06 -0400, Robert Haas wrote: > On Fri, Nov 1, 2019 at 12:00 PM Andres Freund 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. >

Re: On disable_cost

2019-11-01 Thread Robert Haas
On Fri, Nov 1, 2019 at 12:00 PM Andres Freund 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

Re: On disable_cost

2019-11-01 Thread Andres Freund
Hi, On 2019-11-01 19:58:04 +1300, Thomas Munro wrote: > On Fri, Nov 1, 2019 at 7:42 PM Zhenghua Lyu 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

Re: On disable_cost

2019-11-01 Thread Euler Taveira
Em sex, 1 de nov de 2019 às 03:42, Zhenghua Lyu 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

Re: On disable_cost

2019-11-01 Thread Thomas Munro
On Fri, Nov 1, 2019 at 7:42 PM Zhenghua Lyu 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?