-
Notifications
You must be signed in to change notification settings - Fork 1
perf: bulk membership creation with INSERT ON CONFLICT DO NOTHING #149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 19.0
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,11 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Part of OpenSPP. See LICENSE file for full copyright and licensing details. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from odoo import _, fields, models | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from odoo import _, api, fields, models | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from odoo.exceptions import ValidationError | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _logger = logging.getLogger(__name__) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class SPPCycleMembership(models.Model): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _name = "spp.cycle.membership" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -87,6 +91,73 @@ def open_registrant_form(self): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @api.model | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def bulk_create_memberships(self, vals_list, chunk_size=1000, skip_duplicates=False): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Create cycle memberships in bulk with optional duplicate skipping. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param vals_list: List of dicts with membership values | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param chunk_size: Number of records per batch (default 1000) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param skip_duplicates: When True, use INSERT ... ON CONFLICT DO NOTHING | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| to silently skip duplicate (partner_id, cycle_id) pairs. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Returns the count of inserted rows. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :return: Recordset (skip_duplicates=False) or int count (skip_duplicates=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not vals_list: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return 0 if skip_duplicates else self.env["spp.cycle.membership"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if skip_duplicates: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return self._bulk_insert_on_conflict(vals_list, chunk_size) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return self.create(vals_list) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _bulk_insert_on_conflict(self, vals_list, chunk_size=1000): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Insert cycle memberships using raw SQL with ON CONFLICT DO NOTHING. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param vals_list: List of dicts with at least partner_id, cycle_id, state | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param chunk_size: Number of records per SQL INSERT batch | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :return: Total number of rows actually inserted | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cr = self.env.cr | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uid = self.env.uid | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| total_inserted = 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| today = fields.Date.today() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for i in range(0, len(vals_list), chunk_size): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| batch = vals_list[i : i + chunk_size] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| values = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| params = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for v in batch: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| values.append("(%s, %s, %s, %s, %s, %s, now(), now())") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| params.extend( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| v["partner_id"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| v["cycle_id"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| v.get("state", "draft"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| v.get("enrollment_date", today), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uid, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uid, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+122
to
+140
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Calling fields.Date.today() inside the loop for every record is inefficient, especially for large batches. It should be called once outside the loop and stored in a variable.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sql = """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| INSERT INTO spp_cycle_membership | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (partner_id, cycle_id, state, enrollment_date, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| create_uid, write_uid, create_date, write_date) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VALUES {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ON CONFLICT (partner_id, cycle_id) DO NOTHING | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """.format( # noqa: S608 # nosec B608 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ", ".join(values) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cr.execute(sql, params) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| total_inserted += cr.rowcount | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _logger.info( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Bulk inserted %d cycle memberships (%d skipped as duplicates)", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| total_inserted, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| len(vals_list) - total_inserted, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return total_inserted | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def unlink(self): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not self: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,4 +1,5 @@ | ||||||
| # Part of OpenSPP. See LICENSE file for full copyright and licensing details. | ||||||
| import logging | ||||||
|
|
||||||
| from lxml import etree | ||||||
|
|
||||||
|
|
@@ -7,6 +8,8 @@ | |||||
|
|
||||||
| from . import constants | ||||||
|
|
||||||
| _logger = logging.getLogger(__name__) | ||||||
|
|
||||||
|
|
||||||
| class SPPProgramMembership(models.Model): | ||||||
| _inherit = [ | ||||||
|
|
@@ -345,26 +348,26 @@ def action_exit(self): | |||||
| } | ||||||
| ) | ||||||
|
|
||||||
| @api.model_create_multi | ||||||
| def bulk_create_memberships(self, vals_list, chunk_size=1000): | ||||||
| @api.model | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is recommended to use the @api.model_create_multi decorator for methods that accept a list of dictionaries (vals_list). This ensures that the method is always called with a list, providing better consistency with Odoo's standard batch processing patterns.
Suggested change
|
||||||
| def bulk_create_memberships(self, vals_list, chunk_size=1000, skip_duplicates=False): | ||||||
| """Create program memberships in bulk with optional chunking. | ||||||
|
|
||||||
| This helper is intended for large enrollment jobs (e.g. CEL-driven | ||||||
| bulk enrollment) where thousands of memberships need to be created | ||||||
| in a single operation. | ||||||
|
|
||||||
| It preserves the normal create() semantics, including: | ||||||
| - standard ORM validations and constraints | ||||||
| - audit logging (via spp_audit rules) | ||||||
| - source tracking mixins | ||||||
|
|
||||||
| The only optimisation is to: | ||||||
| - accept already-prepared value dicts | ||||||
| - optionally split very large batches into smaller chunks to keep | ||||||
| memory use and per-transaction work bounded. | ||||||
| :param vals_list: List of dicts with membership values | ||||||
| :param chunk_size: Number of records per batch (default 1000) | ||||||
| :param skip_duplicates: When True, use INSERT ... ON CONFLICT DO NOTHING | ||||||
| to silently skip duplicate (partner_id, program_id) pairs instead of | ||||||
| raising IntegrityError. Returns the count of inserted rows. | ||||||
| :return: Recordset (skip_duplicates=False) or int count (skip_duplicates=True) | ||||||
| """ | ||||||
| if not vals_list: | ||||||
| return self.env["spp.program.membership"] | ||||||
| return 0 if skip_duplicates else self.env["spp.program.membership"] | ||||||
|
|
||||||
| if skip_duplicates: | ||||||
| return self._bulk_insert_on_conflict(vals_list, chunk_size) | ||||||
|
|
||||||
| if chunk_size and chunk_size > 0: | ||||||
| all_memberships = self.env["spp.program.membership"] | ||||||
|
|
@@ -386,3 +389,58 @@ def bulk_create_memberships(self, vals_list, chunk_size=1000): | |||||
| SPPProgramMembership, | ||||||
| self.sudo(), # nosemgrep: odoo-sudo-without-context | ||||||
| ).create(vals_list) | ||||||
|
|
||||||
| def _bulk_insert_on_conflict(self, vals_list, chunk_size=1000): | ||||||
| """Insert memberships using raw SQL with ON CONFLICT DO NOTHING. | ||||||
|
|
||||||
| Bypasses ORM for maximum throughput during bulk enrollment. Duplicates | ||||||
| (matching the UNIQUE constraint on partner_id, program_id) are silently | ||||||
| skipped. | ||||||
|
|
||||||
| :param vals_list: List of dicts with at least partner_id, program_id, state | ||||||
| :param chunk_size: Number of records per SQL INSERT batch | ||||||
| :return: Total number of rows actually inserted | ||||||
| """ | ||||||
| cr = self.env.cr | ||||||
| uid = self.env.uid | ||||||
| total_inserted = 0 | ||||||
|
|
||||||
| now = fields.Datetime.now() | ||||||
|
|
||||||
| for i in range(0, len(vals_list), chunk_size): | ||||||
| batch = vals_list[i : i + chunk_size] | ||||||
| values = [] | ||||||
| params = [] | ||||||
| for v in batch: | ||||||
| state = v.get("state", "draft") | ||||||
| enrollment_date = now if state == "enrolled" else None | ||||||
| values.append("(%s, %s, %s, %s, %s, %s, now(), now())") | ||||||
| params.extend( | ||||||
| [ | ||||||
| v["partner_id"], | ||||||
| v["program_id"], | ||||||
| state, | ||||||
| enrollment_date, | ||||||
| uid, | ||||||
| uid, | ||||||
| ] | ||||||
| ) | ||||||
|
|
||||||
| sql = """ | ||||||
| INSERT INTO spp_program_membership | ||||||
| (partner_id, program_id, state, enrollment_date, | ||||||
| create_uid, write_uid, create_date, write_date) | ||||||
| VALUES {} | ||||||
| ON CONFLICT (partner_id, program_id) DO NOTHING | ||||||
| """.format( # noqa: S608 # nosec B608 | ||||||
| ", ".join(values) | ||||||
| ) | ||||||
|
Comment on lines
+410
to
+437
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The enrollment_date field in spp.program.membership is a stored computed field that depends on the state. Since this raw SQL insert bypasses the ORM, the field will not be populated for records inserted with state='enrolled'. To ensure data integrity, you should include enrollment_date in the SQL insert and set it to the current timestamp when the state is enrolled. for i in range(0, len(vals_list), chunk_size):
batch = vals_list[i : i + chunk_size]
values = []
params = []
for v in batch:
state = v.get("state", "draft")
enrollment_date = fields.Datetime.now() if state == "enrolled" else None
values.append("(%s, %s, %s, %s, %s, %s, now(), now())")
params.extend(
[
v["partner_id"],
v["program_id"],
state,
enrollment_date,
uid,
uid,
]
)
sql = """
INSERT INTO spp_program_membership
(partner_id, program_id, state, enrollment_date,
create_uid, write_uid, create_date, write_date)
VALUES {}
ON CONFLICT (partner_id, program_id) DO NOTHING
""".format(
", ".join(values)
) |
||||||
| cr.execute(sql, params) | ||||||
| total_inserted += cr.rowcount | ||||||
|
|
||||||
| _logger.info( | ||||||
| "Bulk inserted %d program memberships (%d skipped as duplicates)", | ||||||
| total_inserted, | ||||||
| len(vals_list) - total_inserted, | ||||||
| ) | ||||||
| return total_inserted | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is recommended to use the @api.model_create_multi decorator for methods that accept a list of dictionaries (vals_list). This ensures that the method is always called with a list, providing better consistency with Odoo's standard batch processing patterns.