Skip to content

Fix avg & unique by reverting to old cartesian join#8770

Merged
pieterbeulque merged 1 commit intomainfrom
pieter/hybrid-model-avg-unique-fix
Feb 2, 2026
Merged

Fix avg & unique by reverting to old cartesian join#8770
pieterbeulque merged 1 commit intomainfrom
pieter/hybrid-model-avg-unique-fix

Conversation

@pieterbeulque
Copy link
Contributor

@pieterbeulque pieterbeulque commented Jan 5, 2026

Just to test. Follow-up to #8769

To be correct, I think we should do something like this, but could run some numbers to assess the impact on performance etc?

@vercel
Copy link

vercel bot commented Jan 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
polar Ready Ready Preview, Comment Feb 2, 2026 9:09am
polar-sandbox Ready Ready Preview, Comment Feb 2, 2026 9:09am

Request Review

@pieterbeulque pieterbeulque requested a review from Yopi January 6, 2026 07:15
Base automatically changed from claude/investigate-meter-api-issue-kxEEr to main January 7, 2026 14:55
@frankie567
Copy link
Member

@pieterbeulque Should we discard this?

@pieterbeulque
Copy link
Contributor Author

pieterbeulque commented Jan 29, 2026

… well. It's not calculating correctly in production right now, but merging this would be a performance regression. @Yopi - since you "broke" it / fixed the performance issue (half empty / half full kind of way to look at it, haha), what do you think?

Not pointing fingers 😊

Actually, not sure how it'd go together with the other optimizations made in main since this PR.

@Yopi
Copy link
Contributor

Yopi commented Jan 29, 2026

I'm also not sure. Maybe we should merge this just to get it working correctly. Given the other changes we've made it might be fine out of a performance improvement perspective.

@pieterbeulque pieterbeulque force-pushed the pieter/hybrid-model-avg-unique-fix branch from ac3183c to 2a2228b Compare February 2, 2026 09:04
@pieterbeulque pieterbeulque marked this pull request as ready for review February 2, 2026 09:04
@pieterbeulque
Copy link
Contributor Author

@Yopi - I rebased this against main so maybe we can merge it now? I think the issue is still in main.

@Yopi
Copy link
Contributor

Yopi commented Feb 2, 2026

@Yopi - I rebased this against main so maybe we can merge it now? I think the issue is still in main.

Lets do it

@pieterbeulque pieterbeulque merged commit 5a93e1e into main Feb 2, 2026
16 checks passed
@pieterbeulque pieterbeulque deleted the pieter/hybrid-model-avg-unique-fix branch February 2, 2026 09:43
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