Skip to content

Commit 0685104

Browse files
ChrisJollyAUChrisJollyAU
committed
Refactor Jet value gen strategy logic and FK constraint drops
Refine JetPropertyExtensions to improve value generation strategy selection, add an extension for property overrides, and clarify handling of shared table roots and FKs. Reformat OnModelCreating in GraphUpdatesJetTestBase for consistency, and update SeedAsync to better document and adjust composite FK constraint drops for Jet's MATCH FULL semantics.
1 parent f351b45 commit 0685104

2 files changed

Lines changed: 83 additions & 74 deletions

File tree

‎src/EFCore.Jet/Extensions/JetPropertyExtensions.cs‎

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -314,16 +314,18 @@ public static JetValueGenerationStrategy GetValueGenerationStrategy(this IReadOn
314314
return (JetValueGenerationStrategy?)annotation.Value ?? JetValueGenerationStrategy.None;
315315
}
316316

317+
var defaultValueGenerationStrategy = GetDefaultValueGenerationStrategy(property);
318+
317319
if (property.ValueGenerated != ValueGenerated.OnAdd
318320
|| property.IsForeignKey()
319321
|| property.TryGetDefaultValue(out _)
320-
|| property.GetDefaultValueSql() != null
322+
|| (defaultValueGenerationStrategy != JetValueGenerationStrategy.Sequence && property.GetDefaultValueSql() != null)
321323
|| property.GetComputedColumnSql() != null)
322324
{
323325
return JetValueGenerationStrategy.None;
324326
}
325327

326-
return GetDefaultValueGenerationStrategy(property);
328+
return defaultValueGenerationStrategy;
327329
}
328330
/// <summary>
329331
/// <para>
@@ -364,16 +366,15 @@ internal static JetValueGenerationStrategy GetValueGenerationStrategy(
364366
if (sharedTableRootProperty != null)
365367
{
366368
return sharedTableRootProperty.GetValueGenerationStrategy(storeObject, typeMappingSource)
367-
== JetValueGenerationStrategy.IdentityColumn
368-
&& table.StoreObjectType == StoreObjectType.Table
369-
&& !property.GetContainingForeignKeys().Any(
370-
fk =>
371-
!fk.IsBaseLinking()
372-
|| (StoreObjectIdentifier.Create(fk.PrincipalEntityType, StoreObjectType.Table)
373-
is StoreObjectIdentifier principal
374-
&& fk.GetConstraintName(table, principal) != null))
375-
? JetValueGenerationStrategy.IdentityColumn
376-
: JetValueGenerationStrategy.None;
369+
== JetValueGenerationStrategy.IdentityColumn
370+
&& table.StoreObjectType == StoreObjectType.Table
371+
&& !property.GetContainingForeignKeys().Any(fk =>
372+
!fk.IsBaseLinking()
373+
|| (StoreObjectIdentifier.Create(fk.PrincipalEntityType, StoreObjectType.Table)
374+
is { } principal
375+
&& fk.GetConstraintName(table, principal) != null))
376+
? JetValueGenerationStrategy.IdentityColumn
377+
: JetValueGenerationStrategy.None;
377378
}
378379

379380
if (property.ValueGenerated != ValueGenerated.OnAdd
@@ -382,12 +383,11 @@ is StoreObjectIdentifier principal
382383
|| property.GetDefaultValueSql(storeObject) != null
383384
|| property.GetComputedColumnSql(storeObject) != null
384385
|| property.GetContainingForeignKeys()
385-
.Any(
386-
fk =>
387-
!fk.IsBaseLinking()
388-
|| (StoreObjectIdentifier.Create(fk.PrincipalEntityType, StoreObjectType.Table)
389-
is StoreObjectIdentifier principal
390-
&& fk.GetConstraintName(table, principal) != null)))
386+
.Any(fk =>
387+
!fk.IsBaseLinking()
388+
|| (StoreObjectIdentifier.Create(fk.PrincipalEntityType, StoreObjectType.Table)
389+
is { } principal
390+
&& fk.GetConstraintName(table, principal) != null)))
391391
{
392392
return JetValueGenerationStrategy.None;
393393
}
@@ -414,6 +414,19 @@ private static JetValueGenerationStrategy GetDefaultValueGenerationStrategy(IRea
414414
: JetValueGenerationStrategy.None;
415415
}
416416

