Обсуждение: Missing conversion error handling in postgres_fdw

Hi,

I noticed that this in make_tuple_from_result_row does conversion error
handling only for the ordinary-column case (ie, errpos.cur_attno is set
for that case, but not for the ctid case).

        /* convert value to internal representation */
        if (i > 0)
        {
            /* ordinary column */
            Assert(i <= tupdesc->natts);
            nulls[i - 1] = (valstr == NULL);
            /* Apply the input function even to nulls, to support domains */
            errpos.cur_attno = i;
            values[i - 1] = InputFunctionCall(&attinmeta->attinfuncs[i - 1],
                                              valstr,
                                              attinmeta->attioparams[i - 1],
                                              attinmeta->atttypmods[i - 1]);
            errpos.cur_attno = 0;
        }
        else if (i == SelfItemPointerAttributeNumber)
        {
            /* ctid --- note we ignore any other system column in result */
            if (valstr != NULL)
            {
                Datum       datum;

                datum = DirectFunctionCall1(tidin, CStringGetDatum(valstr));
                ctid = (ItemPointer) DatumGetPointer(datum);
            }
        }

I think errpos.cur_attno should be set for the ctid case as well.
Attached is a patch for that.

Best regards,
Etsuro Fujita

On Tue, Mar 15, 2016 at 4:06 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
> I noticed that this in make_tuple_from_result_row does conversion error
> handling only for the ordinary-column case (ie, errpos.cur_attno is set
> for that case, but not for the ctid case).
>
>         /* convert value to internal representation */
>         if (i > 0)
>         {
>             /* ordinary column */
>             Assert(i <= tupdesc->natts);
>             nulls[i - 1] = (valstr == NULL);
>             /* Apply the input function even to nulls, to support domains */
>             errpos.cur_attno = i;
>             values[i - 1] = InputFunctionCall(&attinmeta->attinfuncs[i - 1],
>                                               valstr,
>                                               attinmeta->attioparams[i - 1],
>                                               attinmeta->atttypmods[i - 1]);
>             errpos.cur_attno = 0;
>         }
>         else if (i == SelfItemPointerAttributeNumber)
>         {
>             /* ctid --- note we ignore any other system column in result */
>             if (valstr != NULL)
>             {
>                 Datum       datum;
>
>                 datum = DirectFunctionCall1(tidin, CStringGetDatum(valstr));
>                 ctid = (ItemPointer) DatumGetPointer(datum);
>             }
>         }
>
> I think errpos.cur_attno should be set for the ctid case as well.
> Attached is a patch for that.

Hmm, I'd say you are right. Committed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2016/03/16 5:55, Robert Haas wrote:
> On Tue, Mar 15, 2016 at 4:06 AM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.jp> wrote:
>> Attached is a patch for that.

> Hmm, I'd say you are right. Committed.

Thank you for taking care of this!

Best regards,
Etsuro Fujita