Skip to content

fix(clickhouse): preserve legacy soft-delete column casing - #4597

Open
floze-the-genius wants to merge 1 commit into
PeerDB-io:mainfrom
floze-the-genius:fix/clickhouse-soft-delete-case-upgrade
Open

fix(clickhouse): preserve legacy soft-delete column casing#4597
floze-the-genius wants to merge 1 commit into
PeerDB-io:mainfrom
floze-the-genius:fix/clickhouse-soft-delete-case-upgrade

Conversation

@floze-the-genius

Copy link
Copy Markdown

Summary

  • resolve the configured soft-delete column against the physical ClickHouse destination schema before normalization
  • prefer exact matches, then case-insensitive matches, so both legacy lowercase tables and newer uppercase/custom tables keep working
  • apply the same resolution during destination validation

Root cause

Older PeerDB releases stored the UI default _PEERDB_IS_DELETED in mirror config, but ClickHouse table creation and normalization hardcoded the physical column as _peerdb_is_deleted. After #4365 began honoring the stored config, upgraded legacy mirrors generated inserts for the uppercase identifier even though their existing tables only had the lowercase column.

Tests

  • go test ./connectors/clickhouse -skip '^TestCreateRawTableHasTTL$' -count=1
  • go vet ./connectors/clickhouse
  • go test -race ./connectors/clickhouse -run 'Test(ResolveSoftDeleteColumnName|BuildQuery_UsesDestinationSoftDeleteColumnCase|ProcessTableComparison_WithResolvedSoftDeleteColumnCase)$' -count=1

Fixes #4596

@floze-the-genius
floze-the-genius requested a review from a team as a code owner July 19, 2026 02:07
@CLAassistant

CLAassistant commented Jul 19, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@pfcoperez pfcoperez left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for your contribution! I've dropped a comment about the implementation detail.

Comment thread flow/connectors/clickhouse/normalize.go Outdated
@floze-the-genius
floze-the-genius force-pushed the fix/clickhouse-soft-delete-case-upgrade branch from 1bacbfc to ac5fe77 Compare July 29, 2026 08:57
@floze-the-genius

Copy link
Copy Markdown
Author

Narrowed the compatibility behavior in ac5fe774 as requested:

  • exact destination-column matches still win;
  • fallback resolution is now limited to the two known equivalent defaults, _PEERDB_IS_DELETED and _peerdb_is_deleted;
  • mixed-case variants and custom column names remain strict, so my_deleted will not resolve to MY_deleted.

Added positive tests for both legacy directions and negative tests for mixed/default and custom-case variants. Targeted ClickHouse tests and go vet ./connectors/clickhouse pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants