-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: New Functionalities #13
base: main
Are you sure you want to change the base?
Conversation
src/query_generate.rs
Outdated
)); | ||
query_code.push('\n'); | ||
|
||
query_code.push_str(&generate_unique_query_code(table_name, schema_name, rows)); | ||
query_code.push('\n'); | ||
// query_code.push_str(&generate_unique_query_code(table_name, schema_name, &rows)); |
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.
This created duplicate function names for some of my tables. I had to comment this out and forget about it. will look into it soon.
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.
Maybe it's because primary keys are unique - the by_{}
one is meant to be for the pk, where as this is meant to be for all other unique keys.
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.
PK query is sort of important for cases where the PK is two or more columns, which may or may not each be unique
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 guess I fixed it by filtering more based on the given table name and unique columns.
pub(crate) fn rust_type_fix(column_name: &str) -> String { | ||
if column_name == "type" { | ||
String::from("r#type") | ||
} else { | ||
column_name.to_string() | ||
} | ||
} |
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.
Good catch, thanks for this
src/utils.rs
Outdated
let serde_derives = if enable_serde { | ||
", serde::Serialize, serde::Deserialize" | ||
} else { | ||
"" | ||
}; |
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 actually wonder whether instead of hardcoding this for serde, it's worth making more extensible by just allowing people to provide extra headers (sort of like how tonic build works for gRPC)
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.
That's a great idea. We could have new args like --struct-derive
and --enum-derive
that could take multiple values
@@ -543,7 +573,7 @@ fn generate_select_by_fk_query_code( | |||
select_code.push_str(&format!( | |||
" pub async fn all_by_{}_{}<'e, E: PgExecutor<'e>>(executor: E, {}_{}: {}) -> Result<Vec<{}>> {{\n", | |||
to_snake_case(foreign_row_table_name), | |||
foreign_row_column_name, | |||
to_snake_case(column_name), |
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 think this might need to be foreign_row_column_name still
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.
yeah, you're right. I've changed this because I was figuring out what was causing the duplicate functions.
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.
with foreign_row_column_name
:
pub async fn all_by_profile_id<'e, E: PgExecutor<'e>>(executor: E, profile_id: i64) -> Result<Vec<Friendship>> {
query_as::<_, Friendship>(r#"SELECT * FROM "friendship" WHERE from_user = $1"#)
.bind(profile_id)
.fetch_all(executor)
.await
}
pub async fn all_by_profile_id<'e, E: PgExecutor<'e>>(executor: E, profile_id: i64) -> Result<Vec<Friendship>> {
query_as::<_, Friendship>(r#"SELECT * FROM "friendship" WHERE to_user = $1"#)
.bind(profile_id)
.fetch_all(executor)
.await
}
with to_snake_case(column_name)
:
pub async fn all_by_profile_from_user<'e, E: PgExecutor<'e>>(executor: E, profile_id: i64) -> Result<Vec<Friendship>> {
query_as::<_, Friendship>(r#"SELECT * FROM "friendship" WHERE from_user = $1"#)
.bind(profile_id)
.fetch_all(executor)
.await
}
pub async fn all_by_profile_to_user<'e, E: PgExecutor<'e>>(executor: E, profile_id: i64) -> Result<Vec<Friendship>> {
query_as::<_, Friendship>(r#"SELECT * FROM "friendship" WHERE to_user = $1"#)
.bind(profile_id)
.fetch_all(executor)
.await
}
yeah I remember why I changed it :)
.arg( | ||
Arg::with_name("exclude") | ||
.short('e') | ||
.long("exclude") | ||
.takes_value(true) | ||
.value_name("SQLGEN_EXCLUDE") | ||
.multiple(true) | ||
.use_delimiter(true) | ||
.help("Specify the excluded table name(s)"), | ||
) |
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.
great idea
Appreciate the work so far! |
With this library, we could create many more generators for other things as well. For example, I'm working on creating redis json schemas in here: R3-Lab@bdba715. But Since we only focus only on the rust types and helpers for SQL, Redis may not have life in here :) lmk what you think |
Thanks a lot for this. I even had a column name |
Yeah I think my intention is to maybe expand to support other DBs in future but it probably still would be SQL. |
schemas: Option<Vec<&str>>, | ||
date_time_lib: DateTimeLib, | ||
struct_derives: Vec<String>, |
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.
Ty for doing this 🙏
closes #12
I wanted to bypass geo types by making it String on the rust side to store the geom from SQL as a string. I've also added new features to integrate my project more.
Added:
--serde
flag if you wanna derive structs/enums with serde--datetime-lib
to selecttime
orchrono
crate to use. defaults tochrono
Fixed:
type
, it's a reserved keyword in Rust. So, I created a helper functionrust_type_fix
. (naming could be better)Notes: