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 · 9 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

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.

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

6 participants