417+
/// <summary>
418+
/// Returns the <see cref="JetValueGenerationStrategy" /> to use for the property.
419+
/// </summary>
420+
/// <remarks>
421+
/// If no strategy is set for the property, then the strategy to use will be taken from the <see cref="IModel" />.
422+
/// </remarks>
423+
/// <param name="overrides">The property overrides.</param>
424+
/// <returns>The strategy, or <see cref="JetValueGenerationStrategy.None" /> if none was set.</returns>
425+
public static JetValueGenerationStrategy? GetValueGenerationStrategy(
426+
this IReadOnlyRelationalPropertyOverrides overrides)
427+
=> (JetValueGenerationStrategy?)overrides.FindAnnotation(JetAnnotationNames.ValueGenerationStrategy)
428+
?.Value;
429+
417430
private static JetValueGenerationStrategy GetDefaultValueGenerationStrategy(
418431
IReadOnlyProperty property,
419432
in StoreObjectIdentifier storeObject,
@@ -426,7 +439,7 @@ private static JetValueGenerationStrategy GetDefaultValueGenerationStrategy(
426439
? property.DeclaringType.GetMappingStrategy() == RelationalAnnotationNames.TpcMappingStrategy
427440
? JetValueGenerationStrategy.None
428441
: JetValueGenerationStrategy.IdentityColumn
429-
: JetValueGenerationStrategy.None;
442+
: JetValueGenerationStrategy.None;
430443
}
431444

432445
/// <summary>
@@ -522,12 +535,12 @@ public static void SetValueGenerationStrategy(
522535
public static bool IsCompatibleWithValueGeneration(IReadOnlyProperty property)
523536
{
524537
var valueConverter = property.GetValueConverter()
525-
?? property.FindTypeMapping()?.Converter;
538+
?? property.FindTypeMapping()?.Converter;
526539

527540
var type = (valueConverter?.ProviderClrType ?? property.ClrType).UnwrapNullableType();
528541
return type.IsInteger()
529-
|| type.IsEnum
530-
|| type == typeof(decimal);
542+
|| type.IsEnum
543+
|| type == typeof(decimal);
531544
}
532545

533546
private static bool IsCompatibleWithValueGeneration(

‎test/EFCore.Jet.FunctionalTests/GraphUpdates/GraphUpdatesJetTestBase.cs‎

Lines changed: 48 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -157,63 +157,55 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
157157
{
158158
base.OnModelCreating(modelBuilder, context);
159159

160-
modelBuilder.Entity<AccessState>(
161-
b =>
162-
{
163-
b.Property(e => e.AccessStateId).ValueGeneratedNever();
164-
b.HasData(new AccessState { AccessStateId = 1 });
165-
});
166-
167-
modelBuilder.Entity<Cruiser>(
168-
b =>
169-
{
170-
b.Property(e => e.IdUserState).HasDefaultValue(1);
171-
b.HasOne(e => e.UserState).WithMany(e => e.Users).HasForeignKey(e => e.IdUserState);
172-
});
173-
174-
modelBuilder.Entity<AccessStateWithSentinel>(
175-
b =>
176-
{
177-
b.Property(e => e.AccessStateWithSentinelId).ValueGeneratedNever();
178-
b.HasData(new AccessStateWithSentinel { AccessStateWithSentinelId = 1 });
179-
});
180-
181-
modelBuilder.Entity<CruiserWithSentinel>(
182-
b =>
183-
{
184-
b.Property(e => e.IdUserState).HasDefaultValue(1).HasSentinel(667);
185-
b.HasOne(e => e.UserState).WithMany(e => e.Users).HasForeignKey(e => e.IdUserState);
186-
});
160+
modelBuilder.Entity<AccessState>(b =>
161+
{
162+
b.Property(e => e.AccessStateId).ValueGeneratedNever();
163+
b.HasData(new AccessState { AccessStateId = 1 });
164+
});
165+
166+
modelBuilder.Entity<Cruiser>(b =>
167+
{
168+
b.Property(e => e.IdUserState).HasDefaultValue(1);
169+
b.HasOne(e => e.UserState).WithMany(e => e.Users).HasForeignKey(e => e.IdUserState);
170+
});
171+
172+
modelBuilder.Entity<AccessStateWithSentinel>(b =>
173+
{
174+
b.Property(e => e.AccessStateWithSentinelId).ValueGeneratedNever();
175+
b.HasData(new AccessStateWithSentinel { AccessStateWithSentinelId = 1 });
176+
});
177+
178+
modelBuilder.Entity<CruiserWithSentinel>(b =>
179+
{
180+
b.Property(e => e.IdUserState).HasDefaultValue(1).HasSentinel(667);
181+
b.HasOne(e => e.UserState).WithMany(e => e.Users).HasForeignKey(e => e.IdUserState);
182+
});
187183

188184
modelBuilder.Entity<SomethingOfCategoryA>().Property<int>("CategoryId").HasDefaultValue(1);
189185
modelBuilder.Entity<SomethingOfCategoryB>().Property(e => e.CategoryId).HasDefaultValue(2);
190186

191-
modelBuilder.Entity<StringKeyAndIndexParent>(
192-
b =>
193-
{
194-
b.HasOne(e => e.Child)
195-
.WithOne(e => e.Parent)
196-
.HasForeignKey<StringKeyAndIndexChild>(e => e.ParentId)
197-
.HasPrincipalKey<StringKeyAndIndexParent>(e => e.AlternateId);
198-
});
199-
200-
modelBuilder.Entity<CompositeKeyWith<int>>(
201-
b =>
202-
{
187+
modelBuilder.Entity<StringKeyAndIndexParent>(b =>
188+
{
189+
b.HasOne(e => e.Child)
190+
.WithOne(e => e.Parent)
191+
.HasForeignKey<StringKeyAndIndexChild>(e => e.ParentId)
192+
.HasPrincipalKey<StringKeyAndIndexParent>(e => e.AlternateId);
193+
});
194+
195+
modelBuilder.Entity<CompositeKeyWith<int>>(b =>
196+
{
203197
b.Property(e => e.PrimaryGroup).HasDefaultValue(1).HasSentinel(1);
204-
});
198+
});
205199

206-
modelBuilder.Entity<CompositeKeyWith<bool>>(
207-
b =>
208-
{
200+
modelBuilder.Entity<CompositeKeyWith<bool>>(b =>
201+
{
209202
b.Property(e => e.PrimaryGroup).HasDefaultValue(true);
210-
});
203+
});
211204

212-
modelBuilder.Entity<CompositeKeyWith<bool?>>(
213-
b =>
214-
{
215-
b.Property(e => e.PrimaryGroup).HasDefaultValue(true);
216-
});
205+
modelBuilder.Entity<CompositeKeyWith<bool?>>(b =>
206+
{
207+
b.Property(e => e.PrimaryGroup).HasDefaultValue(true);
208+
});
217209

218210
modelBuilder.Entity<SharedFkRoot>().Property(x => x.Id).HasColumnType("int");
219211
modelBuilder.Entity<SharedFkDependant>().Property(x => x.Id).HasColumnType("int");
@@ -226,12 +218,16 @@ protected override async Task SeedAsync(PoolableDbContext context)
226218
{
227219
await base.SeedAsync(context);
228220

221+
// Jet enforces MATCH FULL semantics for composite foreign keys: every column must be null
222+
// or every column must be non-null and satisfy the reference. SQL Server (and EF Core's
223+
// test suite) uses MATCH SIMPLE, where any null column bypasses the constraint.
224+
// Optional relationships can produce a mixed-null FK state (one column null, one not),
225+
// which is valid under MATCH SIMPLE but rejected by Jet, so those composite FK constraints
226+
// are dropped here. Required and owned relationships are always fully null or fully non-null
227+
// (owned FK columns are PK components and therefore non-nullable), so those FKs are kept.
229228
await context.Database.ExecuteSqlAsync($"ALTER TABLE `OptionalComposite2` DROP CONSTRAINT `FK_OptionalComposite2_OptionalAk1_ParentId_ParentAlternateId`");
230229
await context.Database.ExecuteSqlAsync($"ALTER TABLE `OptionalOverlapping2` DROP CONSTRAINT `FK_OptionalOverlapping2_RequiredComposite1_ParentId_ParentAlter~`");
231230
await context.Database.ExecuteSqlAsync($"ALTER TABLE `OptionalSingleComposite2` DROP CONSTRAINT `FK_OptionalSingleComposite2_OptionalSingleAk1_BackId_ParentAlte~`");
232-
await context.Database.ExecuteSqlAsync($"ALTER TABLE `OwnedOptional2` DROP CONSTRAINT `FK_OwnedOptional2_OwnedOptional1_OwnedOptional1OwnerRootId_Owne~`");
233-
await context.Database.ExecuteSqlAsync($"ALTER TABLE `OwnedRequired2` DROP CONSTRAINT `FK_OwnedRequired2_OwnedRequired1_OwnedRequired1OwnerRootId_Owne~`");
234-
await context.Database.ExecuteSqlAsync($"ALTER TABLE `RequiredComposite2` DROP CONSTRAINT `FK_RequiredComposite2_RequiredAk1_ParentId_ParentAlternateId`");
235231
await context.Database.ExecuteSqlAsync($"ALTER TABLE `SharedFkParent` DROP CONSTRAINT `FK_SharedFkParent_SharedFkDependant_RootId_DependantId`");
236232
}
237233
}

0 commit comments

Comments
 (0)