Re: Re: Damage control for planner's get_actual_variable_endpoint() runaway

On Mon, Nov 21, 2022 at 2:15 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On November 21, 2022 10:44:17 AM PST, Simon Riggs <simon.riggs@enterprisedb.com> wrote:
> >> Robert, something like this perhaps? limit on both the index and the heap.
>
> > I don't think we should add additional code / struct members into very common good paths for these limits.
>
> Yeah, I don't like that either: for one thing, it seems completely
> unsafe to back-patch.

I have mixed feelings about back-patching this. On the one hand, the
lack of any limit here can have *really* bad consequences. On the
other hand, it's also possible to imagine someone getting hosed by the
fix. No matter what limit we enforce, someone could in theory get a
much better estimate by searching just a little further.

I agree that adding members to IndexScanDescData doesn't seem very
appealing, but I'm still trying to wrap my head around what exposure
that creates. In the patches thus far, we're basically counting the
number of times that we get an index tuple that's not on the same page
as the previous index tuple. That's not quite the same thing as the
number of heap pages, because we might be just ping-ponging back and
forth between 2 pages and it looks the same. I'm not sure that's a
problem, but it's something to note. If the limit is 100 pages, then
we'll try to fetch at least 100 index tuples, if they're all on
different pages, and perhaps as much as two orders of magnitude more,
if they're all on the same page. That doesn't seem too bad, because we
won't really be doing 100 times as much work. Following 10,000 index
tuples that all point at the same 100 heap pages is probably more work
than following 100 index tuples that each point to a separate heap
page, but it's not anywhere near 100x as much work, especially if real
I/O is involved. All in all, my first reaction is to think that it
sounds fairly OK.

The real sticky wicket is that we don't know how dense the index is.
In a non-pathological case, we expect to find quite a few index tuples
in each index page, so if we're fetching 100 heap pages the number of
index pages fetched is probably much less, like 1 or 2. And even if
we're fetching 10000 index tuples to visit 100 heap pages, they should
still be concentrated in a relatively reasonable number of index
pages. But ... what if they're not? Could the index contain a large
number of pages containing just 1 tuple each, or no tuples at all? If
so, maybe we can read ten bazillion index pages trying to find each
heap tuple and still end up in trouble.

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