-
Notifications
You must be signed in to change notification settings - Fork 33
Postgres: get_by_any getters #15
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
Conversation
closes #7 |
great work! |
yes no problem, will add an example + documentation this weekend : ) |
On another (related) note, I was thinking... Would it make sense to refactor the backend trait towards something like: pub trait Backend: Sized + Clone {
// ...
fn insert_statement(table: &Table<Self>) -> String {
common::insert_statement(table)
}
fn get_statement(table: &Table<Self>) -> String {
common::get_statement(table)
}
// ...
fn get_any_statement(table: &Table<Self>) -> String
} impl Backend for MySqlBackend {
// ...
fn get_any_statement(table: &Table<Self>) -> String {
compile_error!("'get_many' getters are not supported with the MySql driver")
}
} This way we would not end up with feature flags throughout the code base tldr: sqlx's |
7aab6da
to
41b224d
Compare
@NyxCode added small example and note in docs |
ormx-macros/src/lib.rs
Outdated
@@ -63,6 +63,9 @@ mod utils; | |||
/// **`#[ormx(get_many)]`**: | |||
/// `{pub} async fn get_by_{field_name}(&{field_type}) -> Result<Vec<Self>>` | |||
/// | |||
/// **`#[ormx(get_any)]`**: | |||
/// `{pub} async fn get_by_{field_name}(&[{field_type}]) -> Result<Vec<Self>>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if get_by_any
would be a better name? The default name of the method would become get_by_any_{field_name}
.
get_by_any_email(&[String])
is clearer than get_by_email(&[String])
in my opinion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree!
Been inactive here for a while, sry. Do you still want to change this? I think get_by_any_* is a far better name. As soon as you are ready, I can merge this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem :) will push an update somewhere this week
Anything happening with this PR? |
Just waiting for @bram209 to rename it to |
@bram209 are you still working on this? If not, I'd happily rename the attribute myself and merge it. |
Sorry, this one slipped through : / You can do it if you want, I am a bit busy rest of the week |
Sry have been extremely busy lately :/ |
cleaning up my PR list |
Description
This PR adds
get_by_any
getters. These getters will return all records that match any of the provided values/ids.Usage
My current use case is a graphql api that utilizes data loaders to avoid N+1 queries:
Implementation
It generates the following SQL, to be type checked by sqlx:
SELECT {} FROM {} WHERE {} = ANY($1)
.It is only available for postgres since mysql does not support arrays.