google / google/adk-go

session/database: respect GORM NamingStrategy.TablePrefix for table names

Open
#538 1 comment 5 reactions 2 assignees Claimed by @kdroste-google View on GitHub
enhancement needs review
Dominant language
Go
Stars
8.8k
Forks
1k
Avg merge
3d 18h
Merged PRs (30d)
88

Description

** Please make sure you read the contribution guide and file the issues in the right place. **
[Contribution guide.](https://google.github.io/adk-docs/contributing-guide/)

## 🔴 Required Information

_Please ensure all items in this section are completed to allow for efficient
triaging. If an item is not applicable to you - please mark it as N/A_

### Is your feature request related to a specific problem?

Yes. I am unable to configure table names (specifically applying a `TablePrefix`) when using the `session/database` package with GORM.

My use case requires running multiple versions of an application (e.g., different versions of an agent) against the same Google Cloud Spanner instance. To avoid data collisions, I need to prefix table names (e.g., `v1_sessions`, `v2_sessions`).

However, the internal models (e.g., `storageSession`, `storageEvent`) appear to define a `TableName()` method or force a specific table name. This hardcoding overrides the `gorm.Config{NamingStrategy: ...}` settings passed into `NewSessionService`. Additionally, because `AutoMigrate` migrates these unexported structs directly, I cannot intervene to rename the tables during migration.

### Proposed Solution

I propose modifying the internal models in the `database` package to respect the GORM `NamingStrategy`.

**Preferred approach:**
Remove the `TableName()` method from the internal structs (`storageSession`, `storageEvent`, etc.). This would allow the `gorm.Config` passed into `NewSessionService` to control naming (including `TablePrefix`, `SingularTable`, etc.) naturally.

**Alternative approach:**
If hardcoded names are required for some internal reason, please expose a configuration option in `NewSessionService` or a separate functional option to set a table prefix explicitly.

### Impact on your work

This is a blocker for our multi-tenancy and versioning strategy. We are currently unable to deploy new versions of our services side-by-side in the same database instance without risking data corruption or collisions.

We are currently blocked from proceeding with our release pipeline until we can isolate these tables.

---

## 🟡 Recommended Information

### Alternatives Considered

1. **Wrapping the Dialector:** I attempted to wrap the `gorm.Dialector` to intercept `Register` callbacks and rewrite `db.Statement.Table` on the fly. This works for runtime queries but fails for `AutoMigrate`. `AutoMigrate` does not trigger standard query callbacks, meaning it still creates tables with the hardcoded names.
2. **Manual DDL:** Since the models (`storageSession`) are unexported, I cannot run migration manually on them. I would have to reverse-engineer the schema and write raw SQL DDL to create the prefixed tables, which is brittle and prone to breaking on library updates.

### Willingness to contribute

Yes, I am willing to submit a PR if given guidance on the preferred approach (e.g., whether to remove `TableName()` or add a configuration field).

### Proposed API / Implementation

If `TableName()` is removed from internal models, no API change is needed. The existing code would simply start respecting the user's config:

```go
// User code
dbService, _ := database.NewSessionService(dialector, &gorm.Config{
NamingStrategy: schema.NamingStrategy{
TablePrefix: "v2_", // This would now correctly result in "v2_sessions"
},
})

```

If `TableName()` cannot be removed, we could add an option:

```go
// Inside internal/database/models.go
func (s *storageSession) TableName() string {
// Ideally dynamic based on some injected config
return s.config.Prefix + "sessions"
}

```

### Additional Context

Currently using the Spanner dialector. The issue seems to stem from the precedence GORM places on the `TableName()` interface over `NamingStrategy` config. Since we cannot access the unexported structs to override this method, we are locked into the default names.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.