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

Infer ResultSet type hints based on DataFormat #238

Merged
merged 18 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 140 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
## 1.0.0 (Common, Node.js, Web)
# 1.0.0 (Common, Node.js, Web)

Formal stable release milestone. The client will follow the [official semantic versioning](https://docs.npmjs.com/about-semantic-versioning) guidelines.
Formal stable release milestone with a lot of improvements and a fair bit of breaking changes.

### Deprecated API
The client will follow the [official semantic versioning](https://docs.npmjs.com/about-semantic-versioning) guidelines.

## Deprecated API

The following configuration parameters are marked as deprecated:

Expand All @@ -15,7 +17,7 @@ These parameters will be removed in the next major release (2.0.0).

See "New features" section for more details.

### Breaking changes
## Breaking changes

- `request_timeout` default value was incorrectly set to 300s instead of 30s. It is now correctly set to 30s by default. If your code relies on the previous incorrect default value, consider setting it explicitly.
- Client will enable [send_progress_in_http_headers](https://clickhouse.com/docs/en/operations/settings/settings#send_progress_in_http_headers) and set `http_headers_progress_interval_ms` to `20000` (20 seconds) by default. These settings in combination allow to avoid potential load balancer timeout issues in case of long-running queries without data coming in or out, such as `INSERT FROM SELECT` and similar ones, as the connection could be marked as idle by the LB and closed abruptly. In that case, a `socket hang up` error could be thrown on the client side. Currently, 20s is chosen as a safe value, since most LBs will have at least 30s of idle timeout; this is also in line with the default [AWS LB KeepAlive interval](https://docs.aws.amazon.com/elasticloadbalancing/latest/network/network-load-balancers.html#connection-idle-timeout), which is 20s by default. It can be overridden when creating a client instance if your LB timeout value is even lower than that by manually changing the `clickhouse_settings.http_headers_progress_interval_ms` value.
Expand All @@ -37,6 +39,7 @@ const client = createClient({
With `send_progress_in_http_headers` and `http_headers_progress_interval_ms` settings now enabled by default, this is no longer sufficient. If you need to create a client instance for a read-only user, consider this instead:

```ts
// 1.0.0
const client = createClient({
readonly: true,
})
Expand All @@ -48,7 +51,139 @@ NB: this is not necessary if a user has `READONLY = 2` mode as it allows to modi

See also: [readonly documentation](https://clickhouse.com/docs/en/operations/settings/permissions-for-queries#readonly).

### New features
- (TypeScript only) `ResultSet` and `Row` are now more strictly typed, according to the format used during the `query` call. See "New features" section for more details.
- (TypeScript only) Instead of using `ClickHouseClient<StreamType>`, a corresponding type name (`NodeClickHouseClient` or `WebClickHouseClient`) should be used when providing a _type hint_ for your client instance. NB: you should still use `createClient` factory function provided in the package. For example:

```ts
// pre-1.0.0 (Node.js)
class MyClickHouseService {
private client: ClickHouseClient<Stream.Readable>
constructor() {
this.client = createClient({ ... })
}
}

// 1.0.0 (Node.js)
class MyClickHouseService {
private client: NodeClickHouseClient
constructor() {
this.client = createClient({ ... })
}
}
```

## New features

- Advanced TypeScript support for `query` + `ResultSet`. Client will now try its best to figure out the shape of the data based on the DataFormat literal specified to the `query` call, as well as which methods are allowed to be called on the `ResultSet`.

Live demo (see the full description below):

[Screencast from 2024-03-09 08-10-26.webm](https://github.com/ClickHouse/clickhouse-js/assets/3175289/b66afcb2-3a10-4411-af59-51d2754c417e)

Complete reference:

| Format | `ResultSet.json<T>()` | `ResultSet.stream<T>()` | Stream data | `Row.json<T>()` |
| ------------------------------- | --------------------- | --------------------------- | ----------------- | --------------- |
| JSON | ResponseJSON\<T\> | never | never | never |
| JSONObjectsEachRow | Record\<string, T\> | never | never | never |
| JSON\*EachRow | Array\<T\> | Stream\<Array\<Row\<T\>\>\> | Array\<Row\<T\>\> | T |
| CSV/TSV/CustomSeparated/Parquet | never | Stream\<Array\<Row\<?\>\>\> | Array\<Row\<?\>\> | never |

By default, `T` (which represents `JSONType`) is still `unknown`. However, considering `JSONObjectsEachRow` example: prior to 1.0.0, you had to specify the entire type hint, including the shape of the data, manually:

```ts
type Data = { foo: string }

const resultSet = await client.query('SELECT * FROM table', {
format: 'JSONObjectsEachRow',
})

// pre-1.0.0, `resultOld` has type Record<string, Data>
const resultOld = resultSet.json<Record<string, Data>>()
// const resultOld = resultSet.json<Data>() // incorrect! The type hint should've been `Record<string, Data>` here.

// 1.0.0, `resultNew` also has type Record<string, Data>; client inferred that it has to be a Record from the format literal.
const resultNew = resultSet.json<Data>()
```

This is even more handy in case of streaming on the Node.js platform:

```ts
const resultSet = await client.query('SELECT * FROM table', {
format: 'JSONEachRow',
})

// pre-1.0.0
// `streamOld` was just a regular Node.js Stream.Readable
const streamOld = resultSet.stream()
// `rows` were `any`, needed an explicit type hint
streamNew.on('data', (rows: Row[]) => {
rows.forEach((row) => {
// without an explicit type hint to `rows`, calling `forEach` and other array methods resulted in TS compiler errors
const t = row.text
const j = row.json<Data>() // `j` needed a type hint here, otherwise, it's `unknown`
})
})

// 1.0.0
// `streamNew` is now StreamReadable<T> (Node.js Stream.Readable with a bit more type hints);
// type hint for the further `json` calls can be added here (and removed from the `json` calls)
const streamNew = resultSet.stream<Data>()
// `rows` inferred as an Array<Row<Data>> instead of `any`
streamNew.on('data', (rows) => {
rows.forEach((row) => {
// no explicit type hints required, you can use `forEach` straight away and TS compiler will be happy
const t = row.text
const j = row.json() // `j` will be of type Data
})
})
```

Calling `ResultSet.stream` is not allowed for certain data formats, such as `JSON` and `JSONObjectsEachRow`. In these cases, the client throws an error. However, it was previously not reflected on the type level; now, calling `stream` on these formats will result in a TS compiler error. For example:

```ts
const resultSet = await client.query('SELECT * FROM table', {
format: 'JSON',
})
const stream = resultSet.stream() // `stream` is `never`
```

Calling `ResultSet.json` also does not make sense on `CSV` and similar "raw" formats, and the client throws. Again, now, it is typed properly:

```ts
const resultSet = await client.query('SELECT * FROM table', {
format: 'CSV',
})
// `json` is `never`; same if you stream CSV, and call `Row.json` - it will be `never`, too.
const json = resultSet.json()
```

There is one currently known limitation: as the general shape of the data and the methods allowed for calling are inferred from the format literal, there might be situations where it will fail to do so, for example:

```ts
// assuming that `queryParams` has `JSONObjectsEachRow` format inside
async function runQuery(
queryParams: QueryParams
): Promise<Record<string, Data>> {
const resultSet = await client.query(queryParams)
// type hint here will provide a union of all known shapes instead of a specific one
return resultSet.json<Data>()
}
```

In this case, as it is _likely_ that you already know the desired format in advance (otherwise, returning a specific shape like `Record<string, Data>` would've been incorrect), consider helping the client a bit:

```ts
async function runQuery(
queryParams: QueryParams
): Promise<Record<string, Data>> {
const resultSet = await client.query({
...queryParams,
format: 'JSONObjectsEachRow',
})
return resultSet.json<Data>() // TS understands that it is a Record<string, Data[]> now
}
```

- Added `url` configuration parameter. It is intended to replace the deprecated `host`, which was already supposed to be passed as a valid URL.
- Added `http_headers` configuration parameter as a direct replacement for `additional_headers`. Functionally, it is the same, and the change is purely cosmetic, as we'd like to leave an option to implement TCP connection in the future open.
Expand Down
4 changes: 2 additions & 2 deletions examples/select_json_each_row.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ void (async () => {
query: 'SELECT number FROM system.numbers LIMIT 5',
format: 'JSONEachRow',
})
const result = await rows.json<Array<Data>>()
result.map((row: Data) => console.log(row))
const result = await rows.json<Data>()
result.map((row) => console.log(row))
await client.close()
})()
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ClickHouseClient, ResponseJSON } from '@clickhouse/client-common'
import type { ClickHouseClient } from '@clickhouse/client-common'
import { createTestClient, guid, sleep } from '../utils'

describe('abort request', () => {
Expand Down Expand Up @@ -111,8 +111,6 @@ describe('abort request', () => {
})

it('should cancel of the select queries while keeping the others', async () => {
type Res = Array<{ foo: number }>

const controller = new AbortController()
const results: number[] = []

Expand All @@ -127,7 +125,7 @@ describe('abort request', () => {
// we will cancel the request that should've yielded '3'
shouldAbort ? controller.signal : undefined,
})
.then((r) => r.json<Res>())
.then((r) => r.json<{ foo: number }>())
.then((r) => results.push(r[0].foo))
// this way, the cancelled request will not cancel the others
if (shouldAbort) {
Expand Down Expand Up @@ -157,7 +155,7 @@ async function assertActiveQueries(
query: 'SELECT query FROM system.processes',
format: 'JSON',
})
const queries = await rs.json<ResponseJSON<{ query: string }>>()
const queries = await rs.json<{ query: string }>()
if (assertQueries(queries.data)) {
isRunning = false
} else {
Expand Down
15 changes: 7 additions & 8 deletions packages/client-common/__tests__/integration/exec.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ExecParams, ResponseJSON } from '@clickhouse/client-common'
import type { ExecParams } from '@clickhouse/client-common'
import { type ClickHouseClient } from '@clickhouse/client-common'
import {
createTestClient,
Expand Down Expand Up @@ -100,10 +100,7 @@ describe('exec', () => {
format: 'JSON',
})

const json = await result.json<{
rows: number
data: Array<{ name: string }>
}>()
const json = await result.json<{ name: string }>()
expect(json.rows).toBe(1)
expect(json.data[0].name).toBe('numbers')
})
Expand All @@ -120,9 +117,11 @@ describe('exec', () => {
format: 'JSON',
})

const { data, rows } = await selectResult.json<
ResponseJSON<{ name: string; engine: string; create_table_query: string }>
>()
const { data, rows } = await selectResult.json<{
name: string
engine: string
create_table_query: string
}>()

expect(rows).toBe(1)
const table = data[0]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ describe('multiple clients', () => {
})

it('should send multiple parallel selects', async () => {
type Res = Array<{ sum: number }>
const results: number[] = []
await Promise.all(
clients.map((client, i) =>
Expand All @@ -29,8 +28,8 @@ describe('multiple clients', () => {
query: `SELECT toInt32(sum(*)) AS sum FROM numbers(0, ${i + 2});`,
format: 'JSONEachRow',
})
.then((r) => r.json<Res>())
.then((json: Res) => results.push(json[0].sum))
.then((r) => r.json<{ sum: number }>())
.then((json) => results.push(json[0].sum))
)
)
expect(results.sort((a, b) => a - b)).toEqual([1, 3, 6, 10, 15])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('read only user', () => {
.query({
query: `SELECT * FROM ${tableName}`,
})
.then((r) => r.json<{ data: unknown[] }>())
.then((r) => r.json())
expect(result.data).toEqual([{ id: '42', name: 'hello', sku: [0, 1] }])
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('response compression', () => {
format: 'JSONEachRow',
})

const response = await rs.json<{ number: string }[]>()
const response = await rs.json<{ number: string }>()
const last = response[response.length - 1]
expect(last.number).toBe('19999')
})
Expand Down
12 changes: 4 additions & 8 deletions packages/client-common/__tests__/integration/select.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import {
type ClickHouseClient,
type ResponseJSON,
} from '@clickhouse/client-common'
import { type ClickHouseClient } from '@clickhouse/client-common'
import { createTestClient, guid, validateUUID } from '../utils'

describe('select', () => {
Expand Down Expand Up @@ -108,7 +105,7 @@ describe('select', () => {
format: 'JSON',
})

const response = await rs.json<ResponseJSON<{ number: string }>>()
const response = await rs.json<{ number: string }>()
expect(response.data).toEqual([{ number: '0' }, { number: '1' }])
})

Expand Down Expand Up @@ -164,7 +161,6 @@ describe('select', () => {
})

it('can send multiple simultaneous requests', async () => {
type Res = Array<{ sum: number }>
const results: number[] = []
await Promise.all(
[...Array(5)].map((_, i) =>
Expand All @@ -173,8 +169,8 @@ describe('select', () => {
query: `SELECT toInt32(sum(*)) AS sum FROM numbers(0, ${i + 2});`,
format: 'JSONEachRow',
})
.then((r) => r.json<Res>())
.then((json: Res) => results.push(json[0].sum))
.then((r) => r.json<{ sum: number }>())
.then((json) => results.push(json[0].sum))
)
)
expect(results.sort((a, b) => a - b)).toEqual([1, 3, 6, 10, 15])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ describe('select with query binding', () => {
})

describe('NULL parameter binding', () => {
const baseQuery: Pick<QueryParams, 'query' | 'format'> = {
const baseQuery: QueryParams = {
query: 'SELECT number FROM numbers(3) WHERE {n:Nullable(String)} IS NULL',
format: 'CSV',
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('Select ResultSet', () => {
format: 'JSON',
})

const { data: nums } = await rs.json<ResponseJSON<{ number: string }>>()
const { data: nums } = await rs.json<{ number: string }>()
expect(Array.isArray(nums)).toBe(true)
expect(nums.length).toEqual(5)
const values = nums.map((i) => i.number)
Expand All @@ -51,7 +51,7 @@ describe('Select ResultSet', () => {
format: 'JSON',
})

const { meta } = await rs.json<ResponseJSON<{ number: string }>>()
const { meta } = await rs.json<{ number: string }>()

expect(meta?.length).toBe(1)
const column = meta ? meta[0] : undefined
Expand Down
13 changes: 11 additions & 2 deletions packages/client-common/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ import type {
ClickHouseSettings,
Connection,
ConnExecResult,
MakeResultSet,
WithClickHouseSummary,
} from '@clickhouse/client-common'
import { type DataFormat, DefaultLogger } from '@clickhouse/client-common'
import type { InputJSON, InputJSONObjectEachRow } from './clickhouse_types'
import type {
CloseStream,
ImplementationDetails,
MakeResultSet,
ValuesEncoder,
} from './config'
import { getConnectionParams, prepareConfigWithURL } from './config'
Expand All @@ -37,6 +37,12 @@ export interface QueryParams extends BaseQueryParams {
format?: DataFormat
}

/** Same parameters as {@link QueryParams}, but with `format` field as a type */
export type QueryParamsWithFormat<Format extends DataFormat> = Omit<
QueryParams,
'format'
> & { format?: Format }

export interface ExecParams extends BaseQueryParams {
/** Statement to execute. */
query: string
Expand Down Expand Up @@ -135,8 +141,11 @@ export class ClickHouseClient<Stream = unknown> {
* FORMAT clause should be specified separately via {@link QueryParams.format} (default is JSON)
* Consider using {@link ClickHouseClient.insert} for data insertion,
* or {@link ClickHouseClient.command} for DDLs.
* Returns an implementation of {@link BaseResultSet}.
*/
async query(params: QueryParams): Promise<BaseResultSet<Stream>> {
async query<Format extends DataFormat = 'JSON'>(
params: QueryParamsWithFormat<Format>
): Promise<BaseResultSet<Stream, Format>> {
const format = params.format ?? 'JSON'
const query = formatQuery(params.query, format)
const { stream, query_id } = await this.connection.query({
Expand Down
Loading
Loading