On Fri, Jan 08, 2016 at 04:37:36PM +1100, Haribabu Kommi wrote: > On Mon, Dec 21, 2015 at 1:50 PM, David Fetter <da...@fetter.org> wrote: > > On Sun, Dec 20, 2015 at 06:13:33PM -0600, Jim Nasby wrote: > >> On 11/2/15 5:46 PM, David Fetter wrote: > >> >I'd like to add weighted statistics to PostgreSQL > >> > >> Anything happen with this? If community isn't interested, ISTM it'd be good > >> to put this in PGXN. > > > > I think it's already in PGXN as an extension, and I'll get another > > version out this early this week, as it involves mostly adding some > > tests. > > > > I'll do the float8 ones for core this week, too, and unless there's a > > really great reason to do more data types on the first pass, it should > > be in committable shape. > > I reviewed the patch, following are my observations. > > 1. + precision</type>, <type>numeric</type>, or <type>interval</type> > > with interval type it is giving problem. As interval data type is not > supported, > so remove it in the list of supported inputs.
I'd meant to add more, but will make sure that the next version documents exactly the types it supports. > postgres=# select weighted_avg(f7,f1) from tbl; > ERROR: function weighted_avg(interval, smallint) does not exist at character > 8 > HINT: No function matches the given name and argument types. You > might need to add explicit type casts. > > > 2. +float8_weighted_avg(PG_FUNCTION_ARGS) > > It will be helpful, if you provide some information as a function header, > how the weighted average is calculated similar like other weighted functions. Will do. > 3. + transvalues = check_float8_array(transarray, > "float8_weighted_stddev_accum", 4); > > The second parameter to check_float8_array should be "float8_weighted_accum". Oops. Will fix. > 4. There is an OID conflict of 4066 with latest master code. Will fix. > 5.+ A += newvalW * ( newvalX - transvalues ) / W; > + CHECKFLOATVAL(A, isinf(newvalW) || isinf(newvalX - transvalues) > || isinf(1.0/W), true); > > + Q += newvalW * (newvalX - transvalues) * (newvalX - A); > + CHECKFLOATVAL(A, isinf(newvalX - transvalues) || isinf(newvalX - > A) || isinf(1.0/W), true); > > > Is the need of calculation also needs to be passed to CHECKFLOATVAL? > Just passing > the variables involved in the calculation isn't enough? If expressions > are required then > it should be something as follows? > > CHECKFLOATVAL(A, isinf(transvalues) || isinf(newvalW) || > isinf(newvalX - transvalues) || isinf(1.0/W), true); > > CHECKFLOATVAL(Q, isinf(transvalues) || isinf(newvalX - > transvalues) || isinf(newvalX - A) || isinf(1.0/W), true); Will fix. > I verified the stddev transition and final function calculations > according to wikipedia > and they are fine. Thanks for reviewing this! Cheers, David. -- David Fetter <da...@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers