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

fix: Support list scalars from JSON #1530

Merged
merged 3 commits into from
Feb 22, 2024

Conversation

disq
Copy link
Member

@disq disq commented Feb 20, 2024

Without this, lists from the test source (at least, could be more plugins/cases) get persisted as []

I'm still not 100% sure how the lists end up getting encoded to JSON, probably we use Arrow's .ValueStr and/or .GetOneForMarshal somewhere.

Called from table resolver - source plugins need this fix.

Copy link

github-actions bot commented Feb 20, 2024

⏱️ Benchmark results

  • Glob-8 ns/op: 91.03

@candiduslynx
Copy link
Contributor

Could you also verify that the same fix is present in other SDKs?

kodiakhq bot pushed a commit to cloudquery/cloudquery that referenced this pull request Feb 21, 2024
This makes it fully sync the testdata table from the test source plugin (with [fixed](cloudquery/plugin-sdk#1530) list scalars...) using parquet loading.

Also removes `interval` references which didn't work in parquet anyway.
@disq
Copy link
Member Author

disq commented Feb 21, 2024

Could you also verify that the same fix is present in other SDKs?

Opened issues, linked above (Java SDK doesn't support lists at all so added a general issue for that too)

@github-actions github-actions bot added fix and removed fix labels Feb 21, 2024
@disq disq added the automerge label Feb 22, 2024
@kodiakhq kodiakhq bot merged commit cf13dd5 into cloudquery:main Feb 22, 2024
7 checks passed
@disq disq deleted the fix/scalar-lists-from-json branch February 22, 2024 10:24
kodiakhq bot pushed a commit that referenced this pull request Feb 27, 2024
🤖 I have created a release *beep* *boop*
---


## [4.31.0](v4.30.0...v4.31.0) (2024-02-27)


### Features

* Allow homogeneous data types to be configured ([#1533](#1533)) ([ca7cdb8](ca7cdb8))


### Bug Fixes

* Default Plugin logger assumes plugin is a `source` ([#1531](#1531)) ([b7dcd56](b7dcd56))
* **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.18.0 ([#1528](#1528)) ([4cc6ade](4cc6ade))
* **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.19.0 ([#1532](#1532)) ([4b475bb](4b475bb))
* Support list scalars from JSON ([#1530](#1530)) ([cf13dd5](cf13dd5))

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

None yet

3 participants