search

jschaf / pggen

Generate type-safe Go for any Postgres query. If Postgres can run the query, pggen can generate code for it.
MIT License
282 stars 26 forks source link

proposal: panicing function on non-empty-row error #32

Closed aight8 closed 3 years ago

aight8 commented 3 years ago

I propose to generate a panicking alternative function. Panicking in go happens when something go really wrong and the program flow should be disrupted - this is mostly the case on any pg error - with one exception: no row error. The row instead is a pointer to a row, which is nil when no row is found.

Foo() // current query function
FooX() // panicking query function
// https://github.com/ent/ent has this methods
jschaf commented 3 years ago

program flow should be disrupted - this is mostly the case on any pg error

That's correct, but I think "program should crash" is more accurate in terms of what panicking means in practice. That's a pretty high bar for me. I think there's many pg errors which are non-permanent and retryable: lock timeouts come to mind.

If I understand correctly, if pggen returns a pointer type for the row, there's no need to panic on ErrNoRow because pggen can return a nil pointer and a nil error.

I'd like to avoid adding another method per query, pggen already has 3. I probably should have used another interface like BatchQuerier for the Scan and Batch methods.

Long term, I'd like to support use-cases like this but I think the way forward is with hooks or a plugin model.

aight8 commented 3 years ago

I'd like to avoid adding another method per query, pggen already has 3. I probably should have used another interface like BatchQuerier for the Scan and Batch methods.

good idea!

Long term, I'd like to support use-cases like this but I think the way forward is with hooks or a plugin model.

yes this could be a clean way. a callback function could decide to panic in some cases or generally. idea note: maybe it make sense if it would possible to tag queries (-- name: GetUser :one tag:yyy) which available in the callback

jschaf commented 3 years ago

I'm going to close for now. I have an idea for reducing the number of methods in https://github.com/jschaf/pggen/issues/33.

Custom code generation to support use cases like this is on the roadmap but don't hold your breath.