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

Respect precision for intervals #699

Merged

Conversation

greg-rychlewski
Copy link
Member

In the previous PR we hardcoded the precision to 6 because we didn't know how to deal with the type modifier.

I got pointed in the right direction by the nice people at Postgres. The type modifier for intervals takes into account two different things:

  1. The allowed fields (i.e. only days, years, days to years and stuff like that), represented as a 16-bit bit field
  2. The precision represented as a 16 bit integer

The right most 16 bits in the type modifier are the precision. I was pointed to the right places in their code here and here

We could probably do something about the fields modifier as well but that is a lot more complex. And we ignore it already today. So going to think on it a bit and save it for later.

@greg-rychlewski
Copy link
Member Author

Sorry I'm not sure how to get rid of this warning when using lower Elixir versions =(

Check warning on line 9 in lib/postgrex/extensions/interval.ex

GitHub Actions
/ test (9.4, skip_wal, 1.11.4, 23.3.3)

unused import Bitwise

@wojtekmach
Copy link
Member

import Bitwise, warn: false

@greg-rychlewski greg-rychlewski merged commit db2e0a2 into elixir-ecto:master Jul 27, 2024
9 checks passed
@greg-rychlewski greg-rychlewski deleted the interval_precision branch July 27, 2024 22:25
@greg-rychlewski
Copy link
Member Author

Just to be extra cautious:

Is it safe to assume across elixir/erlang versions that -1 &&& 0xFFFF is always 0xFFFF

@josevalim
Copy link
Member

Yes. Otherwise it would have to be a breaking change in Erlang.

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.

3 participants