Skip to content

change soft to hard delete#347

Open
Yostra wants to merge 7 commits intomainfrom
hard_delete
Open

change soft to hard delete#347
Yostra wants to merge 7 commits intomainfrom
hard_delete

Conversation

@Yostra
Copy link
Copy Markdown
Collaborator

@Yostra Yostra commented Apr 27, 2026

Summary

Switches the Postgres destination from soft delete to hard delete. When a record arrives with deleted: true, the matching row is physically removed from the destination table instead of being upserted with a deleted flag.

Changes

packages/destination-postgres

  • New writeMany(pool, schema, table, entries, pk, newerThanField) — the single entry point used by flushStream. Splits the batch by e.deleted === true and dispatches:
  • live records → upsertMany (unchanged semantics, with newer_than_field guard)
  • tombstones → deleteMany (hard DELETE by primary key)
  • upsertMany is now strictly upserts; the softDeleteExpression argument is gone.
  • deleteMany is a plain DELETE … USING (VALUES …) keyed on the catalog's primary key columns. Composite keys are supported. Deletion is terminal: once an object is deleted it cannot be undeleted,
  • schemaProjection no longer creates a deleted GENERATED column on new tables. The deleted field on incoming records is treated purely as a tombstone signal consumed by writeMany.

Backward compatibility

Existing soft-deleted rows from prior deployments are intentionally not cleaned up. no production user is on the soft-delete code path, so we don't need a migration.

Tested

Manual end-to-end: backfill a Stripe account with deletes (delete a customer in the dashboard) and confirm the row disappears from the destination.

@Yostra Yostra marked this pull request as ready for review April 28, 2026 00:02
Copy link
Copy Markdown
Collaborator

@kdhillon-stripe kdhillon-stripe left a comment

Choose a reason for hiding this comment

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

can we add a test case in the e2e test for this?

Comment thread packages/destination-postgres/src/index.ts Fixed
@Yostra
Copy link
Copy Markdown
Collaborator Author

Yostra commented Apr 29, 2026

can we add a test case in the e2e test for this?

added

Comment on lines +86 to +87
// eslint-disable-next-line @typescript-eslint/no-explicit-any
entries: Record<string, any>[],
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.

why type any

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.

i see all of the other helper fns have this too

Comment on lines +21 to 22
'deleted',
])
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.

do we need this anymore? it should not be needed after #347

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.

i suppose the real fix is to stack this pr on that

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, because some objects do have deleted fields in their openAPI schema. (in discussion with tony & steven we decided to drop the column completely)

>() {
return {
record(payload: { stream: string; data: TRecordData; emitted_at: string }): RecordMessage {
record(payload: { stream: string; data: TRecordData; emitted_at: string, recordDeleted?: boolean }): RecordMessage {
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.

not blocking: i wonder if this is better represented as a special type of TRecordData. if it's deleted, the data itself is { ..., deleted:true }

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

data.deleted was stripped by enforceCatalog, that was partially causing some issues that Steven noticed with webhookds

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