[PATCHES] error codes for ln(), power()

[Neil Conway]

SQL2003 mandates that ln() and power() emit particular SQLSTATE error
codes for a few illegal combinations of arguments (in Section 6.27 of my
copy of SQL2003). This patch adds those error codes and changes the
several variants of ln() and power() to emit them as appropriate.

I didn't change log() to emit the same error code as ln() when it is
passed similarly incorrect arguments, on the grounds that the SQLSTATE
code defined by SQL specifically refers to the natural logarithm. Does
anyone think I should make both log() and ln() return the same SQLSTATE
code, invent a new SQLSTATE for log() errors, and just leave things as
they are? (the latter being my preference...)

I didn't bother adding any regression tests for this behavior, but I can
do so if anyone thinks that's worth doing.

Barring any objections, I intend to apply this patch within 24 hours.

-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 -	1.5
--- b/doc/src/sgml/errcodes.sgml	15 May 2004 06:18:45 -
***
*** 311,316 
--- 311,326 
  /row
  
  row
+ entryliteral2201E/literal/entry
+ entryINVALID ARGUMENT FOR NATURAL LOGARITHM/entry
+ /row
+ 
+ row
+ entryliteral2201F/literal/entry
+ entryINVALID ARGUMENT FOR POWER FUNCTION/entry
+ /row
+ 
+ row
  entryliteral2201G/literal/entry
  entryINVALID 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 -	1.104
--- b/src/backend/utils/adt/float.c	15 May 2004 06:32:53 -
***
*** 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  trunc(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_NATURAL_LOG),
   errmsg(cannot take logarithm of zero)));
  	if (arg1  0)
  		ereport(ERROR,
! (errcode(ERRCODE_INVALID_ARGUMENT_FOR_NATURAL_LOG),
   errmsg(cannot take logarithm of a negative number)));
  
  	result = log(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 -	1.74
--- b/src/backend/utils/adt/numeric.c	15 May 2004 06:16:20 -
***
*** 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);
  	

Re: [PATCHES] error codes for ln(), power()

[Tom Lane]

Neil Conway [EMAIL PROTECTED] writes:
 I didn't change log() to emit the same error code as ln() when it is
 passed similarly incorrect arguments, on the grounds that the SQLSTATE
 code defined by SQL specifically refers to the natural logarithm. Does
 anyone think I should make both log() and ln() return the same SQLSTATE
 code, invent a new SQLSTATE for log() errors, and just leave things as
 they are? (the latter being my preference...)

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().  Otherwise you're
essentially arguing that SQL-extension functions should never use
relevant codes defined by the standard because that is usage outside the
scope of the standard.  That leads to massive invention of SQLSTATE
codes, which is hardly what we want.  Certainly there are other places
where we've slightly expanded the meaning of a spec-defined code.

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

 + if ((arg1 == 0  arg2  0) ||
 + (arg1  0  trunc(arg2) != arg2))

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

regards, tom lane

---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster