Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify configuration for temporal tables with owned types #29303

Open
ajcvickers opened this issue Oct 9, 2022 · 4 comments
Open

Simplify configuration for temporal tables with owned types #29303

ajcvickers opened this issue Oct 9, 2022 · 4 comments

Comments

@ajcvickers
Copy link
Member

Ideally, of the owning entity type is configured as temporal, then owned entities mapped to the same table should not need additional configuration to make use of the temporal table, with the same PeriodStart and PeriodEnd columns.

For example, this configuration should be sufficient:

modelBuilder
    .Entity<Employee>()
    .ToTable("Employees", tableBuilder => tableBuilder.IsTemporal())
    .OwnsOne(employee => employee.Info);

But throws:

Entity type 'EmployeeInfo' should be marked as temporal because it shares table mapping with another entity that has been marked as temporal. Alternatively, other entity types that share the same table must be non-temporal.

Attempting to fix this:

modelBuilder
    .Entity<Employee>()
    .ToTable("Employees", tableBuilder => tableBuilder.IsTemporal())
    .OwnsOne(employee => employee.Info)
    .ToTable("Employees", tableBuilder => tableBuilder.IsTemporal());

Results in:

When multiple temporal entities are mapped to the same table, their period start properties must map to the same column. Issue happens for entity type 'EmployeeInfo' with period property 'PeriodStart' which is mapped to column 'EmployeeInfo_PeriodStart'. Ex
pected period column name is 'PeriodStart'.

Attempting the set the column name explicit on one or other type doesn't help. So the simplest config that I can come up with that works is:

modelBuilder
    .Entity<Employee>()
    .ToTable(
        "Employees",
        tableBuilder =>
        {
            tableBuilder.IsTemporal();
            tableBuilder.Property<DateTime>("PeriodStart").HasColumnName("PeriodStart");
            tableBuilder.Property<DateTime>("PeriodEnd").HasColumnName("PeriodEnd");
        })
    .OwnsOne(
        employee => employee.Info,
        ownedBuilder => ownedBuilder.ToTable(
            "Employees",
            tableBuilder =>
            {
                tableBuilder.IsTemporal();
                tableBuilder.Property<DateTime>("PeriodStart").HasColumnName("PeriodStart");
                tableBuilder.Property<DateTime>("PeriodEnd").HasColumnName("PeriodEnd");
            }));
@parvic1
Copy link

parvic1 commented May 10, 2023

Any idea if this is being considered on coming releases?

@parvic1
Copy link

parvic1 commented May 12, 2023

this worked for me:
builder.ToTable($"{nameof(SampleTestException)}s", t =>
{
t.IsTemporal(t1 =>
{
t1.UseHistoryTable($"{nameof(SampleTestException)}s_Audit");
t1.HasPeriodStart("ValidFrom");
t1.HasPeriodEnd("ValidTo");
});
t.Property("ValidFrom").HasColumnName("ValidFrom");
t.Property("ValidTo").HasColumnName("ValidTo");
}).HasKey(x => x.Id);

    builder.OwnsOne(p => p.Fraction1, ownedBilder =>
    {
        ownedBilder.Property(pp => pp.Name).HasMaxLength(50).IsUnicode(false);
        ownedBilder.Property(pp => pp.Value).HasColumnType("real");
        ownedBilder.Property(pp => pp.MinRangeValue).HasColumnType("real");
        ownedBilder.Property(pp => pp.MaxRangeValue).HasColumnType("real");

        ownedBilder.ToTable($"{nameof(SampleTestException)}s", tbuilder =>
        {
            tbuilder.IsTemporal(t1 =>
            {
                t1.UseHistoryTable($"{nameof(SampleTestException)}s_Audit");
                t1.HasPeriodStart("ValidFrom");
                t1.HasPeriodEnd("ValidTo");
            });
            tbuilder.Property<DateTime>("ValidFrom").HasColumnName("ValidFrom");
            tbuilder.Property<DateTime>("ValidTo").HasColumnName("ValidTo");
        });
    }
    );

@davidhenley
Copy link

davidhenley commented May 12, 2023

Thanks. Mine was a little more complicated and since TemporalTableBuilder doesn't have HasColumnType I had to explicitly set the column to datetime2 on the main entity.

These are all things that could be simplified.

@PhilippeWaeber
Copy link

Having encountered this issue, I've use @ajcvickers approach, explained in the initial post and made a generic, reusable method:

private static void ConfigureOwnedNavigationProperty<TModel, TDependent>(OwnedNavigationBuilder<TModel, TDependent> ownedNavigationBuilder)
    where TModel : class
    where TDependent : class
{
    ownedNavigationBuilder.ToTable($"{typeof(TModel).Name}s",
                                   tableBuilder =>
                                   {
                                       tableBuilder.IsTemporal();

                                       tableBuilder.Property<DateTime>(PeriodStart).HasColumnName(PeriodStart);
                                       tableBuilder.Property<DateTime>(PeriodEnd).HasColumnName(PeriodEnd);
                                   });
}

IMO it makes it easier to use and reuse with e.g. this localized description

builder.OwnsOne(product => product.Description, ConfigureOwnedNavigationProperty);

The entity itself, must also be registered as temporal with the same period start/stop columns:

private static void ConfigureBase<TModel>(EntityTypeBuilder<TModel> builder)
    where TModel : BaseModel
{
    builder.HasKey(model => model.Id);

    builder.ToTable($"{typeof(TModel).Name}s",
                    tableBuilder =>
                    {
                        tableBuilder.IsTemporal();

                        tableBuilder.Property<DateTime>(PeriodStart).HasColumnName(PeriodStart);
                        tableBuilder.Property<DateTime>(PeriodEnd).HasColumnName(PeriodEnd);
                    });

    builder.Property(model => model.Id)
           .ValueGeneratedOnAdd();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants