Conversation
There was a problem hiding this comment.
Pull request overview
Simplifies the radionuclides transfer by removing thing_id lookups/requirements and relying only on chemistry_sample_info_id for FK mapping.
Changes:
- Simplified the sample info cache to map legacy sample point UUID →
chemistry_sample_info_id. - Removed skip logic and upsert updates related to missing
thing_id. - Updated row construction to omit
thing_id.
| index_elements=["nma_GlobalID"], | ||
| set_={ | ||
| "thing_id": excluded.thing_id, | ||
| "chemistry_sample_info_id": excluded.chemistry_sample_info_id, |
There was a problem hiding this comment.
The upsert no longer sets thing_id. If NMA_Radionuclides.thing_id is a non-nullable column or is expected to stay in sync with the associated sample/thing, inserts may fail (NULL) and updates may leave stale values. Either (a) keep populating/updating thing_id, or (b) remove/relax the thing_id requirement at the schema/model level and ensure downstream code does not rely on it.
| "chemistry_sample_info_id": excluded.chemistry_sample_info_id, | |
| "chemistry_sample_info_id": excluded.chemistry_sample_info_id, | |
| "thing_id": excluded.thing_id, |
| return { | ||
| # Legacy UUID PK -> nma_global_id (unique audit column) | ||
| "nma_GlobalID": nma_global_id, | ||
| # FKs |
There was a problem hiding this comment.
With thing_id removed from the row payload, it would help to document (in a brief comment here or in the class docstring) why thing_id is no longer transferred (e.g., derived via join, deprecated column, or guaranteed by chemistry_sample_info_id). This makes the intentional schema/behavior change clearer for future maintainers.
| # FKs | |
| # FKs | |
| # NOTE: The legacy `thing_id` column is intentionally not transferred here. | |
| # In the integer-PK schema, `chemistry_sample_info_id` (backed by the | |
| # NMA_Chemistry_SampleInfo row) and the legacy IDs below provide the | |
| # necessary linkage, so a separate `thing_id` value is no longer required. |
Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
Notes
Any special considerations, workarounds, or follow-up work to note?