Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support user defined functions #3440

Closed
Tracked by #3428
lianghanzhen opened this issue Dec 13, 2021 · 18 comments · Fixed by #3655
Closed
Tracked by #3428

Support user defined functions #3440

lianghanzhen opened this issue Dec 13, 2021 · 18 comments · Fixed by #3655
Labels
A-query Area: databend query C-feature Category: feature good first issue Category: good first issue

Comments

@lianghanzhen
Copy link
Contributor

Summary

Currently we need to write different implementation for invertible functions, for example, isnull & isnotnull. Can we create kind of function alias to avoid that? For isnotnull, we can have isnotnull = not(isnull($1)), when we parse the expression isnotnull, it can be rewrited as not(isnull($1)).

@sundy-li
Copy link
Member

Yes, it is an easy task.

We can register isnotnullfunction in function factory with wrapper not ( isnull (args) )

@sundy-li sundy-li added A-query Area: databend query C-feature Category: feature labels Dec 14, 2021
@sundy-li
Copy link
Member

Let's make it more common way.


    pub fn register_reverse(&mut self, name: &str, desc: FunctionDescription) {
        let case_insensitive_desc = &mut self.case_insensitive_desc;

        let reverse_desc = create_reverse_desc(desc);
        case_insensitive_desc.insert(name.to_lowercase(), reverse_desc);
    }

@sundy-li sundy-li added the good first issue Category: good first issue label Dec 14, 2021
@lianghanzhen
Copy link
Contributor Author

We still need to implement the reverse function in order to register in the factory, no?

I was thinking to add something like the following that you don't have to implement the function:

pub fn register_reverse(&mut self, name: &str, substitution: &str)

Also we can provide a CREATE FUNCTION statement to allow users to create their own functions:

CREATE FUNCTION isnotempty AS not(isempty($1))

@sundy-li
Copy link
Member

sundy-li commented Dec 14, 2021

Yes, CREATE Function can be a great feature in the future. But this is custom UDF function belongs to the user.

Currently we are implementing built-in functions which can be used by all users.

@lianghanzhen
Copy link
Contributor Author

Some built-in functions that return boolean seems redundant implementations to me, which can be simply replaced with an alias.

@lianghanzhen
Copy link
Contributor Author

I took a deeper look at the code. I agree with you that it's not a good idea to use alias for built-in functions, because built-in functions are very useful to optimise queries, for example, not ( isnull (args) ) can be simplified to isnotnull(args).

On the other hand, it would be a great feature that we allow users to create their own functions based on the built-in ones.

Do you know how to create a database that can only be accessed by specified users?

@sundy-li
Copy link
Member

Do you know how to create a database that can only be accessed by specified users?

I think you mean create a function that can only be accessed by specified users, now we do not support create function statement.

@lianghanzhen
Copy link
Contributor Author

lianghanzhen commented Dec 21, 2021

Yes, I plan to add it. We need a table to store all the UDFs which might contain 2 columns: name and substitution.

The statement can be:

CREATE FUNCTION isnotnull = not(isnull('$0'))
or
CREATE FUNCTION isnotnull AS not(isnull('$0'))

@sundy-li
Copy link
Member

But CREATE Functione is not just for alias function.
Maybe clickhouse create function using lambda expression is better.

Refer: https://clickhouse.com/docs/zh/sql-reference/statements/create/function/
CREATE FUNCTION linear_equation AS (x, k, b) -> k*x + b;

@sundy-li
Copy link
Member

We need support lambda expression at first...

@lianghanzhen
Copy link
Contributor Author

Thanks for pointing out, I haven't checked the ClickHouse one. Correct me if I'm wrong, the only difference I see between lambda & my proposal is how we specify the params, explicit or implicit. I think the implicit one might be much more friendly for the parser.

@sundy-li
Copy link
Member

You can take a try, it's valuable.

@lianghanzhen
Copy link
Contributor Author

Then back to the question, is there any table/database that can only be accessed by specified users?

@sundy-li
Copy link
Member

No, there is not.

@lianghanzhen
Copy link
Contributor Author

Do you think we can put it in system.functions table? Or is it better to create a new one for UDFs?

@sundy-li
Copy link
Member

It's better to store functions as stage.

create function xxx;
show create function xxx;
desc function xxx;

@lianghanzhen
Copy link
Contributor Author

Well that's a new concept for me. Can you share any documents about it?

@sundy-li
Copy link
Member

sundy-li commented Dec 21, 2021

#3253
#3361

Snowflake:
Console for free: app.snowflake.com

https://docs.snowflake.com/en/sql-reference/sql/create-stage.html
https://docs.snowflake.com/en/sql-reference/sql/create-function.html

image

@lianghanzhen lianghanzhen changed the title Function alias Support user defined functions Dec 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query Area: databend query C-feature Category: feature good first issue Category: good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants