Skip to content

fix(types): compare and hash intervals by PostgreSQL duration - #1559

Merged
jkaczman merged 1 commit into
pgdogdev:mainfrom
dipeshbabu:fix/interval-comparison-semantics
Sep 19, 2026
Merged

jkaczman merged 1 commit into
pgdogdev:mainfrom
dipeshbabu:fix/interval-comparison-semantics

Conversation

@dipeshbabu

@dipeshbabu dipeshbabu commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Cross-shard interval comparisons use individual fields, so 1 mon sorts after 31 days and equivalent values such as 1 day and 24 hours form separate groups. This also produces incorrect MIN/MAX results.

Use one allocation-free i128 duration for ordering, equality, and hashing, matching PostgreSQL's 30-day months and 24-hour days without overflowing the interval range.

Regression tests cover equivalent values, mixed signs, extreme ranges, and multi-shard ordering/grouping/min/max in text and binary formats.

Validation: all 27 jobs pass in CI, including strict formatting/Clippy, unit/doc tests, Linux/macOS builds, and all 21 integration suites on PostgreSQL 18.

@levkk

levkk commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Awesome. Do you want to try stacking the PRs? You basically change the base branch of this PR to your other PR, and GitHub can convert to a stack we can merge together!

@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@dipeshbabu

Copy link
Copy Markdown
Contributor Author

Thanks! I checked, but GitHub currently doesn't support cross-fork stacks. Both PRs come from my fork, and I have read-only access upstream.

This branch already builds directly on #1558, with the interval fix isolated in e261e96d. The available path is to merge #1558 first, then rebase this PR onto main if needed.

@levkk

levkk commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Ah!! That's unfortunate! Okay, let's merge the CI fix first, then let's do the other ones one at a time. Please make sure to sign our CLA; without it, we can't accept contributions. Cheers!

#1560 (comment)

@dipeshbabu

Copy link
Copy Markdown
Contributor Author

The CLA is signed, and its checks are now green on #1558 and #1560. All three PRs' code checks pass. Agreed on merging #1558 first, followed by #1559 and #1560 one at a time.

@dipeshbabu
dipeshbabu force-pushed the fix/interval-comparison-semantics branch from e261e96 to 848cc3d Compare September 19, 2026 00:45
@dipeshbabu

Copy link
Copy Markdown
Contributor Author

Rebased onto main after #1558 merged. This PR now contains only the interval fix. All 27 validation jobs pass.

@jkaczman
jkaczman merged commit 608cb9d into pgdogdev:main Sep 19, 2026
29 checks passed
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