Skip to content

feat(xpansion): db dao de-normalized version#3089

Closed
smailio wants to merge 23 commits intodevfrom
feature/db-dao-xpansion-denormalized
Closed

feat(xpansion): db dao de-normalized version#3089
smailio wants to merge 23 commits intodevfrom
feature/db-dao-xpansion-denormalized

Conversation

@smailio
Copy link
Collaborator

@smailio smailio commented Mar 9, 2026

This implementation has better perf and is less complex than the normalized implementation.

However even if the denormalized version seems relatively faster it's at most one ms faster than the normalized one. For such small gains I'd recommend to go with the normalized version.

benchmark_comparison

@smailio smailio force-pushed the feature/db-dao-xpansion-denormalized branch from 9792d82 to d212dd7 Compare March 9, 2026 16:39
insert(XPANSION_SENSITIVITY_PROJECTION_TABLE),
[{"study_id": self._study_id, "candidate_name": name} for name in sensitivity["projection"]],
)
except IntegrityError:
Copy link
Collaborator Author

@smailio smailio Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file study doesn't handle this validation.

In the denormalized version I don't perform the validation.

However, In the normalized version the validation is enforced by the DB, I could ignore the IntegrityError just to align with the file stufy implementation but I think that would be unwise.

Btw, the IntegrityError can be triggered both by duplicates projections lines or missing candidates, however in this PR I added validation at the model level for duplicates so we're sure at this stage of the code that if the IntegrityError it can only be the missing candidate.

@smailio smailio changed the title Feature/db dao xpansion denormalized feature(xpansion): db dao de-normalized version Mar 9, 2026
@smailio smailio changed the title feature(xpansion): db dao de-normalized version feat(xpansion): db dao de-normalized version Mar 10, 2026
@smailio smailio marked this pull request as ready for review March 10, 2026 13:04
Base automatically changed from feature/db-dao-xpansion-normalized to dev March 17, 2026 14:43
@sylvlecl
Copy link
Member

We kept the normalized version, closing !
Many thanks for the benchmark

@sylvlecl sylvlecl closed this Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants