Skip to content

Commit

Permalink
finish ImplementedByAll multi Id
Browse files Browse the repository at this point in the history
  • Loading branch information
@olmobrutall
olmobrutall committed Apr 5, 2022
1 parent 294c8ec commit 51321d1
Show file tree
Hide file tree
Showing 20 changed files with 277 additions and 162 deletions.
2 changes: 1 addition & 1 deletion Signum.Engine.Extensions/Cache/CachedTable.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ public CachedTable(ICacheLogicController controller, AliasGenerator? aliasGenera
{
object obj = rowReader(fr);
result[idGetter(obj)] = obj; //Could be repeated joins
});
});
tr.Commit();
}
Expand Down
12 changes: 7 additions & 5 deletions Signum.Engine.Extensions/Cache/CachedTableConstructor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,19 +140,21 @@ public Expression MaterializeField(Field field)

if (field is FieldImplementedByAll iba)
{
Expression id = GetTupleProperty(iba.Column);
Expression id = iba.IdColumns.Values
.Select(c => (Expression)Expression.Convert(GetTupleProperty(c), typeof(IComparable)))
.Aggregate((a, b) => Expression.Coalesce(a, b));
Expression typeId = GetTupleProperty(iba.TypeColumn);

if (isLite)
{
var liteCreate = Expression.Call(miGetIBALite.MakeGenericMethod(field.FieldType.CleanType()),
Expression.Constant(Schema.Current),
NewPrimaryKey(typeId.UnNullify()),
id.UnNullify());
id);

var liteRequest = Expression.Call(retriever, miRequestLite.MakeGenericMethod(Lite.Extract(field.FieldType)!), liteCreate);

return Expression.Condition(Expression.NotEqual(WrapPrimaryKey(id), NullId), liteRequest, nullRef);
return Expression.Condition(Expression.NotEqual(typeId.Nullify(), Expression.Constant(null, iba.TypeColumn.Type.Nullify())), liteRequest, nullRef);
}
else
{
Expand Down Expand Up @@ -311,11 +313,11 @@ internal static Expression WrapPrimaryKey(Expression expression)


static MethodInfo miGetIBALite = ReflectionTools.GetMethodInfo((Schema s) => GetIBALite<Entity>(null!, 1, "")).GetGenericMethodDefinition();
public static Lite<T> GetIBALite<T>(Schema schema, PrimaryKey typeId, string id) where T : Entity
public static Lite<T> GetIBALite<T>(Schema schema, PrimaryKey typeId, IComparable id) where T : Entity
{
Type type = schema.GetType(typeId);

return (Lite<T>)Lite.Create(type, PrimaryKey.Parse(id, type));
return (Lite<T>)Lite.Create(type, new PrimaryKey(id));
}

public static MemberExpression peModified = Expression.Property(retriever, ReflectionTools.GetPropertyInfo((IRetriever me) => me.ModifiedState));
Expand Down
3 changes: 2 additions & 1 deletion Signum.Engine/BulkInserter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ public static int BulkInsertTable<T>(IEnumerable<T> entities,
if (!e.IsNew)
throw new InvalidOperationException("Entites should be new");
t.SetToStrField(e);
dt.Rows.Add(t.BulkInsertDataRow(e));
var values = t.BulkInsertDataRow(e);
dt.Rows.Add(values);
}
}

Expand Down
69 changes: 57 additions & 12 deletions Signum.Engine/Engine/SchemaSynchronizer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,14 @@ public static class SchemaSynchronizer
{
var key = Replacements.KeyColumnsForTable(tn);
//START IBA Migration 2022.04.04
var newIBAs = tab.Columns.OfType<ImplementedByAllIdColumn>().Where(a => !diff.Columns.ContainsKey(a.Name) && diff.Columns.ContainsKey(a.Name.BeforeLast("_"))).Select(a => a.Name).ToList();
var oldIBAs = diff.Columns.Values.Where(c => !tab.Columns.ContainsKey(c.Name) && Schema.Current.Settings.ImplementedByAllPrimaryKeyTypes.All(t => tab.Columns.TryGetC(c.Name + "_" + t.Name) is ImplementedByAllIdColumn)).Select(a => a.Name).ToList();
//END
replacements.AskForReplacements(
diff.Columns.Keys.ToHashSet(),
tab.Columns.Keys.ToHashSet(), key);
diff.Columns.Keys.Except(oldIBAs).ToHashSet(),
tab.Columns.Keys.Except(newIBAs).ToHashSet(), key);
var incompatibleTypes = diff.Columns.JoinDictionary(tab.Columns, (cn, diff, col) => new { cn, diff, col }).Values.Where(a => !a.diff.CompatibleTypes(a.col) || a.diff.Identity != a.col.Identity).ToList();
Expand Down Expand Up @@ -233,15 +238,55 @@ public static class SchemaSynchronizer
tab.Columns,
dif.Columns,
createNew: (cn, tabCol) => SqlPreCommand.Combine(Spacing.Simple,
tabCol.PrimaryKey && dif.PrimaryKeyName != null ? sqlBuilder.DropPrimaryKeyConstraint(tab.Name) : null,
AlterTableAddColumnDefault(sqlBuilder, tab, tabCol, replacements,
forceDefaultValue: cn.EndsWith("_HasValue") && dif.Columns.Values.Any(c => c.Name.StartsWith(cn.Before("HasValue")) && c.Nullable == false) ? "1" : null,
hasValueFalse: hasValueFalse)),
createNew: (cn, tabCol) =>
{
//START IBA Migration 2022.04.04
var settings = Schema.Current.Settings;
var isNewImplementedIBAColumn = tabCol is ImplementedByAllIdColumn && settings.ImplementedByAllPrimaryKeyTypes.Any(t =>
{
var before = tabCol.Name.TryBefore("_" + t.Name);
return before != null && dif.Columns.ContainsKey(before) && !tab.Columns.ContainsKey(before);
});
//END
var result = SqlPreCommand.Combine(Spacing.Simple,
tabCol.PrimaryKey && dif.PrimaryKeyName != null ? sqlBuilder.DropPrimaryKeyConstraint(tab.Name) : null,
AlterTableAddColumnDefault(sqlBuilder, tab, tabCol, replacements,
forceDefaultValue: cn.EndsWith("_HasValue") && dif.Columns.Values.Any(c => c.Name.StartsWith(cn.Before("HasValue")) && c.Nullable == false) ? "1" : null,
hasValueFalse: hasValueFalse,
avoidDefault: isNewImplementedIBAColumn && tabCol.Nullable == IsNullable.Forced));
return result;
},
removeOld: (cn, difCol) =>
{
removeOld: (cn, difCol) => SqlPreCommand.Combine(Spacing.Simple,
difCol.DefaultConstraint != null && difCol.DefaultConstraint.Name != null ? sqlBuilder.AlterTableDropConstraint(tab.Name, difCol.DefaultConstraint!.Name) : null,
sqlBuilder.AlterTableDropColumn(tab, cn)),
var result = SqlPreCommand.Combine(Spacing.Simple,
difCol.DefaultConstraint != null && difCol.DefaultConstraint.Name != null ? sqlBuilder.AlterTableDropConstraint(tab.Name, difCol.DefaultConstraint!.Name) : null,
sqlBuilder.AlterTableDropColumn(tab, cn));
//START IBA Migration 2022.04.04
if (difCol.DbType.IsString())
{
var settings = Schema.Current.Settings;
var update = (from t in settings.ImplementedByAllPrimaryKeyTypes
let c = tab.Columns.TryGetC(difCol.Name + "_" + t.Name)
where c is ImplementedByAllIdColumn
select new SqlPreCommandSimple($"UPDATE {tab.Name} SET {c.Name} = TRY_CONVERT({settings.GetSqlDbTypePair(t).DbType}, {difCol.Name})")).Combine(Spacing.Simple);
if (update != null)
{
update.GoBefore = true;
return SqlPreCommandSimple.Combine(Spacing.Double, update, result);
}
}
//END
return result;
},
mergeBoth: (cn, tabCol, difCol) =>
{
Expand Down Expand Up @@ -539,9 +584,9 @@ private static bool DifferentDatabase(ObjectName name, ObjectName name2)

public static Func<SchemaName, bool> IgnoreSchema = s => s.Name.Contains("\\");

private static SqlPreCommand AlterTableAddColumnDefault(SqlBuilder sqlBuilder, ITable table, IColumn column, Replacements rep, string? forceDefaultValue, HashSet<FieldEmbedded.EmbeddedHasValueColumn> hasValueFalse)
private static SqlPreCommand AlterTableAddColumnDefault(SqlBuilder sqlBuilder, ITable table, IColumn column, Replacements rep, string? forceDefaultValue, bool avoidDefault, HashSet<FieldEmbedded.EmbeddedHasValueColumn> hasValueFalse)
{
if (column.Nullable == IsNullable.Yes || column.Identity || column.Default != null || column is ImplementationColumn)
if (column.Nullable == IsNullable.Yes || column.Identity || column.Default != null || column is ImplementationColumn || avoidDefault)
return sqlBuilder.AlterTableAddColumn(table, column);

if (column.Nullable == IsNullable.Forced)
Expand Down
45 changes: 28 additions & 17 deletions Signum.Engine/Linq/DbExpressions.Signum.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ public override string ToString()
ExternalId.ToString());

return constructor +
(Bindings == null ? null : ("\r\n{\r\n " + Bindings.ToString(",\r\n ").Indent(4) + "\r\n}")) +
(Mixins == null ? null : ("\r\n" + Mixins.ToString(m => ".Mixin({0})".FormatWith(m), "\r\n")));
(Bindings == null ? null : ("\n{\n " + Bindings.ToString(",\n ").Indent(4) + "\n}")) +
(Mixins == null ? null : ("\n" + Mixins.ToString(m => ".Mixin({0})".FormatWith(m), "\n")));
}

public Expression GetBinding(FieldInfo fi)
Expand Down Expand Up @@ -131,10 +131,10 @@ public override string ToString()
{
string constructor = "new {0}".FormatWith(Type.TypeName());

string bindings = Bindings?.Let(b => b.ToString(",\r\n ")) ?? "";
string bindings = Bindings?.Let(b => b.ToString(",\n ")) ?? "";

return bindings.HasText() ?
constructor + "\r\n{" + bindings.Indent(4) + "\r\n}" :
constructor + "\n{" + bindings.Indent(4) + "\n}" :
constructor;
}

Expand Down Expand Up @@ -197,10 +197,10 @@ public override string ToString()
{
string constructor = "new {0}".FormatWith(Type.TypeName());

string bindings = Bindings?.Let(b => b.ToString(",\r\n ")) ?? "";
string bindings = Bindings?.Let(b => b.ToString(",\n ")) ?? "";

return bindings.HasText() ?
constructor + "\r\n{" + bindings.Indent(4) + "\r\n}" :
constructor + "\n{" + bindings.Indent(4) + "\n}" :
constructor;
}

Expand Down Expand Up @@ -251,8 +251,8 @@ public ImplementedByExpression(Type type, CombineStrategy strategy, IDictionary<

public override string ToString()
{
return "ImplementedBy({0}){{\r\n{1}\r\n}}".FormatWith(Strategy,
Implementations.ToString(kvp => "{0} -> {1}".FormatWith(kvp.Key.TypeName(), kvp.Value.ToString()), "\r\n").Indent(4)
return "ImplementedBy({0}){{\n{1}\n}}".FormatWith(Strategy,
Implementations.ToString(kvp => "{0} -> {1}".FormatWith(kvp.Key.TypeName(), kvp.Value.ToString()), "\n").Indent(4)
);
}

Expand All @@ -269,20 +269,22 @@ internal class ImplementedByAllExpression : DbExpression
public readonly IntervalExpression? ExternalPeriod;


public ImplementedByAllExpression(Type type, ReadOnlyDictionary<Type/*PrimaryKey type*/, Expression> ids, TypeImplementedByAllExpression typeId, IntervalExpression? externalPeriod)
public ImplementedByAllExpression(Type type, IDictionary<Type/*PrimaryKey type*/, Expression> ids, TypeImplementedByAllExpression typeId, IntervalExpression? externalPeriod)
: base(DbExpressionType.ImplementedByAll, type)
{
if (ids == null)
throw new ArgumentNullException(nameof(ids));

this.Ids = ids;
this.Ids = ids.ToReadOnly();
this.TypeId = typeId ?? throw new ArgumentNullException(nameof(typeId));
this.ExternalPeriod = externalPeriod;
}

public override string ToString()
{
return "ImplementedByAll{{ Ids = {0}, Type = {1} }}".FormatWith(Ids, TypeId);
return "ImplementedByAll{{\n Ids = {0},\n Type = {1}\n}}".FormatWith(
Ids.ToString(kvp => "{0} -> {1}".FormatWith(kvp.Key.TypeName(), kvp.Value.ToString()), "\n"),
TypeId);
}

protected override Expression Accept(DbExpressionVisitor visitor)
Expand Down Expand Up @@ -345,11 +347,11 @@ internal LiteReferenceExpression WithExpandLite(ExpandLite expandLite)
internal class LiteValueExpression : DbExpression
{
public readonly Expression TypeId;
public readonly Expression Id;
public readonly PrimaryKeyExpression Id;
public readonly Expression? ToStr;


public LiteValueExpression(Type type, Expression typeId, Expression id, Expression? toStr) :
public LiteValueExpression(Type type, Expression typeId, PrimaryKeyExpression id, Expression? toStr) :
base(DbExpressionType.LiteValue, type)
{
this.TypeId = typeId ?? throw new ArgumentNullException(nameof(typeId));
Expand All @@ -368,7 +370,15 @@ protected override Expression Accept(DbExpressionVisitor visitor)
}
}

internal class TypeEntityExpression : DbExpression
internal abstract class TypeDbExpression : DbExpression
{
public TypeDbExpression(DbExpressionType dbType, Type type)
: base(dbType, type)
{
}
}

internal class TypeEntityExpression : TypeDbExpression
{
public readonly PrimaryKeyExpression ExternalId;
public readonly Type TypeValue;
Expand All @@ -391,7 +401,7 @@ protected override Expression Accept(DbExpressionVisitor visitor)
}
}

internal class TypeImplementedByExpression : DbExpression
internal class TypeImplementedByExpression : TypeDbExpression
{
public readonly ReadOnlyDictionary<Type, PrimaryKeyExpression> TypeImplementations;

Expand All @@ -415,7 +425,8 @@ protected override Expression Accept(DbExpressionVisitor visitor)
}
}

