-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
chore(person-overrides): send overrides to ClickHouse person_override #13999
Conversation
plugin-server/src/utils/db/db.ts
Outdated
version | ||
) VALUES ($1, $2, $3, $4, $5) | ||
ON CONFLICT (team_id, old_person_id) | ||
DO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can get rid of this ON CONFLICT
as it would not be valid to merge old_person_id
with anything new. Does that make sense @tiina303 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup that can be part of the constraint
820a201
to
1715f71
Compare
This is an example of how we can update the functional_tests to take into consider the person overrides and avoid having the implementation details bleed into the tests.
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
1715f71
to
4707005
Compare
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
270ff15
to
343ba86
Compare
Problem
Changes
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
How did you test this code?