[datastore] impl Selectable and Queryable for Saga#10866
Open
karencfv wants to merge 4 commits into
Open
Conversation
This was referenced Jul 20, 2026
karencfv
commented
Jul 21, 2026
Comment on lines
+510
to
+597
| // The `saga` table's columns, in the order `SagaRow` declares them. Named via | ||
| // the public schema so `Saga`/`LoadedSaga` can use them as their | ||
| // `Selectable::SelectExpression`. We do this to avoid making `SagaRow` public. | ||
| type SagaColumns = ( | ||
| saga::id, | ||
| saga::creator, | ||
| saga::time_created, | ||
| saga::name, | ||
| saga::saga_dag, | ||
| saga::saga_state, | ||
| saga::current_sec, | ||
| saga::adopt_generation, | ||
| saga::adopt_time, | ||
| saga::abandon_time, | ||
| saga::abandon_reason, | ||
| saga::abandon_comment, | ||
| ); | ||
|
|
||
| fn saga_columns() -> SagaColumns { | ||
| ( | ||
| saga::id, | ||
| saga::creator, | ||
| saga::time_created, | ||
| saga::name, | ||
| saga::saga_dag, | ||
| saga::saga_state, | ||
| saga::current_sec, | ||
| saga::adopt_generation, | ||
| saga::adopt_time, | ||
| saga::abandon_time, | ||
| saga::abandon_reason, | ||
| saga::abandon_comment, | ||
| ) | ||
| } | ||
|
|
||
| // The Rust-side tuple that [`SagaColumns`] deserializes into, mirroring | ||
| // `SagaRow`'s fields. Used as `Queryable::Row` for `Saga` and `LoadedSaga`. | ||
| // Spelled out in public types so it doesn't leak the crate-private `SagaRow`. | ||
| type SagaRowColumns = ( | ||
| SagaId, | ||
| SecId, | ||
| DateTime<Utc>, | ||
| String, | ||
| serde_json::Value, | ||
| SagaState, | ||
| Option<SecId>, | ||
| super::Generation, | ||
| DateTime<Utc>, | ||
| Option<DateTime<Utc>>, | ||
| Option<SagaReasonAbandoned>, | ||
| Option<String>, | ||
| ); | ||
|
|
||
| impl SagaRow { | ||
| // Reassemble a raw row from the loaded column tuple, so the (private) | ||
| // validation logic in `TryFrom<SagaRow>` can be reused by the `Queryable` | ||
| // impls without exposing `SagaRow` in any signature. | ||
| fn from_columns(columns: SagaRowColumns) -> Self { | ||
| let ( | ||
| id, | ||
| creator, | ||
| time_created, | ||
| name, | ||
| saga_dag, | ||
| saga_state, | ||
| current_sec, | ||
| adopt_generation, | ||
| adopt_time, | ||
| abandon_time, | ||
| abandon_reason, | ||
| abandon_comment, | ||
| ) = columns; | ||
| SagaRow { | ||
| id, | ||
| creator, | ||
| time_created, | ||
| name, | ||
| saga_dag, | ||
| saga_state, | ||
| current_sec, | ||
| adopt_generation, | ||
| adopt_time, | ||
| abandon_time, | ||
| abandon_reason, | ||
| abandon_comment, | ||
| } | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
I am not 100% on this approach as it feels super unnatural to me. It was either this, or make SagaRow public, which kind of defeats the whole purpose of this PR. Sadly, Diesel doesn't make life easy. Claude and I worked hard on this specific representation, and this is the best we could come up with. If someone has a better idea I would honestly love to hear it
Contributor
Author
|
Asking reviews from @sunshowers because you're great at libraries, and @smklein because it seems you'll bear the brunt of my decisions here. I honestly don't know if either of you are Diesel experts, sorry 😄 If there's someone else I should ask for a review please let me know! |
karencfv
marked this pull request as ready for review
July 21, 2026 06:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes a few changes. It modifies the
Sagatype to make invalid saga states unrepresentable, and makes the raw DB row typeSagaRowcrate-private to make it impossible to attempt to insert invalid state/abandon-metadata combinations. There were already two CHECK constraints to enforce this at the DB level, but it's a better experience to have these at the API level as well.Additionally, there is a new
LoadedSagatype which makes it possible to read invalid saga rows. This is necessary so we can skip invalid rows from a batch rather than failing at the first invalid row. This is something we do in saga recovery and other places.Closes: #10849