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

add ecosystem integration to jiff? (e.g., optional dependencies on icu, diesel, sqlx, etc) #50

Open
BurntSushi opened this issue Jul 26, 2024 · 13 comments
Labels
question Further information is requested

Comments

@BurntSushi
Copy link
Owner

In #4 and in other places, I've resisted adding "ecosystem integration" into Jiff. Not necessarily because I expected others to instead depend on Jiff necessarily right away, but that eventually, as folks started using Jiff (hopefully), it would make sense for them to add a Jiff option.

But I think I should probably flip this script. Getting Jiff adopted is going to be difficult, and I don't know if it has any broadly applicable "killer feature" that will convince folks to jump over from other datetime libraries. (I think Jiff's Span is really amazing, but I haven't figured out the best way of communicating that to a broad audience yet.) I think this means that the best path to Jiff adoption is to reduce friction as much as possible.

I get the sense that folks rely heavily on other crates providing integration points for their chosen datetime types of choice. In particular, it is often the case that converting from one type to another is actually quite tricky. Even tricky enough that I myself get tripped up. So expecting users to just hand-roll these sorts of integrations is probably pretty lofty.

So... I think one way to reduce friction is for Jiff to provide these integrations. One of my initial reservations was introducing public dependencies on crates that are "less stable/mature." While Jiff is currently brand new and I expect to do some breaking change releases, I do hope that in about ~1 year's time, we'll have a Jiff 1.0 and that I'll commit to that for the foreseeable future with no breaking changes. However, I actually don't think this is a huge deal. We can just include the major/minor version of the crate in the feature name itself, just like rust-postgres does.

I have two concerns with this approach. That is, if I start this way by introducing dependencies on crates like diesel, then:

  1. Does this mean it will become very difficult, if not impossible, to coordinate a switch where instead of Jiff providing the integration points, the higher level libraries do it instead?
  2. A datetime library ought to appear "low" in the dependency tree. It is intended to be a building block for other systems. So if Jiff starts depending on "bigger" systems like diesel (diesel is just an example, this applies to most crates I think), then what happens if those bigger systems actually need to depend on a datetime library? Recursive dependencies are a no-go.

Should I pursue this course? Should I limit it to Jiff 0.x versions and then, once 1.0 is out, insist that the integration points get flipped? I'm not sure. That will make 3 datetime choices for a lot of crates, and it's a tough sell.

@BurntSushi BurntSushi added the question Further information is requested label Jul 26, 2024
@lu-zero
Copy link
Contributor

lu-zero commented Jul 26, 2024

Can you make those integrations a separate crate?

@BurntSushi
Copy link
Owner Author

Not easily. Or at least, not without a degraded user experience. Because of the orphan rule. That is, I expect most "integrations" to be implementing traits for Jiff data types. That means the integration needs to be:

  1. In the place where the trait is defined. (So, e.g., in diesel.)
  2. In the place where the type is defined. That is, in jiff.
  3. In some other crate with a newtype wrapper. Then anyone using this crate has to manage the newtype wrapper. This is "doable," but there's friction involved. It can be lessened using various tricks, but I don't think it can be zero friction.

@sslivkoff
Copy link

sslivkoff commented Jul 26, 2024

for (3) maybe a nice macro could minimize the friction

this will make dependency-conscious ppl more likely to integrate jiff

@joshtriplett
Copy link

joshtriplett commented Jul 26, 2024

I don't think jiff should have even optional dependencies on crates like sqlx; having a low-level data type even optionally depend on a database in order to implement the database's trait seems a little too absurd, as well as (as you noted) making it difficult for those libraries to (directly or indirectly) depend on jiff.

One of these days, I hope we figure out a way to relax the orphan rule enough that a sqlx-jiff crate can implement integration between sqlx traits and jiff types. But we don't have that yet, and still need a solution.

Ideally, some of the crates will be willing to accept PRs early on, in the hopes that they'll treat a new low-level data type crate favorably. If they don't want to integrate it because it's pre-1.0, you might be able to get confirmation that they'd be willing to integrate it once it's 1.0.

If that doesn't work, then for something like sqlx, for now, I think it'd be entirely reasonable to have a sqlx-jiff crate that provides 1) a newtype wrapper, 2) bidirectional From impls, 3) a Deref impl, 4) a to_jiff() method on the newtype, and 5) an extension trait with a to_sqlx() method on jiff types, to avoid having to turbofish the From impl for disambiguation (because .into() is ambiguous when dealing with an "accept anything implementing a trait" crate like sqlx). That's not zero-friction, but it's low-friction.

@BurntSushi
Copy link
Owner Author

I like that plan @joshtriplett. I think that's probably best way to go. I think this will be my next focus, because I feel like I hear "jiff doesn't have as much ecosystem integration" as a big friction point. The newtype approach when an integration point downstream is perhaps not yet desirable does seem better than doing it directly from Jiff. Definitely not ideal, but I think there is enough "meat and potatoes" when converting between datetime types that it is actually worth doing.

@thumbert
Copy link

Hi,

If you finished the postgres support for jiff, can you please do the same for DuckDB? It has exactly the same data types as postgres, so I hope it should be trivial. The relevant crate is duckdb-rs.

Thank you very much! I am growing my Rust usage and slowly shedding the chrono dependency.

Tony

@BurntSushi
Copy link
Owner Author

I haven't done anything yet. I'll likely prioritize the crates that folks use the most first. But I'll keep duckdb-rs in mind!

@tisonkun
Copy link

tisonkun commented Sep 24, 2024

I think it'd be entirely reasonable to have a sqlx-jiff crate that provides

Here is a snippet I used in my private project that proves this way works - launchbadge/sqlx#3487 (comment) (wrap Timestamp to bridge from/to PG's TIMESTAMPTZ)

I'm not quite familiar with the wrapper way. If one can provide a code snippet of how we can implement "2) bidirectional From impls, 3) a Deref impl" in a decent way, I may be able to spend some time trying out a sqlx-jiff crate.

It will define feature flags like "postgres", "mysql" and "sqlite" for different backends. Note that some sqlx integration like PgTimeTz is defined at upstream and we cannot extend it with jiff support due to the orphan rule.

@BurntSushi
Copy link
Owner Author

BurntSushi commented Sep 24, 2024

I think I would prefer having the integration crates live in this repository. Some of the integrations (like PostgreSQL, as you've discovered) are pretty subtle, and I think it's important to have them under the umbrella of either the Jiff project or the project that's being integrated with.

I don't have a ton of time unfortunately to do abstract mentoring. With that said, if you want to get started by submitting a PR to this repo with a new jiff-sqlx crate, I can probably do a review.

@azerupi
Copy link

azerupi commented Jan 1, 2025

Could implementing From and Into the chrono types give a first simple solution to this problem?

Obviously it would not allow the user to drop chrono as a dependency, but it could allow the user to work with jiff's API until they have to hand it off to a library that doesn't support jiff yet. The ecosystem already has support for chrono, so it could be a general solution until the ecosystem provides support for jiff directly?

@BurntSushi
Copy link
Owner Author

I don't think it makes sense in really any circumstance to add a dependency on chrono from jiff. Even an optional one. I think the route to go there is something like a jiff-chrono (or chrono-jiff) crate that facilitates conversions to-and-from types between the crates.

It's also not especially straight-forward to just provide From impls. It works in some cases. For example, chrono::DateTime<Utc> and Timestamp make sense. But what about a Zoned? You might need chrono-tz for that. Honestly, it's all very thorny and I haven't thought through all the intricacies of conversions yet. And if you aren't careful, the conversion step could easily lose information along the way.

It's also not totally clear to me that From impls necessarily make anything automatic. I think you still need to insert some kind of explicit conversion code when working with crates that, say, only support Chrono and not Jiff. The From impls provide value by having an expert (probably me) implement them, but those can be in a jiff-chrono crate (or something).

Ecosystem integration is definitely on my list of things I want to do soon. I can definitely see that it is a blocker for some folks. Or at least, is causing folks to bounce off of Jiff. ("I tried using Jiff, but sqlx didn't support it and I didn't want to dig into it so I just dropped it for now." is something I heard recently.) I'm considering doing a jiff 0.2 release ~soon, and then that might be a good time to publish some ecosystem integration crates to bootstrap things.

My hope is that once I get Jiff 1.0 out (I'm still planning on doing that in summer of this year, 2025), folks will be more willing to add integration points with it upstream. But we'll see.

@azerupi
Copy link

azerupi commented Jan 1, 2025

It's also not totally clear to me that From impls necessarily make anything automatic. I think you still need to insert some kind of explicit conversion code when working with crates that, say, only support Chrono and not Jiff.

Yes, the explicit conversion at the boundaries with ecosystem crates was how I imagined it working. To me at least it wouldn't sound too bad to require an explicit conversion for the time being whenever you convert to/from e.g. diesel models.

@BurntSushi
Copy link
Owner Author

Ah okay, yeah, I think we're on the same page then. I think that falls into a jiff-chrono crate (and probably also jiff-time). I think those crates are good ideas and I do plan to make them.

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

No branches or pull requests

7 participants