Skip to content

Commit bd4e642

Browse files
bjesterclaude
andauthored
Remove deferred FK validation API and unused backend bulk helpers
The streaming deserialization pipeline (#392) replaced the only callers of these. Removes SyncableModel.deferred_clean_fields, ForeignKeyReference, TemporaryTable, and the backend _bulk_full_record_upsert and _bulk_update helpers, along with the helpers that only they used: _bulk_insert, _create_temporary_table, _create_placeholder_list, the PostgreSQL _prepare_* methods and get_pk_field. calculate_max_sqlite_variables is kept because Kolibri imports it. Closes #396 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent ecf390e commit bd4e642

7 files changed

Lines changed: 4 additions & 365 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ List of the most important changes for each release.
1111
- Children of a dirty parent that is left out of the deserialization run are now deserialized against the parent's existing app row
1212
- For models whose `clean_fields` loads related objects, a dirty self-referential chain is now deserialized one level per run
1313
- Logs deserialization warnings under `morango.sync.stream.deserialize` instead of `morango.sync.operations`
14+
- Removes `SyncableModel.deferred_clean_fields` and `morango.models.core.ForeignKeyReference`
15+
- Removes `morango.sync.backends.utils.TemporaryTable` and `morango.sync.backends.utils.get_pk_field`
16+
- Removes the unused bulk write helpers `_bulk_full_record_upsert`, `_bulk_update` and `_bulk_insert`, and the temporary table helper `_create_temporary_table`, from the sync backends
1417

1518
## 0.8.15
1619
- Improves buffer serialization performance by performing bulk counter lookup

‎morango/models/core.py‎

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
import json
33
import logging
44
import uuid
5-
from collections import defaultdict
6-
from collections import namedtuple
75
from functools import reduce
86

97
from django.core import exceptions
@@ -745,9 +743,6 @@ class RecordMaxCounterBuffer(AbstractCounter):
745743
model_uuid = UUIDField(db_index=True)
746744

747745

748-
ForeignKeyReference = namedtuple("ForeignKeyReference", ["from_field", "from_pk", "to_pk"])
749-
750-
751746
class SyncableModel(UUIDModelMixin):
752747
"""
753748
``SyncableModel`` is the base model class for syncing. Other models inherit from this class if they want to make
@@ -873,39 +868,6 @@ def cached_clean_fields(self, fk_lookup_cache, exclude=None, sync_filter=None):
873868
)
874869
fk_lookup_cache[self_key] = 1
875870

876-
def deferred_clean_fields(self, exclude=None, sync_filter=None):
877-
"""
878-
Calls `.clean_fields()` but excludes all foreign key fields and instead returns them as a
879-
dictionary for deferred batch processing
880-
881-
:param exclude: A list of field names to exclude from validation
882-
:type exclude: list[str]
883-
:param sync_filter: The current sync's filter, if any
884-
:type sync_filter: Filter|None
885-
:return: A dictionary containing lists of `ForeignKeyReference`s keyed by the name of the
886-
model being referenced by the FK
887-
"""
888-
excluded_fields = exclude or []
889-
deferred_fks = defaultdict(list)
890-
for field in self._meta.fields:
891-
if not isinstance(field, models.ForeignKey):
892-
continue
893-
# by not excluding the field if it's null, the default validation logic will apply
894-
# and should raise a ValidationError if the FK field is not nullable
895-
if getattr(self, field.attname) is None:
896-
continue
897-
excluded_fields.append(field.name)
898-
deferred_fks[field.related_model._meta.verbose_name].append(
899-
ForeignKeyReference(
900-
from_field=field.attname,
901-
from_pk=self.pk,
902-
to_pk=getattr(self, field.attname),
903-
)
904-
)
905-
906-
self.clean_fields(exclude=excluded_fields, sync_filter=sync_filter)
907-
return deferred_fks
908-
909871
def serialize(self):
910872
"""All concrete fields of the ``SyncableModel`` subclass, except for those specifically blacklisted, are returned in a dict."""
911873
# NOTE: code adapted from https://github.com/django/django/blob/master/django/forms/models.py#L75

‎morango/sync/backends/base.py‎

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77

88

99
class BaseSQLWrapper(object):
10-
create_temporary_table_template = "CREATE TEMP TABLE {name} ({fields})"
11-
1210
def __init__(self, connection):
1311
self.connection = connection
1412

@@ -25,31 +23,6 @@ def _set_transaction_repeatable_read(self):
2523
"""Set the current transaction isolation level"""
2624
yield
2725

28-
def _create_placeholder_list(self, fields, db_values):
29-
# number of rows to update
30-
num_of_rows = len(db_values) // len(fields)
31-
# create '%s' placeholders for a single row
32-
placeholder_tuple = tuple(["%s" for _ in range(len(fields))])
33-
# create list of the '%s' tuple placeholders based on number of rows to update
34-
return [str(placeholder_tuple) for _ in range(num_of_rows)]
35-
36-
def _bulk_full_record_upsert(self, cursor, table_name, fields, db_values):
37-
raise NotImplementedError("Subclass must implement this method.")
38-
39-
def _bulk_insert(self, cursor, table_name, fields, db_values):
40-
placeholder_str = ", ".join(self._create_placeholder_list(fields, db_values)).replace(
41-
"'", ""
42-
)
43-
fields_str = str(tuple(str(f.attname) for f in fields)).replace("'", "")
44-
insert = """
45-
INSERT INTO {table_name} {fields}
46-
VALUES {placeholder_str}
47-
""".format(table_name=table_name, fields=fields_str, placeholder_str=placeholder_str)
48-
cursor.execute(insert, db_values)
49-
50-
def _bulk_update(self, cursor, table_name, fields, db_values):
51-
raise NotImplementedError("Subclass must implement this method.")
52-
5326
def _dequeuing_delete_rmcb_records(self, cursor, transfersession_id):
5427
# delete all RMCBs which are a reverse FF (store version newer than buffer version)
5528
delete_rmcb_records = """DELETE FROM {rmcb}
@@ -176,16 +149,6 @@ def _dequeuing_delete_remaining_buffer(self, cursor, transfersession_id):
176149
)
177150
cursor.execute(delete_remaining_buffer)
178151

179-
def _create_temporary_table(self, cursor, name, field_sqls, fields_params):
180-
"""
181-
:param cursor: The database connection cursor
182-
:param name: The str name of the temp table
183-
:param field_sqls: A list of SQL strings representing the fields
184-
:param fields_params: A list of SQL parameters if necessary for the fields SQL
185-
"""
186-
sql = self.create_temporary_table_template.format(name=name, fields=", ".join(field_sqls))
187-
cursor.execute(sql, fields_params)
188-
189152
def _lock_all_partitions(self, shared=False):
190153
"""
191154
Execute a lock within the database for all partitions, if the backend supports it. The lock

‎morango/sync/backends/postgres.py‎

Lines changed: 0 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
from morango.utils import SETTINGS
1111

1212
from .base import BaseSQLWrapper
13-
from .utils import get_pk_field
1413

1514
# advisory lock integers for locking partitions
1615
LOCK_ALL_PARTITIONS = 1
@@ -23,7 +22,6 @@
2322

2423
class SQLWrapper(BaseSQLWrapper):
2524
backend = "postgresql"
26-
create_temporary_table_template = "CREATE TEMP TABLE {name} ({fields}) ON COMMIT DROP"
2725

2826
def _is_transaction_isolation_error(self, error):
2927
"""
@@ -85,96 +83,6 @@ def _set_transaction_repeatable_read(self):
8583
self.connection.connection.set_session(isolation_level=existing_isolation_level)
8684
self.connection.connection.set_session(autocommit=existing_autocommit)
8785

