Обсуждение: Why do we expand tuples in execMain.c?

Hi,

I noticed
                if (HeapTupleHeaderGetNatts(tuple.t_data) <
                    RelationGetDescr(erm->relation)->natts)
                {
                    copyTuple = heap_expand_tuple(&tuple,
                                                  RelationGetDescr(erm->relation));
                }
                else
                {
                    /* successful, copy tuple */
                    copyTuple = heap_copytuple(&tuple);
                }

in EvalPlanQualFetchRowMarks, and I'm somewhat confused why it's there?
If it's required here, why isn't it required in dozens of other places?

Greetings,

Andres Freund

On 08/08/2018 12:20 AM, Andres Freund wrote:
> Hi,
>
> I noticed
>                 if (HeapTupleHeaderGetNatts(tuple.t_data) <
>                     RelationGetDescr(erm->relation)->natts)
>                 {
>                     copyTuple = heap_expand_tuple(&tuple,
>                                                   RelationGetDescr(erm->relation));
>                 }
>                 else
>                 {
>                     /* successful, copy tuple */
>                     copyTuple = heap_copytuple(&tuple);
>                 }
>
> in EvalPlanQualFetchRowMarks, and I'm somewhat confused why it's there?
> If it's required here, why isn't it required in dozens of other places?
>
>



Not dozens, I think, since you can't have short records for catalog 
tables, which account for most of the calls to heap_copytuple().

I will look at the remainder of cases (less than 10) and reply in a day 
or two.

cheers

andrew


-- 

Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On August 9, 2018 1:33:17 AM GMT+05:30, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
>
>
>On 08/08/2018 12:20 AM, Andres Freund wrote:
>> Hi,
>>
>> I noticed
>>                 if (HeapTupleHeaderGetNatts(tuple.t_data) <
>>                     RelationGetDescr(erm->relation)->natts)
>>                 {
>>                     copyTuple = heap_expand_tuple(&tuple,
>>                                                   RelationGetDescr(erm->relation));
>>                 }
>>                 else
>>                 {
>>                     /* successful, copy tuple */
>>                     copyTuple = heap_copytuple(&tuple);
>>                 }
>>
>> in EvalPlanQualFetchRowMarks, and I'm somewhat confused why it's
>there?
>> If it's required here, why isn't it required in dozens of other
>places?
>>
>>
>
>
>
>Not dozens, I think, since you can't have short records for catalog
>tables, which account for most of the calls to heap_copytuple().
>
>I will look at the remainder of cases (less than 10) and reply in a day
>
>or two.

But why is it needed at all, and the deforming code at the site where we access the columns isn't sufficient?

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

On 08/08/2018 09:15 PM, Andres Freund wrote:
>
> On August 9, 2018 1:33:17 AM GMT+05:30, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
>>
>> On 08/08/2018 12:20 AM, Andres Freund wrote:
>>> Hi,
>>>
>>> I noticed
>>>                 if (HeapTupleHeaderGetNatts(tuple.t_data) <
>>>                     RelationGetDescr(erm->relation)->natts)
>>>                 {
>>>                     copyTuple = heap_expand_tuple(&tuple,
>>>                                                   RelationGetDescr(erm->relation));
>>>                 }
>>>                 else
>>>                 {
>>>                     /* successful, copy tuple */
>>>                     copyTuple = heap_copytuple(&tuple);
>>>                 }
>>>
>>> in EvalPlanQualFetchRowMarks, and I'm somewhat confused why it's
>> there?
>>> If it's required here, why isn't it required in dozens of other
>> places?
>>>
>>
>>
>> Not dozens, I think, since you can't have short records for catalog
>> tables, which account for most of the calls to heap_copytuple().
>>
>> I will look at the remainder of cases (less than 10) and reply in a day
>>
>> or two.
> But why is it needed at all, and the deforming code at the site where we access the columns isn't sufficient?
>


Yeah, I think you're right, and this is vestigial code that I neglected 
to remove when cleaning up from development.

I'll fix it.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services