Re: TupleTableSlot abstraction

Hi,

Attached is a heavily revised version of the patchset developed in this
thread. It's based on the last version posted by Amit Khandekar at [1].

The largest issue this resolves is that, for JITing, we need to know
what kind of slot we're going to deal with.  E.g. in contrast to right
now, we do not want to generate bespoke deform function for virtual
slots, and we need to be able to discern between slots we know to deform
and slots where we don't.  Up to now generated a deform function
whenever we can figure out what the tupledesc for an EEOP_*_FETCHSOME
operation is, using the fairly ugly hack for looking at
PlanState->scanslot the scan desc, and the right/left tree's for
INNER/OUTER.  That kind of works as far as descriptors go, but doesn't
for the slot types - it's far from uncommon that the slot type inside a
node differs from the result type from the left/right tree.

I've tried various approaches for this, and what I settled on is that
every PlanState signals in new fields what its scan result type and slot
type is, if known; normally that's automatic due to using
ExecInitScanTupleSlot(), ExecInitResultSlot().  By default, the right /
left (aka inner/outer) are inferred using ExecGetResultSlotOps on the
subsidiary node, but can be overriden too.  This gets rid of similar
logic previously present in llvmjit_expr.c, by moving the relevant
knowledge into ExprEvalOp.d.fetch.

While this primarily benefits JITing, it also allows for future
optimizations like eliding slot_getsomattr() / EEOP_*_FETCHSOME calls
for Vars on slots known to be virtual (which is a large percentage, due
to ExecProject() etc).

To avoid bugs around this, I've added assertions that check that hte
slot type is as expected. This, unsurprisingly, caught a number of bugs.

While this approach isn't perfect (in particular, it adds a few new
fields to PlanState), I've tried hard ot find other solutions, and
didn't come up with anything that's meaningfully better.  If anybody has
better ideas, please call now.



The second bigger change is that I've removed ExecFetchGenericSlotTuple
- I really disliked having both that and ExecFetchSlotTuple. Now
ExecFetchSlotTuple has a new *shouldFree parameter that indicates
whether the returned value is allocated in the current context, or
not. I've also renamed it to ExecFetchSlotHeapTuple.  It's currently
still used for places where we modify the returned tuple in-place (to
set oid etc), but subsequent patches in the pluggable storage patchset,
get rid of that too.


I've done a bit of benchmarking (TPCH and pgench), and after some
performance fixes the patchset either has no performance effect, or a
very small performance benefit (when using JITing, it reduces the
overhead).


Further changes:
- changed the split of patches, so they now all pass the
  tests, and individually make (some) sense.
- virtual tuple table slots can now be materialized (by serializing
  !byval columns into a single allocation, that's then pointed to by
  tts_values).
- The responsibility for filling missing columns is now again in
  slot_getsomeattrs, rather than the per-slot callbacks (which can still
  do so if desired, but normally I don't see much point).
- lots of bugfixes
- comment cleanups
- performance improvements
- Reduction in the number of callbacks. getattr, attisnull are gone.


What I'm now planning to do is to go through the big comment in
tuptable.h and update that to the new world.  While I'm not convinced
that that that's the best place for it, it needs to be accurate.

Furthermore:
- More comment polishing
- I'll probably split the commits up a bit further (particulary JIT
  ignoring virtual tuple slots, inlining the hot path of
  slot_getsomeattrs())
- serious commit message polishing

After this, I hope Amit Khandekar will rebase a patch he's sent me
internally that converts triggers to use slots. I'll work on rebasing
the pluggable storage patch ontop of this.

[1] http://archives.postgresql.org/message-id/CAJ3gD9eq38XhLsLTie%2B3NHsCRkLO0xHLA4MQX_3sr6or7xws4Q%40mail.gmail.com

Greetings,

Andres Freund