Skip to content

Conversation

@candiduslynx
Copy link
Contributor

Follow-up for #1072 & #1002

@candiduslynx candiduslynx requested a review from hermanschaaf July 5, 2023 10:26
@candiduslynx candiduslynx requested a review from yevgenypats as a code owner July 5, 2023 10:26
@github-actions github-actions bot added the fix label Jul 5, 2023
@disq
Copy link
Member

disq commented Jul 5, 2023

Same as #1073

@hermanschaaf
Copy link
Member

This is the same as #1073 but has some more handling for ClickHouse cases that I couldn't test, so we can go with this one. Very similar either way :)

@hermanschaaf
Copy link
Member

I get a nil pointer dereference error in stripNullsFromLists when I run this one against BigQuery destination though

@candiduslynx
Copy link
Contributor Author

It also puts the sorting where it was missing in the inserts

@candiduslynx
Copy link
Contributor Author

I get a nil pointer dereference error in stripNullsFromLists when I run this one against BigQuery destination though

fixed in fff9fe5

kodiakhq bot pushed a commit that referenced this pull request Jul 5, 2023
Shuffling missed a case (I think) because we were using the same seed every time. Iterating over the items in reverse should have the same effect and is still deterministic.

Same fix as in #1074 but also adds a test for it.
@kodiakhq kodiakhq bot merged commit 88f08ee into main Jul 5, 2023
@kodiakhq kodiakhq bot deleted the fix/strip-nulls branch July 5, 2023 15:27
kodiakhq bot pushed a commit that referenced this pull request Jul 5, 2023
🤖 I have created a release *beep* *boop*
---


## [4.8.0-rc1](v4.7.1-rc1...v4.8.0-rc1) (2023-07-05)


### Features

* **transformers:** Add `Apply` to apply extra transformations ([#1069](#1069)) ([a40598e](a40598e))


### Bug Fixes

* Deterministic ordering for records returned by readAll in tests ([#1072](#1072)) ([cf7510f](cf7510f))
* Handle null-related test options ([#1074](#1074)) ([88f08ee](88f08ee))
* **naming:** Rename `SyncMessages.InsertMessage()` to `SyncMessages.GetInserts()` ([#1070](#1070)) ([ab9e768](ab9e768))
* Reset timers on flush ([#1076](#1076)) ([767327f](767327f))
* Reverse order of records in memdb ([#1075](#1075)) ([8356590](8356590))
* **scalar:** Test `AppendTime` on TimestampBuilder ([#1068](#1068)) ([888c9ee](888c9ee))
* **testdata:** Exclude only the correct type ([#1067](#1067)) ([1c72fb2](1c72fb2))

---
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants