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

Add Nested data type support #89

Closed
slvrtrn opened this issue Sep 13, 2022 · 2 comments · Fixed by #227
Closed

Add Nested data type support #89

slvrtrn opened this issue Sep 13, 2022 · 2 comments · Fixed by #227
Labels
enhancement New feature or request

Comments

@slvrtrn
Copy link
Contributor

slvrtrn commented Sep 13, 2022

See https://clickhouse.com/docs/en/sql-reference/data-types/nested-data-structures/nested

Fix the failing test.

@slvrtrn slvrtrn added the enhancement New feature or request label Sep 22, 2022
@antoniovizuete
Copy link

antoniovizuete commented Feb 14, 2023

Hi @slvrtrn!

I'm willing to work on adding support for nested data types.

My approach would be to flatten the nested structures, such as objects. So, I would choose the Record<K, V> TS type to support the Nested data type, because both have the same goal, to handle structured data.

If we assume the explanation, then we face incompatibility with the Map(K, V) data type. According to the official docs, the Map(K, V) data type is supported by the Record<K, V> TS type.

This means that I won’t be able to differentiate between Map and Nested when attempting to flatten them.

In my opinion, the Map(K, V) data type should be supported by the Map<K, V> TS type. There are a couple of advantages to supporting it as a TS Map:

  • It would create semantic coherence between the Clickhouse data type and the TS type.
  • The TS Map API is better suited for handling data map structures than the TS Record type.

What do you think about supporting Clickhouse Map with TS Map?

Many thanks

@slvrtrn
Copy link
Contributor Author

slvrtrn commented Feb 14, 2023

Hi @antoniovizuete, thanks for your interest in the project.

IIRC, JS Maps introduction will likely require additional decoding effort when working with JSON* types. Right now, it just does JSON.stringify/parse (see the source), but with JS Maps, we will likely need to replace it with a better codec. I guess we just wanted to keep it as simple as possible.

Another question, for datasets with large maps, do we expect significant serialization/deserialization performance difference between the current implementation (plain objects) and JS Maps?

As for Nested, I don't think the problem is in the Record<K, V> type that we use; for tests, I could not figure out a proper way to insert records with Nested - again, IIRC, the inserted rows had empty columns no matter what without the actual Nested object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants