Skip to content

Conversation

adamyeats
Copy link
Contributor

@adamyeats adamyeats commented Apr 29, 2024

Fixes #805. This PR does a few things:

  • Handles LowCardinality() data type correctly and adds tests
  • Adds a converter for String to handle the recursion in GetConverter correctly (is there a better way to do this? i guess there was a reason why there wasn't a converter there for String already, but adding one does allow us to do some recursiveness here)
  • Modifies defaultConvert to handle strings and add better handling for pointers and dereferencing them
  • Refactors GetConverter to make it a bit nicer (?)
  • Uses reflect.PointerTo instead of reflect.PtrTo, which is now deprecated

@adamyeats adamyeats added type/bug Something isn't working datasource/ClickHouse go Pull requests that update Go code effort/small labels Apr 29, 2024
@adamyeats adamyeats self-assigned this Apr 29, 2024
@adamyeats adamyeats requested a review from a team as a code owner April 29, 2024 17:31
Copy link

github-actions bot commented Apr 29, 2024

Use the following command to run this PR with Docker at http://localhost:3000:

docker run --rm -p 3000:3000 grafana/plugin-builds:0d457e36bd31428776d1f2716df7a73f6aaff5a7pre

@adamyeats adamyeats requested a review from SpencerTorres April 29, 2024 17:44
Copy link
Contributor

@aangelisc aangelisc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Great stuff @adamyeats 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datasource/ClickHouse effort/small go Pull requests that update Go code type/bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Regression in LowCardinality(Nullable(String)) type processing in v4.0.5
2 participants