Implement query attributes by lidavidm · Pull Request #1784 · go-sql-driver/mysql

lidavidm · 2026-07-17T06:10:58Z

Description

Add support for passing query attributes via a special bind parameter. The intent is to enable passing through OpenTelemetry traceparents which is now possible with MySQL 9.7 Community.

AI usage disclosure: created with GPT 5.6 Sol then reviewed and edited by hand.

Checklist

Code compiles correctly
Created tests which fail without the change (if possible)
All tests passing
Extended the README / documentation, if necessary
Added myself / the copyright holder to the AUTHORS file

AI usage disclosure: created with GPT 5.6 Sol then reviewed and edited by hand.

coderabbitai · 2026-07-17T06:22:33Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a147955f-b64b-47aa-82c4-b651b9d9984f

📥 Commits

Reviewing files that changed from the base of the PR and between 607a838 and c0894ab.

📒 Files selected for processing (2)

packets.go
query_attribute_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

query_attribute_test.go
packets.go

Walkthrough

Query attributes are introduced for direct and prepared MySQL queries. The driver validates and separates attributes from positional parameters, negotiates server support, serializes attributes in COM_QUERY and execute packets, and adds unit and integration coverage.

Changes

Query Attribute Support

Layer / File(s)	Summary
Attribute contract and argument conversion `query_attribute.go`, `utils.go`, `connection.go`, `statement.go`	Defines and validates `QueryAttribute`, separates attributes from positional arguments, parses server versions, and updates named-value and statement handling.
Capability negotiation and direct query packets `const.go`, `packets.go`, `connection.go`, `query_attribute_test.go`	Adds capability negotiation, server-version storage, attribute-aware COM_QUERY writing, and direct query routing with packet serialization tests.
Prepared statement attribute execution `packets.go`, `statement.go`, `connection.go`, `query_attribute_test.go`	Propagates attributes through prepared statements, encodes supported attributes as execute parameters, ignores unsupported versions, and validates direct and live behavior.
Author list update `AUTHORS`	Adds David Li to the author list.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant mysqlConn
  participant mysqlStmt
  participant MySQLServer
  Client->>mysqlConn: ExecContext or QueryContext with QueryAttribute
  mysqlConn->>mysqlStmt: Route prepared execution when applicable
  mysqlConn->>MySQLServer: Send COM_QUERY or prepared execute packet with attributes
  MySQLServer-->>Client: Return query or execution result

Suggested reviewers: methane

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: adding query attribute support.
Description check	✅ Passed	The description is directly related to the change and matches the added query attribute support and checklist items.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packets.go`:
- Around line 205-211: Update the handshake parsing flow around
parseServerVersion and the subsequent fixed-field reads to validate that data
contains the complete connection ID, auth data, filler, and capability bytes
before any slice access. Return ErrMalformPkt when the remaining packet is too
short, while preserving normal parsing for complete packets.

In `@query_attribute_test.go`:
- Around line 179-218: Update the query-attribute test around the version check
to also verify that the optional query_attributes component or
mysql_query_attribute_string function is available before executing either
query. Skip the test with a clear reason when it is unavailable, while
preserving the existing version gate and test flow for supported servers.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 4c786d88-fdd9-466e-abad-d2590af0ee76

📥 Commits

Reviewing files that changed from the base of the PR and between 416cd99 and 172c81f.

📒 Files selected for processing (8)

AUTHORS
connection.go
const.go
packets.go
query_attribute.go
query_attribute_test.go
statement.go
utils.go

ccoVeille

I'm just a random gopher who tooks a look at your PR.

I feel there are some bloats caused by using an agent to code this

ccoVeille · 2026-07-17T06:50:05Z

-	return mc.query(query, args)
+	return mc.queryWithAttributes(query, args, nil)


Why not leaving mc.query here?

I don't think you need this change

Ah, thanks, I'll address these - the agent originally did a much bigger refactor that I tried to back out but evidently I missed spots

ccoVeille · 2026-07-17T06:52:24Z

+	Value any
+}
+
+func validateQueryAttribute(attr QueryAttribute) error {


Maybe a method on the struct instead of this

ccoVeille · 2026-07-17T06:54:06Z

+func checkNoError(t *testing.T, err error) {
+	t.Helper()
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+func checkEqual[T any](t *testing.T, want, got T) {
+	t.Helper()
+	if !reflect.DeepEqual(want, got) {
+		t.Fatalf("want %#v, got %#v", want, got)
+	}
+}


I find odd you added these as all repository was tested with such an helper until now.

methane · 2026-07-17T08:01:17Z

I do not intend to merge this PR immediately.

Merging new features would require maintaining multiple branches for the v1.10 bugfix release. To minimize effort, I will only commit bug fixes for the current version to the master branch until I have a clear idea of the changes I want to release in the next version.
I have not yet studied this feature in MySQL thoroughly, and I am also unaware of how MariaDB handles this feature.
I am still unsure about the standard way for Go's database/sql drivers to handle trace id and span id. Should they be received as parameters, or should context be used, or should a callback function be employed?

So, please do not expect detailed reviews or merges in the near future. Instead of creating a PR right away, it would be more helpful for me if you could create an issue with links to the information needed to make the technical decisions mentioned above.

lidavidm · 2026-07-17T08:25:12Z

Thanks - sorry for dropping this on you. I will follow up with an issue later.

Implement query attributes

172c81f

AI usage disclosure: created with GPT 5.6 Sol then reviewed and edited by hand.

lidavidm force-pushed the query-attributes branch from ada6949 to 172c81f Compare July 17, 2026 06:17

lidavidm marked this pull request as ready for review July 17, 2026 06:17

coderabbitai Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread packets.go

Comment thread query_attribute_test.go

ccoVeille reviewed Jul 17, 2026

View reviewed changes

lidavidm added 2 commits July 17, 2026 16:32

review feedback

607a838

automated review feedback

c0894ab

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Implement query attributes#1784

Implement query attributes#1784
lidavidm wants to merge 3 commits into
go-sql-driver:masterfrom
lidavidm:query-attributes

lidavidm commented Jul 17, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 17, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

ccoVeille left a comment •

edited

Loading

Uh oh!

ccoVeille Jul 17, 2026

Uh oh!

lidavidm Jul 17, 2026

Uh oh!

ccoVeille Jul 17, 2026

Uh oh!

ccoVeille Jul 17, 2026

Uh oh!

methane commented Jul 17, 2026

Uh oh!

lidavidm commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		return mc.query(query, args)
		return mc.queryWithAttributes(query, args, nil)

Uh oh!

Conversation

lidavidm commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

coderabbitai Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ccoVeille left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ccoVeille Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

lidavidm Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

ccoVeille Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

ccoVeille Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

methane commented Jul 17, 2026

Uh oh!

lidavidm commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lidavidm commented Jul 17, 2026 •

edited

Loading

coderabbitai Bot commented Jul 17, 2026 •

edited

Loading

ccoVeille left a comment •

edited

Loading