>>>>> "Fujii" == Fujii Masao <masao.fu...@gmail.com> writes:

 Fujii> Pushed.

Bug found:

regression=# select count(*) from generate_series(1::numeric,10) v, 
generate_series(1,v) w;
 count 
-------
 99990
(1 row)

regression=# select count(*) from generate_series(1::numeric,10) v, 
generate_series(1,v+0) w;
 count 
-------
    55
(1 row)

The error is in the use of PG_GETARG_NUMERIC and init_var_from_num
when setting up the multi-call state; init_var_from_num points at the
original num's digits rather than copying them, but start_num and
stop_num have just been (potentially) detoasted in the per-call
context, in which case the storage will have been freed by the next
call.

Obviously this could also be fixed by not detoasting the input until
after switching to the multi-call context, but it looks to me like
that would be unnecessarily complex.

Suggested patch attached.

(Is it also worth putting an explicit warning about this kind of thing
in the SRF docs?)

-- 
Andrew (irc:RhodiumToad)

diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index c73f9bc..d841b6f 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -1325,11 +1325,16 @@ generate_series_step_numeric(PG_FUNCTION_ARGS)
 
 		/*
 		 * Use fctx to keep state from call to call. Seed current with the
-		 * original start value.
+		 * original start value. We must copy the start_num and stop_num
+		 * values rather than pointing to them, since we may have detoasted
+		 * them in the per-call context.
 		 */
-		init_var_from_num(start_num, &fctx->current);
-		init_var_from_num(stop_num, &fctx->stop);
+		init_var(&fctx->current);
+		init_var(&fctx->stop);
 		init_var(&fctx->step);
+
+		set_var_from_num(start_num, &fctx->current);
+		set_var_from_num(stop_num, &fctx->stop);
 		set_var_from_var(&steploc, &fctx->step);
 
 		funcctx->user_fctx = fctx;
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index ee6cb50..9d68145 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -1461,3 +1461,41 @@ select (i / (10::numeric ^ 131071))::numeric(1,0)
        9
 (4 rows)
 
+-- Check usage with variables
+select * from generate_series(1::numeric, 3::numeric) i, generate_series(i,3) j;
+ i | j 
+---+---
+ 1 | 1
+ 1 | 2
+ 1 | 3
+ 2 | 2
+ 2 | 3
+ 3 | 3
+(6 rows)
+
+select * from generate_series(1::numeric, 3::numeric) i, generate_series(1,i) j;
+ i | j 
+---+---
+ 1 | 1
+ 2 | 1
+ 2 | 2
+ 3 | 1
+ 3 | 2
+ 3 | 3
+(6 rows)
+
+select * from generate_series(1::numeric, 3::numeric) i, generate_series(1,5,i) j;
+ i | j 
+---+---
+ 1 | 1
+ 1 | 2
+ 1 | 3
+ 1 | 4
+ 1 | 5
+ 2 | 1
+ 2 | 3
+ 2 | 5
+ 3 | 1
+ 3 | 4
+(10 rows)
+
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index a7e92ac..1633e4c 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -854,3 +854,7 @@ select (i / (10::numeric ^ 131071))::numeric(1,0)
 	from generate_series(6 * (10::numeric ^ 131071),
 			     9 * (10::numeric ^ 131071),
 			     10::numeric ^ 131071) as a(i);
+-- Check usage with variables
+select * from generate_series(1::numeric, 3::numeric) i, generate_series(i,3) j;
+select * from generate_series(1::numeric, 3::numeric) i, generate_series(1,i) j;
+select * from generate_series(1::numeric, 3::numeric) i, generate_series(1,5,i) j;
-- 
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