Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan - Mailing list pgsql-hackers

From Etsuro Fujita
Subject Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan
Date
Msg-id 5694C092.5030409@lab.ntt.co.jp
Whole thread Raw
In response to Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan  (Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>)
List pgsql-hackers
On 2016/01/12 2:36, Alvaro Herrera wrote:
> I wonder,
>
>> --- 2166,2213 ----
>>        }
>>
>>        /*
>> !      * If rel is a base relation, detect whether any system columns are
>> !      * requested from the rel.  (If rel is a join relation, rel->relid will be
>> !      * 0, but there can be no Var in the target list with relid 0, so we skip
>> !      * this in that case.  Note that any such system columns are assumed to be
>> !      * contained in fdw_scan_tlist, so we never need fsSystemCol to be true in
>> !      * the joinrel case.)  This is a bit of a kluge and might go away someday,
>> !      * so we intentionally leave it out of the API presented to FDWs.
>>         */
>> !     scan_plan->fsSystemCol = false;
>> !     if (scan_relid > 0)
>>        {
>> !         Bitmapset  *attrs_used = NULL;
>> !         ListCell   *lc;
>> !         int            i;
>>
>> !         /*
>> !          * First, examine all the attributes needed for joins or final output.
>> !          * Note: we must look at reltargetlist, not the attr_needed data,
>> !          * because attr_needed isn't computed for inheritance child rels.
>> !          */
>> !         pull_varattnos((Node *) rel->reltargetlist, scan_relid, &attrs_used);
>>
>> !         /* Add all the attributes used by restriction clauses. */
>> !         foreach(lc, rel->baserestrictinfo)
>>            {
>> !             RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
>> !
>> !             pull_varattnos((Node *) rinfo->clause, scan_relid, &attrs_used);
>>            }
>>
>> !         /* Now, are any system columns requested from rel? */
>> !         for (i = FirstLowInvalidHeapAttributeNumber + 1; i < 0; i++)
>> !         {
>> !             if (bms_is_member(i - FirstLowInvalidHeapAttributeNumber, attrs_used))
>> !             {
>> !                 scan_plan->fsSystemCol = true;
>> !                 break;
>> !             }
>> !         }
>> !
>> !         bms_free(attrs_used);
>> !     }
>>
>>        return scan_plan;
>>    }
>
> Would it make sense to call pull_varattnos(reltargetlist), then walk the
> bitmapset and break if we see a system column, then call
> pull_varattnos() on the rinfo->clause?  That way, if the targetlist
> request a system column we don't have to walk the RestrictInfos.

Seems like a good idea.  Will update the patch.

Best regards,
Etsuro Fujita





pgsql-hackers by date:

Previous
From: Fabien COELHO
Date:
Subject: Re: extend pgbench expressions with functions
Next
From: Dilip Kumar
Date:
Subject: Re: Relation extension scalability