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
