-
Notifications
You must be signed in to change notification settings - Fork 1.7k
docs: Improve documentation for FunctionFactory / CREATE FUNCTION #17859
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
Conversation
@milenkovicm since you added this feature initially as I recall, I wonder if you could review this documentation improvement PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @alamb documentation looks much better now
/// and interact with [SessionState] to registers new udf, udaf or udwf. | ||
/// Interface for handling `CREATE FUNCTION` statements and interacting with | ||
/// [SessionState] to create and register functions ([`ScalarUDF`], | ||
/// [`AggregateUDF`], [`WindowUDF`], and [`TableFunctionImpl`]) dynamically. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tried ScalarUDF, and AggregateUDF but never WindowUDF nor TableFunction, I guess users can plug-in whatever they want here
A shameless plug 😃 🔌 I have a few more working examples if they are going to help with the evaluation of functionality in InfluxData. |
/// ```sql | ||
/// CREATE FUNCTION f1(BIGINT) | ||
/// RETURNS BIGINT | ||
/// RETURN $1 + 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one note, not related to this PR, it would be great if we could use variable names instead of $1
and support default values, last time i've tried it did not work
CREATE FUNCTION our_add(A BIGINT DEFAULT 3, B BIGINT DEFAULT 1)
RETURNS BIGINT
RETURN $A - $B
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Marko Milenković <milenkovicm@users.noreply.github.com>
I've merged this, thanks @alamb, great improvement |
Which issue does this PR close?
Rationale for this change
Internlly at InfluxData, we are evaluating using
CREATE FUNCTION
but when I was providing links to the docs, it was a little sparse and unclear what was supported.I think some documentation would make this functionality a little easier to find and use.
What changes are included in this PR?
Add some documentation and links
Are these changes tested?
By CI and I rendered them locally:
Are there any user-facing changes?
I also added
Debug
andClone
toRegisterFunction
for consistency