Skip to content

Conversation

@yevgenypats
Copy link
Contributor

@yevgenypats yevgenypats commented Apr 18, 2023

This should solve bunch of issues that we have with NullValues and StripNulls logic propagated to all our destination plugins. Apparently Valid utf8 string can't have null values in the middle of the string so we should just strip those in our type system (it can have at the end or the start but I suggest actually stripping those down as well as it's really unnecessary as far as I know and it exist prob for some archaic reason like sending a string over the wire without any additional encoding/protocol, which we have via arrow).

Once we move sources to arrow we will add the validation/strip on the source side.

@github-actions github-actions bot added the fix label Apr 18, 2023
@github-actions github-actions bot added fix and removed fix labels Apr 18, 2023
@github-actions
Copy link

⏱️ Benchmark results

Comparing with b1baab6

  • DefaultConcurrencyDFS-2 resources/s: 11,261 ⬆️ 0.85% increase vs. b1baab6
  • DefaultConcurrencyRoundRobin-2 resources/s: 12,399 ⬇️ 0.51% decrease vs. b1baab6
  • Glob-2 ns/op: 193.6 ⬇️ 0.57% decrease vs. b1baab6
  • TablesWithChildrenDFS-2 resources/s: 28,259 ⬆️ 6.90% increase vs. b1baab6
  • TablesWithChildrenRoundRobin-2 resources/s: 25,461 ⬇️ 0.78% decrease vs. b1baab6
  • TablesWithRateLimitingDFS-2 resources/s: 28.25 ⬇️ 1.17% decrease vs. b1baab6
  • TablesWithRateLimitingRoundRobin-2 resources/s: 834.3 ⬆️ 0.50% increase vs. b1baab6
  • BufferedScanner-2 ns/op: 9.399 ⬆️ 0.09% increase vs. b1baab6
  • LogReader-2 ns/op: 30.71 (no change)

@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.02 ⚠️

Comparison is base (b1baab6) 46.73% compared to head (8fdddb7) 46.71%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #797      +/-   ##
==========================================
- Coverage   46.73%   46.71%   -0.02%     
==========================================
  Files          76       76              
  Lines        7830     7832       +2     
==========================================
  Hits         3659     3659              
- Misses       3680     3682       +2     
  Partials      491      491              
Impacted Files Coverage Δ
schema/arrow.go 0.00% <0.00%> (ø)

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.

@kodiakhq kodiakhq bot merged commit 591502f into main Apr 19, 2023
@kodiakhq kodiakhq bot deleted the fix/utf8 branch April 19, 2023 07:50
kodiakhq bot pushed a commit that referenced this pull request Apr 19, 2023
🤖 I have created a release *beep* *boop*
---


## [2.3.2](v2.3.1...v2.3.2) (2023-04-19)


### Bug Fixes

* Arrow Retain and Release fixes ([#795](#795)) ([a893db6](a893db6))
* Disallow null character in strings per utf8 spec ([#797](#797)) ([591502f](591502f))

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

3 participants