/diesel

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

Document `column + 1` needs `numeric_expr!(column)` #966

Closed
killercup opened this Issue Jun 23, 2017 · 0 comments

Comments

Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
1 participant
@killercup
@killercup
Member

killercup commented Jun 23, 2017

See https://gitter.im/diesel-rs/diesel?at=594d2b24743aa6f70890dad8

Full example:

#[macro_use] extern crate diesel;
#[macro_use] extern crate diesel_codegen;

use diesel::prelude::*;

table! {
    files(id) {
        id -> Integer,
        views -> Int8,
    }
}

fn main() {
    // Help people find the next two lines!
    numeric_expr!(files::id);
    numeric_expr!(files::views);

    files::table.select(files::id + 1);

    diesel::update(files::table.filter(files::id.eq(0)))
            .set(files::views.eq(files::views + 1))
            .returning(files::views);
}

@killercup killercup added documentation good first issue help wanted labels Jun 23, 2017

sgrif added a commit that referenced this issue Jul 4, 2017

@sgrif
Automatically generate `numeric_expr!` for known numeric types 
This was a much larger pain in the ass then I expected. It turns out
that capturing a token as a `ty` doesn't just prevent breaking it back
apart later, it prevents any matches against it period. So we have to
carry the ty through as a series of `tt` until the very end.

The problem is that this is impossible to do unambiguously, and we
can't even reliably get just the "common" cases since ty params use `<>`
which aren't a single token tree.

So this implementation basically does a best effort case to match the
"common" cases, and then falls back to just capturing in a `ty`
fragment afterwards. For the cases that don't match `$($x:tt)::*
$(<$($y:tt)::*>)*`, they would never be numeric anyway (we will need to
increase the nesting to 3 for the ty params once unsigned comes back
though, since `Nullable<Unsigned<Integer>>` will be a thing)

Ideally I'd like to eventually revert this, and instead be able to write
the impl with a `where Self::SqlType: types::ops::Add`, but that is
currently invalid in Rust.

Close #966.
Loading status checks…
2f453c9

@sgrif sgrif referenced this issue Jul 4, 2017

Merged

Automatically generate `numeric_expr!` for known numeric types #990

sgrif added a commit that referenced this issue Jul 4, 2017

@sgrif
Automatically generate `numeric_expr!` for known numeric types 
This was a much larger pain in the ass then I expected. It turns out
that capturing a token as a `ty` doesn't just prevent breaking it back
apart later, it prevents any matches against it period. So we have to
carry the ty through as a series of `tt` until the very end.

The problem is that this is impossible to do unambiguously, and we
can't even reliably get just the "common" cases since ty params use `<>`
which aren't a single token tree.

So this implementation basically does a best effort case to match the
"common" cases, and then falls back to just capturing in a `ty`
fragment afterwards. For the cases that don't match `$($x:tt)::*
$(<$($y:tt)::*>)*`, they would never be numeric anyway (we will need to
increase the nesting to 3 for the ty params once unsigned comes back
though, since `Nullable<Unsigned<Integer>>` will be a thing)

Ideally I'd like to eventually revert this, and instead be able to write
the impl with a `where Self::SqlType: types::ops::Add`, but that is
currently invalid in Rust.

Close #966.
Loading status checks…
a226566

sgrif added a commit that referenced this issue Jul 5, 2017

@sgrif
Automatically generate `numeric_expr!` for known numeric types 
This was a much larger pain in the ass then I expected. It turns out
that capturing a token as a `ty` doesn't just prevent breaking it back
apart later, it prevents any matches against it period. So we have to
carry the ty through as a series of `tt` until the very end.

The problem is that this is impossible to do unambiguously, and we
can't even reliably get just the "common" cases since ty params use `<>`
which aren't a single token tree.

So this implementation basically does a best effort case to match the
"common" cases, and then falls back to just capturing in a `ty`
fragment afterwards. For the cases that don't match `$($x:tt)::*
$(<$($y:tt)::*>)*`, they would never be numeric anyway (we will need to
increase the nesting to 3 for the ty params once unsigned comes back
though, since `Nullable<Unsigned<Integer>>` will be a thing)

Ideally I'd like to eventually revert this, and instead be able to write
the impl with a `where Self::SqlType: types::ops::Add`, but that is
currently invalid in Rust.

Close #966.
Loading status checks…
8ac93a8

@sgrif sgrif closed this in #990 Jul 5, 2017

Fiedzia added a commit to Fiedzia/diesel that referenced this issue Jul 6, 2017

@sgrif @Fiedzia
Automatically generate `numeric_expr!` for known numeric types 
This was a much larger pain in the ass then I expected. It turns out
that capturing a token as a `ty` doesn't just prevent breaking it back
apart later, it prevents any matches against it period. So we have to
carry the ty through as a series of `tt` until the very end.

The problem is that this is impossible to do unambiguously, and we
can't even reliably get just the "common" cases since ty params use `<>`
which aren't a single token tree.

So this implementation basically does a best effort case to match the
"common" cases, and then falls back to just capturing in a `ty`
fragment afterwards. For the cases that don't match `$($x:tt)::*
$(<$($y:tt)::*>)*`, they would never be numeric anyway (we will need to
increase the nesting to 3 for the ty params once unsigned comes back
though, since `Nullable<Unsigned<Integer>>` will be a thing)

Ideally I'd like to eventually revert this, and instead be able to write
the impl with a `where Self::SqlType: types::ops::Add`, but that is
currently invalid in Rust.

Close diesel-rs#966.
912d12f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment