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

SQLite custom functions #1103

Closed
Boscop opened this Issue Aug 15, 2017 · 25 comments

Comments

Projects
None yet
4 participants
@Boscop

Boscop commented Aug 15, 2017

Are there plans to support creating custom functions when using SQLite?
It would allow using Rust functions in sqlite queries, and can be made accessible in diesel through the sql_function macro.

E.g. in C you can define custom sqlite functions like this:

int sqlite3_extension_init(sqlite3*db,const char**err,const struct sqlite3_api_routines*api) {
  sqlite3_api=api;
  sqlite3_create_function(db,"PCRE_COMPILE",2,SQLITE_UTF8|SQLITE_DETERMINISTIC,0,fn_pcre_compile,0,0);
  sqlite3_create_function(db,"PCRE_CONFIG",1,SQLITE_UTF8|SQLITE_DETERMINISTIC,0,fn_pcre_config,0,0);
  sqlite3_create_function(db,"PCRE_EXEC",-1,SQLITE_UTF8|SQLITE_DETERMINISTIC,0,fn_pcre_exec,0,0);
  sqlite3_create_function(db,"PCRE_FIRSTCHAR",1,SQLITE_UTF8|SQLITE_DETERMINISTIC,0,fn_pcre_firstchar,0,0);
  sqlite3_create_function(db,"PCRE_QUOTE",1,SQLITE_UTF8|SQLITE_DETERMINISTIC,0,fn_pcre_quote,0,0);
  sqlite3_create_function(db,"PCRE_REPLACE",-1,SQLITE_UTF8|SQLITE_DETERMINISTIC,0,fn_pcre_replace,0,0);
  sqlite3_create_function(db,"REGEXP",2,SQLITE_UTF8|SQLITE_DETERMINISTIC,0,fn_regexp,0,0);
  sqlite3_create_module(db,"PCRE_VTABLE",&the_module,0);
  pcre_callout=my_callout;
  return SQLITE_OK;
}

(Source)

@killercup

This comment has been minimized.

Member

killercup commented Aug 15, 2017

I haven't heard of any plans. But this seems interesting! Are there any sqlite bindings/crates that support this? What is the expected API for this?

@Boscop

This comment has been minimized.

Boscop commented Aug 15, 2017

@killercup

This comment has been minimized.

Member

killercup commented Aug 16, 2017

Good. These are just the bindings, though. Looking a bit further, I found that the rusqlite crate supports this with a semi-nice API:

db.create_scalar_function("halve", 1, true, |ctx| {
    let value = try!(ctx.get::<f64>(0));
    Ok(value / 2f64)
})

https://github.com/jgallagher/rusqlite/blob/d5bd7d960106477f4372ebf7c0ec0c0b479e17bd/src/functions.rs#L281-L310

This lives as long as the connection, which shouldn't be a problem when using SQLite.

@Boscop

This comment has been minimized.

Boscop commented Aug 16, 2017

Nice, there's also create_aggregate_function() and remove_function().
The functions would have to be created in r2d2::CustomizeConnection::on_acquire(), right?

What would have to be done to expose this functionality so that it can be used with diesel?

@killercup

This comment has been minimized.

Member

killercup commented Aug 16, 2017

My first attempt would be to add a crate_scalar_function method to SqliteConnection and see if you can make this work. It needs to live there because it needs to access the raw connection. Its content would probably be the same as this.

Then, I'd try to make the API nice, e.g.,

create_function(diesel::sqlite::ScalarFunction::new("halve").deterministic(true).args(1).function(|ctx| …))

(A builder is much more forward-compatible and easier to use than a method that takes a bunch of parameters IMHO.)

After that, it might be nice to reduce the boilerplate. As you noted, we have a sql_function! macro. Maybe we can add a sqlite_function! macro? Something like

sqlite_function!(halve, (value: f64) -> SqliteFnResult<f64> {
    Ok(value / 2f64)
});

that generates something that allows you to connection.create_function(halve::as_sqlite_fn()) as well as use it in queries like numbers.filter(halve(value).eq(2));

@Boscop

This comment has been minimized.

Boscop commented Aug 16, 2017

Yes that would be great.
I'm planning to use it to impl fuzzy string comparison / search.

@killercup

This comment has been minimized.

Member

killercup commented Aug 16, 2017

@Boscop, do you want to work on this?

Thinking about this some more, if we were to add a method that exposes the raw sqlite connection (i.e., an unsafe pointer), this could all live in an external crate. I'm not sure we really want to do that though. Maybe as part of an extra feature ("for-diesel-extension" or something like that)?

I'm planning to use it to impl fuzzy string comparison / search.

Have you seen/tried SQLite's built-in full text search? https://sqlite.org/fts5.html

@Boscop

This comment has been minimized.

Boscop commented Aug 17, 2017

I'm aware of FTS but it doesn't really support the kind of fuzzy matching I need and it looks like diesel doesn't support FTS's MATCH expression or am I missing something?

EDIT: factored out the rest of this comment into a new issue: #1112

@killercup

This comment has been minimized.

Member

killercup commented Aug 17, 2017

@Boscop

This comment has been minimized.

Boscop commented Aug 20, 2017

Another custom function that I'd like to have in a query is |s| Path::new(s).file_stem()

Where would one have to start when adding this feature?
Here is the relevant code in rusqlite.
Would these functions have to be added here?
And then exposed/forwarded here?

@killercup

This comment has been minimized.

Member

killercup commented Aug 27, 2017

@Boscop sorry, I thought #1103 (comment) answered that already. Yes, that is exactly how I'd try to do that. But I haven't experimented with this at all, so unless someone else has any ideas, you'll just have to try and see what issues you might run into. Start with a small POC and we can have a look and help you out :)

@maghoff

This comment has been minimized.

maghoff commented Dec 15, 2017

@Boscop Did you end up doing anything with this?

@killercup Would you care to mentor me a bit on this issue if I pick it up now?

@killercup

This comment has been minimized.

Member

killercup commented Dec 15, 2017

@maghoff sure! I've never done anything with libsqlite3 directly, but I'll try to help :)

@sgrif

This comment has been minimized.

Member

sgrif commented Dec 16, 2017

To be clear, sql_function! should not need to be affected by this at all. This would only require an API which exposes sqlite3_create_function in some form. I suspect whatever we come up with will end up requiring an additional call to sql_function! to be used with the query builder.

@sgrif

This comment has been minimized.

Member

sgrif commented Dec 16, 2017

We also need to consider safety here. Unlike other databases, a custom SQL function in SQLite is tied to the connection, not the database itself. This means that we should ideally enforce that custom SQL functions are always declared.

Probably the best way we could do this by providing an API where you give us all your custom functions, and we give you a function that has the signature (SqliteConnection) -> YourCustomConnection. I don't know that we can reasonably do this though, since we've had 2 years of encouraging people to write functions like fn do_stuff(conn: &SqliteConnection) not fn do_stuff<C: Connection<Backend = Sqlite>>(conn: &C).

I suspect the best we can do is set this up so people define all their SQL functions in one place, we give them a function that takes &mut SqliteConnection, and document "hey if you want to use these, you better make sure you called this function on your connection"

@Boscop

This comment has been minimized.

Boscop commented Dec 19, 2017

@maghoff Not yet, I was very busy working on other stuff..

@sgrif Wouldn't people usually register their custom sqlite functions in r2d2::CustomizeConnection::on_acquire()?

@sgrif

This comment has been minimized.

Member

sgrif commented Dec 19, 2017

There's no reason to use r2d2 with SQLite. "Connections" don't have any actual cost associated with them like they would for a database with a server.

@maghoff

This comment has been minimized.

maghoff commented Dec 30, 2017

Status update: I have gotten started by translating crate_scalar_function from rusqlite. Application-defined functions can return values of different types, but can not read arguments yet.

Work in progress here: https://github.com/diesel-rs/diesel/compare/master...maghoff:sqlite_custom_function?expand=1

@killercup

This comment has been minimized.

Member

killercup commented Jan 12, 2018

@maghoff Happy new year! That looks like a good start! It also seems to be quite self-contained, right? Could this work as a separate crate that uses an extension crate to SqliteConnection?

@maghoff

This comment has been minimized.

maghoff commented Jan 12, 2018

@killercup Thank you for having a look!

Could this work as a separate crate that uses an extension crate to SqliteConnection?

You tell me 😄 I need access to internal_connection in RawConnection from the raw_connection field of SqliteConnection. If I could access that from an external crate, I think everything could be external, yes.

@killercup

This comment has been minimized.

Member

killercup commented Jan 12, 2018

I need access to internal_connection in RawConnection from the raw_connection field of SqliteConnection.

Ah damn, I missed that one. Okay, I don't think we want to expose that right now.

This is looking pretty good. I'd probably not want to land this create_scalar_function fn as public API (it's too easy to mess up the usage), but do you want to open a WIP pull request? We can easier discuss the implementation there :)

@maghoff

This comment has been minimized.

maghoff commented Jan 12, 2018

At present, the application-defined functions cannot accept arguments. My plan was to implement support for that, and then push for more discussion. Maybe a WIP PR would be appropriate then?

@killercup

This comment has been minimized.

Member

killercup commented Jan 12, 2018

Sure! Probably depends on whether you think it makes sense to review as-is or if you expect to change a lot of stuff around anyway.

@maghoff

This comment has been minimized.

maghoff commented Jan 12, 2018

Right. I'll definitely do a pass of clean-up/refactoring before making a PR, but there are some things I need a little bit of input on how to design properly. For example, this branch contains my first unsafe rust code :)

@sgrif

This comment has been minimized.

Member

sgrif commented May 22, 2018

Fixed by #1691

@sgrif sgrif closed this May 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment