Re: Oddity in tuple routing for foreign partitions

(2018/04/25 4:49), Robert Haas wrote:
> On Tue, Apr 24, 2018 at 12:21 PM, Robert Haas<robertmhaas(at)gmail(dot)com> wrote:
>> On Mon, Apr 23, 2018 at 11:25 AM, Alvaro Herrera
>> <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>>> Robert, I think this is your turf, per 3d956d9562aa. Are you looking
>>> into it?
>>
>> Thanks for the ping. I just had a look over the proposed patch and I
>> guess I don't like it very much. Temporarily modifying the range
>> table in place and then changing it back before we return seems ugly
>> and error-prone. I hope we can come up with a solution that doesn't
>> involve needing to do that.

I see your concern.

> I have done some refactoring to avoid that. See attached.

I like this refactoring.

> I didn't really get beyond the refactoring stage with this today.
> This version still seems to work, but I don't really understand the
> logic in postgresBeginForeignInsert which decides whether to use the
> RTE from the range table or create our own. We seem to need to do one
> sometimes and the other sometimes, but I don't know why that is,
> really. Nor do I understand why we need the other changes in the
> patch. There's probably a good reason, but I haven't figured it out
> yet.

To reduce the cost of creating RTEs, the original patch uses the RTE in
the range table if the partition is an UPDATE subplan partition. One
thing I'm concerned about is this change to postgresBeginForeignInsert:

+ if (resultRelInfo->ri_PartitionRoot && plan)
+ {
+ bool dostuff = true;
+
+ if (resultRelation != plan->nominalRelation)
+ dostuff = false;
+ else
+ resultRelation = mtstate->resultRelInfo[0].ri_RangeTableIndex;
+
+ if (dostuff)
+ {
+ rte = list_nth(estate->es_range_table, resultRelation - 1);
+ rte = copyObject(rte);
+ rte->relid = RelationGetRelid(rel);
+ rte->relkind = RELKIND_FOREIGN_TABLE;
+ }
+ }
+
+ if (rte == NULL)
+ {
+ rte = makeNode(RangeTblEntry);
+ rte->rtekind = RTE_RELATION;
+ rte->relid = RelationGetRelid(rel);
+ rte->relkind = RELKIND_FOREIGN_TABLE;
+ }

IIUC, I think this creates a new RTE for an UPDATE subplan partition by
makeNode, but I'm not sure if that would work well because we decide
which user to do the remote access as by looking at rte->checkAsUser, in
create_foreign_modify.

Thanks for working on this!

Best regards,
Etsuro Fujita