On 2017/08/22 1:08, Robert Haas wrote:
> On Mon, Aug 21, 2017 at 7:45 AM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.jp> wrote:
>> If there are no objections, I'll add this to the open item list for v10.
>
> This seems fairly ad-hoc to me. I mean, now you have
> CheckValidResultRel not being called in just this one case -- but that
> bypasses all the checks that function might do, not just this one. It
> so happens that's OK at the moment because CheckCmdReplicaIdentity()
> doesn't do anything in the insert case.
>
> I'm somewhat inclined to just view this as a limitation of v10 and fix
> it in v11. If you want to fix it in v10, I think we need a different
> approach -- just ripping the CheckValidResultRel checks out entirely
> doesn't seem like a good idea to me.
Before 389af951552f, the relkind check that is now performed by
CheckValidResultRel(), used to be done in InitResultRelInfo(). ISTM, it
was separated out so that certain ResultRelInfos could be initialized
without the explicit relkind check, either because it's taken care of
elsewhere or the table in question is *known* to be a valid result
relation. Maybe, mostly just the former of the two reasons when that
commit went in.
IMO, the latter case applies when initializing ResultRelInfos for
partitions during tuple-routing, because the table types we allow to
become partitions are fairly restricted.
Also, it seems okay to show the error messages that CheckValidResultRel()
shows when the concerned table is *directly* addressed in a query, but the
same error does not seem so user-friendly when emitted for one of the
partitions while tuple-routing is being set up. IMHO, there should be
"tuple routing" somewhere in the error message shown in that case, even if
it's for the total lack of support for inserts by a FDW.
Thanks,
Amit