-
Notifications
You must be signed in to change notification settings - Fork 26
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
More specific types for meta.type
#273
Comments
meta
meta.type
Array, Tuple, Map, DateTime, etc, are a bit tricky in this case cause:
So, the proposed literal won't match the actual type value in certain cases. Another approach is to provide a helper of sorts, possibly utilizing the code from the RowBinary branch (the branch is currently on hold, but the column types parser was ready, just missing some tests). It's the exact same scenario here — we need to parse the appropriate types (preferably with nested types for correctness). export function parseMetaColumnTypes(
meta: Array<{ name: string, type: string> }>
): Array<{ name: string; type: ParsedColumnType }> CC @mshustov - I'm curious to hear your opinion as well. |
Just to clarify: the idea was that the However, we can provide a helper function (essentially implemented already in the RowBinary branch, minus a few leftover types) that will produce some (probably useful) data structures out of the meta type in runtime. For example: console.log('### Simple types')
console.dir(parseColumnType('UInt32'))
console.log('### Enums')
console.dir(parseColumnType("Enum8('a' = 1, 'b' = 2)"), {
depth: 1000,
})
console.log('### Nested types')
console.dir(parseColumnType('Map(String, Array(Tuple(UInt8, String)))'), {
depth: 1000,
})
|
Yes! This would work and be very helpful.
These could actually be typed out via template string literals In the case of type TupleMember = 'String' | 'Number'; // This is just for example and would clearly need many more members
type Tuple = `Tuple(${TupleMember}, ${TupleMember})`; I think the helper proposed by @slvrtrn is actually more helpful and easier to implement/maintain than templating out the strings for every possibility. However, I think it is worth sharing this pattern still in case there is ever value to be had from generating the strings of all possible meta types. Edit to share a screenshot of what this evaluates out to: |
If a middle ground is desirable; you could type out just the outer part of the type and use type Tuple = `Tuple(${string}, ${string})`;
const aTuple: Tuple = 'Tuple(Foo, Bar)'; I think this could be really useful for checking the top level type, then disregarding the problem with depth for this field. I think in most cases, just being able to check if |
Use case
Use case will be for handling formatting of the data value from a response, when
meta
is present on an app.Describe the solution you'd like
https://github.com/ClickHouse/clickhouse-js/blob/main/packages/client-common/src/clickhouse_types.ts#L8
This line ^ could be something more like the following
Describe the alternatives you've considered
This type could be an enum or string literal. I'd have no real preference.
Additional context
What would be nice for me as a consumer of
@clickhouse/client
is to be able to treat the library as a comprehensive source of truth for what data types I should expect to be able to format. This way, I can format them accordingly in my apps (especially if new data types are added later on or renamed, so I'd see the type change).The text was updated successfully, but these errors were encountered: