feat: Set Primary Key(s) for table#20437
Conversation
| for _, oldPk := range oldPks { | ||
| if !slices.Contains(newPks, oldPk) { | ||
| table.Columns.Get(oldPk).PrimaryKey = false | ||
| } | ||
| } |
There was a problem hiding this comment.
Should we actually have it as a single-source-of-truth for PKs, or rather allow toggling of extra fields as PKs?
It might be better not to remove the PKs set by the plugins, but allow adding more. Wdyt?
cc @erezrokah
There was a problem hiding this comment.
We probably need both, add PKs and remove PKs directives, also how would this look like in the spec? I don't think this PR exposes it? (For example renovate has both labels and addLabels configurations (https://docs.renovatebot.com/configuration-options/#addlabels)
There was a problem hiding this comment.
This would be the spec a user would use:
kind: transformer
spec:
name: "basic"
path: "cloudquery/basic"
version: v2.1.0
spec:
transformations:
- kind: set_primary_keys
tables: ["*"]
columns: ["_cq_id", "_cq_source_name"]
There was a problem hiding this comment.
Yeah exactly, I think providing the ability to add PKs columns is the way to go, rather then specify the only columns that should be PKs.
There was a problem hiding this comment.
Changed to AddPrimaryKeys
Done 461f7dd |
| for _, newPk := range newPks { | ||
| newCol := table.Columns.Get(newPk) | ||
| if newCol == nil { | ||
| return nil, errors.New("new primary key column not found in schema") |
There was a problem hiding this comment.
Can we add the missing column name to the error so it's easier to know what caused the error? Maybe the table name too
There was a problem hiding this comment.
Also can we make any column type a PK? Maybe support only the primitive types?
There was a problem hiding this comment.
I would stay away from restricting the columns that can be used as a PK as this is an advanced feature... It all depends on how the destination handles a type. For example SQLite stringifies all complex columns so technically they could be used for a PK
erezrokah
left a comment
There was a problem hiding this comment.
One last question is about JSON Path support. We support it for removal and obfuscation, will this work here too? In case someone wants to use an internal JSON property as a PK?
I guess you'd have to flatten the JSON first?
Yeah I think that is correct |
🤖 I have created a release *beep* *boop* --- ## [2.1.0](plugins-transformer-basic-v2.0.4...plugins-transformer-basic-v2.1.0) (2025-03-25) ### Features * Set Primary Key(s) for table ([#20437](#20437)) ([3d92b5d](3d92b5d)) ### Bug Fixes * **deps:** Update module github.com/apache/arrow-go/v18 to v18.2.0 ([#20410](#20410)) ([ee081fb](ee081fb)) * **deps:** Update module github.com/cloudquery/plugin-sdk/v4 to v4.74.2 ([#20434](#20434)) ([8db20d6](8db20d6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary