Обсуждение: Comment update in BuildTupleFromCStrings()

Hi,
BuildTupleFromCStrings() has comment "/* Call the "in" function for
each non-dropped attribute */". It then calls the in function even
when it's going to set that attribute to NULL.
1189         if (!TupleDescAttr(tupdesc, i)->attisdropped)
1190         {
1191             /* Non-dropped attributes */
1192             dvalues[i] = InputFunctionCall(&attinmeta->attinfuncs[i],
1193                                            values[i],
1194                                            attinmeta->attioparams[i],
1195                                            attinmeta->atttypmods[i]);
1196             if (values[i] != NULL)
1197                 nulls[i] = false;
1198             else
1199                 nulls[i] = true;
1200         }

 If we are setting isnull to true i.e. it's a NULL value, dvalues
value doesn't matter but we still invoke corresponding in function,
which looks strange and the comment doesn't help. But there's code in
make_tuple_from_result_row() which does the same thing and explain why
we need to invoke in() function even on the NULL values. I thought,
the same comment applies here. Here's patch to update the comment in
BuildTupleFromCStrings().

The code in make_tuple_from_result_row() that converts an array of
values string to tuple looks quite similar to what
BuildTupleFromCStrings() is doing with a small difference that
make_tuple_from_result_row() maintains and error context to report the
attribute whose in() function caused an error. May be we could pass an
optional error context to the later and use it.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On Fri, Mar 23, 2018 at 03:00:37PM +0530, Ashutosh Bapat wrote:
> Hi,
> BuildTupleFromCStrings() has comment "/* Call the "in" function for
> each non-dropped attribute */". It then calls the in function even
> when it's going to set that attribute to NULL.
> 1189         if (!TupleDescAttr(tupdesc, i)->attisdropped)
> 1190         {
> 1191             /* Non-dropped attributes */
> 1192             dvalues[i] = InputFunctionCall(&attinmeta->attinfuncs[i],
> 1193                                            values[i],
> 1194                                            attinmeta->attioparams[i],
> 1195                                            attinmeta->atttypmods[i]);
> 1196             if (values[i] != NULL)
> 1197                 nulls[i] = false;
> 1198             else
> 1199                 nulls[i] = true;
> 1200         }
> 
>  If we are setting isnull to true i.e. it's a NULL value, dvalues
> value doesn't matter but we still invoke corresponding in function,
> which looks strange and the comment doesn't help. But there's code in
> make_tuple_from_result_row() which does the same thing and explain why
> we need to invoke in() function even on the NULL values. I thought,
> the same comment applies here. Here's patch to update the comment in
> BuildTupleFromCStrings().

Patch applied.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Thanks Bruce.

On Tue, Apr 3, 2018 at 11:31 PM, Bruce Momjian <bruce@momjian.us> wrote:
> On Fri, Mar 23, 2018 at 03:00:37PM +0530, Ashutosh Bapat wrote:
>> Hi,
>> BuildTupleFromCStrings() has comment "/* Call the "in" function for
>> each non-dropped attribute */". It then calls the in function even
>> when it's going to set that attribute to NULL.
>> 1189         if (!TupleDescAttr(tupdesc, i)->attisdropped)
>> 1190         {
>> 1191             /* Non-dropped attributes */
>> 1192             dvalues[i] = InputFunctionCall(&attinmeta->attinfuncs[i],
>> 1193                                            values[i],
>> 1194                                            attinmeta->attioparams[i],
>> 1195                                            attinmeta->atttypmods[i]);
>> 1196             if (values[i] != NULL)
>> 1197                 nulls[i] = false;
>> 1198             else
>> 1199                 nulls[i] = true;
>> 1200         }
>>
>>  If we are setting isnull to true i.e. it's a NULL value, dvalues
>> value doesn't matter but we still invoke corresponding in function,
>> which looks strange and the comment doesn't help. But there's code in
>> make_tuple_from_result_row() which does the same thing and explain why
>> we need to invoke in() function even on the NULL values. I thought,
>> the same comment applies here. Here's patch to update the comment in
>> BuildTupleFromCStrings().
>
> Patch applied.
>
> --
>   Bruce Momjian  <bruce@momjian.us>        http://momjian.us
>   EnterpriseDB                             http://enterprisedb.com
>
> + As you are, so once was I.  As I am, so you will be. +
> +                      Ancient Roman grave inscription +



-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company