(2018/04/25 17:29), Amit Langote wrote:
On 2018/04/25 16:42, Kyotaro HORIGUCHI wrote:
At Wed, 25 Apr 2018 11:19:23 +0900, Amit Langote wrote:
After the refactoring, it appears to me that we only need this much:
+ rte = makeNode(RangeTblEntry);
+ rte->rtekind = RTE_RELATION;
+ rte->relid = RelationGetRelid(rel);
+ rte->relkind = RELKIND_FOREIGN_TABLE;
Mmm.. That is, only relid is required to deparse (I don't mean
that it should be refactored so.). On the other hand
create_foreign_modify requires rte->checkAsUser as well.
That's right. I took care of this in my version, but unfortuneately,
that was ignored in the updated versions. Maybe the comments I added to
the patch were not enough, though.
Hmm, I missed that we do need information from a proper RTE as well. So,
I suppose we should be doing this instead of creating the RTE for foreign
partition from scratch.
+ rte = list_nth(estate->es_range_table, resultRelation - 1);
+ rte = copyObject(rte);
+ rte->relid = RelationGetRelid(rel);
+ rte->relkind = RELKIND_FOREIGN_TABLE;
As I said upthread, we can use the RTE in the range table as-is if the
foreign table is one of the UPDATE subplan partitions or the target
specified in a COPY command. So, I created the patch that way because
that would save cycles. Why not just use that RTE in those cases?
If we apply the other patch I proposed, resultRelation always points to
the correct RTE.
I tried to do that. So, attached are two patches.
1. Fix for ExecInitPartitionInfo to use the right RT index to pass to
InitResultRelInfo
2. v5 of the patch to fix the bug of foreign partitions
Thoughts?
Actually, I also thought the former when creating the patch, but I left
that as-is because I'm not sure that would be really safe;
ExecConstraints looks at the RT index via
GetInsertedColumns/GetUpdatedColumns, so what I thought was: that might
cause unexpected behavior. Anyway, I think that the former is more like
an improvement rather than a fix, so it would be better to leave that
for another patch for PG12?
Best regards,
Etsuro Fujita
