Re: EvalPlanQual behaves oddly for FDW queries involving system columns

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: EvalPlanQual behaves oddly for FDW queries involving system columns
Date: 2015-01-19 08:10:32
Message-ID: 54BCBBF8.3020103@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2015/01/16 1:24, Alvaro Herrera wrote:
> Etsuro Fujita wrote:
>> *** 817,826 **** InitPlan(QueryDesc *queryDesc, int eflags)
>> --- 818,833 ----
>> break;
>> case ROW_MARK_COPY:
>> /* there's no real table here ... */
>> + relkind = rt_fetch(rc->rti, rangeTable)->relkind;
>> + if (relkind == RELKIND_FOREIGN_TABLE)
>> + relid = getrelid(rc->rti, rangeTable);
>> + else
>> + relid = InvalidOid;
>> relation = NULL;
>> break;

>> --- 2326,2342 ----
>>
>> /* build a temporary HeapTuple control structure */
>> tuple.t_len = HeapTupleHeaderGetDatumLength(td);
>> ! /* if relid is valid, rel is a foreign table; set system columns */
>> ! if (OidIsValid(erm->relid))
>> ! {
>> ! tuple.t_self = td->t_ctid;
>> ! tuple.t_tableOid = erm->relid;
>> ! }
>> ! else
>> ! {
>> ! ItemPointerSetInvalid(&(tuple.t_self));
>> ! tuple.t_tableOid = InvalidOid;
>> ! }
>> tuple.t_data = td;

> I find this arrangement confusing and unnecessary -- surely if you have
> access to the ExecRowMark here, you could just obtain the relid with
> RelationGetRelid instead of saving the OID beforehand?

I don't think so because we don't have the Relation (ie, erm->relation =
NULL) here since InitPlan don't open/lock relations having markType =
ROW_MARK_COPY as shown above, which include foreign tables selected for
update/share. But I noticed we should open/lock such foreign tables as
well in InitPlan because I think ExecOpenScanRelation is assuming that,
IIUC.

> And if you have
> the Relation, you could just consult the relkind at that point instead
> of relying on the relid being set or not as a flag to indicate whether
> the rel is foreign.

Followed your revision. Attached is an updated version of the patch.

Thanks for the comment!

Best regards,
Etsuro Fujita

Attachment Content-Type Size
EvalPlanQual-v2.patch text/x-diff 5.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2015-01-19 08:16:11 Re: Patch: add recovery_timeout option to control timeout of restore_command nonzero status code
Previous Message Pavel Stehule 2015-01-19 07:59:37 Re: proposal: disallow operator "=>" and use it for named parameters