sqllogictest,testdrive: downgrade to stable release of postgres - #802
Merged
Conversation
Using a stable release of the postgres driver means we can use off-the-shelf packages for interval and decimal decoding, rather than rolling our own.
quodlibetor
reviewed
Oct 21, 2019
| // want to get to `significand2 * 10^desired_scale` | ||
| // so `significand2 = significand * 10^(current_scale - desired_scale)` | ||
| let scale_correction = current_scale - (i64::from(desired_scale)); | ||
| let scale_correction = (d.scale as isize) - (desired_scale as isize); |
Contributor
There was a problem hiding this comment.
did they change the upstream types here so we need to change it?
Contributor
There was a problem hiding this comment.
I guess I mean will they have changed the types when we upgrade.
Contributor
Author
There was a problem hiding this comment.
Well we used to be using our own DecimalWrapper thingy (removed from below) which said scales was an i64, whereas now d is a rust_decimal::Decimal which thinks scale is a u32.
Contributor
There was a problem hiding this comment.
ahh okay yeah that makes sense.
quodlibetor
approved these changes
Oct 21, 2019
quodlibetor
left a comment
quodlibetor
left a comment
Contributor
There was a problem hiding this comment.
other than not being sure why some integer types changed this looks good to me
benesch
commented
Oct 21, 2019
| // want to get to `significand2 * 10^desired_scale` | ||
| // so `significand2 = significand * 10^(current_scale - desired_scale)` | ||
| let scale_correction = current_scale - (i64::from(desired_scale)); | ||
| let scale_correction = (d.scale as isize) - (desired_scale as isize); |
Contributor
Author
There was a problem hiding this comment.
Well we used to be using our own DecimalWrapper thingy (removed from below) which said scales was an i64, whereas now d is a rust_decimal::Decimal which thinks scale is a u32.
antiguru
added a commit
that referenced
this pull request
Jul 15, 2026
Our `mz_join_core` is a fork of differential's `JoinCore::join_core` and inherited a correctness bug fixed upstream in TimelyDataflow/differential-dataflow#802 (issue #801). The operator skipped incoming batches by testing `batch.lower()` against the mutable `acknowledged` frontier. `advance_upper` can advance `acknowledged` past an in-flight batch when trace merges consolidate the batch's updates away, e.g. an add/remove pair collapsing once logical compaction equates their times. The trace is legitimately empty there only for readers at or beyond the compaction frontier, while our consumers may read finer times where the batch's updates are still real. Testing against `acknowledged` therefore dropped such batches and silently lost updates. The fix captures fixed `preload_upper` frontiers at start-up and gates the skip on them, and advances `acknowledged` only when a batch sits at or beyond it. This is a one-to-one port of the upstream change. No regression test: deterministically reproducing the race requires trace merges to consolidate an add/remove pair while a batch is in flight and `advance_upper` to then jump ahead, which is timing-dependent and not reliable in testdrive/sqllogictest. Upstream #802 added no direct join test either. Correctness rests on mirroring the upstream reasoning. Note: the alternate `DifferentialDataflow` join implementation calls upstream `join_core` from crates.io `differential-dataflow` 0.24.0, which still carries the bug and will be fixed by a future dependency bump. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Using a stable release of the postgres driver means we can use
off-the-shelf packages for interval and decimal decoding, rather than
rolling our own.