Skip to content

Conversation

eryue0220
Copy link
Contributor

Summary

This pr is try to fix #390

Note

This pr only add example codes, no any other feature or bug fix changes.

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG
  • For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

@mshustov mshustov requested review from Copilot and slvrtrn September 23, 2025 09:18
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds comprehensive examples demonstrating advanced streaming INSERT operations with backpressure handling for the ClickHouse Node.js client. This addresses issue #390 by providing practical guidance for handling high-throughput streaming scenarios.

Key changes:

  • Adds advanced streaming example with event-driven data source and proper backpressure management
  • Includes simplified streaming example showing essential backpressure patterns
  • Updates documentation to reference the new streaming examples

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
examples/node/insert_streaming_with_backpressure.ts Advanced streaming example with event-driven data source, burst mode simulation, and comprehensive backpressure handling
examples/node/insert_streaming_backpressure_simple.ts Simplified streaming example demonstrating core backpressure concepts with interval-based data generation
examples/README.md Documentation updates adding references to the new streaming examples

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

// Push all pending data, but stop if we hit backpressure again
while (this.#pendingData.length > 0 && !this.#streamPaused) {
const data = this.#pendingData.shift()
if (!data || !this.#pushData(data)) {
Copy link
Preview

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition !data will never be true since this.#pendingData.shift() is only called when this.#pendingData.length > 0 is verified on line 97. The !data check is redundant and could mask logic errors.

Suggested change
if (!data || !this.#pushData(data)) {
if (!this.#pushData(data)) {

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not right, because the data might be undefined.


await client.command({
query: `
CREATE TABLE ${tableName}
Copy link
Contributor

@slvrtrn slvrtrn Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: just CREATE OR REPLACE should be sufficient

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@slvrtrn
Copy link
Contributor

slvrtrn commented Sep 23, 2025

Thanks, this is very cool. I will try to find time to review it more thoroughly soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Advanced example with streaming INSERT
2 participants