TL;DR:
==
Before:
--
```rust
sql_function!(lower, lower_t, (x: Text) -> Text, "
This represents the lower function.
Unfortunately, the API of `sql_function!` makes providing a doc
string ugly as all hell.");
```
After:
--
```rust
sql_function! {
/// This represents the `lower` function.
/// Because the syntax of `sql_function!` is identical to a normal
/// extern function in Rust, editor hilighting now works,
/// and this will get picked up by ctags.
fn lower(x: Text) -> Text;
}
```
Actual commit description:
===
This redesign comes from a few main desires:
- Remove the second argument to the macro
- Make it easy to add `#[sql_name]` in the future (see #1596)
- Have the function picked up by ctags
- Generally just look more like Rust.
One other nice side effect is that we're able to pick up any/all meta
items. Right now I'm only applying them to the function, but it may be
worth applying them to the module as well to enable various `#[allow]`s
to get added by users in the event of future lints added to rust/clippy
that we are violating.
The second argument to `sql_function!` has long been a wart on Diesel.
Ultimately we want to generate these three things:
```rust
fn lower<X>(x: X) -> lower<X>
where
X: AsExpression<Text>,
{
...
}
pub struct lower<X> {
x: X,
}
pub type lower<X> = lower<<X as AsExpression<Text>>::Expression>;
```
However, we cannot have the struct and the type exist at the same time.
One of them needs to have a different name. Ultimately we don't care
about the struct. The only time it ever needs to get named is in the
body of the generated function. Virtually everything else can work with
the helper type. We will never be able to automatically generate all of
those things unless we either get stable procedural macros, or a working
form of `concat_idents!`. Ultimately all we need is for that struct to
be some unique name, or otherwise unnameable outside of the macro.
However, as I was working on this, it became clear that the helper type
is pretty rarely needed. Generally the only time you care about having
that helper type is if you are making a library to be consumed by others
(e.g. `diesel_full_text_search`), in which case you're already
re-exporting it from other places. When you're re-exporting, we can take
advantage of how Rust deals with name resolution between glob imports
and locally defined items. We actually use `sql_function!` surprisingly
little in Diesel itself, but you can see the effects of this in `date`.
I've opted to give the struct the same name as the function, and the
helper type the "generic" name. The alternative would be to give the
helper type the same name as the function, and call the struct `Repr` or
something similar. However, the struct appears in error messages, the
helper type does not.
At the end of the day though, very little code actually wants or needs
the helper type. We do use `sql_function!` quite a bit in tests, and
there's literally only one place where we were using the type (and that
place was code that was pre-0.16, which is not code I would have written
that way today in the first place).
Edge cases
---
This doesn't quite handle everything the old form does. 100% of the edge
cases we miss come from the use of `sql_function!` inside of a Rust
function. To make all of this work, we need to wrap most of the code
inside of a module. To resolve items the user expects us to, we do `use
super::*`. However, that will not resolve any items that were imported
in the function itself. To make it more concrete, this code will not
compile:
```
fn foo() {
use std::io::Stdout;
mod bar {
use super::*;
type Out = self::Stdout;
}
}
```
The only place this really affects `sql_function!` is with imported SQL
types. For example, you'll often see `use diesel::sql_types::Text`
before the definition of `lower` to avoid having to qualify it. Because
this is limited to SQL types, I've tried to lower the impact of this
caveat by adding `use diesel::sql_types::*;` at the top of the module.
However, `sql_functions!` which reference types from third party crates
will either need to be declared outside of a function, have the imports
done outside of a function, or just qualify the names.
Finally, if you use `sql_function!` inside of a Rust function, you can
never reference the helper types. There is no workaround. You'll just
have to declare the sql function outside the Rust function.
These edge cases are fine. The TL;DR workaround for all of them is
"don't use `sql_function!` inside of a Rust function". Really the only
reason to have done that in the first place is if you're dealing with a
highly variadic function like `coalesce`, where you want to just encode
the exact signature for the case you have right now. Those cases
generally still work, and once we add support for `#[sql_name]`, those
become easier to handle without abusing Rust's scoping rules anyway.
Future plans
---
I think this design has much more room for extension than the old one
did. Here's what I'd like to do with this in the future:
- Support `#[sql_name]` as a special meta item
- Discussed above. It's the primary motivation for the redesign in the
first place.
- Unify `sql_function!` and `no_arg_sql_function!`
- The main difference between them is that the no-arg form just
generates a unit struct, rather than a function. I think we can
probably generate the function no matter what, but then generate a
unit struct instead of a module if there are no arguments.
- Support where clauses
- Required for unifying `no_arg_sql_function!`. `DB` would be a
"magic" type that you can reference in the where clause to add
backend constraints. I suspect we may also want to support `where DB
== Pg` to specify that it's only available on postgres, but that's
really only useful for PG sepcific libraries, so there's not as much
gain.
- Support generics
- We have an obvious syntax we can use to get rid of `ord_function!`
and `fold_function!`. We should use it.
Fixes #1604
moore3071 commentedMar 27, 2018
Going off of the comments in #1596, this proposal is for a redesign of the sql_function macro. The new macro would be a function-like procedural macro that should support the following syntaxes:
This macro should also temporarily support the old sql_function syntax until a reasonable point (e.g. next major release).
Checklist