Skip to content

Commit 29c055e

Browse files
committed
Default constraint name PR feedback:
� Add model builder extention methods for other types of properties � Uniquify names using the established pattern in SharedTableConvention � Make GetDefaultConstraintName return the generated default constraint name if UseNamedDefaultConstraints was called
1 parent 745536b commit 29c055e

13 files changed

+404
-192
lines changed

src/EFCore.Relational/Extensions/RelationalModelExtensions.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -537,10 +537,10 @@ public static void SetCollation(this IMutableModel model, string? value)
537537
/// Returns the value indicating whether named default constraints should be used.
538538
/// </summary>
539539
/// <param name="model">The model to get the value for.</param>
540-
public static bool? AreNamedDefaultConstraintsUsed(this IReadOnlyModel model)
540+
public static bool AreNamedDefaultConstraintsUsed(this IReadOnlyModel model)
541541
=> (model is RuntimeModel)
542542
? throw new InvalidOperationException(CoreStrings.RuntimeModelMissingData)
543-
: (bool?)model[RelationalAnnotationNames.UseNamedDefaultConstraints];
543+
: (bool?)model[RelationalAnnotationNames.UseNamedDefaultConstraints] ?? false;
544544

545545
/// <summary>
546546
/// Sets the value indicating whether named default constraints should be used.
@@ -558,7 +558,7 @@ public static void UseNamedDefaultConstraints(this IMutableModel model, bool val
558558
/// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
559559
public static bool? UseNamedDefaultConstraints(
560560
this IConventionModel model,
561-
bool value,
561+
bool? value,
562562
bool fromDataAnnotation = false)
563563
=> (bool?)model.SetOrRemoveAnnotation(
564564
RelationalAnnotationNames.UseNamedDefaultConstraints,

src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2092,20 +2092,42 @@ public static void SetJsonPropertyName(this IMutableProperty property, string? n
20922092
/// </summary>
20932093
/// <param name="property">The property.</param>
20942094
public static string? GetDefaultConstraintName(this IReadOnlyProperty property)
2095-
=> (property is RuntimeProperty)
2095+
=> property is RuntimeProperty
2096+
? throw new InvalidOperationException(CoreStrings.RuntimeModelMissingData)
2097+
: (string?)property[RelationalAnnotationNames.DefaultConstraintName]
2098+
?? (ShouldHaveDefaultConstraintName(property)
2099+
&& StoreObjectIdentifier.Create(property.DeclaringType, StoreObjectType.Table) is StoreObjectIdentifier table
2100+
? property.GenerateDefaultConstraintName(table)
2101+
: null);
2102+
2103+
/// <summary>
2104+
/// Gets the default constraint name.
2105+
/// </summary>
2106+
/// <param name="property">The property.</param>
2107+
/// <param name="storeObject">The store object identifier to generate the name for.</param>
2108+
public static string? GetDefaultConstraintName(this IReadOnlyProperty property, in StoreObjectIdentifier storeObject)
2109+
=> property is RuntimeProperty
20962110
? throw new InvalidOperationException(CoreStrings.RuntimeModelMissingData)
2097-
: (string?)property[RelationalAnnotationNames.DefaultConstraintName];
2111+
: (string?)property[RelationalAnnotationNames.DefaultConstraintName]
2112+
?? (ShouldHaveDefaultConstraintName(property)
2113+
? property.GenerateDefaultConstraintName(storeObject)
2114+
: null);
2115+
2116+
private static bool ShouldHaveDefaultConstraintName(IReadOnlyProperty property)
2117+
=> property.DeclaringType.Model.AreNamedDefaultConstraintsUsed()
2118+
&& (property[RelationalAnnotationNames.DefaultValue] is not null
2119+
|| property[RelationalAnnotationNames.DefaultValueSql] is not null);
20982120

20992121
/// <summary>
21002122
/// Generates the default constraint name based on the table and column name.
21012123
/// </summary>
21022124
/// <param name="property">The property.</param>
2103-
/// <param name="storeObject">The store object identifier to generate the name from.</param>
2125+
/// <param name="storeObject">The store object identifier to generate the name for.</param>
21042126
public static string GenerateDefaultConstraintName(this IReadOnlyProperty property, in StoreObjectIdentifier storeObject)
21052127
{
21062128
var candidate = $"DF_{storeObject.Name}_{property.GetColumnName(storeObject)}";
21072129

2108-
return candidate.Length > 120 ? candidate[..120] : candidate;
2130+
return Uniquifier.Truncate(candidate, property.DeclaringType.Model.GetMaxIdentifierLength());
21092131
}
21102132

21112133
/// <summary>

src/EFCore.Relational/Metadata/Conventions/Infrastructure/RelationalConventionSetBuilder.cs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,6 @@ public override ConventionSet CreateConventionSet()
8989
conventionSet.Replace<RuntimeModelConvention>(
9090
new RelationalRuntimeModelConvention(Dependencies, RelationalDependencies));
9191

92-
var defaultConstraintConvention = new RelationalDefaultConstraintConvention(Dependencies, RelationalDependencies);
93-
ConventionSet.AddAfter(
94-
conventionSet.ModelFinalizingConventions,
95-
defaultConstraintConvention,
96-
typeof(SharedTableConvention));
97-
9892
return conventionSet;
9993
}
10094
}

src/EFCore.Relational/Metadata/Conventions/RelationalDefaultConstraintConvention.cs

Lines changed: 0 additions & 150 deletions
This file was deleted.

src/EFCore.Relational/Metadata/Conventions/SharedTableConvention.cs

Lines changed: 115 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ public virtual void ProcessModelFinalizing(
5151
var foreignKeys = new Dictionary<string, (IConventionForeignKey, StoreObjectIdentifier)>();
5252
var indexes = new Dictionary<string, (IConventionIndex, StoreObjectIdentifier)>();
5353
var checkConstraints = new Dictionary<(string, string?), (IConventionCheckConstraint, StoreObjectIdentifier)>();
54+
var defaultConstraints = new Dictionary<(string, string?), (IConventionProperty, StoreObjectIdentifier)>();
5455
var triggers = new Dictionary<string, (IConventionTrigger, StoreObjectIdentifier)>();
5556
foreach (var ((tableName, schema), conventionEntityTypes) in tables)
5657
{
@@ -76,6 +77,11 @@ public virtual void ProcessModelFinalizing(
7677
checkConstraints.Clear();
7778
}
7879

80+
if (!DefaultConstraintsUniqueAcrossTables)
81+
{
82+
defaultConstraints.Clear();
83+
}
84+
7985
if (!TriggersUniqueAcrossTables)
8086
{
8187
triggers.Clear();
@@ -89,6 +95,7 @@ public virtual void ProcessModelFinalizing(
8995
UniquifyForeignKeyNames(entityType, foreignKeys, storeObject, maxLength);
9096
UniquifyIndexNames(entityType, indexes, storeObject, maxLength);
9197
UniquifyCheckConstraintNames(entityType, checkConstraints, storeObject, maxLength);
98+
UniquifyDefaultConstraintNames(entityType, defaultConstraints, storeObject, maxLength);
9299
UniquifyTriggerNames(entityType, triggers, storeObject, maxLength);
93100
}
94101
}
@@ -124,14 +131,19 @@ protected virtual bool CheckConstraintsUniqueAcrossTables
124131
protected virtual bool TriggersUniqueAcrossTables
125132
=> true;
126133

134+
/// <summary>
135+
/// Gets a value indicating whether default constraint names should be unique across tables.
136+
/// </summary>
137+
protected virtual bool DefaultConstraintsUniqueAcrossTables
138+
=> false;
139+
127140
private static void TryUniquifyTableNames(
128141
IConventionModel model,
129142
Dictionary<(string Name, string? Schema), List<IConventionEntityType>> tables,
130143
int maxLength)
131144
{
132145
Dictionary<(string Name, string? Schema), Dictionary<(string Name, string? Schema), List<IConventionEntityType>>>?
133-
clashingTables
134-
= null;
146+
clashingTables = null;
135147
foreach (var entityType in model.GetEntityTypes())
136148
{
137149
var tableName = entityType.GetTableName();
@@ -646,6 +658,107 @@ protected virtual bool AreCompatible(
646658
return null;
647659
}
648660

661+
private void UniquifyDefaultConstraintNames(
662+
IConventionEntityType entityType,
663+
Dictionary<(string, string?), (IConventionProperty, StoreObjectIdentifier)> defaultConstraints,
664+
in StoreObjectIdentifier storeObject,
665+
int maxLength)
666+
{
667+
foreach (var property in entityType.GetProperties())
668+
{
669+
var constraintName = property.GetDefaultConstraintName(storeObject);
670+
if (constraintName == null)
671+
{
672+
continue;
673+
}
674+
675+
var columnName = property.GetColumnName(storeObject);
676+
if (columnName == null)
677+
{
678+
continue;
679+
}
680+
681+
if (!defaultConstraints.TryGetValue((constraintName, storeObject.Schema), out var otherPropertyPair))
682+
{
683+
defaultConstraints[(constraintName, storeObject.Schema)] = (property, storeObject);
684+
continue;
685+
}
686+
687+
var (otherProperty, otherStoreObject) = otherPropertyPair;
688+
if (storeObject == otherStoreObject
689+
&& columnName == otherProperty.GetColumnName(storeObject)
690+
&& AreCompatibleDefaultConstraints(property, otherProperty, storeObject))
691+
{
692+
continue;
693+
}
694+
695+
var newConstraintName = TryUniquifyDefaultConstraint(property, constraintName, storeObject.Schema, defaultConstraints, storeObject, maxLength);
696+
if (newConstraintName != null)
697+
{
698+
defaultConstraints[(newConstraintName, storeObject.Schema)] = (property, storeObject);
699+
continue;
700+
}
701+
702+
var newOtherConstraintName = TryUniquifyDefaultConstraint(otherProperty, constraintName, storeObject.Schema, defaultConstraints, otherStoreObject, maxLength);
703+
if (newOtherConstraintName != null)
704+
{
705+
defaultConstraints[(constraintName, storeObject.Schema)] = (property, storeObject);
706+
defaultConstraints[(newOtherConstraintName, otherStoreObject.Schema)] = otherPropertyPair;
707+
}
708+
}
709+
}
710+
711+
/// <summary>
712+
/// Gets a value indicating whether two default constraints with the same name are compatible.
713+
/// </summary>
714+
/// <param name="property">A property with a default constraint.</param>
715+
/// <param name="duplicateProperty">Another property with a default constraint.</param>
716+
/// <param name="storeObject">The identifier of the store object.</param>
717+
/// <returns><see langword="true" /> if compatible</returns>
718+
protected virtual bool AreCompatibleDefaultConstraints(
719+
IReadOnlyProperty property,
720+
IReadOnlyProperty duplicateProperty,
721+
in StoreObjectIdentifier storeObject)
722+
=> property.GetDefaultValue(storeObject) == duplicateProperty.GetDefaultValue(storeObject)
723+
&& property.GetDefaultValueSql(storeObject) == duplicateProperty.GetDefaultValueSql(storeObject);
724+
725+
private static string? TryUniquifyDefaultConstraint(
726+
IConventionProperty property,
727+
string constraintName,
728+
string? schema,
729+
Dictionary<(string, string?), (IConventionProperty, StoreObjectIdentifier)> defaultConstraints,
730+
in StoreObjectIdentifier storeObject,
731+
int maxLength)
732+
{
733+
var mappedTables = property.GetMappedStoreObjects(StoreObjectType.Table);
734+
if (mappedTables.Count() > 1)
735+
{
736+
// For TPC and some entity splitting scenarios we end up with multiple tables having to define the constraint.
737+
// Since constraint name has to be unique, we can't keep the same name for all
738+
// Disabling this scenario until we have better way to configure the constraint name
739+
// see issue #27970
740+
if (property.GetDefaultConstraintNameConfigurationSource() == null)
741+
{
742+
throw new InvalidOperationException(
743+
RelationalStrings.ImplicitDefaultNamesNotSupportedForTpcWhenNamesClash(constraintName));
744+
}
745+
else
746+
{
747+
throw new InvalidOperationException(
748+
RelationalStrings.ExplicitDefaultConstraintNamesNotSupportedForTpc(constraintName));
749+
}
750+
}
751+
752+
if (property.Builder.CanSetAnnotation(RelationalAnnotationNames.DefaultConstraintName, null))
753+
{
754+
constraintName = Uniquifier.Uniquify(constraintName, defaultConstraints, n => (n, schema), maxLength);
755+
property.Builder.HasAnnotation(RelationalAnnotationNames.DefaultConstraintName, constraintName);
756+
return constraintName;
757+
}
758+
759+
return null;
760+
}
761+
649762
private void UniquifyTriggerNames(
650763
IConventionEntityType entityType,
651764
Dictionary<string, (IConventionTrigger, StoreObjectIdentifier)> triggers,

0 commit comments

Comments
 (0)