go-gorm / gorm

The fantastic ORM library for Golang, aims to be developer friendly
https://gorm.io
MIT License
36.93k stars 3.93k forks source link

Rationale for the DryRun guard on callbacks.RowQuery #6222

Open emetsger opened 1 year ago

emetsger commented 1 year ago

Your Question

TL;DR: can you tell me why callbacks.RowQuery is not supported in dry run mode? Is there a possibility (I'm happy to contribute a PR) that it could support dry run mode?

Use case

My use case is to use DryRun to capture table creation DDL with Migrator running in dry mode. A promising approach has been to implement a custom logger to capture the generated SQL to an io sink (I've also tried wrapping a Dialector) but otherwise leave the target database's state alone (i.e. dry mode).

Blocker

However, this approach seems incompatible with callbacks.RowQuery, which guards the write to Statement.Dest with a if !db.DryRun block†. The presence of this guard implies that callbacks.RowQuery is generally incompatible with dry mode, and I'm wondering if you can tell me why that is? I just don't have enough experience with Gorm to reason about why that might be the case.

†(this actually causes a panic because the finisher_api's func (db *DB) Row() *sql.Row will return a nil *sql.Row instead of populating a *sql.Row with an error)

The document you expected this should be explained

This is a pretty low-level question, and might be my ignorance. So I don't expect it to be documented. For reference I did read the following:

Expected answer

That callbacks.RowQuery is compatible with dry run, you just need a PR and some tests :)

a631807682 commented 1 year ago

TL;DR: can you tell me why callbacks.RowQuery is not supported in dry run mode? Is there a possibility (I'm happy to contribute a PR) that it could support dry run mode?

Since Row does not return grom.DB, we do not support it, but in any case we should not panic.

https://gorm.io/docs/sql_builder.html#ToSQL https://gorm.io/docs/session.html#DryRun

When (*sql.Row).Scan has err, it returns err first. At present, I don't know if there is any way to handle it without changing the api.

github-actions[bot] commented 6 months ago

This issue has been automatically marked as stale because it has been open 360 days with no activity. Remove stale label or comment or this will be closed in 180 days