Missing conversion error handling in postgres_fdw

Started by Etsuro Fujitaalmost 10 years ago3 messages
#1Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
1 attachment(s)

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

Attachments:

postgres-fdw-conv-error-handling.patchapplication/x-patch; name=postgres-fdw-conv-error-handling.patchDownload
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
***************
*** 3755,3772 **** make_tuple_from_result_row(PGresult *res,
  			valstr = PQgetvalue(res, row, j);
  
  		/* 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)
  		{
--- 3755,3771 ----
  			valstr = PQgetvalue(res, row, j);
  
  		/* convert value to internal representation */
+ 		errpos.cur_attno = i;
  		if (i > 0)
  		{
  			/* ordinary column */
  			Assert(i <= tupdesc->natts);
  			nulls[i - 1] = (valstr == NULL);
  			/* Apply the input function even to nulls, to support domains */
  			values[i - 1] = InputFunctionCall(&attinmeta->attinfuncs[i - 1],
  											  valstr,
  											  attinmeta->attioparams[i - 1],
  											  attinmeta->atttypmods[i - 1]);
  		}
  		else if (i == SelfItemPointerAttributeNumber)
  		{
***************
*** 3779,3784 **** make_tuple_from_result_row(PGresult *res,
--- 3778,3784 ----
  				ctid = (ItemPointer) DatumGetPointer(datum);
  			}
  		}
+ 		errpos.cur_attno = 0;
  
  		j++;
  	}
#2Robert Haas
robertmhaas@gmail.com
In reply to: Etsuro Fujita (#1)
Re: Missing conversion error handling in postgres_fdw

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Robert Haas (#2)
Re: Missing conversion error handling in postgres_fdw

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers