Skip to content

fix(backfill): populate cdc timestamp for snapshot records when cdc enabled - #1059

Open
krishanu7 wants to merge 3 commits into
datazip-inc:stagingfrom
krishanu7:fix/cdc-timestamp-null-error
Open

fix(backfill): populate cdc timestamp for snapshot records when cdc enabled#1059
krishanu7 wants to merge 3 commits into
datazip-inc:stagingfrom
krishanu7:fix/cdc-timestamp-null-error

Conversation

@krishanu7

Copy link
Copy Markdown
Contributor

Description

This PR resolves an issue where the _cdc_timestamp metadata column was generated as null (or missing values) during the initial backfill/snapshot phase when the source connector configuration had CDC enabled (cdc: true), but the sync mode was set to incremental or full_refresh.

Key Changes

Updated drivers/abstract/backfill.go to dynamically check the target schema. If the destination schema includes the _cdc_timestamp column, we write the Unix epoch 1970-01-01 00:00:00 UTC (time.Unix(0, 0)) as a placeholder. This satisfies downstream schema constraints (e.g., Iceberg) and clearly marks historical snapshot rows from live CDC events.
Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

The changes were validated using a local multi-container test environment running Postgres source, MinIO S3 object storage, and an Apache Spark SQL engine connecting to the local Iceberg catalog.

  • Scenario A (CDC Enabled): Verified that when cdc: true is configured and the _cdc_timestamp is present in the schema, the backfilled tables are successfully generated with 1970-01-01 05:30:00+05:30 instead of null (or failing due to Iceberg schema constraints).
  • Scenario B (CDC Disabled): Verified that when CDC is disabled and the _cdc_timestamp column is stripped from the schema config, the sync successfully executes and writes the Parquet files without adding any unexpected _cdc_timestamp data columns (avoiding schema mismatch issues).

Screenshots or Recordings

*** Test 1: CDC ON (Schema contains _cdc_timestamp) ***
For this run, we used streams.json which includes _cdc_timestamp in the catalog schema definition.

image

*** Test 2: CDC OFF (Schema does NOT contain _cdc_timestamp) ***

image

Documentation

  • Documentation Link: [link to README, olake.io/docs, or olake-docs]
  • N/A (bug fix, refactor, or test changes only)

Related PR's (If Any):

None

@ImDoubD-datazip

Copy link
Copy Markdown
Collaborator

Hi @krishanu7 , maybe you understood the issue incorrectly. The issue states that if cdc is enabled, but the sync mode is full_refresh or incremental, the cdc_timestamp column comes in the destination table.

The thing is when cdc is enabled (cdc config provided), the cdc timestamp column comes in the discover phase i.e. in streams.json. If the cdc timestamp column is present in streams.json, then it will always come whatever the sync mode and its value is.

What we require is during discover even if the cdc is enabled we will not include cdc timestamp in the discover phase i.e. in streams.json. by default the sync mode is cdc, where user can change it to incremental or full refresh. If cdc sync mode, then we have to configure somehow during the sync running that cdc timestamp column and its respective value to be included. Otherwise if sync mode changed to full refresh or incremental (even if cdc enabled) , since in streams.json cdc timestamp not present, it won't come in destination table.

also in case of s3 parquet destination specifically, i think cdc column in destination (null valued) always comes even if not in streams.json. so this has to be fixed as well here.

@krishanu7

Copy link
Copy Markdown
Contributor Author

@ImDoubD-datazip Hi, Thanks for explaing the requirement, i updated code and raised new PR #1068 can you please review this

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