feat(persons): Add simple backfill command for distinct ID overrides. #20562
Conversation
This file contains 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
Also adds a fairly contrived test case, as I'd already written most of it anyway...
tkaemming
commented
Feb 26, 2024
Comment on lines
+22
to
+32
| SELECT | ||
| team_id, | ||
| distinct_id, | ||
| argMax(person_id, version), | ||
| argMax(is_deleted, version), | ||
| max(version) | ||
| FROM person_distinct_id2 | ||
| WHERE | ||
| team_id = %(team_id)s | ||
| AND version > 0 | ||
| GROUP BY team_id, distinct_id |
There was a problem hiding this comment.
Some arbitrary decisions:
GROUP BY …is optional here — both the source and destination tables will replace onversionanyway, so the cardinality is likely similar before and after aggregation here, and the end state would eventually be the same after table optimization.team_idcould probably be optional as well (particularly if aggregation is dropped) and we're okay with inserting a few hundred million rows at once — this might be simpler than keeping track of a progressive rollout?- This doesn't retain the Kafka engine columns from the original table (
_timestamp,_partition,_offset), though I suppose it'd be safe to do so given they share the same input topic… maybe worth keeping that around?
There was a problem hiding this comment.
Could this query time out with the group by?
There was a problem hiding this comment.
It could (though it could also time out without it as well, even though the GROUP BY does make it more likely.) I ran the SELECT without INSERT on some of the larger teams and didn't run into any issues, but past performance not indicative of future results, etc. If we run into any issues, it's easy to remove.
This was referenced Feb 26, 2024
Closed
tiina303
approved these changes
Feb 27, 2024
Comment on lines
+22
to
+32
| SELECT | ||
| team_id, | ||
| distinct_id, | ||
| argMax(person_id, version), | ||
| argMax(is_deleted, version), | ||
| max(version) | ||
| FROM person_distinct_id2 | ||
| WHERE | ||
| team_id = %(team_id)s | ||
| AND version > 0 | ||
| GROUP BY team_id, distinct_id |
There was a problem hiding this comment.
Could this query time out with the group by?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
We need a convenient way to populate the
person_distinct_id_overridestable (#20326) with data existed prior to the Kafka engine and materialized view (#20349.)This is a simplified version of the initial backfill procedure idea. More context about the change in approach can be found here. This essentially acts as a limited
POPULATE.How did you test this code?
Added a (somewhat contrived) test.