Implement database/sql driver with parameter binding and pooling by wkk778 · Pull Request #162 · apache/iotdb-client-go

wkk778 · 2026-06-12T08:31:12Z

No description provided.

… binding and connection pooling - Add complete database/sql driver interface implementation

# Conflicts: # .github/workflows/go.yml # go.mod # go.sum

shuwenwei · 2026-06-15T08:44:52Z

+	return std, nil
+}
+
+func (std *stdDriver) BeginTx(ctx context.Context, opts driver.TxOptions) (driver.Tx, error) {


This transaction implementation is unsafe as-is. Begin/BeginTx return a valid driver.Tx, but Commit is effectively a no-op and Rollback only clears local state, so callers using db.Begin() will
believe rollback semantics are supported while all executed statements have already taken effect.

If IoTDB does not support transactions through this client path, Begin/BeginTx should return an explicit unsupported error instead of a no-op transaction. Otherwise this needs real transaction/session
semantics before exposing driver.Tx.

Copilot

Pull request overview

This PR introduces a database/sql-compatible driver layer for the IoTDB Go client, including DSN parsing, parameter binding, and row/column scanning support, plus a suite of tests to validate basic query/scan behavior.

Changes:

Register and implement an iotdb database/sql/driver (connect/open, ping, exec, query, rows).
Add DSN parsing + query parameter binding helpers (positional, numeric, named).
Add column adapters and integration/unit tests for querying and scanning common IoTDB data types.

Reviewed changes

Copilot reviewed 33 out of 34 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
go.mod	Adds `github.com/pkg/errors` dependency used by the new driver/binding code.
go.sum	Updates sums for new dependencies.
client/session.go	Adds `Session.Ping(ctx)` used by the sql driver pinger.
database/iotdb.go	Introduces shared error values and transport abstractions used by the new driver.
database/iotdb_std.go	Implements `database/sql/driver` connection behavior and registers the `iotdb` driver.
database/std_connect.go	Defines the internal connection interface used by the stdlib driver wrapper.
database/std_conn_opener.go	Implements a `driver.Connector`-style opener with address selection strategy.
database/conn.go	Implements dialing and the pooled connection wrapper on top of `client.SessionPool`.
database/conn_exec.go	Adds Exec path that binds parameters then executes statements.
database/rows.go	Introduces internal `rows` wrapper around `client.SessionDataSet`.
database/rows_std.go	Implements `driver.Rows` for database/sql iteration and scanning.
database/options.go	Adds DSN parsing into `Options` and exposes driver/pool config fields.
database/query_parameters.go	Adds helper to decide between native parameters and legacy bind interpolation.
database/bind.go	Implements parameter interpolation (positional, numeric, named) and value formatting.
database/batch.go	Adds a statement-like type intended for batch/prepared usage (currently stubbed).
database/context.go	Defines query options stored in context and cloning/timeout behavior.
database/driver/driver.go	Adds internal parameter wrapper types (`NamedValue`, `NamedDateValue`).
database/column/column.go	Maps IoTDB type strings to column adapters for row value extraction.
database/column/bool.go	BOOLEAN column adapter.
database/column/int32.go	INT32 column adapter.
database/column/int64.go	INT64 column adapter.
database/column/float.go	FLOAT column adapter.
database/column/double.go	DOUBLE column adapter.
database/column/string.go	TEXT/STRING column adapter.
database/column/timestamp.go	TIMESTAMP column adapter.
database/column/date.go	DATE column adapter.
database/column/blob.go	BLOB column adapter.
database/options_test.go	Unit tests for DSN parsing behavior.
database/bind_test.go	Unit tests for binding/interpolation and formatting.
database/conn_integration_test.go	Integration tests for ping/pool/close/basic round trip.
database/query_test.go	Integration tests for core SELECT/WHERE query behaviors.
database/query_agg_test.go	Integration tests for aggregations/group-by/limit/align/metadata queries.
database/scan_types_test.go	Integration tests for scanning IoTDB primitive types and text values.
database/iotdb_std_test.go	Integration tests validating basic INSERT/SELECT via database/sql.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+	if opt.Debug {
+		if opt.Debugf != nil {
+			debugf = func(format string, v ...any) {
+				opt.Debugf(
+					"[iotdb][%s][id=%d] "+format,
+					append([]interface{}{opt.Addr, num}, v...)...,
+				)
+			}
+		} else {
+			debugf = log.New(os.Stdout, fmt.Sprintf("[iotdb][%s][id=%d]", opt.Addr, num), 0).Printf
+		}
+	}


+	var debugf = func(format string, v ...any) {}
+	if opt.Debug {
+		debugf = log.New(os.Stdout, "[iotdb-std][opener] ", 0).Printf
+	}
+	opt.ClientInfo.Comment = []string{"database/sql"}


+func (b *Blob) Row(stat *client.SessionDataSet, ptr bool) any {
+	if stat == nil {
+		if ptr {
+			return nil
+		}
+		return 0
+	}
+	value, err := stat.GetBlob(b.name)
+	if err != nil {
+		if ptr {
+			return nil
+		}
+		return 0
+	}
+	if ptr {
+		return &value
+	}
+	return value
+}


+		config = &client.PoolConfig{
+			Host:     host,
+			Port:     port,
+			UserName: opt.UserName,
+			Password: opt.Password,
+		}


HTHou

Thanks for taking on database/sql support. I found several correctness blockers around the driver contracts, result/session lifetime, NULL handling, and context propagation, so I’m requesting changes.

I verified the new package with go test -short ./database and go vet ./database. A full go test -short ./... attempt was killed locally during the Go linker step, so I do not have a full-repo test result from this machine.

HTHou · 2026-06-15T08:51:24Z

+	return std, nil
+}
+
+func (std *stdDriver) BeginTx(ctx context.Context, opts driver.TxOptions) (driver.Tx, error) {


This advertises transaction support, but no transaction is actually started. Commit is usually a no-op and Rollback only clears a callback, so operations executed through *sql.Tx are already applied and cannot be rolled back. If IoTDB does not support transactions here, Begin/BeginTx should return an explicit unsupported error instead of returning this connection as driver.Tx.

HTHou · 2026-06-15T08:51:24Z

+		release(c, err)
+		return nil, err
+	}
+	defer c.conn.PutBack(session)


The session is returned to the pool before the returned rows are consumed. SessionDataSet keeps using this session’s RPC client/session id for later FetchResultsV2 and CloseOperation calls from Rows.Next/Rows.Close, so another goroutine can borrow the same session and use the transport concurrently while the result set is still active. Please hold the session until rows are closed/EOF, or fully materialize the result before PutBack.

HTHou · 2026-06-15T08:51:24Z

+		return nil, driver.ErrBadConn
+	}
+
+	std.commit = std.conn.commit


Prepared statements are currently unusable: PrepareContext ignores the query and returns stdBatch, whose ExecContext returns driver.ErrSkip, Exec returns not implemented, and Query errors. Normal db.Prepare(...).Exec/Query calls will fail even though the driver advertises ConnPrepareContext. Please implement a statement that stores the query and delegates to the connection, or do not advertise prepare support.

HTHou · 2026-06-15T08:51:24Z

+	}
+	defer c.conn.PutBack(session)
+
+	_, err = session.ExecuteStatement(body)


ExecContext receives a caller context, but this call goes into Session.ExecuteStatement, which uses context.Background() internally for the RPC. That means cancellation and deadlines from database/sql are ignored after this point. QueryContext has the same issue through ExecuteQueryStatement; the context needs to be threaded down to the actual RPC calls.

HTHou · 2026-06-15T08:51:24Z

+				dest[i] = nil
+				continue
+			}
+			switch value := s.rows.columns[i].Row(s.rows.set, false).(type) {


This loses SQL NULLs. The column getters return typed zero values when the underlying SessionDataSet value is NULL, and Next assigns those zero values into dest. database/sql expects dest[i] = nil for NULL so sql.Null*, pointer scans, and *any behave correctly. Please check nullness before reading the value or have the column layer return nil for NULL.

HTHou · 2026-06-15T08:51:24Z

+	for i := 0; i < len(query); i++ {
+		// It's fine looping through the query string as bytes, because the (fixed) characters we're looking for
+		// are in the ASCII range to won't take up more than one byte.
+		if query[i] == '?' {


This binding code treats every ? byte as a placeholder, including ? inside quoted strings or comments. The numeric and named bind paths have the same class of issue because they regex-replace globally. Statements like SELECT '?' or WHERE text = '@name' will be rewritten incorrectly. Binding needs to skip SQL string literals/comments, preferably via a small lexer rather than raw byte/regex replacement.

asf-gitbox-commits and others added 2 commits June 12, 2026 15:52

feat(database): add database/sql driver implementation with parameter…

a0d3373

… binding and connection pooling - Add complete database/sql driver interface implementation

Merge remote-tracking branch 'upstream/main'

f7251ca

# Conflicts: # .github/workflows/go.yml # go.mod # go.sum

LeiZhang-Hunter mentioned this pull request Jun 13, 2026

feat: support go database client #150

Closed

refactor(client): remove redundant null checks in rpc dataset methods

353e9d4

HTHou requested a review from Copilot June 15, 2026 08:40

Copilot started reviewing on behalf of HTHou June 15, 2026 08:40 View session

shuwenwei reviewed Jun 15, 2026

View reviewed changes

Copilot AI reviewed Jun 15, 2026

View reviewed changes

HTHou requested changes Jun 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement database/sql driver with parameter binding and pooling#162

Implement database/sql driver with parameter binding and pooling#162
wkk778 wants to merge 3 commits into
apache:mainfrom
wkk778:main

wkk778 commented Jun 12, 2026

Uh oh!

shuwenwei Jun 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HTHou left a comment

Uh oh!

HTHou Jun 15, 2026

Uh oh!

HTHou Jun 15, 2026

Uh oh!

HTHou Jun 15, 2026

Uh oh!

HTHou Jun 15, 2026

Uh oh!

HTHou Jun 15, 2026

Uh oh!

HTHou Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

wkk778 commented Jun 12, 2026

Uh oh!

shuwenwei Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HTHou left a comment

Choose a reason for hiding this comment

Uh oh!

HTHou Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

HTHou Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

HTHou Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

HTHou Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

HTHou Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

HTHou Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants