Skip to content

fix: Error if both PKs and PK components are set#2113

Merged
kodiakhq[bot] merged 1 commit intomainfrom
feat/error_on_both_pks_and_pk_components
Apr 2, 2025
Merged

fix: Error if both PKs and PK components are set#2113
kodiakhq[bot] merged 1 commit intomainfrom
feat/error_on_both_pks_and_pk_components

Conversation

@erezrokah
Copy link
Member

@erezrokah erezrokah commented Apr 1, 2025

Summary

I spent too much time debugging a bug I had when I (meaning Cursor AI) auto completed PrimaryKeyComponent: true instead of PrimaryKey: true for me. Which means I had both PKs and PK components on the same table, causing _cq_id to only be calculated from the PK component, losing data when pk_mode: cq-id-only was enabled.

See https://github.com/cloudquery/cloudquery-private/pull/6902/commits/ef8762eb1f1a6624913e5ee9ff7a88e9e0944537

This PR adds a validation to ensure only either ones are defined, or not defined at all. This has to happen after the TransformTables call since columns can be configured manually as well


Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines 🧑‍🎓
  • Run go fmt to format your code 🖊
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests 🧪
  • Ensure the status checks below are successful ✅

@erezrokah erezrokah requested review from a team, murarustefaan and przste-go April 1, 2025 17:22
@github-actions github-actions bot added feat and removed feat labels Apr 1, 2025
@disq
Copy link
Member

disq commented Apr 1, 2025

Would this be a feat, though?

@erezrokah
Copy link
Member Author

erezrokah commented Apr 1, 2025

Would this be a feat, though?

It's a new validation so yes I think. Could be a fix too if you consider it a bug (changed to fix)

@erezrokah erezrokah changed the title feat: Error if both PKs and PK components are set fix: Error if both PKs and PK components are set Apr 1, 2025
@github-actions github-actions bot added fix and removed feat fix labels Apr 1, 2025
@disq
Copy link
Member

disq commented Apr 1, 2025

I think fix is better because it's preventing a bug to happen by adding a validation to check against.

@kodiakhq kodiakhq bot merged commit 4f0b312 into main Apr 2, 2025
19 checks passed
@kodiakhq kodiakhq bot deleted the feat/error_on_both_pks_and_pk_components branch April 2, 2025 08:35
kodiakhq bot pushed a commit that referenced this pull request Apr 2, 2025
🤖 I have created a release *beep* *boop*
---


## [4.76.0](v4.75.0...v4.76.0) (2025-04-02)


### Features

* Pass installation ID from env to usage report ([#2106](#2106)) ([0bea6e7](0bea6e7))


### Bug Fixes

* **deps:** Update golang.org/x/exp digest to 054e65f ([#2110](#2110)) ([f9875f8](f9875f8))
* **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.26.9 ([#2112](#2112)) ([abd2117](abd2117))
* Error if both PKs and PK components are set ([#2113](#2113)) ([4f0b312](4f0b312))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@github-actions github-actions bot added fix and removed fix labels Apr 2, 2025
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