georgysavva / scany

Library for scanning data from a database into Go structs and more
MIT License
1.23k stars 67 forks source link

Feature request: NotSingular error type #125

Closed leejuyuu closed 4 months ago

leejuyuu commented 5 months ago

Is your feature request related to a problem? Please describe.

Currently, when ScanOne gets multiple rows, scany returns an error from fmt.Errorf: https://github.com/georgysavva/scany/blob/d60e58cb78e0af1de94f467e757821d6189e36f6/dbscan/dbscan.go#L257

This makes inferring the error cause require string comparison.

Describe the solution you'd like

Is it feasible to make this into a custom error type? This helps confirm the error cause to be getting multiple rows when expecting one.

Such as

type ErrNotSingular struct {
    RowsAffected int
}

func (e *ErrNotSingular) Error() string {
    return fmt.Sprintf("scany: expected 1 row, got: %d", e.RowsAffected)
}

func NotSingular(err error) bool {
    var e *ErrNotSingular
    return errors.As(err, &e)
}
georgysavva commented 5 months ago

That's an interesting use case. If your code expects this error to happen, it makes more sense to use ScanAll() and branch your logic based on the number of rows received. If your code calls ScanOne(), the situation when it returns more than one row is unexpected, and there is no benefit to inferring the error programmatically. But I might be missing something, and adding a distinct error type, as you suggest, seems like a good idea. Would you be interested in submitting a PR?

leejuyuu commented 5 months ago

I do not expect this error to happen. I have this question because I came from Ent ORM and it separates this error, so I made a NotSingular error code specifically for this. After reading your comment, I am starting to think that maybe this granularity is not necessary, it is an internal error anyway.

If you still think adding an error type will be helpful, I would love to make a PR.

georgysavva commented 4 months ago

Got it. If you don't have a strong use case for adding this error type then let's not create the PR. Let me know if anything changes.