Hi, Oleksandr! On Nov 21, Oleksandr Byelkin wrote: > revision-id: 49b63dcc0482bcb0fc7f642b0c19c8e24740ab27 > (mariadb-10.4.0-20-g49b63dcc048) > parent(s): b5ac863f1494920b5e7035c9dfa0ebfdaa50a15d > author: Oleksandr Byelkin <[email protected]> > committer: Oleksandr Byelkin <[email protected]> > timestamp: 2018-11-20 10:58:34 +0100 > message: > > MDEV-15073: Generic UDAF parser code in server for windows functions > > Added support for usual agreggate UDF (UDAF) > Added remove() call support for more efficient window function processing > Added example of aggregate UDF with efficient windows function support
Looks good, see a couple of minor comments below. I wouldn't extend UDF API, really. It's ancient and it should be removed and replaced with a new plugin type, not extended. But if you still want to do it make sure it's documented https://mariadb.com/kb/en/user-defined-functions/ > diff --git a/mysql-test/main/udf.test b/mysql-test/main/udf.test > index c3a25c6bcce..e72f8b219af 100644 > --- a/mysql-test/main/udf.test > +++ b/mysql-test/main/udf.test > @@ -528,3 +528,62 @@ DROP FUNCTION METAPHON; > #INSERT INTO t1 (a) VALUES ('Hello'); > #SELECT * FROM t1; > DROP TABLE t1; > + > +--echo > +--echo MDEV-15073: Generic UDAF parser code in server for windows functions > +--echo use --echo # so that your echo commands would stand out in the result file. currently they look like an output from some query and it's confusing. The test itself is good. > diff --git a/sql/item_sum.cc b/sql/item_sum.cc > index 0e52b2988a3..d19645b5020 100644 > --- a/sql/item_sum.cc > +++ b/sql/item_sum.cc > @@ -3235,6 +3235,25 @@ bool Item_udf_sum::add() > DBUG_RETURN(0); > } > > + > +bool Item_udf_sum::supports_removal() > +{ > + DBUG_ENTER("Item_udf_sum::supports_remove"); > + DBUG_PRINT("info", ("support: %d", udf.supports_removal())); > + DBUG_RETURN(udf.supports_removal()); > +} > + > + > +void Item_udf_sum::remove() > +{ > + my_bool tmp_null_value; > + DBUG_ENTER("Item_udf_sum::remove"); > + udf.remove(&tmp_null_value); > + null_value= tmp_null_value; > + DBUG_VOID_RETURN; > +} > + > + > void Item_udf_sum::cleanup() > { > /* > diff --git a/sql/item_sum.h b/sql/item_sum.h > index 1a21c257221..663dea1ed7b 100644 > --- a/sql/item_sum.h > +++ b/sql/item_sum.h > @@ -574,7 +574,7 @@ class Item_sum :public Item_func_or_sum > virtual bool add()= 0; > virtual bool setup(THD *thd) { return false; } > > - virtual bool supports_removal() const { return false; } > + virtual bool supports_removal() { return false; } why? > virtual void remove() { DBUG_ASSERT(0); } Regards, Sergei Chief Architect MariaDB and [email protected] _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
