patch for xidin

Started by Zhang Zqover 10 years ago3 messages
#1Zhang Zq
zqzhangmail@163.com
1 attachment(s)

hi,
The implements of 'xidin' use only ‘strtoul’ to cast from string to xid. So in some cases, may cause confusion, for example,
The sql 'select c1 from test where xmin='abc' can be executed. and sometimes will make mistakes, I want to query "select c1 from test where xmin='0x10'" ,but write 'Ox10', '0' to 'O',The result is obviously wrong.

The patch will correct it. I have justly copy some code of 'OID'. Whether we need to extract the common code?

Thanks.

Attachments:

xid.patchapplication/octet-stream; name=xid.patchDownload
*** a/src/backend/utils/adt/xid.c
--- b/src/backend/utils/adt/xid.c
***************
*** 32,40 ****
  Datum
  xidin(PG_FUNCTION_ARGS)
  {
! 	char	   *str = PG_GETARG_CSTRING(0);
  
! 	PG_RETURN_TRANSACTIONID((TransactionId) strtoul(str, NULL, 0));
  }
  
  Datum
--- 32,83 ----
  Datum
  xidin(PG_FUNCTION_ARGS)
  {
! 	unsigned long cvt;
! 	char	   *endptr;
! 	char	   *s = PG_GETARG_CSTRING(0);
! 	
! 	if (*s == '\0')
! 		ereport(ERROR,
! 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! 				 errmsg("invalid input syntax for type xid: \"%s\"",
! 						s)));
  
! 	errno = 0;
! 	cvt = strtoul(s, &endptr, 0);
! 
! 	/*
! 	 * strtoul() normally only sets ERANGE.  On some systems it also may set
! 	 * EINVAL, which simply means it couldn't parse the input string. This is
! 	 * handled by the second "if" consistent across platforms.
! 	 */
! 	if (errno && errno != ERANGE && errno != EINVAL)
! 		ereport(ERROR,
! 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! 				 errmsg("invalid input syntax for type xid: \"%s\"",
! 						s)));
! 
! 	if (endptr == s)
! 		ereport(ERROR,
! 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! 				 errmsg("invalid input syntax for type xid: \"%s\"",
! 						s)));
! 
! 	if (errno == ERANGE)
! 		ereport(ERROR,
! 				(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
! 				 errmsg("value \"%s\" is out of range for type xid", s)));
! 
! 		/* allow only whitespace after number */
! 	while (*endptr && isspace((unsigned char) *endptr))
! 		endptr++;
! 	if (*endptr)
! 		ereport(ERROR,
! 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
! 				 errmsg("invalid input syntax for type xid: \"%s\"",
! 						s)));
! 
! 
! 	PG_RETURN_TRANSACTIONID((TransactionId) cvt);
  }
  
  Datum
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Zhang Zq (#1)
Re: patch for xidin

"Zhang Zq" <zqzhangmail@163.com> writes:

The implements of 'xidin' use only ¡®strtoul¡¯ to cast from string to xid. So in some cases, may cause confusion, for example,
The sql 'select c1 from test where xmin='abc' can be executed. and sometimes will make mistakes, I want to query "select c1 from test where xmin='0x10'" ,but write 'Ox10', '0' to 'O',The result is obviously wrong.

The patch will correct it. I have justly copy some code of 'OID'. Whether we need to extract the common code?

This seems like an awful lot of code to solve a problem that will never
occur in practice.

regards, tom lane

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

#3Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#2)
Re: patch for xidin

On Fri, Apr 17, 2015 at 10:27 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

The patch will correct it. I have justly copy some code of 'OID'. Whether we need to extract the common code?

This seems like an awful lot of code to solve a problem that will never
occur in practice.

It does seem like an awful lot of code. We should be able to come up
with something shorter. But the bug report is legitimate. It's not
too much to ask that data types sanity check their inputs.

--
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