fix: Require unique FK columns to be nullable (#95)
This commit is contained in:
parent
a9fa6b6b91
commit
c6a873d04c
5 changed files with 184 additions and 20 deletions
|
|
@ -294,24 +294,28 @@ losing-side release in that scope's CRDT chain. "Whoever syncs first" is not
|
||||||
an acceptable CRDT rule. The only permitted global unique indexes are
|
an acceptable CRDT rule. The only permitted global unique indexes are
|
||||||
foreign-key-only indexes: every index element must be a column, and the
|
foreign-key-only indexes: every index element must be a column, and the
|
||||||
indexed columns must be composed only of complete foreign-key column sets whose
|
indexed columns must be composed only of complete foreign-key column sets whose
|
||||||
referenced tables are also synced. This covers Serverpod's relation indexes,
|
referenced tables are also synced. Under the current `flag` unique-conflict
|
||||||
including one-to-one and composite FK relation indexes, without forcing
|
policy, every indexed FK column in that exception must be nullable. A required
|
||||||
duplicate per-scope indexes for include-query performance. Those columns point
|
unique FK has no safe loser value: `null` violates the column, a synthetic UUID
|
||||||
at globally unique synced row ids, and FK enforcement already repairs or
|
violates the FK constraint, and keeping the original value violates uniqueness.
|
||||||
rejects cross-scope references, so they are not treated as global
|
This covers optional Serverpod relation indexes, including optional one-to-one
|
||||||
application-level arbitration. The model must still declare scoped composite
|
and composite FK relation indexes, without forcing duplicate per-scope indexes
|
||||||
unique indexes for application uniqueness. On a deployment that happens to
|
for include-query performance. Those columns point at globally unique synced row
|
||||||
contain one scope, a composite unique with a constant `scopeId` accepts exactly
|
ids, and FK enforcement already repairs or rejects cross-scope references, so
|
||||||
the same rows as a plain unique; nothing degrades.
|
they are not treated as global application-level arbitration. The model must
|
||||||
|
still declare scoped composite unique indexes for application uniqueness. On a
|
||||||
|
deployment that happens to contain one scope, a composite unique with a constant
|
||||||
|
`scopeId` accepts exactly the same rows as a plain unique; nothing degrades.
|
||||||
|
|
||||||
The unique-conflict resolver discovers indexes from the table definitions,
|
The unique-conflict resolver discovers indexes from the table definitions,
|
||||||
uses `scopeId` as part of the lookup predicate, and releases only the
|
uses `scopeId` as part of the lookup predicate for scoped indexes, and releases
|
||||||
application columns in scoped indexes. Allowed FK-only global unique indexes
|
only releasable non-`scopeId` columns. For allowed FK-only global unique
|
||||||
are ignored by the CRDT resolver; same-scope application conflicts are handled
|
indexes, nullable FK columns may be released to `null`; required FK columns are
|
||||||
by scoped composite indexes, and cross-scope references are handled by FK
|
rejected during schema initialization. Same-scope application conflicts are
|
||||||
repair. Rows in different scopes never form a conflict group under the
|
handled by scoped composite indexes, and cross-scope references are handled by
|
||||||
required composite index. The flag policy (see
|
FK repair. Rows in different scopes never form a conflict group under the
|
||||||
`unique-constraint-invariants.md`) is otherwise unchanged.
|
required composite index. The flag policy (see `unique-constraint-invariants.md`)
|
||||||
|
is otherwise unchanged.
|
||||||
|
|
||||||
## Enforcement mechanics
|
## Enforcement mechanics
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -45,6 +45,11 @@ is supported when at least one non-`scopeId` column can be released
|
||||||
deterministically. Stable discriminator columns may remain unchanged; changing
|
deterministically. Stable discriminator columns may remain unchanged; changing
|
||||||
the releasable column or columns is enough to make the unique tuple conflict-free.
|
the releasable column or columns is enough to make the unique tuple conflict-free.
|
||||||
|
|
||||||
|
For the global foreign-key-only unique indexes that Serverpod emits for synced
|
||||||
|
relations, every indexed FK column must be nullable under the current `flag`
|
||||||
|
policy. Required FK columns cannot be released without violating either the
|
||||||
|
non-null constraint, the FK constraint, or the unique constraint itself.
|
||||||
|
|
||||||
The rewritten value is a materialized data value, not a synced conflict class.
|
The rewritten value is a materialized data value, not a synced conflict class.
|
||||||
The core CRDT protocol does not need to expose unique conflict objects to users.
|
The core CRDT protocol does not need to expose unique conflict objects to users.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -58,6 +58,19 @@ class CrdtSchemaRegistry {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
final requiredFKeyUniqueColumns = crdtRequiredForeignKeyOnlyUniqueColumnViolations(
|
||||||
|
syncTables,
|
||||||
|
tableDefinitionsByName,
|
||||||
|
);
|
||||||
|
if (requiredFKeyUniqueColumns.isNotEmpty) {
|
||||||
|
throw StateError(
|
||||||
|
'CRDT can only synchronize 1:1 relations when the foreign key column is '
|
||||||
|
'nullable, but the following foreign keys are non-nullable: '
|
||||||
|
'"${requiredFKeyUniqueColumns.join("', '")}". '
|
||||||
|
'Make the relation optional/nullable.',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
final nonReleasableUniqueIndexes = crdtNonReleasableUniqueIndexViolations(
|
final nonReleasableUniqueIndexes = crdtNonReleasableUniqueIndexViolations(
|
||||||
syncTables,
|
syncTables,
|
||||||
tableDefinitionsByName,
|
tableDefinitionsByName,
|
||||||
|
|
|
||||||
|
|
@ -6,14 +6,27 @@ bool isCrdtAllowedForeignKeyOnlyUniqueIndex(
|
||||||
TableDefinition tableDefinition,
|
TableDefinition tableDefinition,
|
||||||
IndexDefinition index,
|
IndexDefinition index,
|
||||||
Set<String> syncTableNames,
|
Set<String> syncTableNames,
|
||||||
|
) {
|
||||||
|
return _foreignKeyOnlyUniqueIndexColumns(
|
||||||
|
tableDefinition,
|
||||||
|
index,
|
||||||
|
syncTableNames,
|
||||||
|
) !=
|
||||||
|
null;
|
||||||
|
}
|
||||||
|
|
||||||
|
List<String>? _foreignKeyOnlyUniqueIndexColumns(
|
||||||
|
TableDefinition tableDefinition,
|
||||||
|
IndexDefinition index,
|
||||||
|
Set<String> syncTableNames,
|
||||||
) {
|
) {
|
||||||
final indexColumns = <String>[];
|
final indexColumns = <String>[];
|
||||||
for (final element in index.elements) {
|
for (final element in index.elements) {
|
||||||
if (element.type != IndexElementDefinitionType.column) return false;
|
if (element.type != IndexElementDefinitionType.column) return null;
|
||||||
indexColumns.add(element.definition);
|
indexColumns.add(element.definition);
|
||||||
}
|
}
|
||||||
if (indexColumns.isEmpty || indexColumns.length != indexColumns.toSet().length) {
|
if (indexColumns.isEmpty || indexColumns.length != indexColumns.toSet().length) {
|
||||||
return false;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
final indexColumnSet = indexColumns.toSet();
|
final indexColumnSet = indexColumns.toSet();
|
||||||
|
|
@ -27,7 +40,47 @@ bool isCrdtAllowedForeignKeyOnlyUniqueIndex(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return indexColumns.every(coveredForeignKeyColumns.contains);
|
return indexColumns.every(coveredForeignKeyColumns.contains) ? indexColumns : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
@internal
|
||||||
|
List<String> crdtRequiredForeignKeyOnlyUniqueColumnViolations(
|
||||||
|
List<Table> syncTables,
|
||||||
|
Map<String, TableDefinition> tableDefinitionsByName,
|
||||||
|
) {
|
||||||
|
final syncTableNames = {for (final table in syncTables) table.tableName};
|
||||||
|
final violations = <String>{};
|
||||||
|
|
||||||
|
for (final tableName in syncTableNames) {
|
||||||
|
final table = tableDefinitionsByName[tableName];
|
||||||
|
if (table == null) continue;
|
||||||
|
|
||||||
|
final columnsByName = {for (final column in table.columns) column.name: column};
|
||||||
|
for (final index in table.indexes) {
|
||||||
|
if (!index.isUnique || index.isPrimary || isCrdtScopedUniqueIndex(index)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
final foreignKeyOnlyColumns = _foreignKeyOnlyUniqueIndexColumns(
|
||||||
|
table,
|
||||||
|
index,
|
||||||
|
syncTableNames,
|
||||||
|
);
|
||||||
|
if (foreignKeyOnlyColumns == null) continue;
|
||||||
|
|
||||||
|
for (final columnName in foreignKeyOnlyColumns) {
|
||||||
|
final column = columnsByName[columnName];
|
||||||
|
if (column == null) {
|
||||||
|
throw StateError(
|
||||||
|
'No column definition found for ${table.name}.$columnName.',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
if (!column.isNullable) violations.add('${table.name}.$columnName');
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return violations.toList();
|
||||||
}
|
}
|
||||||
|
|
||||||
@internal
|
@internal
|
||||||
|
|
|
||||||
|
|
@ -198,7 +198,7 @@ void main() {
|
||||||
);
|
);
|
||||||
|
|
||||||
test(
|
test(
|
||||||
'Given a CRDT schema registry with a synced foreign-key-only unique index, '
|
'Given a CRDT schema registry with a synced nullable foreign-key-only unique index, '
|
||||||
'when syncAndGetSchema is called, '
|
'when syncAndGetSchema is called, '
|
||||||
'then the index is accepted.',
|
'then the index is accepted.',
|
||||||
() async {
|
() async {
|
||||||
|
|
@ -214,6 +214,42 @@ void main() {
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
|
test(
|
||||||
|
'Given a CRDT schema registry with a synced required foreign-key-only unique index, '
|
||||||
|
'when the registry is created, '
|
||||||
|
'then an error is thrown.',
|
||||||
|
() async {
|
||||||
|
final addressDefinition = testSession.db.serializationManager
|
||||||
|
.getTargetTableDefinitions()
|
||||||
|
.firstWhere((definition) => definition.name == Address.t.tableName);
|
||||||
|
final requiredForeignKeyDefinition = addressDefinition.copyWith(
|
||||||
|
columns: [
|
||||||
|
for (final column in addressDefinition.columns)
|
||||||
|
column.name == 'inhabitantId' ? column.copyWith(isNullable: false) : column,
|
||||||
|
],
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(
|
||||||
|
() => CrdtSchemaRegistry(
|
||||||
|
session,
|
||||||
|
syncTables: [Address.t, Person.t],
|
||||||
|
tableDefinitions: [requiredForeignKeyDefinition],
|
||||||
|
),
|
||||||
|
throwsA(
|
||||||
|
isA<StateError>().having(
|
||||||
|
(e) => e.message,
|
||||||
|
'message',
|
||||||
|
contains(
|
||||||
|
'CRDT can only synchronize 1:1 relations when the foreign key column is '
|
||||||
|
'nullable, but the following foreign keys are non-nullable: '
|
||||||
|
'"address.inhabitantId". Make the relation optional/nullable.',
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
test(
|
test(
|
||||||
'Given a CRDT schema registry with a global unique index containing only foreign keys to synced tables, '
|
'Given a CRDT schema registry with a global unique index containing only foreign keys to synced tables, '
|
||||||
'when syncAndGetSchema is called, '
|
'when syncAndGetSchema is called, '
|
||||||
|
|
@ -256,6 +292,59 @@ void main() {
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
|
test(
|
||||||
|
'Given a CRDT schema registry with a global unique index containing a required foreign key to a synced table, '
|
||||||
|
'when the registry is created, '
|
||||||
|
'then an error is thrown.',
|
||||||
|
() async {
|
||||||
|
final tableDefinitions = testSession.db.serializationManager
|
||||||
|
.getTargetTableDefinitions();
|
||||||
|
final townDefinition = tableDefinitions.firstWhere(
|
||||||
|
(definition) => definition.name == Town.t.tableName,
|
||||||
|
);
|
||||||
|
final addressDefinition = tableDefinitions.firstWhere(
|
||||||
|
(definition) => definition.name == Address.t.tableName,
|
||||||
|
);
|
||||||
|
final indexTemplate = addressDefinition.indexes.singleWhere(
|
||||||
|
(index) => index.indexName == 'address__inhabitantId__unique_idx',
|
||||||
|
);
|
||||||
|
final requiredForeignKeyDefinition = townDefinition.copyWith(
|
||||||
|
columns: [
|
||||||
|
for (final column in townDefinition.columns)
|
||||||
|
column.name == 'cityId' ? column.copyWith(isNullable: false) : column,
|
||||||
|
],
|
||||||
|
indexes: [
|
||||||
|
indexTemplate.copyWith(
|
||||||
|
indexName: 'town_global_city_mayor_unique_idx',
|
||||||
|
elements: [
|
||||||
|
indexTemplate.elements.single.copyWith(definition: 'cityId'),
|
||||||
|
indexTemplate.elements.single.copyWith(definition: 'mayorId'),
|
||||||
|
],
|
||||||
|
),
|
||||||
|
],
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(
|
||||||
|
() => CrdtSchemaRegistry(
|
||||||
|
session,
|
||||||
|
syncTables: [Town.t, City.t, Person.t],
|
||||||
|
tableDefinitions: [requiredForeignKeyDefinition],
|
||||||
|
),
|
||||||
|
throwsA(
|
||||||
|
isA<StateError>().having(
|
||||||
|
(e) => e.message,
|
||||||
|
'message',
|
||||||
|
contains(
|
||||||
|
'CRDT can only synchronize 1:1 relations when the foreign key column is '
|
||||||
|
'nullable, but the following foreign keys are non-nullable: '
|
||||||
|
'"town.cityId". Make the relation optional/nullable.',
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
test(
|
test(
|
||||||
'Given a CRDT schema registry with a global unique index mixing a foreign key and an application column, '
|
'Given a CRDT schema registry with a global unique index mixing a foreign key and an application column, '
|
||||||
'when the registry is created, '
|
'when the registry is created, '
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue