Skip to content
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.

Sign up for GitHub

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

Mention calling of SQL functions in Selectable docs #3751

Merged
merged 9 commits into from
Apr 5, 2024
Merged

Mention calling of SQL functions in Selectable docs #3751

Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
42e21bb
Mention calling of SQL functions in `Selectable` docs.
ISibboI Aug 16, 2023
651910d
Remove trailing spaces.
ISibboI Aug 16, 2023
ea0281e
`Selectable` docs: Specify what most query fragments are supported for.
ISibboI Aug 19, 2023
b6ac032
Extend doctest for `Selectable` with field initialised by SQL function.
ISibboI Aug 19, 2023
502d06b
Merge branch 'diesel-rs:master' into master
ISibboI Aug 29, 2023
0dfe7f9
Merge branch 'diesel-rs:master' into master
ISibboI Oct 20, 2023
9c979b7
Merge branch 'master' into master
pksunkara Mar 29, 2024
19a318b
Update diesel/src/expression/mod.rs
weiznich Apr 5, 2024
8b215fd
Fix compiling the doc test without the chrono feature enabled
weiznich Apr 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions diesel/src/expression/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,12 +544,18 @@ where
/// # }
/// ```
///
/// If you want to avoid nesting types, you can use the
/// [`Selectable`](derive@Selectable) derive macro's
/// `select_expression` and `select_expression_type` attributes to
/// flatten the fields. If the `select_expression` is simple enough,
/// it is not necessary to specify `select_expression_type`
/// (most query fragments are supported for this).
/// It is also possible to specify an entirely custom select expression
/// for fields when deriving [`Selectable`](derive@Selectable).
/// This is useful for example to
///
/// * avoid nesting types, or to
/// * populate fields with values other than table columns, such as
/// the result of an SQL function like `CURRENT_TIMESTAMP()`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until we generate the appropriate type from sql_function I don't think that currently works, does it? (#3745)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! I have now added a field to the doctest below this comment that gets initialised with diesel::dsl::now. It seems to complain about some trait not being implemented:

error[E0277]: the trait bound `expression::select_by::SelectBy<UserPost, _>: load_dsl::private::CompatibleType<_, _>` is not satisfied
    --> src/expression/mod.rs:596:12
     |
37   |     .first(connection)?;
     |      ----- ^^^^^^^^^^ the trait `load_dsl::private::CompatibleType<_, _>` is not implemented for `expression::select_by::SelectBy<UserPost, _>`
     |      |
     |      required by a bound introduced by this call
     |
     = help: the trait `load_dsl::private::CompatibleType<U, DB>` is implemented for `expression::select_by::SelectBy<U, DB>`
     = note: required for `SelectStatement<FromClause<JoinOn<Join<table, table, Inner>, Grouped<Eq<Nullable<user_id>, Nullable<id>>>>>, ..., ..., ..., ..., ...>` to implement `LoadQuery<'_, _, _>`
     = note: the full type name has been written to '/tmp/rustdoctestBQRQaQ/rust_out.long-type-2905737123615719141.txt'
note: required by a bound in `first`
    --> /home/sibbo/git/diesel/diesel/src/query_dsl/mod.rs:1780:22
     |
1780 |         Limit<Self>: LoadQuery<'query, Conn, U>,
     |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `RunQueryDsl::first`

Is this what you mean?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually for the particular case of dsl::now given how it's defined it would work, however if a user were to define a custom function that does CURRENT_TIMESTAMP() using sql_function! that wouldn't work.

I suspect your error comes from the type you are selecting to, and you may get a more explicit error message using check_for_backend.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, thanks. Then I should update the documentation.

/// or a custom SQL function.
///
/// The select expression is specified via the `select_expression` parameter.
/// For more complex expressions, it may be required to also specify the result type
/// of the expression with `select_expression_type` (most query fragments are supported for this).
pksunkara marked this conversation as resolved.
Show resolved Hide resolved
///
/// ```rust
/// # include!("../doctest_setup.rs");
Expand Down