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
Changes from 1 commit
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
Prev Previous commit
Next Next commit
More experiments
slvrtrn committed Mar 9, 2024
commit 7281ba3d82800716b6d9347d7cf12be95402f98a
Original file line number Diff line number Diff line change
@@ -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',
}
36 changes: 27 additions & 9 deletions packages/client-common/src/client.ts
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@ import type {
ClickHouseSettings,
Connection,
ConnExecResult,
MakeResultSet,
WithClickHouseSummary,
} from '@clickhouse/client-common'
import { type DataFormat, DefaultLogger } from '@clickhouse/client-common'
@@ -36,6 +37,15 @@ 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 }

/** Same parameters as {@link QueryParams}, but with `format` field omitted */
export type QueryParamsWithoutFormat = Omit<QueryParams, 'format'>

export interface ExecParams extends BaseQueryParams {
/** Statement to execute. */
query: string
@@ -101,7 +111,7 @@ export interface InsertParams<Stream = unknown, T = unknown>
export class ClickHouseClient<Stream = unknown> {
private readonly clientClickHouseSettings: ClickHouseSettings
private readonly connection: Connection<Stream>
private readonly makeResultSet: ImplementationDetails<Stream>['impl']['make_result_set']
private readonly makeResultSet: MakeResultSet<Stream>
private readonly valuesEncoder: ValuesEncoder<Stream>
private readonly closeStream: CloseStream<Stream>
private readonly sessionId?: string
@@ -129,20 +139,28 @@ export class ClickHouseClient<Stream = unknown> {
this.closeStream = config.impl.close_stream
}

/** Overloads for proper {@link DataFormat} variants handling.
* See the implementation: {@link ClickHouseClient.query} */
async query(
params: QueryParamsWithoutFormat & { format: undefined }
): Promise<BaseResultSet<Stream, 'JSON'>>
async query(
params: QueryParamsWithoutFormat
): Promise<BaseResultSet<Stream, 'JSON'>>
async query<Format extends DataFormat>(
params: QueryParamsWithFormat<Format>
): Promise<BaseResultSet<Stream, Format>>

/**
* Used for most statements that can have a response, such as SELECT.
* 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<Format extends DataFormat | undefined = undefined>(
params: Omit<QueryParams, 'format'> & { format?: Format }
): Promise<
BaseResultSet<
Stream,
Format extends undefined ? 'JSON' : NonNullable<Format>
>
> {
async query<Format extends DataFormat>(
params: QueryParams | QueryParamsWithFormat<Format>
) {
const format = params.format ?? 'JSON'
const query = formatQuery(params.query, format)
const { stream, query_id } = await this.connection.query({
21 changes: 7 additions & 14 deletions packages/client-common/src/config.ts
Original file line number Diff line number Diff line change
@@ -88,11 +88,14 @@ export type MakeConnection<
Config = BaseClickHouseClientConfigOptionsWithURL
> = (config: Config, params: ConnectionParams) => Connection<Stream>

export type MakeResultSet<
Stream,
export type MakeResultSet<Stream> = <
Format extends DataFormat,
ResultSet extends BaseResultSet<Stream, Format>
> = (stream: Stream, format: Format, query_id: string) => ResultSet
>(
stream: Stream,
format: Format,
query_id: string
) => ResultSet

export interface ValuesEncoder<Stream> {
validateInsertValues<T = unknown>(
@@ -141,17 +144,7 @@ export type HandleImplSpecificURLParams = (
export interface ImplementationDetails<Stream> {
impl: {
make_connection: MakeConnection<Stream>
make_result_set: <
Format extends DataFormat | undefined,
ResultSet extends BaseResultSet<
Stream,
Format extends undefined ? 'JSON' : NonNullable<Format>
>
>(
stream: Stream,
format: Format,
query_id: string
) => ResultSet
make_result_set: MakeResultSet<Stream>
values_encoder: ValuesEncoder<Stream>
close_stream: CloseStream<Stream>
handle_specific_url_params?: HandleImplSpecificURLParams
10 changes: 3 additions & 7 deletions packages/client-common/src/index.ts
Original file line number Diff line number Diff line change
@@ -13,13 +13,7 @@ export {
type PingResult,
} from './client'
export { type BaseClickHouseClientConfigOptions } from './config'
export type {
Row,
BaseResultSet,
ResultJSONType,
RowJSONType,
StreamWithFormat,
} from './result'
export type { Row, BaseResultSet, ResultJSONType, RowJSONType } from './result'
export { type DataFormat } from './data_formatter'
export { ClickHouseError } from './error'
export {
@@ -47,6 +41,7 @@ export {
isSupportedRawFormat,
decode,
validateStreamFormat,
StreamableDataFormat,
} from './data_formatter'
export {
type ValuesEncoder,
@@ -86,3 +81,4 @@ export {
formatQuerySettings,
formatQueryParams,
} from './data_formatter'
export type { QueryParamsWithFormat, QueryParamsWithoutFormat } from './client'
12 changes: 4 additions & 8 deletions packages/client-common/src/result.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import type { ResponseJSON } from './clickhouse_types'
import {
import type {
DataFormat,
RawDataFormat,
RecordsJSONFormat,
SingleDocumentJSONFormat,
StreamableDataFormat,
StreamableJSONDataFormat,
} from './data_formatter'

@@ -33,8 +32,8 @@ export type RowJSONType<T, F extends DataFormat | unknown> =
: T // technically, should never happen

export interface Row<
Format extends DataFormat | unknown = unknown,
JSONType = unknown
JSONType = unknown,
Format extends DataFormat | unknown = unknown
> {
/** A string representation of a row. */
text: string
@@ -47,10 +46,7 @@ export interface Row<
json<T = JSONType>(): RowJSONType<T, Format>
}

export interface BaseResultSet<
Stream,
Format extends DataFormat | unknown = unknown
> {
export interface BaseResultSet<Stream, Format extends DataFormat> {
/**
* The method waits for all the rows to be fully loaded
* and returns the result as a string.
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
import type { ClickHouseClient } from '@clickhouse/client-common'
import { createSimpleTable } from '@test/fixtures/simple_table'
import { createTestClient, guid, sleep } from '@test/utils'
import type Stream from 'stream'
import { guid, sleep } from '@test/utils'
import type { NodeClickHouseClient } from '../../src'
import type { NodeClickHouseClientConfigOptions } from '../../src/config'
import { createNodeTestClient } from '../utils/node_client'

/**
* FIXME: Works fine during the local runs, but it is flaky on GHA,
* maybe because of Jasmine test runner vs Jest and tests isolation
* To be revisited in https://github.com/ClickHouse/clickhouse-js/issues/177
*/
xdescribe('[Node.js] Keep Alive', () => {
let client: ClickHouseClient<Stream.Readable>
let client: NodeClickHouseClient
const socketTTL = 2500 // seems to be a sweet spot for testing Keep-Alive socket hangups with 3s in config.xml
afterEach(async () => {
await client.close()
})

describe('query', () => {
it('should recreate the request if socket is potentially expired', async () => {
client = createTestClient({
client = createNodeTestClient({
max_open_connections: 1,
keep_alive: {
enabled: true,
@@ -33,7 +33,7 @@ xdescribe('[Node.js] Keep Alive', () => {
})

it('should disable keep alive', async () => {
client = createTestClient({
client = createNodeTestClient({
max_open_connections: 1,
keep_alive: {
enabled: false,
@@ -46,7 +46,7 @@ xdescribe('[Node.js] Keep Alive', () => {
})

it('should use multiple connections', async () => {
client = createTestClient({
client = createNodeTestClient({
keep_alive: {
enabled: true,
socket_ttl: socketTTL,
@@ -80,7 +80,7 @@ xdescribe('[Node.js] Keep Alive', () => {
describe('insert', () => {
let tableName: string
it('should not duplicate insert requests (single connection)', async () => {
client = createTestClient({
client = createNodeTestClient({
max_open_connections: 1,
keep_alive: {
enabled: true,
@@ -105,7 +105,7 @@ xdescribe('[Node.js] Keep Alive', () => {
})

it('should not duplicate insert requests (multiple connections)', async () => {
client = createTestClient({
client = createNodeTestClient({
max_open_connections: 2,
keep_alive: {
enabled: true,
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type { ClickHouseClient } from '@clickhouse/client-common'
import { createSimpleTable } from '@test/fixtures/simple_table'
import { createTestClient, guid, sleep } from '@test/utils'
import { guid, sleep } from '@test/utils'
import type { NodeClickHouseClient } from '../../src'
import { createNodeTestClient } from '../utils/node_client'

describe('[Node.js] max_open_connections config', () => {
let client: ClickHouseClient
let client: NodeClickHouseClient
let results: number[] = []

afterEach(async () => {
@@ -22,7 +23,7 @@ describe('[Node.js] max_open_connections config', () => {
}

it('should use only one connection', async () => {
client = createTestClient({
client = createNodeTestClient({
max_open_connections: 1,
})
void select('SELECT 1 AS x, sleep(0.3)')
@@ -39,7 +40,7 @@ describe('[Node.js] max_open_connections config', () => {

it('should use only one connection for insert', async () => {
const tableName = `node_connections_single_connection_insert_${guid()}`
client = createTestClient({
client = createNodeTestClient({
max_open_connections: 1,
request_timeout: 3000,
})
@@ -74,7 +75,7 @@ describe('[Node.js] max_open_connections config', () => {
})

it('should use several connections', async () => {
client = createTestClient({
client = createNodeTestClient({
max_open_connections: 2,
})
void select('SELECT 1 AS x, sleep(0.3)')
10 changes: 10 additions & 0 deletions packages/client-node/__tests__/utils/node_client.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { createTestClient } from '@test/utils'
import type Stream from 'stream'
import type { NodeClickHouseClient } from '../../src'
import { type BaseClickHouseClientConfigOptions } from '../../src'

export function createNodeTestClient(
config: BaseClickHouseClientConfigOptions = {}
): NodeClickHouseClient {
return createTestClient<Stream.Readable>(config) as NodeClickHouseClient
}
37 changes: 24 additions & 13 deletions packages/client-node/src/client.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,37 @@
import {
ClickHouseClient,
type DataFormat,
QueryParams,
import type {
DataFormat,
QueryParamsWithFormat,
QueryParamsWithoutFormat,
} from '@clickhouse/client-common'
import { ClickHouseClient } from '@clickhouse/client-common'
import type Stream from 'stream'
import type { NodeClickHouseClientConfigOptions } from './config'
import { NodeConfigImpl } from './config'
import { ResultSet } from './result_set'
import type { ResultSet } from './result_set'

export type NodeClickHouseClient = Omit<
ClickHouseClient<Stream.Readable>,
'query'
> & {
query<Format extends DataFormat | undefined = undefined>(
params: Omit<QueryParams, 'format'> & { format?: Format }
): Promise<ResultSet<Format extends undefined ? 'JSON' : NonNullable<Format>>>
export class NodeClickHouseClient extends ClickHouseClient<Stream.Readable> {
/** Overloads for proper {@link DataFormat} variants handling.
* See the implementation: {@link ClickHouseClient.query} */
async query(
params: QueryParamsWithoutFormat & { format: undefined }
): Promise<ResultSet<'JSON'>>
async query(params: QueryParamsWithoutFormat): Promise<ResultSet<'JSON'>>
async query<Format extends DataFormat>(
params: QueryParamsWithFormat<Format>
): Promise<ResultSet<Format>>

/** See the base implementation: {@link ClickHouseClient.query} */
query<Format extends DataFormat>(
params: QueryParamsWithFormat<Format>
): Promise<ResultSet<Format>> {
return super.query(params) as any
}
}

export function createClient(
config?: NodeClickHouseClientConfigOptions
): NodeClickHouseClient {
return new ClickHouseClient({
return new ClickHouseClient<Stream.Readable>({
impl: NodeConfigImpl,
...(config || {}),
}) as any
2 changes: 1 addition & 1 deletion packages/client-node/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export type { NodeClickHouseClient } from './client'
slvrtrn marked this conversation as resolved.
Show resolved Hide resolved
export { createClient } from './client'
export { NodeClickHouseClientConfigOptions as ClickHouseClientConfigOptions } from './config'
export { ResultSet, ResultStream } from './result_set'
export { ResultSet, StreamReadable } from './result_set'

/** Re-export @clickhouse/client-common types */
export {
Loading