internal class TypeImplementedByAllExpression : DbExpression

internal class TypeImplementedByAllExpression : TypeDbExpression
{
public readonly PrimaryKeyExpression TypeColumn;

Expand Down Expand Up @@ -539,7 +550,7 @@ public MListElementExpression(PrimaryKeyExpression rowId, EntityExpression paren

public override string ToString()
{
return "MListElement({0})\r\n{{\r\nParent={1},\r\nOrder={2},\r\nElement={3}}})".FormatWith(
return "MListElement({0})\n{{\nParent={1},\nOrder={2},\nElement={3}}})".FormatWith(
RowId.ToString(),
Parent.ToString(),
Order?.ToString(),
Expand Down
7 changes: 2 additions & 5 deletions Signum.Engine/Linq/ExpressionVisitor/DbExpressionNominator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ protected internal override Expression VisitLiteReference(LiteReferenceExpressio
if (lite.Reference is ImplementedByExpression ib)
return Add(GetImplmentedById(ib));
if (lite.Reference is ImplementedByAllExpression iba)
return Add(iba.Id);
return Add(iba.TypeId.TypeColumn.Value);
}

return base.VisitLiteReference(lite);
Expand Down Expand Up @@ -187,7 +187,7 @@ protected internal override Expression VisitTypeImplementedBy(TypeImplementedByE
protected internal override Expression VisitImplementedByAll(ImplementedByAllExpression iba)
{
if (iba == isNotNullRoot)
return Add(iba.Id);
return Add(iba.TypeId.TypeColumn.Value);

return base.VisitImplementedByAll(iba);
}
Expand Down Expand Up @@ -1416,9 +1416,6 @@ protected override Expression VisitMember(MemberExpression m)
if (exp is UnaryExpression ue)
exp = ue.Operand;

if (exp is PrimaryKeyStringExpression)
return null;

var pk = (PrimaryKeyExpression)exp;

return Visit(pk.Value);
Expand Down
8 changes: 4 additions & 4 deletions Signum.Engine/Linq/ExpressionVisitor/DbExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ protected internal virtual Expression VisitLiteReference(LiteReferenceExpression
protected internal virtual Expression VisitLiteValue(LiteValueExpression lite)
{
var newTypeId = Visit(lite.TypeId);
var newId = Visit(lite.Id);
var newId = (PrimaryKeyExpression)Visit(lite.Id);
var newToStr = Visit(lite.ToStr);
if (newTypeId != lite.TypeId || newId != lite.Id || newToStr != lite.ToStr)
return new LiteValueExpression(lite.Type, newTypeId, newId, newToStr);
Expand Down Expand Up @@ -205,12 +205,12 @@ protected internal virtual Expression VisitColumn(ColumnExpression column)

protected internal virtual Expression VisitImplementedByAll(ImplementedByAllExpression iba)
{
var id = Visit(iba.Id);
var ids = Visit(iba.Ids, v => Visit(v));
var typeId = (TypeImplementedByAllExpression)Visit(iba.TypeId);
var externalPeriod = (IntervalExpression?)Visit(iba.ExternalPeriod);

if (id != iba.Id || typeId != iba.TypeId || externalPeriod != iba.ExternalPeriod)
return new ImplementedByAllExpression(iba.Type, id, typeId, externalPeriod);
if (ids != iba.Ids || typeId != iba.TypeId || externalPeriod != iba.ExternalPeriod)
return new ImplementedByAllExpression(iba.Type, ids, typeId, externalPeriod);
return iba;
}

Expand Down
5 changes: 1 addition & 4 deletions Signum.Engine/Linq/ExpressionVisitor/EntityCompleter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,7 @@ protected internal override Expression VisitLiteReference(LiteReferenceExpressio
if (lite.EagerEntity)
return base.VisitLiteReference(lite);

var id = lite.Reference is ImplementedByAllExpression ||
lite.Reference is ImplementedByExpression ib && ib.Implementations.Select(imp=>imp.Value.ExternalId.ValueType.Nullify()).Distinct().Count() > 1 ?
(Expression)binder.GetIdString(lite.Reference) :
(Expression)binder.GetId(lite.Reference);
var id = binder.GetId(lite.Reference);

var typeId = binder.GetEntityType(lite.Reference);
var toStr = LiteToString(lite, typeId);
Expand Down
Loading

1 comment on commit 51321d1

@olmobrutall
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ImplementedByAll with multiple ID columns

ImplementedByAllAttribute enables a entity or lite reference to point to any entity in the database.

[EntityKind(EntityKind.System, EntityData.Master), TicksColumn(false)]
public class ChartColorEntity : Entity
{
    [ImplementedByAll, UniqueIndex]
    public Lite<Entity> Related { get; set; }

    [StringLengthValidator(Max = 100), NotNullValidator(DisabledInModelBinder = true)]
    public string Color { get; set; }
}

Old Database Implementation

Of course you can not make a FK to every table, so traditionally it was implemented by having two columns:

  • RelatedID_Type: A ForeignKey to the types table
  • RelatedID: An untyped Id field.
CREATE TABLE [chart].[ChartColor](
	[ID] [int] IDENTITY(1,1) NOT NULL PRIMARY KEY,
	[ToStr] [nvarchar](200) NULL,
	[RelatedID] [nvarchar](40) NOT NULL,  -- NVARCHAR!!
	[RelatedID_Type] [int] NOT NULL,
	[Color] [nvarchar](100) NOT NULL
)
GO

ALTER TABLE [chart].[ChartColor] ADD  CONSTRAINT [FK_ChartColor_RelatedID_Type] FOREIGN KEY([RelatedID_Type]) REFERENCES [framework].[Type] ([ID])

Originally this Id field was of type int, but once Signum Framework supported other types as primary keys (like 8 years ago) the ID fields of the ImplementedBy all changed to be of type NVARCHAR(40) to accomodate int, GUID, etc...

Of course this has some performance penalty when you try to join tables due to conversions from int / GUID to NVARCHAR(40).

New Database Implementation

This change removes this NVARCHAR(40) column and instead adds one column for each distinct ID type in the database:

CREATE TABLE [chart].[ChartColor](
	[ID] [int] IDENTITY(1,1) NOT NULL,
	[ToStr] [nvarchar](200) NULL,
	[RelatedID_Type] [int] NULL,
	[Color] [nvarchar](100) NOT NULL,
	[RelatedID_Int32] [int] NULL,                     -- for ID inttables
	[RelatedID_Guid] [uniqueidentifier] NULL, -- for ID uniqueidentifier tables
)
GO

ALTER TABLE [chart].[ChartColor] ADD  CONSTRAINT [FK_ChartColor_RelatedID_Type] FOREIGN KEY([RelatedID_Type]) REFERENCES [framework].[Type] ([ID])

The LINQ Provider/Saver/BulkInserter etc has been updated to make this change transparent in your code.

How to upgrade

The list of distinct ID type is not automatically detected by the framework, instead it has to be declared at the beginning of the Starter.Start method.

The default is int:

public class SchemaSettings 
{
      ...
      public List<Type> ImplementedByAllPrimaryKeyTypes = new List<Type> { typeof(int) };
       ...
}

If you happen to add some entity with other primary key type (Guid) like Southwind does:

[PrimaryKey(typeof(Guid))]
public abstract class CustomerEntity : Entity
{
    public AddressEmbedded Address { get; set; }

    [StringLengthValidator(Min = 3, Max = 24), TelephoneValidator]
    public string Phone { get; set; }

    [StringLengthValidator(Min = 3, Max = 24), TelephoneValidator]
    public string? Fax { get; set; }
}

Then an exception will be thrown on SchemaCompleted suggesting to add a line like this one to your Starter.Start method:

sb.Schema.Settings.ImplementedByAllPrimaryKeyTypes.Add(typeof(Guid));

The SchemaSynchronizer has now some hard-coded rules to detect the transition of ImplementedByAll columns and create the required SQL Script, no questions asked.

ALTER TABLE chart.ChartColor ALTER COLUMN [RelatedID_Type] INT NULL;
ALTER TABLE chart.ChartColor ADD [RelatedID_Int32] INT NULL;
ALTER TABLE chart.ChartColor ADD [RelatedID_Guid] UNIQUEIDENTIFIER NULL;
GO
UPDATE chart.ChartColor SET RelatedID_Int32 = TRY_CONVERT(INT, RelatedID)
UPDATE chart.ChartColor SET RelatedID_Guid = TRY_CONVERT(UNIQUEIDENTIFIER, RelatedID)

Enjoy!

Please sign in to comment.