add mode to DuckDB driver

rilldata · grahamplata · Aug 5, 2025 · Aug 5, 2025 · Aug 6, 2025 · Aug 7, 2025
commit e368d6512e0d51b4d96531cdb28a1a6179fffde2
diff --git a/runtime/drivers/duckdb/config.go b/runtime/drivers/duckdb/config.go
@@ -10,6 +10,9 @@ import (
 const (
 	poolSizeMin int = 2
 	poolSizeMax int = 5
+
+	modeReadOnly  = "read"
+	modeReadWrite = "readwrite"
 )
 
 // config represents the DuckDB driver config
@@ -35,6 +38,8 @@ type config struct {
 	// Secrets is a comma-separated list of connector names to create temporary secrets for before executing models.
 	// The secrets are not created for read queries.
 	Secrets string `mapstructure:"secrets"`
+	// Mode specifies the mode in which to open the database. It can be "read" (default) or "readwrite".
+	Mode string `mapstructure:"mode"`
 
 	// Path switches the implementation to use a generic rduckdb implementation backed by the db used in the Path
 	Path string `mapstructure:"path"`
@@ -55,6 +60,11 @@ func newConfig(cfgMap map[string]any) (*config, error) {
 		return nil, fmt.Errorf("could not decode config: %w", err)
 	}
 
+	// Validate mode if specified
+	if cfg.Mode != "" && cfg.Mode != modeReadOnly && cfg.Mode != modeReadWrite {
+		return nil, fmt.Errorf("invalid mode '%s': must be 'read' or 'readwrite'", cfg.Mode)
+	}
+
 	// Set pool size
 	poolSize := cfg.PoolSize
 	if poolSize == 0 && cfg.CPU != 0 {

diff --git a/runtime/drivers/duckdb/config_test.go b/runtime/drivers/duckdb/config_test.go
@@ -51,3 +51,44 @@ func Test_specialCharInPath(t *testing.T) {
 	require.NoError(t, res.Close())
 	require.NoError(t, conn.Close())
 }
+
+func TestModeConfigValidation(t *testing.T) {
+	tests := []struct {
+		name        string
+		config      map[string]any
+		expectError bool
+	}{
+		{
+			name:        "valid read mode",
+			config:      map[string]any{"mode": modeReadOnly},
+			expectError: false,
+		},
+		{
+			name:        "valid readwrite mode",
+			config:      map[string]any{"mode": modeReadWrite},
+			expectError: false,
+		},
+		{
+			name:        "empty mode is valid",
+			config:      map[string]any{},
+			expectError: false,
+		},
+		{
+			name:        "invalid mode should fail",
+			config:      map[string]any{"mode": "invalid"},
+			expectError: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := newConfig(tt.config)
+			if tt.expectError {
+				require.Error(t, err)
+				require.Contains(t, err.Error(), "invalid mode")
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
diff --git a/runtime/drivers/duckdb/duckdb.go b/runtime/drivers/duckdb/duckdb.go
@@ -48,6 +48,14 @@ var spec = drivers.Spec{
 			Description: "Path to external DuckDB database.",
 			Placeholder: "/path/to/main.db",
 		},
+		{
+			Key:         "mode",
+			Type:        drivers.StringPropertyType,
+			Required:    false,
+			DisplayName: "Mode",
+			Description: "Set the mode for the DuckDB connection. By default, it is set to 'read' which allows only read operations. Set to 'readwrite' to enable model creation and table mutations.",
+			NoPrompt:    true,
+		},
 	},
 	// Important: Any edits to the below properties must be accompanied by changes to the client-side form validation schemas.
 	SourceProperties: []*drivers.PropertySpec{
@@ -127,6 +135,16 @@ var motherduckSpec = drivers.Spec{
 			Placeholder: "my_new_source",
 			Required:    true,
 		},
+		{
+			Key:         "mode",
+			Type:        drivers.StringPropertyType,
+			Required:    false,
+			DisplayName: "Mode",
+			Description: "Set the mode for the DuckDB connection. By default, it is set to 'read' which allows only read operations. Set to 'readwrite' to enable model creation and table mutations.",
+			Placeholder: modeReadOnly,
+			Default:     modeReadOnly,
+			NoPrompt:    true,
+		},
 	},
 }
 
@@ -155,6 +173,18 @@ func (d Driver) Open(instanceID string, cfgMap map[string]any, st *storage.Clien
 		olapSemSize = 1
 	}
 
+	// Set the mode for the connection
+	if cfg.Mode == "" {
+		// The default mode depends on the connection type:
+		// - For generic connections (Motherduck/DuckLake with Path/Attach), default to "read"
+		// - For connections using the embedded DuckDB, default to "readwrite" to maintain compatibility
+		if cfg.Path != "" || cfg.Attach != "" {
+			cfg.Mode = modeReadOnly
+		} else {
+			cfg.Mode = modeReadWrite
+		}
+	}
+
 	ctx, cancel := context.WithCancel(context.Background())
 	c := &connection{
 		instanceID:     instanceID,
@@ -384,6 +414,11 @@ func (c *connection) AsObjectStore() (drivers.ObjectStore, bool) {
 
 // AsModelExecutor implements drivers.Handle.
 func (c *connection) AsModelExecutor(instanceID string, opts *drivers.ModelExecutorOptions) (drivers.ModelExecutor, bool) {
+	if opts.OutputHandle == c && c.config.Mode != modeReadWrite {
+		c.logger.Warn("Model execution is disabled. To enable modeling on this DuckDB database, set 'mode: readwrite' in your connector configuration. WARNING: This will allow Rill to create and overwrite tables in your database.")
+		return nil, false
+	}
+
 	if opts.InputHandle == c && opts.OutputHandle == c {
 		return &selfToSelfExecutor{c}, true
 	}
@@ -422,6 +457,10 @@ func (c *connection) AsModelExecutor(instanceID string, opts *drivers.ModelExecu
 
 // AsModelManager implements drivers.Handle.
 func (c *connection) AsModelManager(instanceID string) (drivers.ModelManager, bool) {
+	if c.config.Mode != modeReadWrite {
+		c.logger.Warn("Model execution is disabled. To enable modeling on this DuckDB database, set 'mode: readwrite' in your connector configuration. WARNING: This will allow Rill to create and overwrite tables in your database.")
+		return nil, false
+	}
 	return c, true
 }
 

diff --git a/runtime/drivers/duckdb/duckdb_test.go b/runtime/drivers/duckdb/duckdb_test.go
@@ -179,3 +179,139 @@ func TestNoFatalErrConcurrent(t *testing.T) {
 	err = handle.Close()
 	require.NoError(t, err)
 }
+
+func TestDuckDBModeDefaults(t *testing.T) {
+	tests := []struct {
+		name         string
+		config       map[string]any
+		expectedMode string
+	}{
+		{
+			name:         "embedded duckdb defaults to readwrite",
+			config:       map[string]any{"dsn": ":memory:"},
+			expectedMode: modeReadWrite,
+		},
+		{
+			name:         "external duckdb with path defaults to read",
+			config:       map[string]any{"path": "/tmp/test.db"},
+			expectedMode: modeReadOnly,
+		},
+		{
+			name:         "external duckdb with attach defaults to read",
+			config:       map[string]any{"attach": "'/path/to/db.db' AS external"},
+			expectedMode: modeReadOnly,
+		},
+		{
+			name:         "explicit readwrite mode",
+			config:       map[string]any{"dsn": ":memory:", "mode": "readwrite"},
+			expectedMode: modeReadWrite,
+		},
+		{
+			name:         "explicit read mode",
+			config:       map[string]any{"dsn": ":memory:", "mode": "read"},
+			expectedMode: modeReadOnly,
+		},
+		{
+			name:         "explicit readwrite mode overrides path default",
+			config:       map[string]any{"path": "/tmp/test.db", "mode": "readwrite"},
+			expectedMode: modeReadWrite,
+		},
+		{
+			name:         "motherduck defaults to readonly",
+			config:       map[string]any{"path": "md:my_db"},
+			expectedMode: modeReadOnly,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg, err := newConfig(tt.config)
+			require.NoError(t, err)
+
+			// Apply default mode logic
+			if cfg.Mode == "" {
+				if cfg.Path != "" || cfg.Attach != "" {
+					cfg.Mode = modeReadOnly
+				} else {
+					cfg.Mode = modeReadWrite
+				}
+			}
+
+			require.Equal(t, tt.expectedMode, cfg.Mode)
+		})
+	}
+}
+
+func TestDuckDBModeEnforcement(t *testing.T) {
+	ctx := context.Background()
+
+	t.Run("read mode blocks model execution", func(t *testing.T) {
+		handle, err := drivers.Open("duckdb", "test", map[string]any{
+			"dsn":  ":memory:",
+			"mode": "read",
+		}, storage.MustNew(t.TempDir(), nil), activity.NewNoopClient(), zap.NewNop())
+		require.NoError(t, err)
+		defer handle.Close()
+
+		// Test AsModelExecutor
+		opts := &drivers.ModelExecutorOptions{
+			InputHandle:  handle,
+			OutputHandle: handle,
+		}
+		executor, ok := handle.AsModelExecutor("test", opts)
+		require.False(t, ok)
+		require.Nil(t, executor)
+
+		// Test AsModelManager
+		manager, ok := handle.AsModelManager("test")
+		require.False(t, ok)
+		require.Nil(t, manager)
+	})
+
+	t.Run("readwrite mode allows model execution", func(t *testing.T) {
+		handle, err := drivers.Open("duckdb", "test", map[string]any{
+			"dsn":  ":memory:",
+			"mode": "readwrite",
+		}, storage.MustNew(t.TempDir(), nil), activity.NewNoopClient(), zap.NewNop())
+		require.NoError(t, err)
+		defer handle.Close()
+
+		// Test AsModelExecutor
+		opts := &drivers.ModelExecutorOptions{
+			InputHandle:  handle,
+			OutputHandle: handle,
+		}
+		executor, ok := handle.AsModelExecutor("test", opts)
+		require.True(t, ok)
+		require.NotNil(t, executor)
+
+		// Test AsModelManager
+		manager, ok := handle.AsModelManager("test")
+		require.True(t, ok)
+		require.NotNil(t, manager)
+	})
+
+	t.Run("read mode allows reading", func(t *testing.T) {
+		handle, err := drivers.Open("duckdb", "test", map[string]any{
+			"dsn":  ":memory:",
+			"mode": "read",
+		}, storage.MustNew(t.TempDir(), nil), activity.NewNoopClient(), zap.NewNop())
+		require.NoError(t, err)
+		defer handle.Close()
+
+		// Should still allow OLAP queries
+		olap, ok := handle.AsOLAP("test")
+		require.True(t, ok)
+		require.NotNil(t, olap)
+
+		// Test a simple query
+		res, err := olap.Query(ctx, &drivers.Statement{Query: "SELECT 1 as test"})
+		require.NoError(t, err)
+		require.True(t, res.Next())
+		var value int
+		err = res.Scan(&value)
+		require.NoError(t, err)
+		require.Equal(t, 1, value)
+		require.NoError(t, res.Close())
+	})
+}