Hi, Nice patch! And welcome here.
On Mon, Sep 29, 2014 at 12:42 PM, Платон Малюгин <malugi...@gmail.com> wrote: > Could you help to find mistakes? > This implementation is rather broken, particularly when thinking that this code could be used with a negative step... I also see no point in saving explicitly the step sign in the function context. > Some questions: > 1) Is correct using Numeric in generate_series_numeric_fctx instead of > NumericVar? > I'd rather go with NumericVar to facilitate the arithmetic operations at each step between the current and to avoid recomputing at the finish and current values all the time, saving a bit of process for each loop. It also simplifies the calculation of each value. This way you could as well use cmp_var with const_zero to be sure that a given NumericVar is positive or negative, simplifying process. 2) How do you determine object id for new function? Maybe you're looking > for last object id in catalog directory (src/include/catalog/pg_*.h) and > increase by one last object id. > You can use the script unused_oids in src/include/catalog/ to find unused oids. For example after applying with your patch: $ cd src/include/catalog && ./unused_oids 2 - 9 32 86 - 88 90 3154 3156 3259 - 3453 3573 - 3591 3787 - 3801 3952 3954 3994 - 3999 4051 - 5999 6001 - 9999 Btw, while looking at your patch, I actually hacked it a bit and finished with the attached: - changed process to use NumericVar instead of Numeric - addition of custom step values with a function generate_series(numeric,numeric,numeric) - some cleanup and some comments here and there That's still WIP, but feel free to use it for future work. If you are able to add documentation and regression tests to this patch, I would recommend that you register it to the next commit fest, where it would get more review, and hopefully it will get committed. The next commit fest begins on the 15th of October: https://commitfest.postgresql.org/action/commitfest_view?id=24 Regards, -- Michael
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index 2d6a4cb..975843d 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -28,6 +28,7 @@ #include "access/hash.h" #include "catalog/pg_type.h" +#include "funcapi.h" #include "libpq/pqformat.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" @@ -260,6 +261,13 @@ typedef struct NumericVar } NumericVar; +typedef struct +{ + NumericVar current; + NumericVar finish; + NumericVar step; +} generate_series_numeric_fctx; + /* ---------- * Some preinitialized constants * ---------- @@ -1221,6 +1229,111 @@ numeric_floor(PG_FUNCTION_ARGS) PG_RETURN_NUMERIC(res); } + +/* + * generate_series_numeric() - + * + * Generate series of numeric. + */ +Datum +generate_series_numeric(PG_FUNCTION_ARGS) +{ + generate_series_numeric_fctx *fctx; + FuncCallContext *funcctx; + Numeric res; + NumericVar current_var; + NumericVar finish_var; + NumericVar step_var; + + if (SRF_IS_FIRSTCALL()) + { + Numeric start = PG_GETARG_NUMERIC(0); + Numeric finish = PG_GETARG_NUMERIC(1); + NumericVar steploc; + MemoryContext oldcontext; + + /* see if we were given an explicit step size */ + if (PG_NARGS() == 3) + { + Numeric step; + step = PG_GETARG_NUMERIC(2); + + /* Transform step into a variable that can be manipulated */ + init_var_from_num(step, &steploc); + } + else + { + init_var(&steploc); + set_var_from_var(&steploc, &const_one); + } + + /* Step cannot be zero */ + if (cmp_var(&steploc, &const_zero) == 0) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("step size cannot equal zero"))); + + /* + * switch to memory context appropriate for multiple function calls + */ + funcctx = SRF_FIRSTCALL_INIT(); + + /* allocate memory for user context */ + oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx); + + /* allocate memory for user context */ + fctx = (generate_series_numeric_fctx *) + palloc(sizeof(generate_series_numeric_fctx)); + + /* + * Use fctx to keep state from call to call. Seed current with the + * original start value + */ + init_var_from_num(start, ¤t_var); + init_var_from_num(finish, &finish_var); + set_var_from_var(&steploc, &step_var); + free_var(&steploc); + fctx->current = current_var; + fctx->finish = finish_var; + fctx->step = step_var; + funcctx->user_fctx = fctx; + MemoryContextSwitchTo(oldcontext); + } + + /* stuff done on every call of the function */ + funcctx = SRF_PERCALL_SETUP(); + + /* + * get the saved state and use current as the result for this iteration + */ + fctx = funcctx->user_fctx; + current_var = fctx->current; + finish_var = fctx->finish; + step_var = fctx->step; + res = make_result(¤t_var); + + if ((cmp_var(&step_var, &const_zero) > 0 && + cmp_var(¤t_var, &finish_var) <= 0) || + (cmp_var(&step_var, &const_zero) < 0 && + cmp_var(¤t_var, &finish_var) >= 0)) + { + NumericVar tmp; + init_var(&tmp); + + /* Increment for next iteration */ + add_var(¤t_var, &step_var, &tmp); + set_var_from_var(&tmp, ¤t_var); + fctx->current = current_var; + free_var(&tmp); + + SRF_RETURN_NEXT(funcctx, NumericGetDatum(res)); + } + else + { + SRF_RETURN_DONE(funcctx); + } +} + /* * Implements the numeric version of the width_bucket() function * defined by SQL2003. See also width_bucket_float8(). diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h index 3ce9849..ccdf3e8 100644 --- a/src/include/catalog/pg_proc.h +++ b/src/include/catalog/pg_proc.h @@ -3923,6 +3923,10 @@ DATA(insert OID = 1068 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t DESCR("non-persistent series generator"); DATA(insert OID = 1069 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t i 2 0 20 "20 20" _null_ _null_ _null_ _null_ generate_series_int8 _null_ _null_ _null_ )); DESCR("non-persistent series generator"); +DATA(insert OID = 6000 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t i 3 0 1700 "1700 1700 1700" _null_ _null_ _null_ _null_ generate_series_numeric _null_ _null_ _null_ )); +DESCR("non-persistent series generator"); +DATA(insert OID = 6001 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t i 2 0 1700 "1700 1700" _null_ _null_ _null_ _null_ generate_series_numeric _null_ _null_ _null_ )); +DESCR("non-persistent series generator"); DATA(insert OID = 938 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t i 3 0 1114 "1114 1114 1186" _null_ _null_ _null_ _null_ generate_series_timestamp _null_ _null_ _null_ )); DESCR("non-persistent series generator"); DATA(insert OID = 939 ( generate_series PGNSP PGUID 12 1 1000 0 0 f f f f t t s 3 0 1184 "1184 1184 1186" _null_ _null_ _null_ _null_ generate_series_timestamptz _null_ _null_ _null_ )); diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h index d88e7a3..511518d 100644 --- a/src/include/utils/builtins.h +++ b/src/include/utils/builtins.h @@ -1040,6 +1040,7 @@ extern Datum int8_avg(PG_FUNCTION_ARGS); extern Datum int2int4_sum(PG_FUNCTION_ARGS); extern Datum width_bucket_numeric(PG_FUNCTION_ARGS); extern Datum hash_numeric(PG_FUNCTION_ARGS); +extern Datum generate_series_numeric(PG_FUNCTION_ARGS); /* ri_triggers.c */ extern Datum RI_FKey_check_ins(PG_FUNCTION_ARGS);
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers