Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(nested): Simplify constructing nested ClickHouse value #11205

Merged
merged 3 commits into from
Jun 5, 2023

Conversation

candiduslynx
Copy link
Contributor

@candiduslynx candiduslynx commented Jun 4, 2023

Requires cloudquery/plugin-sdk#948

Benchmarks

Performed by the following command in typeconv/ch/values dir:

go test \
  -test.run=BenchmarkFromArray \
  -test.bench=BenchmarkFromArray \
  -test.count 10 -test.benchmem -test.benchtime 10000x
Before this update
goos: darwin
goarch: arm64
pkg: github.com/cloudquery/cloudquery/plugins/destination/clickhouse/typeconv/ch/values
BenchmarkFromArray-10    	   10000	    465316 ns/op	  508005 B/op	    7044 allocs/op
BenchmarkFromArray-10    	   10000	    442365 ns/op	  508002 B/op	    7044 allocs/op
BenchmarkFromArray-10    	   10000	    434197 ns/op	  508006 B/op	    7044 allocs/op
BenchmarkFromArray-10    	   10000	    430641 ns/op	  508006 B/op	    7044 allocs/op
BenchmarkFromArray-10    	   10000	    406410 ns/op	  507999 B/op	    7044 allocs/op
BenchmarkFromArray-10    	   10000	    429302 ns/op	  508002 B/op	    7044 allocs/op
BenchmarkFromArray-10    	   10000	    399611 ns/op	  508005 B/op	    7044 allocs/op
BenchmarkFromArray-10    	   10000	    425171 ns/op	  508004 B/op	    7044 allocs/op
BenchmarkFromArray-10    	   10000	    423713 ns/op	  508007 B/op	    7044 allocs/op
BenchmarkFromArray-10    	   10000	    413656 ns/op	  508006 B/op	    7044 allocs/op
PASS
ok  	github.com/cloudquery/cloudquery/plugins/destination/clickhouse/typeconv/ch/values	108.016s
After this update
goos: darwin
goarch: arm64
pkg: github.com/cloudquery/cloudquery/plugins/destination/clickhouse/typeconv/ch/values
BenchmarkFromArray-10    	   10000	    326041 ns/op	  235184 B/op	    4512 allocs/op
BenchmarkFromArray-10    	   10000	    310762 ns/op	  235187 B/op	    4512 allocs/op
BenchmarkFromArray-10    	   10000	    324254 ns/op	  235182 B/op	    4512 allocs/op
BenchmarkFromArray-10    	   10000	    320649 ns/op	  235188 B/op	    4512 allocs/op
BenchmarkFromArray-10    	   10000	    326467 ns/op	  235183 B/op	    4512 allocs/op
BenchmarkFromArray-10    	   10000	    310552 ns/op	  235189 B/op	    4512 allocs/op
BenchmarkFromArray-10    	   10000	    310553 ns/op	  235188 B/op	    4512 allocs/op
BenchmarkFromArray-10    	   10000	    313457 ns/op	  235187 B/op	    4512 allocs/op
BenchmarkFromArray-10    	   10000	    327252 ns/op	  235185 B/op	    4512 allocs/op
BenchmarkFromArray-10    	   10000	    312083 ns/op	  235188 B/op	    4512 allocs/op
PASS
ok  	github.com/cloudquery/cloudquery/plugins/destination/clickhouse/typeconv/ch/values	97.273s

@candiduslynx candiduslynx force-pushed the feat/simplify-sanitize-nested branch 2 times, most recently from 83095d0 to 20ce29e Compare June 4, 2023 15:49
@candiduslynx candiduslynx marked this pull request as ready for review June 4, 2023 15:50
@candiduslynx candiduslynx requested a review from a team as a code owner June 4, 2023 15:50
@candiduslynx candiduslynx requested review from disq and removed request for a team June 4, 2023 15:50
@candiduslynx candiduslynx added the automerge Automatically merge once required checks pass label Jun 5, 2023
@erezrokah erezrokah removed the automerge Automatically merge once required checks pass label Jun 5, 2023
@erezrokah
Copy link
Contributor

Just one comment, what makes this a feat? What is the user facing change here? The SDK update?

@candiduslynx
Copy link
Contributor Author

Just one comment, what makes this a feat? What is the user facing change here? The SDK update?

It changes a bit how the empty struct value is passed to the database (basically, the Nullable field will still get the value now).

@candiduslynx candiduslynx added the automerge Automatically merge once required checks pass label Jun 5, 2023
@kodiakhq kodiakhq bot merged commit dd6b275 into main Jun 5, 2023
10 checks passed
@kodiakhq kodiakhq bot deleted the feat/simplify-sanitize-nested branch June 5, 2023 12:23
kodiakhq bot pushed a commit that referenced this pull request Jun 6, 2023
🤖 I have created a release *beep* *boop*
---


## [3.2.0](plugins-destination-clickhouse-v3.1.1...plugins-destination-clickhouse-v3.2.0) (2023-06-06)


### Features

* **nested:** Simplify constructing nested ClickHouse value ([#11205](#11205)) ([dd6b275](dd6b275))


### Bug Fixes

* **deps:** Update `github.com/cloudquery/plugin-sdk/v3` to `v3.8.1` ([#11078](#11078)) ([0a1764f](0a1764f))
* **deps:** Update github.com/apache/arrow/go/v13 digest to e07e22c ([#11151](#11151)) ([5083cf7](5083cf7))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to 20b0de9 ([#11199](#11199)) ([dc3565d](dc3565d))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to 88d5dc2 ([#11226](#11226)) ([9f306bc](9f306bc))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to a7aad4c ([#11184](#11184)) ([8a0822e](8a0822e))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to c67fb39 ([#11169](#11169)) ([dcb0f92](dcb0f92))
* **deps:** Update golang.org/x/exp digest to 2e198f4 ([#11155](#11155)) ([c46c62b](c46c62b))
* **deps:** Update google.golang.org/genproto digest to e85fd2c ([#11156](#11156)) ([dbe7e92](dbe7e92))
* **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.0.9 ([#11240](#11240)) ([f92cd4b](f92cd4b))
* **deps:** Update module github.com/cloudquery/plugin-sdk/v3 to v3.10.4 ([#11244](#11244)) ([8fceef6](8fceef6))

---
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
Labels
automerge Automatically merge once required checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants