Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 726b780342
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[cfg(feature = "with-chrono")] | ||
| break 'value Value::ChronoDateTime( | ||
| row.try_get::<Option<chrono::NaiveDateTime>, _>(c.ordinal()) | ||
| .expect("Failed to get timestamp"), | ||
| ); |
There was a problem hiding this comment.
SeaORM's default feature set already includes with-chrono, so many users will enable with-jiff on top of the defaults. In that configuration this proxy-row conversion still breaks out on the chrono/time branch first, which means from_query_result_to_proxy_row stores Value::Chrono*/Value::Time* instead of the new Value::Jiff*. ProxyRow::try_get and the new try_getable_jiff! impl only accept Value::Jiff*, so Jiff reads through the proxy feature start failing on Postgres (and the same ordering issue exists in the SQLite proxy path).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32fed93277
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f11d8c58f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31a0824521
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| with-jiff = [ | ||
| "jiff", | ||
| "jiff-sqlx", | ||
| "sea-query/with-jiff", | ||
| "sea-query-sqlx?/with-jiff", | ||
| ] |
There was a problem hiding this comment.
with-jiff away from Arrow conversions
When a project enables both with-jiff and with-arrow, Arrow round-trips for Jiff-backed models are still unsupported even though this feature set is now exposed. ActiveModel::from_arrow only retries chrono→time conversions (src/entity/active_model.rs:563-586), and sea-orm-arrow/src/lib.rs:154-170,1272-1403 only produces/extracts chrono/time Value variants, never Value::Jiff*. As a result, entities with JiffDate/JiffTime/JiffDateTime/JiffTimestamp cannot be imported from or exported to Arrow batches.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c54003d6f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 725398b5ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96663c8c39
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| it_impl_traits!(jiff::civil::Date); | ||
| it_impl_traits!(jiff::civil::Time); | ||
| it_impl_traits!(jiff::civil::DateTime); | ||
| it_impl_traits!(jiff::Timestamp); |
There was a problem hiding this comment.
with-jiff
These new assertions are unconditional, but the self dev-dependency in sea-orm-sync/Cargo.toml:92-97 still enables only debug-print, mock, postgres-array, and tests-cfg. That means a plain cargo test in sea-orm-sync will try to compile references to jiff::*/Jiff* without turning on with-jiff, so the sync crate's default test target is now broken unless the caller remembers to pass that feature manually. CI currently hides this by always using --features tests-features,rusqlite for the sync crate.
Useful? React with 👍 / 👎.
1 participant
PR Info
Closes Support jiff #2896
Dpendencies:
New Features