On 02.11.2020 18:59, Peter Eisentraut wrote:
I have committed 0003.
For 0001, normal_rand(), I think you should reject negative arguments
with an error.
I've updated 0001. The change is trivial, see attached.
For 0002, I think you should change the block number arguments to
int8, same as other contrib modules do.
Agree. It will need a bit more work, though. Probably a new version of
pageinspect contrib, as the public API will change.
Ashutosh, are you going to continue working on it?
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
commit b42df63c757bf5ff715052a401aba37691a7e2b8
Author: anastasia <a.lubennik...@postgrespro.ru>
Date: Wed Nov 25 17:19:33 2020 +0300
Handle negative number of tuples passed to normal_rand()
The function converts the first argument i.e. the number of tuples to
return into an unsigned integer which turns out to be huge number when a
negative value is passed. This causes the function to take much longer
time to execute. Instead throw an error about invalid parameter value.
While at it, improve SQL test to test the number of tuples returned by
this function.
Authors: Ashutosh Bapat, Anastasia Lubennikova
diff --git a/contrib/tablefunc/expected/tablefunc.out b/contrib/tablefunc/expected/tablefunc.out
index fffadc6e1b4..97bfd690841 100644
--- a/contrib/tablefunc/expected/tablefunc.out
+++ b/contrib/tablefunc/expected/tablefunc.out
@@ -3,10 +3,20 @@ CREATE EXTENSION tablefunc;
-- normal_rand()
-- no easy way to do this for regression testing
--
-SELECT avg(normal_rand)::int FROM normal_rand(100, 250, 0.2);
- avg
------
- 250
+SELECT avg(normal_rand)::int, count(*) FROM normal_rand(100, 250, 0.2);
+ avg | count
+-----+-------
+ 250 | 100
+(1 row)
+
+-- negative number of tuples
+SELECT avg(normal_rand)::int, count(*) FROM normal_rand(-1, 250, 0.2);
+ERROR: invalid number of tuples: -1
+-- zero number of tuples
+SELECT avg(normal_rand)::int, count(*) FROM normal_rand(0, 250, 0.2);
+ avg | count
+-----+-------
+ | 0
(1 row)
--
diff --git a/contrib/tablefunc/sql/tablefunc.sql b/contrib/tablefunc/sql/tablefunc.sql
index ec375b05c63..5341c005f91 100644
--- a/contrib/tablefunc/sql/tablefunc.sql
+++ b/contrib/tablefunc/sql/tablefunc.sql
@@ -4,7 +4,11 @@ CREATE EXTENSION tablefunc;
-- normal_rand()
-- no easy way to do this for regression testing
--
-SELECT avg(normal_rand)::int FROM normal_rand(100, 250, 0.2);
+SELECT avg(normal_rand)::int, count(*) FROM normal_rand(100, 250, 0.2);
+-- negative number of tuples
+SELECT avg(normal_rand)::int, count(*) FROM normal_rand(-1, 250, 0.2);
+-- zero number of tuples
+SELECT avg(normal_rand)::int, count(*) FROM normal_rand(0, 250, 0.2);
--
-- crosstab()
diff --git a/contrib/tablefunc/tablefunc.c b/contrib/tablefunc/tablefunc.c
index 02f02eab574..c6d32d46f01 100644
--- a/contrib/tablefunc/tablefunc.c
+++ b/contrib/tablefunc/tablefunc.c
@@ -174,6 +174,7 @@ normal_rand(PG_FUNCTION_ARGS)
FuncCallContext *funcctx;
uint64 call_cntr;
uint64 max_calls;
+ int32 num_tuples;
normal_rand_fctx *fctx;
float8 mean;
float8 stddev;
@@ -193,7 +194,14 @@ normal_rand(PG_FUNCTION_ARGS)
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
/* total number of tuples to be returned */
- funcctx->max_calls = PG_GETARG_UINT32(0);
+ num_tuples = PG_GETARG_INT32(0);
+
+ if (num_tuples < 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid number of tuples: %d", num_tuples)));
+
+ funcctx->max_calls = num_tuples;
/* allocate memory for user context */
fctx = (normal_rand_fctx *) palloc(sizeof(normal_rand_fctx));
