Skip to content

feat: Add DeleteRecord handling to SQLite destination#21102

Merged
kodiakhq[bot] merged 6 commits intocloudquery:mainfrom
jeromewir:feat/sqlite-delete-record
Jul 23, 2025
Merged

feat: Add DeleteRecord handling to SQLite destination#21102
kodiakhq[bot] merged 6 commits intocloudquery:mainfrom
jeromewir:feat/sqlite-delete-record

Conversation

@jeromewir
Copy link
Copy Markdown
Contributor

Summary

Following up on https://community.cloudquery.io/t/incremental-tables-how-to-handle-deletion/1205/6

This PR adds support for DeleteRecord messages so sources can specify which items can be deleted specifically.
I used a lot of the existing code present in the ClickHouse PR: https://github.com/cloudquery/cloudquery/pull/20772/files

@jeromewir jeromewir marked this pull request as ready for review July 23, 2025 08:26
@jeromewir jeromewir requested a review from a team as a code owner July 23, 2025 08:26
@jeromewir jeromewir requested a review from hermanschaaf July 23, 2025 08:26
@hermanschaaf hermanschaaf changed the title feat!: Add DeleteRecord handling to SQLite destination feat: Add DeleteRecord handling to SQLite destination Jul 23, 2025
Copy link
Copy Markdown
Contributor

@hermanschaaf hermanschaaf left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request @jeromewir! We'll review this a bit more closely, I just left one comment for now after a quick read through. Also, I changed the title from feat! to feat because I think this isn't a breaking change, right?

continue
}
sb.WriteString("(")
for i, predicate := range predicateGroup.Predicates {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This inner loop is re-using i as an index variable which should work, I think, due to how variables can be shadowed in Go, but I would say is not recommended, mostly for readability reasons.

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.

Fixed!

@jeromewir
Copy link
Copy Markdown
Contributor Author

Also, I changed the title from feat! to feat because I think this isn't a breaking change, right?

Oh, I didn't realize the ! was for breaking change, sorry about that.
That's correct, the code is backward compatible

I just added a missing folder typeconv, which is mostly the same as the linked PR in the description with a slight change: I'm returning native objects instead of pointers.

@erezrokah erezrokah requested a review from hermanschaaf July 23, 2025 10:01
Copy link
Copy Markdown
Contributor

@hermanschaaf hermanschaaf left a comment

Choose a reason for hiding this comment

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

Looks great! 🚀

@hermanschaaf hermanschaaf added the automerge Automatically merge once required checks pass label Jul 23, 2025
@kodiakhq kodiakhq bot merged commit e1bf276 into cloudquery:main Jul 23, 2025
9 checks passed
kodiakhq bot pushed a commit that referenced this pull request Jul 24, 2025
🤖 I have created a release *beep* *boop*
---


## [2.11.0](plugins-destination-sqlite-v2.10.22...plugins-destination-sqlite-v2.11.0) (2025-07-24)


### Features

* Add DeleteRecord handling to SQLite destination ([#21102](#21102)) ([e1bf276](e1bf276))


### Bug Fixes

* **deps:** Update module github.com/cloudquery/plugin-sdk/v4 to v4.87.3 ([#21010](#21010)) ([c78cff9](c78cff9))
* **deps:** Update module github.com/cloudquery/plugin-sdk/v4 to v4.87.4 ([#21104](#21104)) ([44f77c8](44f77c8))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@jeromewir jeromewir deleted the feat/sqlite-delete-record branch July 24, 2025 08:34
kodiakhq bot pushed a commit that referenced this pull request Jul 25, 2025
#### Summary

Following up on #21102
There was a leftover from the debugging session that got merged into the release.
This removes a `fmt.Println(...)` left in the `unpackArray` function from the SQLite plugin, used when sending DeleteRecord messages.
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.

3 participants