On Mon, Mar 14, 2016 at 1:29 PM, Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> wrote:
> Hi, > > On 2016/02/09 14:09, Ashutosh Bapat wrote: > >> Sorry, I was wrong. For public user mapping userid is 0 (InvalidOid), >> which is returned as is in UserMapping object. I confused InvalidOid >> with -1. >> > > I think the following umid handling in postgresGetForeignPlan has the same > issue: > > /* > * Build the fdw_private list that will be available to the executor. > * Items in the list must match order in enum FdwScanPrivateIndex. > */ > fdw_private = list_make4(makeString(sql.data), > retrieved_attrs, > makeInteger(fpinfo->fetch_size), > makeInteger(foreignrel->umid)); > > I don't think it's correct to use makeInteger for the foreignrel's umid. > As long as we are using makeInteger() and inVal() pair to set and extract the values, it should be fine. > > You store the umid in the fdw_private list here and extract it from the > list in postgresBeginForeignScan, to get the user mapping. But we really > need that? We have a validated plan when getting called from > postgresBeginForeignScan, so if foreign join, we can simply pick any of the > plan's fs_relids and use it to identify which user to do the remote access > as, in the same way as for foreign tables. > We have done that calculation ones while creating the plan, why do we want to do that again? For a base relation, the user mapping needs to be found out at the time of execution, since it could change between plan creation and execution. But for a join plan invalidation takes care of this change. > Attached is a patch for that. > > Best regards, > Etsuro Fujita > -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company