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.

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: added chrono support as optional feature and extend bindable! #10

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

kaiserbh
Copy link
Contributor

Description:

Enhances the bindable! macro to handle Rust types that require type parameters. This is a significant improvement because it allows us to use the bindable! macro with complex types that have parameters, like chrono::DateTimechrono::Utc.

Changes:

  1. Update bindable! Macro: The bindable! macro has been modified to accept a type parameter. Previously, it was not capable of handling Rust types with parameters. This update makes the macro more versatile and broadens its applicability.
  2. Add Chrono Support Module: added a new module chrono_support that uses our updated bindable! macro to handle the DateTime, NaiveDateTime, NaiveDate, and NaiveTime types from the chrono crate. This enhancement allows our application to correctly bind these types when constructing SQL queries, ensuring accurate and efficient interaction with the PostgreSQL database.

Tests
I have run cargo test and everything seem to be working fine and passed.

Note: I am new to rust so if there is anything I did wrong or can be improved do let me know.

replaced $t:ident with $t:ty that way it matches any Rust type including DateTime<Utc>
@jeremychone
Copy link
Owner

This is awesome. I will look at it by the end of next week. If I forget by the end of next week, feel free to ping me.

@jeremychone
Copy link
Owner

jeremychone commented Aug 3, 2023

@kaiserbh wow, this is very high quality, concise PR. Thanks a lot!!! I am planning to merge it and release 0.3.8 today!

@jeremychone jeremychone merged commit 6b0769e into jeremychone:main Aug 3, 2023
@jeremychone
Copy link
Owner

Released in 0.3.8.

@kaiserbh can you give me an example of how you use the type parameter with the new bindable?

@kaiserbh
Copy link
Contributor Author

kaiserbh commented Aug 3, 2023

Released in 0.3.8.

@kaiserbh can you give me an example of how you use the type parameter with the new bindable?

Thank you I updated my crate now using it directly from crates.io, no issues so far. @jeremychone

Also this is how I am using it personally.
Updating subscription timestamp etc.

cargo.toml
sqlb = { version = "0.3.4", features = ["chrono-support"] }
#[derive(sqlb::Fields, Default, Clone, Deserialize)]
pub struct SubscriptionPatch {
    pub client_id: Option<i32>,
    pub plan_id: Option<i32>,
    pub subscription_start: Option<chrono::DateTime<chrono::Utc>>,
    pub subscription_end: Option<chrono::DateTime<chrono::Utc>>,
    pub status: Option<SubscriptionStatus>,
}

#[derive(sqlx::Type, Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[sqlx(type_name = "subscription_status")]
pub enum SubscriptionStatus {
    Active,
    Expired,
    Cancelled,
}
sqlb::bindable!(SubscriptionStatus);

@jeremychone
Copy link
Owner

@kaiserbh super cool! thanks!

@jeremychone
Copy link
Owner

@kaiserbh hum, but this code (or simplified one still work with the old bindable! macro). Trying to find a code that would not.

@kaiserbh
Copy link
Contributor Author

kaiserbh commented Aug 3, 2023

@kaiserbh hum, but this code (or simplified one still work with the old bindable! macro). Trying to find a code that would not.

Hmm for some reason when I added DateTime<Utc> it didn't work before using the old bindable macro because of the type, I could be wrong tho I don't remember much since. @jeremychone but if the old one works maybe change it back to it and keep the support for chorno?

@jeremychone
Copy link
Owner

jeremychone commented Aug 3, 2023

@kaiserbh ho yes, you are right. My bad. I did not replicate this part.

Trying to do my own custom generic type to show the new feature.

Anyway, do not worry about it, I will find a way.

Uou actually fixed a "bug" I had, it should have been ($t:ty) from the start. So well done.

@kaiserbh
Copy link
Contributor Author

kaiserbh commented Aug 3, 2023

@kaiserbh ho yes, you are right. My bad. I did not replicate this part.

Trying to do my own custom generic type to show the new feature.

Anyway, do not worry about it, I will find a way.

Uou actually fixed a "bug" I had, it should have been ($t:ty) from the start. So well done.

No worries, thanks for making the project.
The new bindable should work with any valid Rust type (e.g., i32, String, Vec<u8>, MyStruct, Option<T>, etc.).

@jeremychone
Copy link
Owner

Cool, find a way to reproduce the error of 0.3.7:

// Note: Just a compile check.
#[test]
pub fn test_bindable_generic() -> Result<()> {
	#[derive(Debug, Clone, sqlx::Type)]
	#[sqlx(transparent)]
	pub struct ClientRef<T>(T);

	sqlb::bindable!(ClientRef<String>);

	Ok(())
}

Works perfectly with the 0.3.8.

Just adding it as a unit test main branch (currently 0.3.9-WIP)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants