Re: [Maria-developers] 49b63dcc048: MDEV-15073: Generic UDAF parser code in server for windows functions
Hi, Oleksandr! On Nov 21, Oleksandr Byelkin wrote:
revision-id: 49b63dcc0482bcb0fc7f642b0c19c8e24740ab27 (mariadb-10.4.0-20-g49b63dcc048) parent(s): b5ac863f1494920b5e7035c9dfa0ebfdaa50a15d author: Oleksandr Byelkin <sanja@mariadb.com> committer: Oleksandr Byelkin <sanja@mariadb.com> 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 security@mariadb.org
Am 21.11.18 um 20:00 schrieb Sergei Golubchik:
Hi, Oleksandr!
On Nov 21, Oleksandr Byelkin wrote:
revision-id: 49b63dcc0482bcb0fc7f642b0c19c8e24740ab27 (mariadb-10.4.0-20-g49b63dcc048) parent(s): b5ac863f1494920b5e7035c9dfa0ebfdaa50a15d author: Oleksandr Byelkin <sanja@mariadb.com> committer: Oleksandr Byelkin <sanja@mariadb.com> 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/ OK I will.
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 # OK 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? for UDF it will be defined for each instance (object) separately, it can not be more one (const) function for the class. virtual void remove() { DBUG_ASSERT(0); } Regards, Sergei Chief Architect MariaDB and security@mariadb.org
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Hi, Oleksandr! On Nov 22, Oleksandr Byelkin wrote:
revision-id: 49b63dcc0482bcb0fc7f642b0c19c8e24740ab27 (mariadb-10.4.0-20-g49b63dcc048) parent(s): b5ac863f1494920b5e7035c9dfa0ebfdaa50a15d author: Oleksandr Byelkin <sanja@mariadb.com> committer: Oleksandr Byelkin <sanja@mariadb.com> 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
ok to push after restoring `const` as discussed. Regards, Sergei Chief Architect MariaDB and security@mariadb.org
participants (2)
-
Oleksandr Byelkin -
Sergei Golubchik