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

Support for expressions when configuring composite keys #33114

Closed
dylanbeattie opened this issue Feb 16, 2024 · 8 comments
Closed

Support for expressions when configuring composite keys #33114

dylanbeattie opened this issue Feb 16, 2024 · 8 comments
Labels
area-model-building closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. customer-reported type-enhancement

Comments

@dylanbeattie
Copy link

dylanbeattie commented Feb 16, 2024

EF Core currently doesn't support configuring composite keys using expressions.

For a "normal" (atomic?) key, you can specify:

modelBuilder.Entity<Customer>().HasKey(c => c.AccountNumber);

or

modelBuilder.Entity<Customer>().HasKey("AccountNumber"); 

Consider this schema: a Venue is defined by a regular Id. A Show is defined by the combination of a Venue and a ShowDate (can't have two shows at the same venue on the same day). Each Show can have zero or more numbered SupportSlot entities, so each SupportSlot is identifed by (show, slotNumber) -- but show is identified by (venue, date) and venue is identified by Id

Currently, the only way to configure this composite key is via string column names:

modelBuilder.Entity<SupportSlot>().HasKey("ShowVenueId", "ShowDate", "SlotNumber");

What I'd like to do is:

modelBuilder.Entity<SupportSlot>().HasKey(
  slot => slot.Show.Venue.Id,
  slot => slot.Show.Date,
  slot => slot.SlotNumber
);

This isn't natively supported in EF Core, but I have an extension method which I use for this on my own projects:

public static class EntityTypeBuilderExtensions {

	public static KeyBuilder<TEntity> HasKey<TEntity>(
		this EntityTypeBuilder<TEntity> builder,
		params Expression<Func<TEntity, object?>>[] keyExpressions
	) where TEntity : class =>
		builder.HasKey(keyExpressions.Select(ColumnName).ToArray());

	public static string ColumnName<TEntity>(Expression<Func<TEntity, object?>> expr) =>
		expr.Body switch {
			MemberExpression mx => ColumnName(mx),
			UnaryExpression { Operand: MemberExpression mx } => ColumnName(mx),
			_ => throw new("Only member expressions are supported")
		};

	private static string ColumnName(Expression? expr) {
		if (expr is not MemberExpression mx) return String.Empty;
		return ColumnName(mx.Expression) + mx.Member.Name;
	}
}

Would there be interest in a PR to add this code to EF Core?

@stevendarby
Copy link
Contributor

You can configure composite keys like this

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.Entity<Car>()
        .HasKey(c => new { c.State, c.LicensePlate });
}

Are you asking for something different to that?

@dylanbeattie
Copy link
Author

Sorry, I could have explained this more clearly. I'm specifically talking about the scenario where the composite key includes columns which are part of a foreign key relationship, so the key properties are actually members of a related entity.

Given these entities:

public class Venue {
	public Guid Id { get; set; }
	public string Name { get; set; } = String.Empty;
}

public class Artist {
	public Guid Id { get; set; }
	public string Name { get; set; } = String.Empty;
}

public class Show {
	public Venue Venue { get; set; } = default!;
	public DateOnly Date { get; set; }
	public Artist HeadlineArtist { get; set; } = default!;
	public List<SupportSlot> SupportSlots { get; set; } = [];
}

The primary key for Show in the database here should be (VenueId, Date), but using the anonymous object syntax:

modelBuilder.Entity<Show>().HasKey(show => new { show.Venue.Id, show.Date });

throws an ArgumentException:

System.ArgumentException: The expression 'show => new <>f__AnonymousType1`2(Id = show.Venue.Id, Date = show.Date)' is not a valid member access expression. The expression should represent a simple property or field access: 't => t.MyProperty'. When specifying multiple properties or fields, use an anonymous type: 't => new { t.MyProperty, t.MyField }'.

To extend it one step further, the SupportSlot:

public class SupportSlot {
	public Show Show { get; set; } = default!;
	public int SlotNumber { get; set; }
	public Artist Artist { get; set; } = default!;
}

In this example, SupportSlot should be created as a table with a three-column composite key (ShowVenueId, ShowDate, SlotNumber), where the (ShowVenueId, ShowDate) tuple is both part of the composite key, and the foreign key relationship identifying which Show this SupportSlot belongs to.

Using the anonymous object syntax:

modelBuilder.Entity<SupportSlot>().HasKey(slot => new { slot.Show.Venue.Id, slot.Show.Date, slot.SlotNumber });

throws an exception:

System.ArgumentException: The expression 'slot => new <>f__AnonymousType1`3(Id = slot.Show.Venue.Id, Date = slot.Show.Date, SlotNumber = slot.SlotNumber)' is not a valid member access expression. The expression should represent a simple property or field access: 't => t.MyProperty'. When specifying multiple properties or fields, use an anonymous type: 't => new { t.MyProperty, t.MyField }'. (Parameter 'memberAccessExpression')

because it doesn't translate the expression slot.Show.Venue.Id into the column name ShowVenueId.

Consequently, the only way (as far as I can see) to handle this kind of key in EF Core is to use string column names - but I don't see any reason it couldn't support using indirect member access expressions, either via the slot => new { slot.Show.Venue.Id, slot.Show.Date, slot.SlotNumber } syntax, or via the HasKey(slot => slot.Show.Venue.Id, slot => slot.Show.Date, slot => slot.SlotNumber) syntax.

Hence the question: would there be interest in a PR adding support for either/both of these syntaxes?

Thanks :)

@roji
Copy link
Member

roji commented Feb 17, 2024

I'm specifically talking about the scenario where the composite key includes columns which are part of a foreign key relationship, so the key properties are actually members of a related entity.

I'm not following here: is Venue mapped to a different table? If so, how can its Id column (in table Venus) be part of the primary key of Show (in table Shows)? A composite key can only reference columns on the table on which it is defined.

If Venue is mapped to the same table as Shows - as an owned entity or cmoplex type - then this is a variation on #11336 (which is about doing the same for indexes).

@dylanbeattie
Copy link
Author

dylanbeattie commented Feb 17, 2024

Each entity here is mapped to its own table; I'm not using owned entities or complex types. The DB schema diagram looks like this:

image

A composite key can only reference columns on the table on which it is defined.

Yes, but those columns can be part of a foreign key. Every Show must be associated with exactly one Venue, so the table Shows has a column VenueId, which exists in the database but is only present in the EF model as a shadow property. Likewise, a SupportSlot is associated with exactly one Show, so in the database, SupportSlots has ShowVenueId and ShowDate columns, but in the EF model, there's only the SupportSlot.Show property; the show venue ID and show date are shadow properties.

I've created a complete repro which might illustrate the issue more clearly; the DB schema diagram above is the actual database created by running this code:

https://github.com/dylanbeattie/efcore-composite-key-examples

For what it's worth, EF Core is doing exactly the right thing here: the database schema is correct, the foreign key relationships are correct; my frustration is with having to rely on string column names to make this happen:

modelBuilder.Entity<Show>().HasKey("VenueId", "Date");
modelBuilder.Entity<SupportSlot>().HasKey("ShowVenueId", "ShowDate", "SlotNumber");

I'm proposing extending HasKey to support indirect property expressions when specifying key columns so we don't have to rely on string column names to configure this behaviour.

@ajcvickers
Copy link
Member

@dylanbeattie I'm trying to understand how this would work. Can you write an example of the complete OnModelCreating code that you would like to write to map the types above to the schema shown. I've tried to put the fragments above together, but I can't make sense of them.

@dylanbeattie
Copy link
Author

Sure - the code I'd like to be able to write is either of the following syntaxes:

Syntax option 1: multiple expressions

protected override void OnModelCreating(ModelBuilder modelBuilder) {
  base.OnModelCreating(modelBuilder);

  modelBuilder.Entity<Show>().HasKey(
    show => show.Venue.Id, 
    show => show.Date
  );

  modelBuilder.Entity<SupportSlot>().HasKey(
    slot => slot.Show.Venue.Id, 
    slot => slot.Show.Date, 
    slot => slot.SlotNumber
  );

  modelBuilder.Entity<Show>()
    .HasMany(show => show.SupportSlots)
    .WithOne(slot => slot.Show).OnDelete(DeleteBehavior.Cascade);

  modelBuilder.Entity<Artist>()
    .HasMany(artist => artist.HeadlineShows)
    .WithOne(slot => slot.HeadlineArtist).OnDelete(DeleteBehavior.SetNull);
  
    modelBuilder.Entity<Artist>()
    .HasMany(artist => artist.SupportSlots)
    .WithOne(slot => slot.Artist).OnDelete(DeleteBehavior.SetNull);}

Syntax option 2: anonymous type with multiple expressions

protected override void OnModelCreating(ModelBuilder modelBuilder) {
  base.OnModelCreating(modelBuilder);

  modelBuilder.Entity<Show>().HasKey(show => new { 
    show.Venue.Id, 
    show.Date 
  });
  
  modelBuilder.Entity<SupportSlot>().HasKey(slot => new { 
    slot.Show.Venue.Id, 
    slot.Show.Date, 
    slot.SlotNumber 
  });

  modelBuilder.Entity<Show>()
    .HasMany(show => show.SupportSlots)
    .WithOne(slot => slot.Show).OnDelete(DeleteBehavior.Cascade);

  modelBuilder.Entity<Artist>()
    .HasMany(artist => artist.HeadlineShows)
    .WithOne(slot => slot.HeadlineArtist).OnDelete(DeleteBehavior.SetNull);
  
    modelBuilder.Entity<Artist>()
    .HasMany(artist => artist.SupportSlots)
    .WithOne(slot => slot.Artist).OnDelete(DeleteBehavior.SetNull);
}

@roji
Copy link
Member

roji commented Mar 4, 2024

  modelBuilder.Entity<Show>().HasKey(show => new { 
    show.Venue.Id, 
    show.Date 
  });

Are you basically asking to be able to define a key column on Shows - which happens to also be a foreign key pointing to Values - by referencing the column on Values which that foreign key is pointing to? For one thing, what happens if there are more than one columns on Show pointing to the same key column on Venue, how would we know which one to resolve it to?

Even without that problem, it seems very odd (and confusing) to me to use a destination key property as a way to configure the source foreign key. If all this is only to avoid using string property names, you always have the option of having a CLR property for the foreign key, at which point you can simply use that, just like you're using the key itself.

@AndriySvyryd
Copy link
Member

I agree with @roji

If the purpose of this proposal is to avoid depending on the FK property names generated by EF than it can be accomplished by explicitly configuring them before configuring the PK.

@AndriySvyryd AndriySvyryd closed this as not planned Won't fix, can't repro, duplicate, stale Mar 15, 2024
@AndriySvyryd AndriySvyryd removed their assignment Mar 15, 2024
@AndriySvyryd AndriySvyryd added the closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. label Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-model-building closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. customer-reported type-enhancement
Projects
None yet
Development

No branches or pull requests

5 participants