Re: Declarative partitioning - another take

On Thu, Dec 22, 2016 at 3:35 AM, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> While working on that, I discovered yet-another-bug having to do with the
> tuple descriptor that's used as we route a tuple down a partition tree. If
> attnums of given key attribute(s) are different on different levels, it
> would be incorrect to use the original slot's (one passed by ExecInsert())
> tuple descriptor to inspect the original slot's heap tuple, as we go down
> the tree. It might cause spurious "partition not found" at some level due
> to looking at incorrect field in the input tuple because of using the
> wrong tuple descriptor (root table's attnums not always same as other
> partitioned tables in the tree). Patch 0001 fixes that including a test.

I committed this, but I'm a bit uncomfortable with it: should the
TupleTableSlot be part of the ModifyTableState rather than the EState?

> It also addresses the problem I mentioned previously that once
> tuple-routing is done, we failed to switch to a slot with the leaf
> partition's tupdesc (IOW, continued to use the original slot with root
> table's tupdesc causing spurious failures due to differences in attums
> between the leaf partition and the root table).
>
> Further patches 0002, 0003 and 0004 fix bugs that I sent one-big-patch for
> in my previous message. Each patch has a test for the bug it's meant to fix.

Regarding 0002, I think that this is kind of a strange fix. Wouldn't
it be better to get hold of the original tuple instead of reversing
the conversion? And what of the idea of avoiding the conversion in
the (probably very common) case where we can?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company