Skip to content

Conversation

@iseki0
Copy link

@iseki0 iseki0 commented Nov 20, 2025

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

Since some bugly database is popular, everyone have to workaround to it.

User Case Description

I have a library https://github.com/iseki0/goda, I don't think it's a better choice to use the GormValuer interface directly, it will make a strong dependency to gorm, and it's unwise since not everyone use gorm. So please give me a chance to register the handler dynamiclly, if someone need, they can register the gorm-enhance-module by the effect import statments, just time time/tzdata.

About the workaround

The MySQL has a very strange behavior:

prepare aa from 'select cast(? as time), ?';
set @a '09:00:00';
execute aa using @a, @a;
+-----------------+----------+
| cast(? as time) | ?        |
+-----------------+----------+
| 00:00:00        | 09:00:00 |
+-----------------+----------+

The problem is on MySQL server, not driver. The Go MySQL driver do paratermized query like the prepare command, I refer to the https://github.com/go-sql-driver/mysql.

The solution is:

prepare aa from 'select cast(cast(? as char) as time)';

Do you have any suggestions? Maybe we should fix it in the MySQL dialect. 毕竟,又不是只有我会倒这个霉,所有传字符串的都会完蛋。

@propel-code-bot
Copy link
Contributor

Dynamic runtime registration of custom GORM valuers

Adds a lightweight mechanism that lets external packages register custom GORM valuers at init time and have them picked up automatically by the core Statement.AddVar() logic. This avoids having third-party libraries depend directly on the Valuer interface while still enabling driver/dialect-specific work-arounds (e.g., MySQL TIME cast issue). The change touches only two files and introduces 23 LoC.

Key Changes

• Created dynamicRegisteredGormValuer map and helper functions RegisterDynamicGormValuerInit() and GetDynamicGormValuer() in valuer.go
• Hooked lookup if valuer := GetDynamicGormValuer(reflect.TypeOf(v)) into statement.go inside AddVar() to translate user values before default handling

Affected Areas

gorm/valuer.go (new infrastructure)
gorm/statement.go (value binding path)

This summary was automatically generated by @propel-code-bot

Comment on lines +10 to +14
var dynamicRegisteredGormValuer = make(map[reflect.Type]func(context.Context, *DB, any) clause.Expr)

// RegisterDynamicGormValuerInit shouldn't be called outside the init function
func RegisterDynamicGormValuerInit(valueType reflect.Type, valuer func(context.Context, *DB, any) clause.Expr) {
dynamicRegisteredGormValuer[valueType] = valuer
Copy link
Contributor

Choose a reason for hiding this comment

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

[CriticalError]

Race condition: The global map dynamicRegisteredGormValuer is written to without synchronization. If multiple packages call RegisterDynamicGormValuerInit concurrently during init() (which can happen when Go initializes packages in parallel), this will cause a data race. While the comment says "shouldn't be called outside the init function", Go doesn't guarantee sequential execution of different packages' init() functions.

Fix: Use sync.Map or protect with sync.RWMutex:

var (
    dynamicRegisteredGormValuer = make(map[reflect.Type]func(context.Context, *DB, any) clause.Expr)
    dynamicValuerMutex sync.RWMutex
)

func RegisterDynamicGormValuerInit(valueType reflect.Type, valuer func(context.Context, *DB, any) clause.Expr) {
    dynamicValuerMutex.Lock()
    defer dynamicValuerMutex.Unlock()
    dynamicRegisteredGormValuer[valueType] = valuer
}

func GetDynamicGormValuer(valueType reflect.Type) func(context.Context, *DB, any) clause.Expr {
    dynamicValuerMutex.RLock()
    defer dynamicValuerMutex.RUnlock()
    return dynamicRegisteredGormValuer[valueType]
}
Context for Agents
**Race condition**: The global map `dynamicRegisteredGormValuer` is written to without synchronization. If multiple packages call `RegisterDynamicGormValuerInit` concurrently during `init()` (which can happen when Go initializes packages in parallel), this will cause a data race. While the comment says "shouldn't be called outside the init function", Go doesn't guarantee sequential execution of different packages' `init()` functions.

**Fix**: Use `sync.Map` or protect with `sync.RWMutex`:

```go
var (
    dynamicRegisteredGormValuer = make(map[reflect.Type]func(context.Context, *DB, any) clause.Expr)
    dynamicValuerMutex sync.RWMutex
)

func RegisterDynamicGormValuerInit(valueType reflect.Type, valuer func(context.Context, *DB, any) clause.Expr) {
    dynamicValuerMutex.Lock()
    defer dynamicValuerMutex.Unlock()
    dynamicRegisteredGormValuer[valueType] = valuer
}

func GetDynamicGormValuer(valueType reflect.Type) func(context.Context, *DB, any) clause.Expr {
    dynamicValuerMutex.RLock()
    defer dynamicValuerMutex.RUnlock()
    return dynamicRegisteredGormValuer[valueType]
}
```

File: valuer.go
Line: 14

@iseki0
Copy link
Author

iseki0 commented Nov 20, 2025

@jinzhu Just a proposal, implementation is a simple demo... Please just ignore the AI reviews😁

@jinzhu
Copy link
Member

jinzhu commented Nov 22, 2025

I don't get what the PR for, maybe you can just use the database/sql's Valuer interface?

@iseki0
Copy link
Author

iseki0 commented Nov 22, 2025

I don't get what the PR for, maybe you can just use the database/sql's Valuer interface?

@jinzhu
The root cause is, on MySQL:

PREPARE aa FROM 'select cast(? as time), ?';
SET @a '09:00:00';
EXECUTE aa USING @a, @a;

The result is:

+-----------------+----------+
| cast(? as time) | ?        |
+-----------------+----------+
| 00:00:00        | 09:00:00 |
+-----------------+----------+

MySQL is bugly, I found a solution:

PREPARE aa FROM 'select cast(cast(? as char) as time)'

I don't know why MySQL is that exactly. Just workaround it,
I consider use your GormValuer interface, but it will make the library hard-dependent to gorm. It's not a good choice.

If the gorm allowing register Valuer dynamically, we can make it like import _ "time/tzdata".

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