Skip to content

Yeeeeeeeeeeeeeeeeeeeeeet#1054

Merged
sgrif merged 3 commits into
mainfrom
sg-yeet
Jun 11, 2026
Merged

Yeeeeeeeeeeeeeeeeeeeeeet#1054
sgrif merged 3 commits into
mainfrom
sg-yeet

Conversation

@sgrif

@sgrif sgrif commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

The deleted tests are either redundant with unit tests in the refactored code, or sufficiently covered by integration tests. CodeCov will still complain

image

@sgrif sgrif requested a review from levkk June 10, 2026 22:02
); // 32x on my M1

assert!(faster > 10.0);
assert!(faster > 9.0);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See y'all when I have to lower this to 8

@sgrif sgrif marked this pull request as ready for review June 10, 2026 22:03

@levkk levkk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Red is good. Green is evil. This is good.

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.11628% with 15 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pgdog/src/backend/pool/connection/aggregate.rs 64.10% 14 Missing ⚠️
pgdog/src/backend/pool/connection/buffer.rs 66.66% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

The deleted tests are either redundant with unit tests in the refactored
code, or sufficiently covered by integration tests. CodeCov will still
complain

This doesn't quite delete as much as I'd like. Surprisingly, we still
call `buffer.aggregate` on queries that absolutely should not be doing
aggregation, such as direct-to-shard and explain queries. The old code
treated missing helper columns as an indication we're running one of
those, instead of an error condition.

Direct to shard is easy to filter out, but explain needs a bit more
digging.
@sgrif sgrif merged commit 0482b6d into main Jun 11, 2026
24 checks passed
@sgrif sgrif deleted the sg-yeet branch June 11, 2026 14:53
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.

2 participants