Re: index-only scans

I wrote:
> There are at least four things we could do: ...
> 2. Set varno = OUTER (or maybe INNER).  This is safe because there's no
> other use for OUTER/INNER in a table scan node.  We would have to hack
> things so that the index tuple gets put into econtext->ecxt_outertuple
> (resp. ecxt_innertuple) at runtime, but that seems like no big problem.
> In both setrefs.c and ruleutils.c, it would be desirable to have a
> TargetEntry list somewhere representing the index columns, which setrefs
> would want so it could set up the special Var nodes with fix_upper_expr,
> and ruleutils would want so it could interpret the Vars using existing
> machinery.  I'm not sure whether to hang that list on the index-only
> plan node or expect EXPLAIN to regenerate it at need.

> 3. Invent another special varno value similar to OUTER/INNER but
> representing an index reference.  This is just about like #2 except that
> we could still put the index tuple into econtext->ecxt_scantuple, and
> ExecEvalVar would do the right thing as it stands.

I have mostly-working code for approach #3, but I haven't tried to make
EXPLAIN work yet.  While looking at that I realized that there's a
pretty good argument for adding the above-mentioned explicit TargetEntry
list representing the index columns to index-only plan nodes.  Namely,
that if we don't do it, EXPLAIN will have to go to the catalogs to find
out what's in that index, and this will fall down for "hypothetical"
indexes injected into the planner by index advisors.  We could imagine
adding some more hooks to let the advisor inject bogus catalog data at
EXPLAIN time, but on the whole it seems easier and less fragile to just
have the planner include a data structure it has to build anyway into
the finished plan.

The need for this additional node list field also sways me in a
direction that I'd previously been on the fence about, namely that
I think index-only scans need to be their own independent plan node type
instead of sharing a node type with regular indexscans.  It's just too
weird that a simple boolean indexonly property would mean completely
different contents/interpretation of the tlist and quals.

I've run into one other thing that's going to need to be hacked up
a bit: index-only scans on "name" columns fall over with this modified
code, because there's now tighter checking of the implied tuple
descriptors:

regression=# select relname from pg_class where relname = 'tenk1';
ERROR:  attribute 1 has wrong type
DETAIL:  Table has type cstring, but query expects name.

The reason for this is the hack we put in some time back to conserve
space in system catalog indexes by having "name" columns be indexed as
though they were "cstring", cf commit
5f6f840e93a3649e0d07e85bad188d163e96ec0e.  We will probably need some
compensatory hack in index-only scans, unless we can think of a less
klugy way of representing that optimization.  (Basically, the index-only
code is assuming that btrees don't have storage type distinct from input
type, and that's not the case for the name opclass.  I had kind of
expected the original patch to have some issues with that too, and I'm
still not fully convinced that there aren't corner cases where it'd be
an issue even with the currently committed code.)
        regards, tom lane