Conversation
elixir-ecto#4775 normalized a non-UTC %DateTime{} to UTC for :date, :naive_datetime and :naive_datetime_usec, but an ISO 8601 string with the same offset still went through NaiveDateTime.from_iso8601, which drops the offset. The same instant cast two ways gave two values: cast(:naive_datetime, "2020-06-01T00:30:07+02:00") ~N[2020-06-01 00:30:07] cast(:naive_datetime, <the same instant as %DateTime{}>) ~N[2020-05-31 22:30:07] Parse with DateTime.from_iso8601 first and fall back to NaiveDateTime.from_iso8601, so strings without an offset are unchanged.
Member
|
Thank you. Upon further inspection, I am reverting the other two PRs, because the dropping of offsets is exactly how Elixir behaves for NaiveDateTime, Time, Date when they pass a DateTime. In other words, we respect the wall time of said datetimes. Which makes sense, if you ask Date.days_in_month, you want the current datetime, not the UTC variant. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#4775 normalized a non-UTC
%DateTime{}to UTC for:date,:naive_datetimeand:naive_datetime_usec, and #4792 did the same for:time. An ISO 8601 string carrying the same offset still goes throughNaiveDateTime.from_iso8601, which drops the offset, so one instant casts two ways::naive_datetime:date:utc_datetime"2020-06-01T00:30:07+02:00"~N[2020-06-01 00:30:07]~D[2020-06-01]~U[2020-05-31 22:30:07Z]%DateTime{}~N[2020-05-31 22:30:07]~D[2020-05-31]~U[2020-05-31 22:30:07Z]This parses with
DateTime.from_iso8601first and falls back toNaiveDateTime.from_iso8601, so strings without an offset, including Phoenixdatetime-localvalues, are unchanged. It follows the conclusion in #2054 that a dropped offset should either be converted or rejected. If you would rather reject non-zero offsets for the naive types, that is a small change and I can switch to it.Verification
Base
f4d84a1a. New rows go into the existing:date,:naive_datetimeand:naive_datetime_useccast tests, mirroring the:utc_datetimeoffset rows attest/ecto/type_test.exs:1006and:1011.lib/ecto/type.exmd5test/ecto/type_test.exse91eacf61b27ba9a170243d4775fc69957058100cast(:date, "2015-12-31T00:00:00")The CI unit-test steps on Elixir 1.18 / OTP 27:
mix deps.unlock --check-unused,mix compile --warnings-as-errors,mix test(97 doctests, 1512 tests, 0 failures);mix format --check-formattedon both files. The same on Elixir 1.14.5 / OTP 26 passes. Not run: the exact OTP pins of the matrix and the Earthly integration job.Written with AI assistance (Claude); the measurements above were run locally and I have reviewed the change.