Add statement-level query tag support#341
Add statement-level query tag support#341joohoyeo-dev wants to merge 3 commits intodatabricks:mainfrom
Conversation
|
⏺ Test 1 — Session-level only: Test 2 — Statement-level tags: Test 3 — Different statement-level tags: Test 4 — No statement-level tags (session-level still applies): |
| // NewContextWithQueryTags creates a new context with per-statement query tags. | ||
| // These tags are serialized and passed via confOverlay as "query_tags" in TExecuteStatementReq. | ||
| // They apply only to the statement executed with this context and do not persist across queries. | ||
| func NewContextWithQueryTags(ctx context.Context, queryTags map[string]string) context.Context { |
There was a problem hiding this comment.
Not sure if this is the best way to create a context object with an optional param in Go. I'm not familiar w/ Go. Perhaps ask Claude to double check.
There was a problem hiding this comment.
Good question! This is actually the idiomatic Go pattern for per-request metadata in database/sql drivers. Since the QueryContext(ctx, query, args) interface is fixed by the standard library, context values are the standard way to pass per-request options.
This driver already uses the same pattern for other per-request data:
driverctx.NewContextWithConnIddriverctx.NewContextWithCorrelationIddriverctx.NewContextWithQueryIddriverctx.NewContextWithStagingInfo
So NewContextWithQueryTags follows the established convention.
examples/query_tags/main.go
Outdated
There was a problem hiding this comment.
Either in this PR or as follow up, we should allow user to pass in a map as query tags so it's consistent with the statement level. I did it in Python and plan to do in NodeJS, too.
There was a problem hiding this comment.
Great point — addressed in 606f44e. Added WithQueryTags(map[string]string) as a connector option that accepts a structured map and handles serialization internally, consistent with the statement-level API and your Python approach (databricks/databricks-sql-python@e916f71).
// Before: pre-serialized string
dbsql.WithSessionParams(map[string]string{
"QUERY_TAGS": "team:engineering,app:etl",
})
// After: structured map (preferred)
dbsql.WithQueryTags(map[string]string{
"team": "engineering",
"app": "etl",
})WithSessionParams still works for backward compatibility. If both are used, WithQueryTags takes precedence (applied after in the options chain). Tests added in connector_test.go.
|
checked the tags still work as expected in Logfood |
vikrantpuppala
left a comment
There was a problem hiding this comment.
please fix failing checks, lgtm otherwise, thanks!
Previously, query tags could only be set at the session level via WithSessionParams during connection creation. This adds per-statement query tag support, allowing different tags for each query execution. Users pass query tags through context using the new driverctx.NewContextWithQueryTags function. The tags are serialized into the TExecuteStatementReq.ConfOverlay["query_tags"] field, consistent with the Python and NodeJS connector implementations. Co-authored-by: Isaac Signed-off-by: Jooho Yeo <jooho.yeo@databricks.com>
Verifies that session-level tags (TOpenSessionReq.Configuration) and statement-level tags (TExecuteStatementReq.ConfOverlay) are independent: session params don't leak into ConfOverlay, and statement-level tags are correctly set even when session-level tags exist. Co-authored-by: Isaac Signed-off-by: Jooho Yeo <jooho.yeo@databricks.com>
Addresses review feedback from jiabin-hu: 1. Add WithQueryTags(map[string]string) as a connector option that accepts a structured map and serializes it internally, consistent with the statement-level API and the Python connector approach (databricks/databricks-sql-python@e916f71). 2. Context values pattern is the idiomatic Go approach for per-request metadata in database/sql drivers (same pattern used by ConnId, CorrelationId, QueryId, and StagingInfo in this driver). Co-authored-by: Isaac Signed-off-by: Jooho Yeo <jooho.yeo@databricks.com>
606f44e to
04704e1
Compare




Summary
driverctx.NewContextWithQueryTags, allowing users to attach query tags to individual SQL statements through contextTExecuteStatementReq.ConfOverlay["query_tags"], consistent with the Python (#736) and NodeJS (#339) connector implementationsWithSessionParamsat connection time)Usage
Changes
driverctx/ctx.goNewContextWithQueryTags,QueryTagsFromContext, propagation inNewContextFromBackgroundquery_tags.go(new)SerializeQueryTags— map to wire format with escapingconnection.goConfOverlay["query_tags"]driverctx/ctx_test.goquery_tags_test.go(new)connection_test.goexamples/query_tags/main.goTest plan
SerializeQueryTagscovering nil, empty, single/multi tags, escaping of\,:,,in values and keysNewContextWithQueryTags/QueryTagsFromContextincluding nil context, missing key, timeout preservation, background propagationConfOverlay["query_tags"]is correctly set (or absent) in capturedTExecuteStatementReqThis pull request was AI-assisted by Isaac.