Skip to content

Add Close function to batch interface #1566

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

Merged
merged 2 commits into from
Jun 3, 2025
Merged

Add Close function to batch interface #1566

merged 2 commits into from
Jun 3, 2025

Conversation

SpencerTorres
Copy link
Member

Summary

closes #1558

If a batch falls out of scope, it's possible that the connection will stay open. If the connection is never used again, the server sees an EOF in the query log. It starves the client and server of connections when these are left open.

This function can safely be called via defer, which will verify that the INSERT is completed and the connection is released back to the pool. A side effect of this is you may see some 0 row inserts in the query log, but this is better than erroring.

Example:

batch, _ := conn.PrepareBatch(ctx, "INSERT INTO ...")
defer batch.Close()

This is similar to other Go modules/structs that depend on Close being called for resource disposal.

Checklist

  • Unit and integration tests covering the common scenarios were added

@mshustov mshustov requested review from Copilot and jkaflik June 3, 2025 07:47
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 a Close method to the batch interface and its implementations to ensure connections are returned to the pool even if Send isn't explicitly called. It includes interface updates, implementations for HTTP and native batches, example usage updates, a new test for Close, and additional debug logging.

  • Introduce Close() on the BatchResult/Batch interface
  • Implement Close() in conn_http_batch.go and conn_batch.go
  • Update example code and add unit tests to cover Close()
  • Add debug logging around connection acquire/release

Reviewed Changes

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

Show a summary per file
File Description
lib/driver/driver.go Added Close() error to the batch interface
conn_http_batch.go Implemented Close() that marks HTTP batch as sent
conn_batch.go Implemented Close() to flush query and release conn
examples/clickhouse_api/batch.go Added defer batch.Close() in example usage
tests/abort_test.go Added TestBatchClose to verify Close() behavior
clickhouse.go Added debug logging on connection acquire/release

@mshustov mshustov mentioned this pull request Jun 3, 2025
7 tasks
@SpencerTorres SpencerTorres merged commit 06d64b3 into main Jun 3, 2025
12 checks passed
@SpencerTorres SpencerTorres deleted the fix_cancel_query branch June 3, 2025 19:46
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.

Batch interface may leak connections
1 participant