88-
def _prepare_with_values(self, name, fields, db_values):
89-
placeholder_list = self._create_placeholder_list(fields, db_values)
90-
# convert this list to a string to be passed into raw sql query
91-
placeholder_str = ", ".join(placeholder_list).replace("'", "")
92-
return """
93-
WITH {name} {fields} as
94-
(
95-
VALUES {placeholder_str}
96-
)
97-
""".format(
98-
name=name,
99-
fields=str(tuple(str(f.column) for f in fields)).replace("'", ""),
100-
placeholder_str=placeholder_str,
101-
)
102-
103-
def _prepare_casted_fields(self, fields):
104-
return ", ".join(
105-
map(
106-
lambda f: "{f}::{type}".format(f=f.column, type=f.rel_db_type(self.connection)),
107-
fields,
108-
)
109-
)
110-
111-
def _prepare_set_casted_values(self, fields, source_table):
112-
return ", ".join(
113-
map(
114-
lambda f: "{f} = {src}.{f}::{type}".format(
115-
f=f.attname,
116-
type=f.rel_db_type(self.connection),
117-
src=source_table,
118-
),
119-
fields,
120-
)
121-
)
122-
123-
def _bulk_full_record_upsert(self, cursor, table_name, fields, db_values):
124-
pk = get_pk_field(fields)
125-
126-
cte_name = "new_values"
127-
upsert = """
128-
{cte},
129-
updated as
130-
(
131-
UPDATE {table_name} model
132-
SET {set_values}
133-
FROM {cte_name} cte
134-
WHERE model.id = cte.{pk_field}::{pk_type}
135-
RETURNING model.{pk_field}
136-
)
137-
INSERT INTO {table_name} {fields}
138-
SELECT {select_fields}
139-
FROM {cte_name} cte
140-
WHERE cte.{pk_field}::{pk_type} NOT IN (SELECT {pk_field} FROM updated)
141-
""".format(
142-
cte=self._prepare_with_values(cte_name, fields, db_values),
143-
cte_name=cte_name,
144-
table_name=table_name,
145-
fields=str(tuple(str(f.column) for f in fields)).replace("'", ""),
146-
set_values=self._prepare_set_casted_values(fields, "cte"),
147-
select_fields=self._prepare_casted_fields(fields),
148-
pk_field=pk.column,
149-
pk_type=pk.rel_db_type(self.connection),
150-
)
151-
# use DB-APIs parameter substitution (2nd parameter expects a sequence)
152-
cursor.execute(upsert, db_values)
153-
154-
def _bulk_update(self, cursor, table_name, fields, db_values):
155-
pk = get_pk_field(fields)
156-
157-
insert = """
158-
{cte}
159-
UPDATE {table_name} model
160-
SET {set_values}
161-
FROM {cte_name} cte
162-
WHERE model.{pk_field} = cte.{pk_field}::{pk_type}
163-
"""
164-
165-
cte_name = "new_values"
166-
insert = insert.format(
167-
cte=self._prepare_with_values(cte_name, fields, db_values),
168-
cte_name=cte_name,
169-
table_name=table_name,
170-
fields=str(tuple(str(f.column) for f in fields)).replace("'", ""),
171-
set_values=self._prepare_set_casted_values(fields, "cte"),
172-
pk_field=pk.column,
173-
pk_type=pk.rel_db_type(self.connection),
174-
)
175-
# use DB-APIs parameter substitution (2nd parameter expects a sequence)
176-
cursor.execute(insert, db_values)
177-
17886
def _dequeuing_merge_conflict_rmcb(self, cursor, transfersession_id):
17987
# transfer record max counters for records with merge conflicts + perform max
18088
merge_conflict_rmc = """UPDATE {rmc} as rmc SET counter

‎morango/sync/backends/sqlite.py‎

Lines changed: 0 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -4,104 +4,11 @@
44
from morango.models.core import Store
55

66
from .base import BaseSQLWrapper
7-
from .utils import calculate_max_sqlite_variables
8-
from .utils import get_pk_field
97

108

119
class SQLWrapper(BaseSQLWrapper):
1210
backend = "sqlite"
1311

14-
def _bulk_full_record_upsert(self, cursor, table_name, fields, db_values):
15-
"""
16-
Example query:
17-
`REPLACE INTO model (F1,F2,F3) VALUES (%s, %s, %s), (%s, %s, %s), (%s, %s, %s)`
18-
where values=[1,2,3,4,5,6,7,8,9]
19-
"""
20-
placeholder_list = self._create_placeholder_list(fields, db_values)
21-
# calculate and create equal sized chunks of data to insert incrementally
22-
num_of_rows_able_to_insert = calculate_max_sqlite_variables() // len(fields)
23-
num_of_values_able_to_insert = num_of_rows_able_to_insert * len(fields)
24-
value_chunks = [
25-
db_values[x : x + num_of_values_able_to_insert]
26-
for x in range(0, len(db_values), num_of_values_able_to_insert)
27-
]
28-
placeholder_chunks = [
29-
placeholder_list[x : x + num_of_rows_able_to_insert]
30-
for x in range(0, len(placeholder_list), num_of_rows_able_to_insert)
31-
]
32-
# insert data chunks
33-
fields_str = str(tuple(str(f.attname) for f in fields)).replace("'", "")
34-
for values, params in zip(value_chunks, placeholder_chunks):
35-
placeholder_str = ", ".join(params).replace("'", "")
36-
insert = """
37-
REPLACE INTO {table_name} {fields}
38-
VALUES {placeholder_str}
39-
""".format(
40-
table_name=table_name,
41-
fields=fields_str,
42-
placeholder_str=placeholder_str,
43-
)
44-
# use DB-APIs parameter substitution (2nd parameter expects a sequence)
45-
cursor.execute(insert, values)
46-
47-
def _bulk_insert(self, cursor, table_name, fields, db_values):
48-
num_of_rows_able_to_insert = calculate_max_sqlite_variables() // len(fields)
49-
num_of_values_able_to_insert = num_of_rows_able_to_insert * len(fields)
50-
value_chunks = [
51-
db_values[x : x + num_of_values_able_to_insert]
52-
for x in range(0, len(db_values), num_of_values_able_to_insert)
53-
]
54-
for value_chunk in value_chunks:
55-
super(SQLWrapper, self)._bulk_insert(cursor, table_name, fields, value_chunk)
56-
57-
def _bulk_update(self, cursor, table_name, fields, db_values):
58-
"""
59-
Example query:
60-
`UPDATE model SET F1=(CASE id WHEN %s THEN %s ... END) ...`
61-
WHERE id IN [...]
62-
"""
63-
# calculate and create equal sized chunks of data to update incrementally
64-
# for every field we're updating, we'll require 3 parameters
65-
num_update_fields = len(fields) - 1
66-
num_of_rows_able_to_update = calculate_max_sqlite_variables() // num_update_fields // 3
67-
num_of_values_able_to_update = num_of_rows_able_to_update * len(fields)
68-
value_chunks = [
69-
db_values[x : x + num_of_values_able_to_update]
70-
for x in range(0, len(db_values), num_of_values_able_to_update)
71-
]
72-
pk = get_pk_field(fields)
73-
74-
# insert data chunks
75-
for values in value_chunks:
76-
set_sql = ""
77-
params = []
78-
pk_params = []
79-
for field in fields:
80-
if field == pk:
81-
continue
82-
set_field_sql = " {field} = (CASE {pk_field}".format(
83-
field=field.column, pk_field=pk.column
84-
)
85-
for y in range(0, len(values), len(fields)):
86-
value_set = values[y : y + len(fields)]
87-
set_field_sql += " WHEN %s THEN %s"
88-
pk_params.append(value_set[fields.index(pk)])
89-
params.append(value_set[fields.index(pk)])
90-
params.append(value_set[fields.index(field)])
91-
set_field_sql += " END),"
92-
set_sql += set_field_sql
93-
params.extend(pk_params)
94-
update = """
95-
UPDATE {table_name} SET {set_sql} WHERE {pk_field} IN {placeholder_str}
96-
""".format(
97-
table_name=table_name,
98-
set_sql=set_sql[:-1],
99-
pk_field=pk.column,
100-
placeholder_str="({})".format(",".join("%s" for _ in range(len(pk_params)))),
101-
)
102-
# use DB-APIs parameter substitution (2nd parameter expects a sequence)
103-
cursor.execute(update, params)
104-
10512
def _dequeuing_merge_conflict_rmcb(self, cursor, transfersession_id):
10613
# transfer record max counters for records with merge conflicts + perform max
10714
merge_conflict_rmc = """REPLACE INTO {rmc} (instance_id, counter, store_model_id)

0 commit comments

Comments
 (0)