[SQL] Implementation for user-defined functions by mihaibudiu · Pull Request #1129 · feldera/feldera

mihaibudiu · 2023-12-12T02:46:09Z

Is this a user-visible change (yes/no): yes

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>

mihaibudiu · 2023-12-12T02:50:50Z

This solution is not 100% because it doesn't allow people to specify additional Rust libraries they can use in their code.
For that we will need to make some tweaks, like supplying a directory with a Cargo.toml and a rust file file instead of a rust file with the --udf command-line option.

mihaibudiu · 2023-12-12T02:53:21Z

+        this.udf = new HashMap<>();
+    }
+
+    static class RlikeFunction extends SqlFunction {


The static classes here were moved from CalciteCompiler.

Shouldn't we be attributing them?

mihaibudiu · 2023-12-12T03:02:48Z

@ryzhyr please review only the rust api and the documentation

ryzhyk

This is great stuff.

We will need to discuss with @lalithsuresh how to best integrate this with the API.

I think we shouldn't publish doc changes until then, since there is now way for users to provide those Rust files to Feldera or to invoke the compiler directly.

ryzhyk · 2023-12-12T16:41:17Z

+  :   type [NOT NULL]
+
+createFunctionStatement
+  :   CREATE FUNCTION name '(' [ columnDecl [, columnDecl ]* ] ')' RETURNS generalType


Are arguments immutable, so these are pure functions?

In SQL everything is immutable

I see other dialects have additional attributes like the language type etc, but those look like they'd be easy to add over time.

ryzhyk · 2023-12-12T16:45:58Z

+standard Rust types that the compiler uses to implement SQL datatypes.
+The next section explains what these types are.
+
+Notice the function implemented has an all-capitals name (which is not


Seems repetitive. Didn't you explain this above?

ryzhyk · 2023-12-12T16:53:30Z

+a standard convention for Rust), dictated by the default SQL
+capitalization rules.
+
+There is no type casting or type inference performed for the function


Is this a limitation of the current implementation or is this standard for SQL?

ryzhyk · 2023-12-12T16:55:31Z

+The SQL family of `INTERVAL` types translates to one of two Rust
+types: `ShortInterval` (representing intervals from days to seconds),
+and `LongInterval` (representing intervals from years to months).
+(Our dialect of SQL does allow mixing the two kinds of intervals in a


did you mean does not?

ryzhyk · 2023-12-12T16:57:49Z

+
+In the Rust implementation the function always has to return the type
+`Result<T, Box<dyn std::error::Error>>`, where `T` is the Rust
+equivalent expected return type of the SQL function.  The Rust


OF THE expected?

ryzhyk · 2023-12-12T16:59:31Z

+equivalent expected return type of the SQL function.  The Rust
+function should return an `Err` only when the function fails at
+runtime; in this case the returned error can use the source position
+information to indicate where the error has originated in the code.


What happens to the program when the function returns an error?

ryzhyk · 2023-12-12T17:01:26Z

+
+The first argument passed to the Rust function is always `pos:
+&SourcePositionRange`.  This argument indicates where in the source
+position is placed the code that generated the call to this


ryzhyk · 2023-12-12T17:15:07Z

    }
 }
+
+pub fn st_distance_geopoint_geopoint(left: GeoPoint, right: GeoPoint) -> F64 {


Why do we need to pass these arguments by value?

Three more functions are generated by a macro, with signatures like
st_distance_geopointN_geopoint(left: Option<GeoPoint>, right: GeoPoint) -> Option<F64>. The signature of the other functions is related to this one. They all also call this function.

ryzhyk · 2023-12-12T17:26:12Z

@@ -0,0 +1,9 @@
+//! Crate which just re-exports all the types that are used to implement


This is a module, not a crate. But more importantly I'm not sure it's useful. Anything that needs to be exported should be exported from lib.rs using pub use ....

What I want is people who use this stuff to only write one use line.
If there's a simple way to do this, great.

Right. I don't see how this file helps with this.

mihaibudiu · 2023-12-12T17:37:31Z

I think we shouldn't publish doc changes until then, since there is now way for users to provide those Rust files to Feldera or to invoke the compiler directly.

There are multiple layers of documentation, and I do need to document the grammar and the compiler flags. I think this is fine. if we postpone documentation we forget about it.

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>

mihaibudiu · 2023-12-13T18:32:03Z

We should move this discussion to #1134

lalithsuresh · 2023-12-14T01:13:56Z

+  :   type [NOT NULL]
+
+createFunctionStatement
+  :   CREATE FUNCTION name '(' [ columnDecl [, columnDecl ]* ] ')' RETURNS generalType


I see other dialects have additional attributes like the language type etc, but those look like they'd be easy to add over time.

lalithsuresh · 2023-12-14T01:17:05Z

+        this.udf = new HashMap<>();
+    }
+
+    static class RlikeFunction extends SqlFunction {


Shouldn't we be attributing them?

mihaibudiu added 5 commits December 11, 2023 00:15

[SQL] Parser support for user-defined functions

c0a4578

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>

[SQL] Document user-defined functions

537d288

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>

[SQL] Changelog for UDFs

195f354

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>

[SQL] Minor fixes from self-review

528e8ae

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>

Address review comments

80c89d5

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>

mihaibudiu added the User-facing For PRs that lead to Feldera-user visible changes label Dec 12, 2023

mihaibudiu requested review from lalithsuresh and ryzhyk December 12, 2023 02:46

mihaibudiu force-pushed the udf branch from 40f7b5b to 195f354 Compare December 12, 2023 02:49

mihaibudiu commented Dec 12, 2023

View reviewed changes

ryzhyk approved these changes Dec 12, 2023

View reviewed changes

[SQL] Revert incorrect change to st_point function

dec67ba

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>

abhizer mentioned this pull request Dec 13, 2023

[SQL] Support User Defined Functions organized as a Rust Library #1134

Closed

mihaibudiu added the SQL compiler Related to the SQL compiler label Dec 14, 2023

lalithsuresh approved these changes Dec 14, 2023

View reviewed changes

mihaibudiu merged commit e28181d into main Dec 14, 2023

mihaibudiu deleted the udf branch December 14, 2023 01:41

		@@ -0,0 +1,9 @@
		//! Crate which just re-exports all the types that are used to implement

Conversation

mihaibudiu commented Dec 12, 2023

Uh oh!

mihaibudiu commented Dec 12, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mihaibudiu commented Dec 12, 2023

Uh oh!

ryzhyk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mihaibudiu commented Dec 12, 2023

Uh oh!

mihaibudiu commented Dec 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants