Skip to content

feat(spanner): add spanner-update-database-schema tool - #3819

Open
anubhav756 wants to merge 1 commit into
anubhav-spanner-get-ddlfrom
anubhav-spanner-update-schema
Open

feat(spanner): add spanner-update-database-schema tool#3819
anubhav756 wants to merge 1 commit into
anubhav-spanner-get-ddlfrom
anubhav-spanner-update-schema

Conversation

@anubhav756

@anubhav756 anubhav756 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Description

This PR introduces the spanner-update-database-schema tool for executing DDL schema mutations (CREATE TABLE, ALTER TABLE, CREATE INDEX, etc.) on Cloud Spanner databases, along with automatic agent suppression on read-only sources.

Changes:

  • Implemented UpdateDatabaseDdl(ctx, statements, tokenString) on Spanner Source using DatabaseAdminClient.UpdateDatabaseDdl and waiting for long-running operation completion via op.Wait(ctx).
  • Created spanner-update-database-schema with destructive annotations (readOnlyHint: false, destructiveHint: true).
  • Enabled automatic tool suppression via tools.ShouldSuppress so the tool is hidden from MCP agents when connected to a read-only Spanner source.
  • Added update_database_schema and get_database_ddl to the data and data_with_discovery toolsets in spanner.yaml and spanner-postgres.yaml.
  • Updated config_test.go and unit tests to validate schema updating and suppression behavior.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the spanner-update-database-schema tool to execute DDL statements on Cloud Spanner databases, along with corresponding updates to prebuilt configurations, tests, and documentation. Feedback on the implementation highlights a potential issue where non-string or empty elements in the statements array are silently skipped, which could lead to partial migration execution; it is recommended to return an error instead.

Comment on lines +133 to +138
case []any:
for _, elem := range v {
if str, ok := elem.(string); ok && str != "" {
statements = append(statements, str)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Silently skipping non-string or empty elements in the statements array can lead to partial execution of a DDL migration, which can leave the database schema in an inconsistent or broken state. It is safer to return an error if any element in the array is not a valid, non-empty string.

	case []any:
		for _, elem := range v {
			str, ok := elem.(string)
			if !ok {
				return nil, util.NewAgentError(fmt.Sprintf("invalid element type in 'statements': expected string, got %T", elem), nil)
			}
			if str == "" {
				return nil, util.NewAgentError("empty statement is not allowed", nil)
			}
			statements = append(statements, str)
		}

@anubhav756
anubhav756 requested review from a team as code owners August 13, 2026 13:17
@anubhav756
anubhav756 force-pushed the anubhav-spanner-update-schema branch from 4b2939c to 79ccea2 Compare August 13, 2026 13:17
@anubhav756
anubhav756 force-pushed the anubhav-spanner-update-schema branch from 79ccea2 to 913c8a6 Compare August 13, 2026 15:49
@anubhav756
anubhav756 force-pushed the anubhav-spanner-update-schema branch from 913c8a6 to 851134e Compare August 13, 2026 15:50
@anubhav756
anubhav756 force-pushed the anubhav-spanner-update-schema branch from 851134e to 69ac557 Compare August 14, 2026 06:05
@anubhav756
anubhav756 force-pushed the anubhav-spanner-update-schema branch from 69ac557 to 78e17a7 Compare August 14, 2026 15:11
@anubhav756
anubhav756 force-pushed the anubhav-spanner-update-schema branch from 78e17a7 to c79c149 Compare August 17, 2026 05:45
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.

2 participants