>>>>> "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