refactor(plugin-server): reduce createPerson to a single query #18126
Conversation
bretthoerner
force-pushed
the
brett/ps-queries2
branch
2 times, most recently
from
d6ae520 to
840017f
Compare
| isIdentified, | ||
| uuid, | ||
| version, | ||
| ...distinctIds.slice().reverse(), |
There was a problem hiding this comment.
So it looks like PG does inserts and/or assigns row IDs in reverse order of what you provide in the CTEs (meaning the last INSERT gets the earlier ID). This doesn't matter at all for actual code correctness, but we have a lot of tests that depend on ORDER BY id and so I'd rather just prove I am not changing anything at all behavior-wise. That's why the reverse() is done here.
There was a problem hiding this comment.
please add a comment about that
| const person = { | ||
| ...personCreated, | ||
| created_at: DateTime.fromISO(personCreated.created_at).toUTC(), | ||
| version, |
There was a problem hiding this comment.
nit (for follow-up PR): maybe we should change RawPerson ... same as with PluginEvent stuff I think we have too many types for the same thing and it's confusing.
| distinctIds | ||
| .map( | ||
| (_, index) => `, distinct_id_${index} AS ( | ||
| INSERT INTO posthog_persondistinctid (distinct_id, person_id, team_id, version) |
There was a problem hiding this comment.
I worry about this getting out of sync with addDistinctIdPooled, for lack of a better solution maybe a comment to keep them in sync like we have done in other places 🤷♀️
bretthoerner
force-pushed
the
brett/ps-queries2
branch
2 times, most recently
from
42f8f54 to
011c47f
Compare
bretthoerner
force-pushed
the
brett/ps-queries2
branch
from
011c47f to
5e5e0bd
Compare
Problem
createPersonused a txn to insert the person row and then insert (1 or 2) distinct ID rows, each with its own round trip to the DB, while the txn was held open.Changes
Do all of
createPersonin a single DB round-trip.👉 Stay up-to-date with PostHog coding conventions for a smoother review.
How did you test this code?
Existing tests, this is a drop in for well tested, existing behavior.