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

Short-circuit when insert values are empty #229

Merged
merged 2 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
## 0.2.10 (Common, Node.js, Web)

### New features

- If `InsertParams.values` is an empty array, no request is sent to the server and `ClickHouseClient.insert` short-circuits itself. In this scenario, the newly added `InsertResult.executed` flag will be `false`, and `InsertResult.query_id` will be an empty string.

### Bug fixes

- Client no longer produces `Code: 354. inflate failed: buffer error` exception if request compression is enabled and `InsertParams.values` is an empty array (see above).

## 0.2.9 (Common, Node.js, Web)

### New features
Expand Down
10 changes: 6 additions & 4 deletions packages/client-common/__tests__/integration/insert.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('insert', () => {
})

it('inserts values using JSON format', async () => {
const { query_id } = await client.insert({
const result = await client.insert({
table: tableName,
values: {
meta: [
Expand All @@ -39,12 +39,13 @@ describe('insert', () => {
format: 'JSON',
})
await assertJsonValues(client, tableName)
expect(validateUUID(query_id)).toBeTruthy()
expect(validateUUID(result.query_id)).toBeTruthy()
expect(result.executed).toBeTruthy()
})

it('should use provide query_id', async () => {
const query_id = guid()
const { query_id: q_id } = await client.insert({
const result = await client.insert({
table: tableName,
query_id,
values: {
Expand All @@ -67,7 +68,8 @@ describe('insert', () => {
format: 'JSON',
})
await assertJsonValues(client, tableName)
expect(query_id).toEqual(q_id)
expect(result.query_id).toEqual(query_id)
expect(result.executed).toBeTruthy()
})

it('inserts values using JSONObjectEachRow format', async () => {
Expand Down
24 changes: 21 additions & 3 deletions packages/client-common/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import type {
Connection,
ConnectionParams,
ConnExecResult,
ConnInsertResult,
Logger,
WithClickHouseSummary,
} from '@clickhouse/client-common'
Expand Down Expand Up @@ -128,7 +127,21 @@ export interface ExecParams extends BaseQueryParams {
export type CommandParams = ExecParams
export type CommandResult = { query_id: string } & WithClickHouseSummary

export type InsertResult = ConnInsertResult
export type InsertResult = {
/**
* Indicates whether the INSERT statement was executed on the server.
* Will be `false` if there was no data to insert.
* For example: if {@link InsertParams.values} was an empty array,
* the client does not any requests to the server, and {@link executed} is false.
*/
executed: boolean
/**
* Empty string if {@link executed} is false.
* Otherwise, either {@link InsertParams.query_id} if it was set, or the id that was generated by the client.
*/
query_id: string
} & WithClickHouseSummary

export type ExecResult<Stream> = ConnExecResult<Stream>
export type PingResult = ConnPingResult

Expand Down Expand Up @@ -249,15 +262,20 @@ export class ClickHouseClient<Stream = unknown> {
* consider using {@link ClickHouseClient.command}, passing the entire raw query there (including FORMAT clause).
*/
async insert<T>(params: InsertParams<Stream, T>): Promise<InsertResult> {
if (Array.isArray(params.values) && params.values.length === 0) {
return { executed: false, query_id: '' }
}

const format = params.format || 'JSONCompactEachRow'
this.valuesEncoder.validateInsertValues(params.values, format)

const query = getInsertQuery(params, format)
return await this.connection.insert({
const result = await this.connection.insert({
query,
values: this.valuesEncoder.encodeValues(params.values, format),
...this.getQueryParams(params),
})
return { ...result, executed: true }
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/client-common/src/version.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '0.2.9'
export default '0.2.10'
64 changes: 46 additions & 18 deletions packages/client-node/__tests__/integration/node_insert.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,57 @@ describe('[Node.js] insert', () => {
let client: ClickHouseClient
let tableName: string

beforeEach(async () => {
client = await createTestClient()
tableName = `insert_test_${guid()}`
await createSimpleTable(client, tableName)
})
afterEach(async () => {
await client.close()
})
it('should provide error details about a dataset with an invalid type', async () => {
await expectAsync(
client.insert({

describe('without request compression', () => {
beforeEach(async () => {
client = createTestClient()
tableName = `node_insert_test_${guid()}`
await createSimpleTable(client, tableName)
})

it('should provide error details about a dataset with an invalid type', async () => {
await expectAsync(
client.insert({
table: tableName,
values: Stream.Readable.from(['42,foobar,"[1,2]"'], {
objectMode: false,
}),
format: 'TabSeparated',
})
).toBeRejectedWith(
jasmine.objectContaining({
message: jasmine.stringContaining('Cannot parse input'),
code: '27',
type: 'CANNOT_PARSE_INPUT_ASSERTION_FAILED',
})
)
})
})

describe('with request compression', () => {
beforeEach(async () => {
client = createTestClient({
compression: {
request: true,
},
})
tableName = `node_insert_test_${guid()}`
await createSimpleTable(client, tableName)
})

it('should not fail if the values array is empty', async () => {
const result = await client.insert({
table: tableName,
values: Stream.Readable.from(['42,foobar,"[1,2]"'], {
objectMode: false,
}),
format: 'TabSeparated',
values: [],
format: 'JSONEachRow',
})
).toBeRejectedWith(
jasmine.objectContaining({
message: jasmine.stringContaining('Cannot parse input'),
code: '27',
type: 'CANNOT_PARSE_INPUT_ASSERTION_FAILED',
expect(result).toEqual({
executed: false,
query_id: '',
})
)
})
})
})
2 changes: 1 addition & 1 deletion packages/client-node/src/version.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '0.2.9'
export default '0.2.10'
2 changes: 1 addition & 1 deletion packages/client-web/src/version.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '0.2.9'
export default '0.2.10'
Loading