On Sat, 2004-05-15 at 11:52, Tom Lane wrote:
> But the spec hasn't even got log(), so it can hardly be thought to be
> taking a position on what error codes log() should return.  I think
> log() should use the same error codes as ln().

Point taken, I've made this change.

> You might consider changing the explication of the state code to INVALID
> ARGUMENT FOR LOGARITHM, omitting the word NATURAL.

Done.

> I don't think trunc() is portable ... we certainly don't use it anywhere
> else.  May I suggest rint() instead?  Or floor()?

trunc() is C99, but we may as well use floor(), which is C89.

I've attached a revised patch. It incorporates your suggestions, and
makes one additional change: the SQL standard defines sqrt(x) as
power(x, 0.5) -- therefore, ISTM that sqrt(-whatever) should emit the
"invalid argument to power function" SQLSTATE code.

Barring any objections, I'll apply this patch tonight (... or as soon as
CVS is back up).

-Neil

Index: doc/src/sgml/errcodes.sgml
===================================================================
RCS file: /var/lib/cvs/pgsql-server/doc/src/sgml/errcodes.sgml,v
retrieving revision 1.5
diff -c -r1.5 errcodes.sgml
*** a/doc/src/sgml/errcodes.sgml	14 May 2004 21:42:27 -0000	1.5
--- b/doc/src/sgml/errcodes.sgml	15 May 2004 21:29:37 -0000
***************
*** 311,316 ****
--- 311,326 ----
  </row>
  
  <row>
+ <entry><literal>2201E</literal></entry>
+ <entry>INVALID ARGUMENT FOR LOGARITHM</entry>
+ </row>
+ 
+ <row>
+ <entry><literal>2201F</literal></entry>
+ <entry>INVALID ARGUMENT FOR POWER FUNCTION</entry>
+ </row>
+ 
+ <row>
  <entry><literal>2201G</literal></entry>
  <entry>INVALID ARGUMENT FOR WIDTH BUCKET FUNCTION</entry>
  </row>
Index: src/backend/utils/adt/float.c
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/backend/utils/adt/float.c,v
retrieving revision 1.104
diff -c -r1.104 float.c
*** a/src/backend/utils/adt/float.c	7 May 2004 00:24:58 -0000	1.104
--- b/src/backend/utils/adt/float.c	15 May 2004 21:39:03 -0000
***************
*** 1415,1421 ****
  
  	if (arg1 < 0)
  		ereport(ERROR,
! 				(errcode(ERRCODE_FLOATING_POINT_EXCEPTION),
  				 errmsg("cannot take square root of a negative number")));
  
  	result = sqrt(arg1);
--- 1415,1421 ----
  
  	if (arg1 < 0)
  		ereport(ERROR,
! 				(errcode(ERRCODE_INVALID_ARGUMENT_FOR_POWER_FUNCTION),
  				 errmsg("cannot take square root of a negative number")));
  
  	result = sqrt(arg1);
***************
*** 1450,1455 ****
--- 1450,1465 ----
  	float8		result;
  
  	/*
+ 	 * The SQL spec requires that we emit a particular SQLSTATE error
+ 	 * code for certain error conditions.
+ 	 */
+ 	if ((arg1 == 0 && arg2 < 0) ||
+ 		(arg1 < 0 && floor(arg2) != arg2))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_INVALID_ARGUMENT_FOR_POWER_FUNCTION),
+ 				 errmsg("invalid argument for power function")));
+ 
+ 	/*
  	 * We must check both for errno getting set and for a NaN result, in
  	 * order to deal with the vagaries of different platforms...
  	 */
***************
*** 1501,1507 ****
  
  /*
   *		dlog1			- returns the natural logarithm of arg1
-  *						  ("dlog" is already a logging routine...)
   */
  Datum
  dlog1(PG_FUNCTION_ARGS)
--- 1511,1516 ----
***************
*** 1509,1522 ****
  	float8		arg1 = PG_GETARG_FLOAT8(0);
  	float8		result;
  
  	if (arg1 == 0.0)
  		ereport(ERROR,
! 				(errcode(ERRCODE_FLOATING_POINT_EXCEPTION),
  				 errmsg("cannot take logarithm of zero")));
- 
  	if (arg1 < 0)
  		ereport(ERROR,
! 				(errcode(ERRCODE_FLOATING_POINT_EXCEPTION),
  				 errmsg("cannot take logarithm of a negative number")));
  
  	result = log(arg1);
--- 1518,1534 ----
  	float8		arg1 = PG_GETARG_FLOAT8(0);
  	float8		result;
  
+ 	/*
+ 	 * Emit particular SQLSTATE error codes for ln(). This is required
+ 	 * by the SQL standard.
+ 	 */
  	if (arg1 == 0.0)
  		ereport(ERROR,
! 				(errcode(ERRCODE_INVALID_ARGUMENT_FOR_LOG),
  				 errmsg("cannot take logarithm of zero")));
  	if (arg1 < 0)
  		ereport(ERROR,
! 				(errcode(ERRCODE_INVALID_ARGUMENT_FOR_LOG),
  				 errmsg("cannot take logarithm of a negative number")));
  
  	result = log(arg1);
***************
*** 1535,1548 ****
  	float8		arg1 = PG_GETARG_FLOAT8(0);
  	float8		result;
  
  	if (arg1 == 0.0)
  		ereport(ERROR,
! 				(errcode(ERRCODE_FLOATING_POINT_EXCEPTION),
  				 errmsg("cannot take logarithm of zero")));
- 
  	if (arg1 < 0)
  		ereport(ERROR,
! 				(errcode(ERRCODE_FLOATING_POINT_EXCEPTION),
  				 errmsg("cannot take logarithm of a negative number")));
  
  	result = log10(arg1);
--- 1547,1565 ----
  	float8		arg1 = PG_GETARG_FLOAT8(0);
  	float8		result;
  
+ 	/*
+ 	 * Emit particular SQLSTATE error codes for log(). The SQL spec
+ 	 * doesn't define log(), but it does define ln(), so it makes
+ 	 * sense to emit the same error code for an analogous error
+ 	 * condition.
+ 	 */
  	if (arg1 == 0.0)
  		ereport(ERROR,
! 				(errcode(ERRCODE_INVALID_ARGUMENT_FOR_LOG),
  				 errmsg("cannot take logarithm of zero")));
  	if (arg1 < 0)
  		ereport(ERROR,
! 				(errcode(ERRCODE_INVALID_ARGUMENT_FOR_LOG),
  				 errmsg("cannot take logarithm of a negative number")));
  
  	result = log10(arg1);
Index: src/backend/utils/adt/numeric.c
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/backend/utils/adt/numeric.c,v
retrieving revision 1.74
diff -c -r1.74 numeric.c
*** a/src/backend/utils/adt/numeric.c	14 May 2004 21:42:28 -0000	1.74
--- b/src/backend/utils/adt/numeric.c	15 May 2004 21:30:18 -0000
***************
*** 1668,1673 ****
--- 1668,1674 ----
  	Numeric		res;
  	NumericVar	arg1;
  	NumericVar	arg2;
+ 	NumericVar	arg2_trunc;
  	NumericVar	result;
  
  	/*
***************
*** 1681,1690 ****
--- 1682,1707 ----
  	 */
  	init_var(&arg1);
  	init_var(&arg2);
+ 	init_var(&arg2_trunc);
  	init_var(&result);
  
  	set_var_from_num(num1, &arg1);
  	set_var_from_num(num2, &arg2);
+ 	set_var_from_var(&arg2, &arg2_trunc);
+ 
+ 	trunc_var(&arg2_trunc, 0);
+ 
+ 	/*
+ 	 * Return special SQLSTATE error codes for a few conditions
+ 	 * mandated by the standard.
+ 	 */
+ 	if ((cmp_var(&arg1, &const_zero) == 0 &&
+ 		 cmp_var(&arg2, &const_zero) < 0) ||
+ 		(cmp_var(&arg1, &const_zero) < 0 &&
+ 		 cmp_var(&arg2, &arg2_trunc) != 0))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_INVALID_ARGUMENT_FOR_POWER_FUNCTION),
+ 				 errmsg("invalid argument for power function")));
  
  	/*
  	 * Call power_var() to compute and return the result; note it handles
***************
*** 1696,1701 ****
--- 1713,1719 ----
  
  	free_var(&result);
  	free_var(&arg2);
+ 	free_var(&arg2_trunc);
  	free_var(&arg1);
  
  	PG_RETURN_NUMERIC(res);
***************
*** 4408,4417 ****
  	NumericVar	elem;
  	NumericVar	fact;
  	int			local_rscale;
  
! 	if (cmp_var(arg, &const_zero) <= 0)
  		ereport(ERROR,
! 				(errcode(ERRCODE_FLOATING_POINT_EXCEPTION),
  				 errmsg("cannot take logarithm of a negative number")));
  
  	local_rscale = rscale + 8;
--- 4426,4441 ----
  	NumericVar	elem;
  	NumericVar	fact;
  	int			local_rscale;
+ 	int			cmp;
  
! 	cmp = cmp_var(arg, &const_zero);
! 	if (cmp == 0)
  		ereport(ERROR,
! 				(errcode(ERRCODE_INVALID_ARGUMENT_FOR_LOG),
! 				 errmsg("cannot take logarithm of zero")));
! 	else if (cmp < 0)
! 		ereport(ERROR,
! 				(errcode(ERRCODE_INVALID_ARGUMENT_FOR_LOG),
  				 errmsg("cannot take logarithm of a negative number")));
  
  	local_rscale = rscale + 8;
Index: src/include/utils/errcodes.h
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/include/utils/errcodes.h,v
retrieving revision 1.10
diff -c -r1.10 errcodes.h
*** a/src/include/utils/errcodes.h	14 May 2004 21:42:30 -0000	1.10
--- b/src/include/utils/errcodes.h	15 May 2004 21:29:54 -0000
***************
*** 116,122 ****
  #define ERRCODE_ESCAPE_CHARACTER_CONFLICT	MAKE_SQLSTATE('2','2', '0','0','B')
  #define ERRCODE_INDICATOR_OVERFLOW			MAKE_SQLSTATE('2','2', '0','2','2')
  #define ERRCODE_INTERVAL_FIELD_OVERFLOW		MAKE_SQLSTATE('2','2', '0','1','5')
! #define ERRCODE_INVALID_ARGUMENT_FOR_WIDTH_BUCKET_FUNCTION		MAKE_SQLSTATE('2','2', '0', '1', 'G')
  #define ERRCODE_INVALID_CHARACTER_VALUE_FOR_CAST		MAKE_SQLSTATE('2','2', '0','1','8')
  #define ERRCODE_INVALID_DATETIME_FORMAT		MAKE_SQLSTATE('2','2', '0','0','7')
  #define ERRCODE_INVALID_ESCAPE_CHARACTER	MAKE_SQLSTATE('2','2', '0','1','9')
--- 116,124 ----
  #define ERRCODE_ESCAPE_CHARACTER_CONFLICT	MAKE_SQLSTATE('2','2', '0','0','B')
  #define ERRCODE_INDICATOR_OVERFLOW			MAKE_SQLSTATE('2','2', '0','2','2')
  #define ERRCODE_INTERVAL_FIELD_OVERFLOW		MAKE_SQLSTATE('2','2', '0','1','5')
! #define ERRCODE_INVALID_ARGUMENT_FOR_LOG	MAKE_SQLSTATE('2','2', '0','1','E')
! #define ERRCODE_INVALID_ARGUMENT_FOR_POWER_FUNCTION	MAKE_SQLSTATE('2','2', '0', '1', 'F')
! #define ERRCODE_INVALID_ARGUMENT_FOR_WIDTH_BUCKET_FUNCTION	MAKE_SQLSTATE('2','2', '0', '1', 'G')
  #define ERRCODE_INVALID_CHARACTER_VALUE_FOR_CAST		MAKE_SQLSTATE('2','2', '0','1','8')
  #define ERRCODE_INVALID_DATETIME_FORMAT		MAKE_SQLSTATE('2','2', '0','0','7')
  #define ERRCODE_INVALID_ESCAPE_CHARACTER	MAKE_SQLSTATE('2','2', '0','1','9')
---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
      joining column's datatypes do not match

Reply via email to