Design meeting notes - January 29, 2015

[ajcvickers]

Key value sentinels

Background

EF has traditionally allowed a key property to be any value other than null. EF never reasoned about keys values--all values were treated equally.

In EF7 we have started treated the default value for the CLR type (default(T); e.g. zero) differently:

• When value generation is used, values are only generated when the property value is not the default
• When store defaults are configured, the store default is only used when the property value is the default

Also, because we do fixup on Add in places we didn't in EF6 default values add additional complexity to avoid unexpected behaviors. For example, new created entities will have default FK values. Care must then be taken not to try to fix these up to tracked entities with default PK values, for example in the transition from default to a real value in value generation.

Proposal

The proposal is to require some value to be a sentinel, defaulting to default(T). The sentinel value can be configured to be something other than default(T)--for example -1. However, it may then be necessary to ensure new objects are created with the invalid sentinel set for FK properties.

This has the following advantages:

• Simplifies code because state manager will never be tracking principal with that value
• Means we don’t need to look-up value generation metadata and behave differently base on it
• Allows any FK to have an invalid value. This means that when a relationship has been severed using navigation properties, this change can be tracked in the entity by setting the FK to the invalid sentinel value. This removes the need to track "conceptually null" FK values as we did in the old stack.

The main disadvantages are:

• It may not be possible to use EF with legacy databases where the entire key space is considered valid. However, it should possible to map such columns to nullable CLR types or we could provide an option that would allow EF to be used with such a database at the expense of some of the higher-level behaviors that make use of the sentinel value.
• The CLR default for enums usually needs to be treated as a valid value. The mitigation for this is to configure the sentinel for enum keys to some other value--for example, -1.

Sentinel values must be consistent across an entire key chain--that is any foreign keys must use the same sentinel as their primary key. However, it is probably simpler to just set the sentinel per property and then rely on model conventions and validation to ensure consistency.

Discussion

Please comment on this issue to provide feedback, ask questions, etc.

[FransBouma]

The minute you start using magic values for something, there will be a moment when that magic value will be given a context other than the one you gave it at the start.

IMHO a key can have any value other than NULL (i.e.: not set), I don't see why there has to be a magic value default(T) (which for string is still null btw) to determine something, as one can do that without that magic value without issues (i.e.: entity new? property set to a value or not? etc.) I don't use this at all in my framework.

[oatkins]

Sometimes when writing a service that permits multiple related records to be added in one operation (e.g. master-detail-subdetail) I've used temporary client generated keys to preserve the relationships across the service call. These were then replaced with the store gernerated values by EF6. I don't know how common this kind of thing is, but it sounds as though it would be broken in EF7.

[ajcvickers]

@oatkins That will still work in EF7.

[popcatalin81]

I work with a large database (1200 tables) where 0s (default(int)) are valid foreign keys to "Not applicable" rows. These were done to avoid to many null conditionals in queries, because data in the rows is localized ("No Applicable-> DE: "Nicht Zutreffend") in translation tables, and also most of the times the FKs are also nullable and allowed. Even more -1,-2,-3 etc, are values given to new objects FKs in disconnected scenarios before the graph is attached and saved.

I see this feature as problematic considering that migration to EF7 is a must due to performance reasons.

[rowanmiller]

@popcatalin81 The 'sentinal' can be changed, so if zero is a valid key value for integer properties, you can just assign some other value that represents 'not set'. Setting your own temporary values will still work too, the temporary key generator only kicks in if a value has not already been set. At the moment you would need to call API to let EF know that it's a temporary value that should be replaced with a database value during save.

[FransBouma]

@popcatalin81 https://github.com/popcatalin81 The 'sentinal' can be changed, so if zero is a valid key value for integer properties, you can just assign some other value that represents 'not set'. Setting your own temporary values will still work too, the temporary key generator only kicks in if a value has not already been set. At the moment you would need to call API to let EF know that it's a temporary value that should be replaced with a database value during save.

Back in the 90-ies some companies used '99' as year not set. Till it became a valid value and they had a lot of trouble getting rid of it. Magic values are a problem waiting to come out to show its ugly head.

    FB

[popcatalin81]

@rowanmiller "let EF know that it's a temporary value that should be replaced with a database value during save"

Is this going to be done by changing all PKs and FKs to the sentinel value or is it enough to have the change tracker entry to Added? If it will require changing all the values to the sentinel value then this is a very cumbersome programming model. (Requires constantly patching your objects to please EF)

[petarrepac]

IMO "sentinel key value" breaks "single responsibility principle"

key values have just 1 responsibility - identify the entity "sentinel key value" - additionally (because it is still a key value) are trying to tell something else

I strongly agree with @FransBouma that this is problematic.

And it is not discover-able, developers will not know this behavior and will spend time again and again debugging.

Additionally an organisation will need to "spread the word" do DBAs and other developers that value XYZ is not to be used. This is VERY difficult to manage.

It may not be possible to use EF with legacy databases where the entire key space is considered valid.

Take into account that in big organisation you can start with a table that has no 0 for PK value, but then some other department implements some solution using a different technology set and they use 0 as PK value. Your database became legacy.

So, now we should refactor our EF entities ?

[MrJul]

I think the main problem is The proposal is to require some value to be a sentinel. Please make it an option instead, defaulted to on for new developments, but that be turned off for legacy databases. When off no value is considered to be a temporary one.

[jnm2]

• How would this work with composite keys?
• What about byte, ushort, uint and ulong enums? Regardless of the underlying type, if it's a flags enum and all bits are set, the value is going to look a lot like -1.
• What about a bool in a key or composite key? Is default(bool) the sentinel?

If you're going to require some sentinel, I think it's safer and clearer to require the key properties to be nullable. That way no one is guessing the actual meaning of certain values- a null key value is immediately clear to anyone. (In fact, in past experience with EF, not allowing your key properties to be nullable is shooting yourself in the foot. You know they're going to be invalid at some point. At least make it clean to work with.)

In response to all of the proposal's three advantages: As a general principle, the cognitive burden should be hidden by EF rather than exposed in the entities. If EF has to track extra state in order to reduce that burden, great! That's its job in the first place, after all. The question is, does this reduce or increase developer productivity?

• I think requiring a magic value to be a sentinel will reduce developer productivity and leak unnecessary constraints into the database arena.
• And as I said above, I think requiring a null value to be a sentinel will increase developer productivity while avoiding the database dev issues.

[Sebazzz]

For my clarification, a sentinel is conceptually the same as NHibernate's unsaved-value?

[jnm2]

Has any thought been given to the three edge cases I listed above, and the pros and cons of nullable primary keys that represent the expected lack of primary key semantically with null instead of a magic sentinel value?

[rowanmiller]

@jnm2 regarding composite keys, the value for each property is generated separately, so it really just works the same as with single keys.

Regarding cases where you want to specify values and not have them generated, we discussed and opened https://github.com/aspnet/EntityFramework/issues/3072 to better support this.

[ajcvickers]

We are closing this issue because no further action is planned for this issue. If you still have any issues or questions, please log a new issue with any additional details that you have.