Skip to content

Commit

Permalink
Infer ResultSet type hints based on DataFormat (#238)
Browse files Browse the repository at this point in the history
  • Loading branch information
slvrtrn authored Mar 26, 2024
1 parent b740afe commit 0c73398
Show file tree
Hide file tree
Showing 40 changed files with 1,146 additions and 154 deletions.
3 changes: 2 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@
"env": {
"node": true
},
"plugins": ["@typescript-eslint", "prettier"],
"plugins": ["@typescript-eslint", "prettier", "eslint-plugin-expect-type"],
"extends": [
"eslint:recommended",
"plugin:@typescript-eslint/eslint-recommended",
"plugin:@typescript-eslint/recommended",
"plugin:expect-type/recommended",
"prettier"
],
"rules": {
Expand Down
2 changes: 1 addition & 1 deletion .scripts/jasmine.sh
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
#!/bin/bash
ts-node -r tsconfig-paths/register --project=tsconfig.dev.json node_modules/jasmine/bin/jasmine --config=$1
ts-node -r tsconfig-paths/register --transpileOnly --project=tsconfig.dev.json node_modules/jasmine/bin/jasmine --config=$1
180 changes: 158 additions & 22 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,27 +51,156 @@ 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 [this section](#advanced-typescript-support-for-query--resultset) for more details.
- (TypeScript only) Both Node.js and Web versions now uniformly export correct `ClickHouseClient` and `ClickHouseClientConfigOptions` types, specific to each implementation. Exported `ClickHouseClient` now does not have a `Stream` type parameter, as it was unintended to expose it there. NB: you should still use `createClient` factory function provided in the package.

## 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](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 |
| JSONObjectEachRow | Record\<string, T\> | never | never | never |
| All other 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({
query: 'SELECT * FROM my_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({
query: 'SELECT * FROM my_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` are inferred as an Array<Row<Data, "JSONEachRow">> instead of `any`
streamNew.on('data', (rows) => {
// `row` is inferred as Row<Data, "JSONEachRow">
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
})
})

// async iterator now also has type hints
// similarly to the `on(data)` example above, `rows` are inferred as Array<Row<Data, "JSONEachRow">>
for await (const rows of streamNew) {
// `row` is inferred as Row<Data, "JSONEachRow">
rows.forEach((row) => {
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` (unlike `JSONEachRow` and the rest of `JSON*EachRow`, these formats return a single object). 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
}
```

### URL configuration

- 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.
- It is now possible to configure most of the client instance parameters with a URL. The URL format is `http[s]://[username:password@]hostname:port[/database][?param1=value1&param2=value2]`. In almost every case, the name of a particular parameter reflects its path in the config options interface, with a few exceptions. The following parameters are supported:

| Parameter | Type |
| --------------------------------------------------- | ----------------------------------------------------------------- |
| `readonly` | boolean. See below [1]. |
| `application_id` | non-empty string. |
| `session_id` | non-empty string. |
| `request_timeout` | non-negative number. |
| `max_open_connections` | non-negative number, greater than zero. |
| `compression_request` | boolean. |
| `compression_response` | boolean. |
| `log_level` | allowed values: `OFF`, `TRACE`, `DEBUG`, `INFO`, `WARN`, `ERROR`. |
| `keep_alive_enabled` | boolean. |
| `clickhouse_setting_*` or `ch_*` | see below [2]. |
| `http_header_*` | see below [3]. |
| (Node.js only) `keep_alive_socket_ttl` | non-negative number. |
| (Node.js only) `keep_alive_retry_on_expired_socket` | boolean. |
| Parameter | Type |
| ------------------------------------------- | ----------------------------------------------------------------- |
| `readonly` | boolean. See below [1]. |
| `application_id` | non-empty string. |
| `session_id` | non-empty string. |
| `request_timeout` | non-negative number. |
| `max_open_connections` | non-negative number, greater than zero. |
| `compression_request` | boolean. |
| `compression_response` | boolean. |
| `log_level` | allowed values: `OFF`, `TRACE`, `DEBUG`, `INFO`, `WARN`, `ERROR`. |
| `keep_alive_enabled` | boolean. |
| `clickhouse_setting_*` or `ch_*` | see below [2]. |
| `http_header_*` | see below [3]. |
| (Node.js only) `keep_alive_idle_socket_ttl` | non-negative number. |

[1] For booleans, valid values will be `true`/`1` and `false`/`0`.

Expand Down Expand Up @@ -102,6 +234,10 @@ Currently not supported via URL:

See also: [URL configuration example](./examples/url_configuration.ts).

### Miscellaneous

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

## 0.3.0 (Node.js only)

This release primarily focuses on improving the Keep-Alive mechanism's reliability on the client side.
Expand Down Expand Up @@ -348,7 +484,7 @@ await client.exec('CREATE TABLE foo (id String) ENGINE Memory')

// correct: stream does not contain any information and just destroyed
const { stream } = await client.exec(
'CREATE TABLE foo (id String) ENGINE Memory'
'CREATE TABLE foo (id String) ENGINE Memory',
)
stream.destroy()

Expand Down
1 change: 1 addition & 0 deletions examples/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ We aim to cover various scenarios of client usage with these examples.

#### General usage

- [url_configuration.ts](url_configuration.ts) - client configuration using the URL parameters.
- [clickhouse_settings.ts](clickhouse_settings.ts) - ClickHouse settings on the client side, both global and per operation.
- [ping.ts](ping.ts) - sample checks if the server can be reached.
- [abort_request.ts](abort_request.ts) - cancelling an outgoing request or a read-only query.
Expand Down
2 changes: 1 addition & 1 deletion examples/async_insert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ void (async () => {
query: `SELECT count(*) AS count FROM ${table}`,
format: 'JSONEachRow',
})
const [{ count }] = await resultSet.json<[{ count: string }]>()
const [{ count }] = await resultSet.json<{ count: string }>()
// It is expected to have 10k records in the table.
console.info('Select count result:', count)
})()
2 changes: 1 addition & 1 deletion examples/async_insert_without_waiting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ void (async () => {
query: `SELECT count(*) AS count FROM ${tableName}`,
format: 'JSONEachRow',
})
const [{ count }] = await resultSet.json<[{ count: string }]>()
const [{ count }] = await resultSet.json<{ count: string }>()
console.log(
'Rows inserted so far:',
`${rowsInserted};`,
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()
})()
2 changes: 2 additions & 0 deletions examples/url_configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ void (async () => {
'log_level=TRACE',
// sets keep_alive.enabled = false
'keep_alive_enabled=false',
// (Node.js only) sets keep_alive.idle_socket_ttl = 1500
'keep_alive_idle_socket_ttl=1500',
// all values prefixed with clickhouse_setting_ will be added to clickhouse_settings
// this will set clickhouse_settings.async_insert = 1
'clickhouse_setting_async_insert=1',
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"apache-arrow": "^15.0.2",
"eslint": "^8.57.0",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-expect-type": "^0.3.0",
"eslint-plugin-prettier": "^5.1.3",
"husky": "^9.0.11",
"jasmine": "^5.1.0",
Expand Down Expand Up @@ -92,7 +93,7 @@
"lint-staged": {
"*.ts": [
"prettier --write",
"eslint --fix"
"npm run lint:fix"
],
"*.json": [
"prettier --write"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ export async function createTableWithFields(
client: ClickHouseClient,
fields: string,
clickhouse_settings?: ClickHouseSettings,
table_name?: string,
): Promise<string> {
const tableName = `test_table__${guid()}`
const tableName = table_name ?? `test_table__${guid()}`
await createTable(
client,
(env) => {
Expand Down
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
19 changes: 7 additions & 12 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,13 +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
Loading

0 comments on commit 0c73398

Please sign in to comment.