Skip to content

Conversation

@shivam2680
Copy link
Contributor

Description

This PR adds support for metric view metadata in the Databricks SQL Go driver, following the JDBC driver implementation.

Changes

  • Added EnableMetricViewMetadata boolean configuration parameter to UserConfig
  • Added DSN parsing support for enableMetricViewMetadata query parameter
  • Added WithEnableMetricViewMetadata(bool) connector option
  • Session automatically injects spark.sql.thriftserver.metadata.metricview.enabled=true when enabled
  • Added comprehensive unit tests

Behavior

When EnableMetricViewMetadata=true:

  • getTables() and getTableTypes() will include METRIC_VIEW entries
  • getColumns() will return measure columns with <data_type> measure format in TYPE_NAME

Usage

// Via DSN
db, err := sql.Open("databricks", "token:dapi***@host:443/path?enableMetricViewMetadata=true")

// Via connector options
connector, err := dbsql.NewConnector(
    dbsql.WithServerHostname("host"),
    dbsql.WithAccessToken("token"),
    dbsql.WithHTTPPath("/path"),
    dbsql.WithEnableMetricViewMetadata(true),
)

Testing

All unit tests pass including 2 new tests for this feature.

Backward Compatibility

Feature is disabled by default - fully backward compatible.

@shivam2680 shivam2680 force-pushed the metric-view-metadata-support branch from 65efc20 to 5a113cb Compare October 15, 2025 19:32
vikrantpuppala
vikrantpuppala previously approved these changes Oct 21, 2025
Copy link

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 support for metric view metadata in the Databricks SQL Go driver, enabling getTables(), getTableTypes(), and getColumns() to return METRIC_VIEW entries when enabled. The feature is disabled by default for backward compatibility.

  • Added EnableMetricViewMetadata configuration parameter with DSN parsing and connector option support
  • Session automatically injects the required Spark configuration when enabled
  • Added comprehensive unit tests verifying enabled and disabled states

Reviewed Changes

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

File Description
internal/config/config.go Added EnableMetricViewMetadata field to UserConfig with DSN parsing support
connector.go Implemented session configuration injection and WithEnableMetricViewMetadata option
connector_test.go Added tests for enabled and default disabled states

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

gopalldb
gopalldb previously approved these changes Oct 23, 2025
Copy link
Collaborator

@gopalldb gopalldb left a comment

Choose a reason for hiding this comment

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

Check suggestions from copilot

Address PR review comments by simplifying the session configuration logic.
Always create a new map and copy existing params regardless of whether
metric view metadata is enabled.

This makes the code cleaner and more consistent:
- No nil check needed (ranging over nil map is safe in Go)
- Consistent behavior in all cases
- More readable code

Resolves reviewer comments from @vikrantpuppala, @copilot-pull-request-reviewer, and @gopalldb

Signed-off-by: Shivam Raj <[email protected]>
@shivam2680 shivam2680 dismissed stale reviews from gopalldb and vikrantpuppala via 3edd436 October 24, 2025 11:50
@shivam2680 shivam2680 merged commit d0ebd21 into main Oct 24, 2025
2 of 3 checks passed
@shivam2680 shivam2680 deleted the metric-view-metadata-support branch October 24, 2025 16:46
dgiagio pushed a commit to grafana/databricks-sql-go that referenced this pull request Nov 4, 2025
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.

4 participants