Skip to content

fix(transform): Use path instead of field name for PK options#739

Merged
kodiakhq[bot] merged 3 commits into
mainfrom
fix/pk-opt
Mar 31, 2023
Merged

fix(transform): Use path instead of field name for PK options#739
kodiakhq[bot] merged 3 commits into
mainfrom
fix/pk-opt

Conversation

@candiduslynx
Copy link
Copy Markdown
Contributor

@candiduslynx candiduslynx commented Mar 29, 2023

While using transformers.WithPrimaryKeys option there is an odd behavior being observed, where the unwrapped struct field also gets promoted to be PK.

Consider the following example:

type A struct {
  ID string
  B B
}

type B struct {
  ID string
}

Currently, if transformers.WithPrimaryKeys("ID") option was used to transform struct A we would get the following columns:

  • id: PK, taken from A
  • b_id: PK, taken from B

However, with the current change, the following becomes possible:

transformers.WithPrimaryKeys id is PK b_id is PK
"ID" true false
"B.ID" false true
"ID", "B.ID" true true

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 29, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.05 🎉

Comparison is base (72c2cc0) 43.15% compared to head (0157878) 43.21%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #739      +/-   ##
==========================================
+ Coverage   43.15%   43.21%   +0.05%     
==========================================
  Files          80       80              
  Lines        7886     7889       +3     
==========================================
+ Hits         3403     3409       +6     
+ Misses       4015     4013       -2     
+ Partials      468      467       -1     
Impacted Files Coverage Δ
transformers/struct.go 71.66% <100.00%> (+1.62%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 29, 2023

⏱️ Benchmark results

Comparing with 72c2cc0

  • DefaultConcurrencyDFS-2 resources/s: 10,898 ⬇️ 0.06% decrease vs. 72c2cc0
  • DefaultConcurrencyRoundRobin-2 resources/s: 12,016 ⬇️ 0.33% decrease vs. 72c2cc0
  • Glob-2 ns/op: 263.9 ⬆️ 34.98% increase vs. 72c2cc0
  • TablesWithChildrenDFS-2 resources/s: 24,975 ⬇️ 23.36% decrease vs. 72c2cc0
  • TablesWithChildrenRoundRobin-2 resources/s: 23,743 ⬇️ 23.85% decrease vs. 72c2cc0
  • TablesWithRateLimitingDFS-2 resources/s: 28.37 ⬆️ 0.78% increase vs. 72c2cc0
  • TablesWithRateLimitingRoundRobin-2 resources/s: 826.2 ⬇️ 5.99% decrease vs. 72c2cc0
  • BufferedScanner-2 ns/op: 11.58 ⬆️ 19.84% increase vs. 72c2cc0
  • LogReader-2 ns/op: 37.57 ⬆️ 18.79% increase vs. 72c2cc0

@hermanschaaf
Copy link
Copy Markdown
Contributor

Could you please add a description to explain what this fixes? It's useful for reviewers and for users who might later need to understand what this change did.

@github-actions github-actions Bot added fix and removed fix labels Mar 29, 2023
@candiduslynx
Copy link
Copy Markdown
Contributor Author

@hermanschaaf I've added a more detailed explanation for this change in the PR description

@hermanschaaf
Copy link
Copy Markdown
Contributor

Nice, thanks for the detailed description @candiduslynx, makes sense 👍

Copy link
Copy Markdown
Member

@erezrokah erezrokah 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, thanks for adding the detailed description

@kodiakhq kodiakhq Bot merged commit d7649d8 into main Mar 31, 2023
@kodiakhq kodiakhq Bot deleted the fix/pk-opt branch March 31, 2023 07:37
kodiakhq Bot pushed a commit that referenced this pull request Mar 31, 2023
🤖 I have created a release *beep* *boop*
---


## [1.44.1](v1.44.0...v1.44.1) (2023-03-31)


### Bug Fixes

* **transform:** Use path instead of field name for PK options ([#739](#739)) ([d7649d8](d7649d8))

---
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 to cloudquery/cloudquery that referenced this pull request Mar 31, 2023
daniel-garcia pushed a commit to infobloxopen/ibcq-source-k8s that referenced this pull request Feb 24, 2026
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