Re: planner missing a trick for foreign tables w/OR conditions

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Dec 17, 2013 at 12:28 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> So at this point I'm pretty much talked into it. We could eliminate the
>> dependence on indexes entirely, and replace this code with a step that
>> simply tries to pull single-base-relation quals out of ORs wherever it can
>> find one. You could argue that the produced quals would sometimes not be
>> worth testing for, but we could apply a heuristic that says to forget it
>> unless the estimated selectivity of the extracted qual is less than,
>> I dunno, 0.5 maybe.

> This is an area where I think things are very different from local and
> remote tables. For a local table, the cost of transmitting a row from
> one plan node to another is basically zero. For a remote table, it's
> potentially far higher, although unfortunately it's hard to know
> exactly. But if the qual is cheap to evaluate, and we're getting back
> a lot of rows, I suspect even eliminating 5-10% of them could work out
> to a win. With a local table, 50% sounds like a reasonable number.

I used 90% as a cutoff in the attached draft patch. Not sure if it's
worth expending a lot of sweat on the point.

Preliminary testing of this makes it look like a good idea. In
particular, it will now extract restriction clauses from ORs even if
those clauses have nothing to do with indexes, as exhibited in the
second new regression test case. (I was unhappy to find that there were
no regression tests covering this behavior at all; though I guess that's
not so surprising given that orindxpath.c dates from before we could put
EXPLAIN output into the regression tests.)

A cosmetic issue that has to be resolved is where to put the new code.
orindxpath.c is clearly now a misnomer; in fact, I don't think that the
code in this form belongs in optimizer/path/ at all. There's some
argument for putting it in initsplan.c, but that file is pretty big
already. Maybe a new file in optimizer/util/? Any thoughts on that?

Also, there's some janitorial cleanup work that remains to be done.
I believe that make_restrictinfo_from_bitmapqual is now dead code,
and generate_bitmap_or_paths could now be static-ified and probably
simplified by dropping its restriction_only option. I've not investigated
in detail what we could get rid of once create_or_index_quals is gone.

regards, tom lane