Hi,

Indeed, this is quite strange...


 I don't want to go too deep into it, but you get stuff like this:

Select pow(2.0, -3)::text = pow(2, -3)::text;
 ?column?
----------
 f
(1 row)


 - you can simplify the ipow function by removing handling of y<0 case,
>    maybe add an assert to be sure to avoid it.

I agree, done.

 - you should add more symmetry and simplify the evaluation:

Done too.

 Add a test case to show what happens on NULL arguments, hopefully the
> result is NULL.

Done and it does.

Thanks again for the review.

On Mon, Nov 6, 2017 at 4:14 PM, Fabien COELHO <coe...@cri.ensmp.fr> wrote:

>
> Hello,
>
> Sorry for the confusion, I wasn't aware that SQL pow changed types
>> depending on the input value.
>>
>
> Indeed, this is quite strange...
>
>   fabien=# SELECT i, POW(2, i) FROM generate_series(-2, 2) AS i;
>    -2 | 0.25
>    -1 | 0.5
>     0 | 1
>     1 | 2
>     2 | 4
>
> I've modified the function to match more closely the behaviour of SQL,
>> except that 0^(negative) returns 'double inf'. Do you think there is any
>> value in raising an error instead?
>>
>
>   fabien=# SELECT POW(0,-1);
>   ERROR:  zero raised to a negative power is undefined
>
> Hmmmm... I'm fine with double inf, because exception in pgbench means the
> end of the script, which is not desirable for benchmarking purposes.
>
> I think that:
>
>  - you can simplify the ipow function by removing handling of y<0 case,
>    maybe add an assert to be sure to avoid it.
>
>  - you should add more symmetry and simplify the evaluation:
>
>    if (int & int)
>    {
>       i1, i2 = ...;
>       if (i2 >= 0)
>         setIntValue(retval, ipow(i1, i2));
>       else
>         // conversion is done by C, no need to coerce again
>         setDoubleValue(retval, pow(i1, i2));
>    }
>    else
>    {
>      d1, d2 = ...;
>      setDoubleValue(retval, pow(d1, d2));
>    }
>
> Add a test case to show what happens on NULL arguments, hopefully the
> result is NULL.
>
> --
> Fabien.
>



-- 

*Raúl Marín Rodríguez *carto.com
From 47f6ec396d3bc11c39066dbd4b31e75b76102094 Mon Sep 17 00:00:00 2001
From: Raul Marin <rmrodrig...@cartodb.com>
Date: Fri, 13 Oct 2017 17:42:23 +0200
Subject: [PATCH] Add pow() support to pgbench

---
 doc/src/sgml/ref/pgbench.sgml                |  7 ++++
 src/bin/pgbench/exprparse.y                  |  3 ++
 src/bin/pgbench/pgbench.c                    | 62 ++++++++++++++++++++++++++++
 src/bin/pgbench/pgbench.h                    |  3 +-
 src/bin/pgbench/t/001_pgbench_with_server.pl | 15 +++++++
 5 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 1f55967e40a..32c94ba0dc1 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -1233,6 +1233,13 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
        <entry><literal>sqrt(2.0)</literal></entry>
        <entry><literal>1.414213562</literal></entry>
       </row>
+      <row>
+       <entry><literal><function>pow(<replaceable>x</replaceable>, <replaceable>y</replaceable>)</function></literal></entry>
+       <entry>integer if <replaceable>x</replaceable> and <replaceable>y</replaceable> are integers and <literal><replaceable>y</replaceable> >= 0</literal>, else double</entry>
+       <entry>Numeric exponentiation</entry>
+       <entry><literal>pow(2.0, 10)</literal></entry>
+       <entry><literal>1024.0</literal></entry>
+      </row>
      </tbody>
      </tgroup>
    </table>
diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y
index 770be981f06..290bca99d12 100644
--- a/src/bin/pgbench/exprparse.y
+++ b/src/bin/pgbench/exprparse.y
@@ -334,6 +334,9 @@ static const struct
 	{
 		"!case_end", -2, PGBENCH_CASE
 	},
+	{
+		"pow", 2, PGBENCH_POW
+	},
 	/* keep as last array element */
 	{
 		NULL, 0, 0
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index add653bf90c..3781e75721d 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -739,6 +739,27 @@ getPoissonRand(TState *thread, int64 center)
 }
 
 /*
+ * pow() for integer values with exp >= 0. Matches SQL pow() behaviour
+ */
+static int64
+ipow(int64 base, int64 exp)
+{
+	int64 result = 1;
+
+	Assert(exp >= 0);
+
+	while (exp)
+	{
+		if (exp & 1)
+			result *= base;
+		exp >>= 1;
+		base *= base;
+	}
+
+	return result;
+}
+
+/*
  * Initialize the given SimpleStats struct to all zeroes
  */
 static void
@@ -1918,6 +1939,47 @@ evalFunc(TState *thread, CState *st,
 				return true;
 			}
 
+		case PGBENCH_POW:
+			{
+				PgBenchValue *lval = &vargs[0];
+				PgBenchValue *rval = &vargs[1];
+
+				Assert(nargs == 2);
+
+				/*
+				 * If both operands are int and exp >= 0 use
+				 * the ipow() function, else use pow()
+				 */
+				if (lval->type == PGBT_INT &&
+					 rval->type == PGBT_INT)
+				{
+
+					int64		li,
+								ri;
+
+					if (!coerceToInt(lval, &li) ||
+						!coerceToInt(rval, &ri))
+						return false;
+
+					if (ri >= 0)
+						setIntValue(retval, ipow(li, ri));
+					else
+						setDoubleValue(retval, pow(li, ri));
+				}
+				else
+				{
+					double		ld,
+								rd;
+
+					if (!coerceToDouble(lval, &ld) ||
+						!coerceToDouble(rval, &rd))
+						return false;
+
+					setDoubleValue(retval, pow(ld, rd));
+				}
+				return true;
+			}
+
 		default:
 			/* cannot get here */
 			Assert(0);
diff --git a/src/bin/pgbench/pgbench.h b/src/bin/pgbench/pgbench.h
index e1277a1dde6..9f26af92bf6 100644
--- a/src/bin/pgbench/pgbench.h
+++ b/src/bin/pgbench/pgbench.h
@@ -94,7 +94,8 @@ typedef enum PgBenchFunction
 	PGBENCH_LE,
 	PGBENCH_LT,
 	PGBENCH_IS,
-	PGBENCH_CASE
+	PGBENCH_CASE,
+	PGBENCH_POW
 } PgBenchFunction;
 
 typedef struct PgBenchExpr PgBenchExpr;
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 8e19bbd3f45..644add6ea63 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -237,6 +237,13 @@ sub pgbench
 		qr{command=36.: int 36\b},
 		qr{command=37.: boolean true\b},
 		qr{command=38.: boolean true\b},
+		qr{command=44.: int -27\b},
+		qr{command=45.: double 1024\b},
+		qr{command=46.: int 1\b},
+		qr{command=47.: double 1\b},
+		qr{command=48.: double -0.125\b},
+		qr{command=49.: double -0.125\b},
+		qr{command=50.: null\b},
 	],
 	'pgbench expressions',
 	{   '001_pgbench_expressions' => q{-- integer functions
@@ -299,6 +306,14 @@ sub pgbench
 \set v2 5432
 \set v3 -54.21E-2
 SELECT :v0, :v1, :v2, :v3;
+--- pow() operator
+\set poweri debug(pow(-3,3))
+\set powerd debug(pow(2.0,10))
+\set poweriz debug(pow(0,0))
+\set powerdz debug(pow(0.0,0.0))
+\set powernegi debug(pow(-2,-3))
+\set powernegd debug(pow(-2.0,-3.0))
+\set powernull debug(pow(NULL, NULL))
 } });
 
 =head
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to