Re: Add generate_series(numeric, numeric)

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Платон Малюгин <malugin(dot)p(at)gmail(dot)com>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Add generate_series(numeric, numeric)
Date: 2014-09-29 07:19:54
Message-ID: CAB7nPqSrPK_TtUPLdGRdYR6PeNhkrd8OwnAWZ5apu3abNNT4FA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Nice patch! And welcome here.

On Mon, Sep 29, 2014 at 12:42 PM, Платон Малюгин <malugin(dot)p(at)gmail(dot)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

Attachment Content-Type Size
20140929_generate_series_numeric.patch text/x-patch 5.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2014-09-29 07:48:43 Re: Collation-aware comparisons in GIN opclasses
Previous Message Andres Freund 2014-09-29 07:02:35 Re: INSERT ... ON CONFLICT {UPDATE | IGNORE}