Skip to content

feat!: Add DeleteRecord handling to Clickhouse destination#20772

Merged
kodiakhq[bot] merged 16 commits intomainfrom
feat/clickhouse-delete-record
May 20, 2025
Merged

feat!: Add DeleteRecord handling to Clickhouse destination#20772
kodiakhq[bot] merged 16 commits intomainfrom
feat/clickhouse-delete-record

Conversation

@maaarcelino
Copy link
Copy Markdown
Contributor

@maaarcelino maaarcelino commented May 16, 2025

This adds handling for DeleteRecord messages in ClickHouse along with tests. I've also enabled the DeleteRecord tests in Postgres as they were excluded by mistake.

BEGIN_COMMIT_OVERRIDE
chore: Add DeleteRecord handling to Clickhouse destination
END_COMMIT_OVERRIDE

@cq-bot
Copy link
Copy Markdown
Contributor

cq-bot commented May 16, 2025

@maaarcelino maaarcelino changed the title WIP Clickhouse feat: Add DeleteRecord handling to Clickhouse destination May 19, 2025
@maaarcelino maaarcelino marked this pull request as ready for review May 19, 2025 09:42
@maaarcelino maaarcelino requested review from a team, jon-s58, murarustefaan and przste-go May 19, 2025 09:42
@cq-bot
Copy link
Copy Markdown
Contributor

cq-bot commented May 19, 2025

@cq-bot cq-bot added the area/ci label May 19, 2025
return err
}

if err = c.conn.Exec(ctx, sql, params...); err != nil {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd like to be able to use some kind of prepared statement or batching here, but CH does not support this for DELETE statements. Batching is only supported for INSERT queries.

services:
clickhouse:
image: clickhouse/clickhouse-server:22.1.2
image: clickhouse/clickhouse-server:24.12.1
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.

Is this needed to support deletion? If so we should do a major version bump and update the docs https://hub.cloudquery.io/plugins/destination/cloudquery/clickhouse/latest/docs#overview

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes that's to support the lightweight delete statements. Those were added in CH 23.3 (release notes). The current latest version of CH is 25.4.4. I'm wondering what we should set as the new minimum version for this plugin. WDYT? 23.3?

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.

Per https://clickhouse.com/docs/whats-new/changelog/2023 23.3 was released over 2 years ago and no longer supported (LTS are supported for 1 year per https://clickhouse.com/docs/knowledgebase/production#how-to-choose-between-clickhouse-releases).

Maybe we can move to 24.8 LTS which is the latest LTS which is still supported https://clickhouse.com/docs/whats-new/changelog/2024#a-id248a-clickhouse-release-248-lts-2024-08-20

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.

Maybe even 25.3 as 24.8 will go out of support soon, and 25.3 has JSON type production level support

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.

Regardless needs to be a breaking change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah it does need to be breaking, agreed. I think jumping to 25.x will be a problem for our platform tenants. On platform-preview we run version 24.12.1 of CH. I think I'll bump the plugin to 24.8.1.

@maaarcelino maaarcelino changed the title feat: Add DeleteRecord handling to Clickhouse destination feat!: Add DeleteRecord handling to Clickhouse destination May 20, 2025
@maaarcelino maaarcelino added the automerge Automatically merge once required checks pass label May 20, 2025
@kodiakhq kodiakhq bot merged commit 13e9573 into main May 20, 2025
24 checks passed
@kodiakhq kodiakhq bot deleted the feat/clickhouse-delete-record branch May 20, 2025 07:45
kodiakhq bot pushed a commit that referenced this pull request May 20, 2025
🤖 I have created a release *beep* *boop*
---


## [7.0.0](plugins-destination-clickhouse-v6.2.7...plugins-destination-clickhouse-v7.0.0) (2025-05-20)


### ⚠ BREAKING CHANGES

* Add DeleteRecord handling to Clickhouse destination ([#20772](#20772))

### Features

* Add DeleteRecord handling to Clickhouse destination ([#20772](#20772)) ([13e9573](13e9573))


### Bug Fixes

* **deps:** Update module github.com/cloudquery/plugin-sdk/v4 to v4.80.2 ([#20778](#20778)) ([525352c](525352c))
* **deps:** Update module github.com/cloudquery/plugin-sdk/v4 to v4.80.3 ([#20802](#20802)) ([2ba2f8e](2ba2f8e))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
kodiakhq bot pushed a commit that referenced this pull request May 20, 2025
kodiakhq bot pushed a commit that referenced this pull request May 21, 2025
🤖 I have created a release *beep* *boop*
---


## [9.0.0](plugins-destination-postgresql-v8.8.7...plugins-destination-postgresql-v9.0.0) (2025-05-21)


### ⚠ BREAKING CHANGES

* Add DeleteRecord handling to Clickhouse destination ([#20772](#20772))

### Features

* Add DeleteRecord handling to Clickhouse destination ([#20772](#20772)) ([13e9573](13e9573))


### Bug Fixes

* **deps:** Update module github.com/cloudquery/plugin-sdk/v4 to v4.80.2 ([#20778](#20778)) ([525352c](525352c))
* **deps:** Update module github.com/cloudquery/plugin-sdk/v4 to v4.80.3 ([#20802](#20802)) ([2ba2f8e](2ba2f8e))
* **deps:** Update module github.com/cloudquery/plugin-sdk/v4 to v4.81.0 ([#20806](#20806)) ([567e252](567e252))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants