Subject: Re: Parameter for planner estimate of recursive queries

From: Peter Eisentraut <[email-masked]>
Date: Tue, 25 Jan 2022 10:44:21 +0100

On 31.12.21 15:10, Simon Riggs wrote:
>> The factor 10 is a reasonably safe assumption and helps avoid worst
>> case behavior in bigger graph queries. However, the factor 10 is way
>> too large for many types of graph query, such as where the path
>> through the data is tight, and/or the query is written to prune bushy
>> graphs, e.g. shortest path queries. The factor 10 should not be
>> hardcoded in the planner, but should be settable, just as
>> cursor_tuple_fraction is.
> If you think this should be derived without parameters, then we would
> want a function that starts at 1 for 1 input row and gets much larger
> for larger input. The thinking here is that Graph OLTP is often a
> shortest path between two nodes, whereas Graph Analytics and so the
> worktable will get much bigger.

On the one hand, this smells like a planner hint.  But on the other 
hand, it doesn't look like we will come up with proper graph-aware 
selectivity estimation system any time soon, so just having all graph 
OLTP queries suck until then because the planner hint is hardcoded 
doesn't seem like a better solution.  So I think this setting can be ok. 
  I think the way you have characterized it makes sense, too: for graph 
OLAP, you want a larger value, for graph OLTP, you want a smaller value.

---

From: Hamid Akhtar <[email-masked]>
Date: Fri, 28 Jan 2022 18:40:20 +0500

> OLAP, you want a larger value, for graph OLTP, you want a smaller value.

Do you think there is a case to replace the 10x multiplier with
"recursive_worktable_estimate" for total_rows calculation in the
cost_recursive_union function too?

---

From: Robert Haas <[email-masked]>
Date: Fri, 28 Jan 2022 09:07:32 -0500

> doesn't seem like a better solution.  So I think this setting can be ok.

I agree. It's a bit lame, but seems pretty harmless, and I can't see
us realistically doing a lot better with any reasonable amount of
work.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: Simon Riggs <[email-masked]>
Date: Thu, 10 Mar 2022 17:42:14 +0000

> us realistically doing a lot better with any reasonable amount of

Shall I set this as Ready For Committer?

-- 
Simon Riggs                http://www.EnterpriseDB.com/

---

From: Andres Freund <[email-masked]>
Date: Mon, 21 Mar 2022 17:04:54 -0700

Hi,

> Shall I set this as Ready For Committer?

Currently this CF entry fails on cfbot: https://cirrus-ci.com/task/4531771134967808?logs=test_world#L1158

[16:27:35.772] #   Failed test 'no parameters missing from postgresql.conf.sample'
[16:27:35.772] #   at t/003_check_guc.pl line 82.
[16:27:35.772] #          got: '1'
[16:27:35.772] #     expected: '0'
[16:27:35.772] # Looks like you failed 1 test of 3.
[16:27:35.772] [16:27:35] t/003_check_guc.pl ..............

Marked as waiting on author.

- Andres

---

From: Simon Riggs <[email-masked]>
Date: Wed, 23 Mar 2022 14:53:04 +0000

> Marked as waiting on author.

Thanks.

I've corrected that by adding a line to postgresql.conf.sample, as
well as adding docs.

---

From: Tom Lane <[email-masked]>
Date: Wed, 23 Mar 2022 13:36:29 -0400

> us realistically doing a lot better with any reasonable amount of

Yeah, agreed on all counts.  The thing that makes it lame is that
there's no reason to expect that the same multiplier is good for
every recursive query done in an installation, or even in a session.

One could imagine dealing with that by adding custom syntax to WITH,
as we have already done once:

WITH RECURSIVE cte1 AS SCALE 1.0 (SELECT ...

But I *really* hesitate to go there, mainly because once we do
something like that we can't ever undo it.  I think Simon's
proposal is a reasonable low-effort compromise.

Some nitpicks:

* The new calculation needs clamp_row_est(), since the float
GUC could be fractional or even zero.

* Do we want to prevent the GUC value from being zero?  It's not
very sensible, plus I think we might want to reserve that value
to mean "use the built-in calculation", in case we ever do put
in some smarter logic here.  But I'm not sure what a reasonable
non-zero lower bound would be.

* The proposed docs claim that a smaller setting works by biasing
the planner towards fast-start plans, but I don't think I believe
that explanation.  I'd venture that we want text more along the
lines of "This may help the planner choose the most appropriate
method for joining the work table to the query's other tables".

			regards, tom lane

---

From: Simon Riggs <[email-masked]>
Date: Wed, 23 Mar 2022 18:20:09 +0000

> GUC could be fractional or even zero.

True, will do.

> non-zero lower bound would be.

Agreed, makes sense.

> method for joining the work table to the query's other tables".

OK, will improve.

[New patch version pending]

---

From: Simon Riggs <[email-masked]>
Date: Wed, 23 Mar 2022 19:37:02 +0000

---

From: Tom Lane <[email-masked]>
Date: Wed, 23 Mar 2022 16:25:36 -0400

>> [New patch version pending]

Do you have any objection if I rename the GUC to
recursive_worktable_factor?  That seems a bit clearer as to what
it does, and it leaves more room for other knobs in the same area
if we decide we need any.

			regards, tom lane

---

From: Simon Riggs <[email-masked]>
Date: Thu, 24 Mar 2022 10:32:00 +0000

> if we decide we need any.

None, I think your proposal is a better name.

---

From: Tom Lane <[email-masked]>
Date: Thu, 24 Mar 2022 11:48:21 -0400

> None, I think your proposal is a better name.

Pushed that way.

			regards, tom lane

---

From: Simon Riggs <[email-masked]>
Date: Thu, 24 Mar 2022 16:04:55 +0000

> > None, I think your proposal is a better name.

Ok, thanks.