fix(oracle): enable DML operations and resolve incorrect array type error (googleapis#2323)

This pull request resolves an issue where executing DML statements
(UPDATE, INSERT, DELETE) caused "incorrect array type" errors.
Previously, the Oracle source forced all operations to use QueryContext,
expecting rows to be returned. This caused failures when running write
operations that do not return rows.

**Changes Implemented:**
The modification updates the Oracle source and tool definitions to
distinguish between Read-Only (SELECT) and Action (DML) operations.

- **internal/sources/oracle/oracle.go:**

1. Updated RunSQL signature to accept a readOnly boolean.
2. Added conditional logic: If readOnly is false, it now uses
ExecContext and returns rows_affected instead of attempting to scan
non-existent rows.

- **internal/tools/oracle/oraclesql/oraclesql.go:**

1. Added a readonly field to the tool configuration (YAML).
2. Updated the Invoke method to pass this flag to the source. Defaults
to true (Read-Only) for backward compatibility.


🛠️ Fixes googleapis#2026

---------

Co-authored-by: Wenxin Du <117315983+duwenxin99@users.noreply.github.com>

Author: ishatilwani1301

docs/en/resources/tools/oracle/oracle-sql.md

@@ -19,6 +19,10 @@ The specified SQL statement is executed using [prepared statements][oracle-stmt]
 for security and performance. It expects parameter placeholders in the SQL query
 to be in the native Oracle format (e.g., `:1`, `:2`).
 
+By default, tools are configured as **read-only** (SAFE mode). To execute data modification 
+statements (INSERT, UPDATE, DELETE), you must explicitly set the `readOnly` 
+field to `false`.
+
 [oracle-stmt]: https://docs.oracle.com/javase/tutorial/jdbc/basics/prepared.html
 
 ## Example
@@ -29,29 +33,46 @@ to be in the native Oracle format (e.g., `:1`, `:2`).
 > names, or other parts of the query.
 
 ```yaml
-kind: tools
-name: search_flights_by_number
-type: oracle-sql
-source: my-oracle-instance
-statement: |
-  SELECT * FROM flights
-  WHERE airline = :1
-  AND flight_number = :2
-  FETCH FIRST 10 ROWS ONLY
-description: |
-  Use this tool to get information for a specific flight.
-  Takes an airline code and flight number and returns info on the flight.
-  Do NOT use this tool with a flight id. Do NOT guess an airline code or flight number.
-  Example:
-  {{
-      "airline": "CY",
-      "flight_number": "888",
-  }}
-parameters:
-  - name: airline
-    type: string
-    description: Airline unique 2 letter identifier
-  - name: flight_number
-    type: string
-    description: 1 to 4 digit number
-```
+tools:
+  search_flights_by_number:
+    kind: oracle-sql
+    source: my-oracle-instance
+    statement: |
+      SELECT * FROM flights
+      WHERE airline = :1
+      AND flight_number = :2
+      FETCH FIRST 10 ROWS ONLY
+    description: |
+      Use this tool to get information for a specific flight.
+      Takes an airline code and flight number and returns info on the flight.
+      Do NOT use this tool with a flight id. Do NOT guess an airline code or flight number.
+      Example:
+      {{
+          "airline": "CY",
+          "flight_number": "888",
+      }}
+    parameters:
+      - name: airline
+        type: string
+        description: Airline unique 2 letter identifier
+      - name: flight_number
+        type: string
+        description: 1 to 4 digit number
+
+  update_flight_status:
+    kind: oracle-sql
+    source: my-oracle-instance
+    readOnly: false  # Required for INSERT/UPDATE/DELETE
+    statement: |
+      UPDATE flights 
+      SET status = :1 
+      WHERE airline = :2 AND flight_number = :3
+    description: Updates the status of a specific flight.
+    parameters:
+      - name: status
+        type: string
+      - name: airline
+        type: string
+      - name: flight_number
+        type: string
+

internal/sources/oracle/oracle.go

@@ -136,7 +136,23 @@ func (s *Source) OracleDB() *sql.DB {
 	return s.DB
 }
 
-func (s *Source) RunSQL(ctx context.Context, statement string, params []any) (any, error) {
+func (s *Source) RunSQL(ctx context.Context, statement string, params []any, readOnly bool) (any, error) {
+	if !readOnly {
+		result, err := s.OracleDB().ExecContext(ctx, statement, params...)
+		if err != nil {
+			return nil, fmt.Errorf("unable to execute DML statement: %w", err)
+		}
+
+		rowsAffected, err := result.RowsAffected()
+		if err != nil {
+			return nil, fmt.Errorf("unable to get rows affected: %w", err)
+		}
+
+		return map[string]any{
+			"status":        "success",
+			"rows_affected": rowsAffected,
+		}, nil
+	}
 	rows, err := s.OracleDB().QueryContext(ctx, statement, params...)
 	if err != nil {
 		return nil, fmt.Errorf("unable to execute query: %w", err)

internal/sources/oracle/oracle_test.go

@@ -4,6 +4,7 @@ package oracle_test
 
 import (
 	"context"
+	"database/sql"
 	"strings"
 	"testing"
 
@@ -191,3 +192,36 @@ func TestFailParseFromYaml(t *testing.T) {
 		})
 	}
 }
+
+// TestRunSQLExecutesDML verifies that RunSQL correctly routes operations to
+// ExecContext instead of QueryContext when the readOnly flag is set to false.
+func TestRunSQLExecutesDML(t *testing.T) {
+	// Initialize a mock database connection.
+	// This connection is not established with a real backend but
+	// satisfies the interface requirements for the test.
+	db, err := sql.Open("oracle", "oracle://user:pass@localhost:1521/service")
+	if err != nil {
+		t.Fatalf("failed to open mock db: %v", err)
+	}
+	defer db.Close()
+
+	src := &oracle.Source{
+		Config: oracle.Config{
+			Name: "test-dml-source",
+			Type: oracle.SourceType,
+			User: "test-user",
+		},
+		DB: db,
+	}
+
+	// Invoke RunSQL with readOnly=false to force the DML execution path.
+	_, err = src.RunSQL(context.Background(),
+		"UPDATE users SET email='x' WHERE id=1", nil, false)
+
+	// We expect an error because the mock database cannot execute the query.
+	// If err is nil, it implies the logic skipped the execution block.
+	if err == nil {
+		t.Fatal("expected error from fake DB execution, but got nil; " +
+			"DML path may not have been executed")
+	}
+}

internal/tools/oracle/oraclesql/oraclesql.go

@@ -34,7 +34,7 @@ func newConfig(ctx context.Context, name string, decoder *yaml.Decoder) (tools.T
 
 type compatibleSource interface {
 	OracleDB() *sql.DB
-	RunSQL(context.Context, string, []any) (any, error)
+	RunSQL(context.Context, string, []any, bool) (any, error)
 }
 
 type Config struct {
@@ -43,6 +43,7 @@ type Config struct {
 	Source             string                `yaml:"source" validate:"required"`
 	Description        string                `yaml:"description" validate:"required"`
 	Statement          string                `yaml:"statement" validate:"required"`
+	ReadOnly           *bool                 `yaml:"readOnly"`
 	AuthRequired       []string              `yaml:"authRequired"`
 	Parameters         parameters.Parameters `yaml:"parameters"`
 	TemplateParameters parameters.Parameters `yaml:"templateParameters"`
@@ -105,7 +106,14 @@ func (t Tool) Invoke(ctx context.Context, resourceMgr tools.SourceProvider, para
 		fmt.Printf("[%d]=%T ", i, p)
 	}
 	fmt.Printf("\n")
-	resp, err := source.RunSQL(ctx, newStatement, sliceParams)
+
+	isReadOnly := true
+	if t.ReadOnly != nil {
+		isReadOnly = *t.ReadOnly
+	}
+
+	resp, err := source.RunSQL(ctx, newStatement, sliceParams, isReadOnly)
+
 	if err != nil {
 		return nil, util.ProcessGeneralError(err)
 	}

internal/tools/oracle/oraclesql/oraclesql_test.go

@@ -15,6 +15,10 @@ func TestParseFromYamlOracleSql(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
+
+	valTrue := true
+	valFalse := false
+
 	tcs := []struct {
 		desc string
 		in   string
@@ -39,6 +43,7 @@ func TestParseFromYamlOracleSql(t *testing.T) {
 					Source:       "my-oracle-instance",
 					Description:  "Retrieves user details by ID.",
 					Statement:    "SELECT id, name, email FROM users WHERE id = :1",
+					ReadOnly:     nil,
 					AuthRequired: []string{"my-google-auth-service"},
 				},
 			},
@@ -60,6 +65,53 @@ func TestParseFromYamlOracleSql(t *testing.T) {
 					Source:       "db-prod",
 					Description:  "Gets orders for a customer with optional filtering.",
 					Statement:    "SELECT * FROM ${SCHEMA}.ORDERS WHERE customer_id = :customer_id AND status = :status",
+					ReadOnly:     nil,
+					AuthRequired: []string{},
+				},
+			},
+		},
+		{
+			desc: "explicit: readOnly set to true",
+			in: `
+			kind: tools
+			name: safe_query
+			type: oracle-sql
+			source: db-prod
+			description: Safe read operation.
+			readOnly: true
+			statement: "SELECT * FROM orders"
+			`,
+			want: server.ToolConfigs{
+				"safe_query": oraclesql.Config{
+					Name:         "safe_query",
+					Type:         "oracle-sql",
+					Source:       "db-prod",
+					Description:  "Safe read operation.",
+					Statement:    "SELECT * FROM orders",
+					ReadOnly:     &valTrue,
+					AuthRequired: []string{},
+				},
+			},
+		},
+		{
+			desc: "example with readonly flag set to false (DML)",
+			in: `
+			kind: tools
+			name: update_user
+			type: oracle-sql
+			source: db-prod
+			description: Updates user email.
+			readOnly: false
+			statement: "UPDATE users SET email = :1 WHERE id = :2"
+			`,
+			want: server.ToolConfigs{
+				"update_user": oraclesql.Config{
+					Name:         "update_user",
+					Type:         "oracle-sql",
+					Source:       "db-prod",
+					Description:  "Updates user email.",
+					Statement:    "UPDATE users SET email = :1 WHERE id = :2",
+					ReadOnly:     &valFalse,
 					AuthRequired: []string{},
 				},
 			},

tests/oracle/oracle_integration_test.go

@@ -3,9 +3,12 @@
 package oracle
 
 import (
+	"bytes"
 	"context"
 	"database/sql"
 	"fmt"
+	"io"
+	"net/http"
 	"os"
 	"regexp"
 	"strings"
@@ -103,6 +106,34 @@ func TestOracleSimpleToolEndpoints(t *testing.T) {
 	tmplSelectCombined, tmplSelectFilterCombined := tests.GetMySQLTmplToolStatement()
 	toolsFile = tests.AddTemplateParamConfig(t, toolsFile, OracleToolType, tmplSelectCombined, tmplSelectFilterCombined, "")
 
+	// Configure a DML tool to verify the 'readonly: false' logic.
+	updateStmt := fmt.Sprintf(`UPDATE %s SET "name" = :1 WHERE "id" = :2`, tableNameParam)
+
+	toolsMap, ok := toolsFile["tools"].(map[string]any)
+	if !ok {
+		t.Fatal("Configuration error: 'tools' key is missing or is not a map")
+	}
+
+	toolsMap["my-update-tool"] = map[string]any{
+		"type":        "oracle-sql",
+		"source":      "my-instance",
+		"statement":   updateStmt,
+		"readOnly":    false,
+		"description": "Update user name by ID.",
+		"parameters": []map[string]any{
+			{
+				"name":        "name",
+				"type":        "string",
+				"description": "The new name for the user.",
+			},
+			{
+				"name":        "id",
+				"type":        "integer",
+				"description": "The user ID to update.",
+			},
+		},
+	}
+
 	cmd, cleanup, err := tests.StartCmd(ctx, toolsFile, args...)
 	if err != nil {
 		t.Fatalf("command initialization returned an error: %s", err)
@@ -119,7 +150,7 @@ func TestOracleSimpleToolEndpoints(t *testing.T) {
 
 	// Get configs for tests
 	select1Want := "[{\"1\":1}]"
-	mcpMyFailToolWant := `{"jsonrpc":"2.0","id":"invoke-fail-tool","result":{"content":[{"type":"text","text":"error processing request: unable to execute query: dpiStmt_execute: ORA-00900: invalid SQL statement"}],"isError":true}}`
+	mcpMyFailToolWant := `{"jsonrpc":"2.0","id":"invoke-fail-tool","result":{"content":[{"type":"text","text":"unable to execute query: dpiStmt_execute: ORA-00900: invalid SQL statement\nHelp: https://docs.oracle.com/error-help/db/ora-00900/"}],"isError":true}}`
 	createTableStatement := `"CREATE TABLE t (id NUMBER GENERATED AS IDENTITY PRIMARY KEY, name VARCHAR2(255))"`
 	mcpSelect1Want := `{"jsonrpc":"2.0","id":"invoke my-auth-required-tool","result":{"content":[{"type":"text","text":"{\"1\":1}"}]}}`
 
@@ -133,6 +164,11 @@ func TestOracleSimpleToolEndpoints(t *testing.T) {
 	tests.RunMCPToolCallMethod(t, mcpMyFailToolWant, mcpSelect1Want)
 	tests.RunExecuteSqlToolInvokeTest(t, createTableStatement, select1Want)
 	tests.RunToolInvokeWithTemplateParameters(t, tableNameTemplateParam)
+
+	// Invoke the 'my-update-tool' and verify the result.
+	testDmlQueries(t, "my-update-tool",
+		`{"name": "UpdatedAlice", "id": 1}`,
+		`\"rows_affected\":1`)
 }
 
 func setupOracleTable(t *testing.T, ctx context.Context, pool *sql.DB, createStatement, insertStatement, tableName string, params []any) func(*testing.T) {
@@ -242,3 +278,47 @@ func dropAllUserTables(t *testing.T, ctx context.Context, db *sql.DB) {
 		}
 	}
 }
+
+// testDmlQueries sends a JSON-RPC request to the running test server
+// and asserts that the response body contains the expected substring.
+func testDmlQueries(t *testing.T, toolName, paramsJSON, wantResponseSubStr string) {
+	t.Helper()
+
+	// The test server typically runs on port 8080
+	url := "http://localhost:5000/mcp"
+
+	// Construct the JSON-RPC request body
+	reqBody := fmt.Sprintf(`{
+		"jsonrpc": "2.0", 
+		"method": "tools/call", 
+		"params": {
+			"name": "%s", 
+			"arguments": %s
+		}, 
+		"id": 1
+	}`, toolName, paramsJSON)
+
+	req, err := http.NewRequest("POST", url, bytes.NewBuffer([]byte(reqBody)))
+	if err != nil {
+		t.Fatalf("Failed to create HTTP request: %v", err)
+	}
+	req.Header.Set("Content-Type", "application/json")
+
+	client := &http.Client{Timeout: 5 * time.Second}
+	resp, err := client.Do(req)
+	if err != nil {
+		t.Fatalf("Failed to send request to %s: %v", url, err)
+	}
+	defer resp.Body.Close()
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		t.Fatalf("Failed to read response body: %v", err)
+	}
+
+	// Verify that the response contains the expected success message (e.g., "rows_affected")
+	if !strings.Contains(string(body), wantResponseSubStr) {
+		t.Errorf("Tool invocation '%s' failed.\nGot response: %s\nExpected substring: %s",
+			toolName, string(body), wantResponseSubStr)
+	}
+}