search

nhibernate / nhibernate-core

NHibernate Object Relational Mapper
https://nhibernate.info
GNU Lesser General Public License v2.1
2.13k stars 927 forks source link

Natural key mapping no longer functions. #2423

Open deAtog opened 4 years ago

deAtog commented 4 years ago

1875 changed the way that duplicated column names were handled when generating insert and update statements. Prior to this change it was possible to map a column multiple times for a given object. When multiple many-to-one relationships exist that utilize one or more columns from the natural key for related objects an exception is now thrown from here:

https://github.com/nhibernate/nhibernate-core/blob/752fd626ca407c41e4a841997b4f9d1c6974bd0a/src/NHibernate/SqlCommand/SqlInsertBuilder.cs#L106

I have a database that uses natural keys for related tables. While this breaking change is documented, there is no workable solution as individual columns of a many-to-one relationship cannot have insert or update set to false.

The issues with composite natural keys have been well known to cause inconsistent insert/updates if special care was not taken. In most cases there is no need to update the duplicated column from the many-to-one mappings and it is well understood that changing the column would also invalidate objects loaded using the original value. This problem has never really been addressed by NHibernate as surrogate keys are the preferred method for creating relationships.

This change put us and others in the precarious situation of doing one of the following:

  1. Creating a fork without this change
  2. Adding surrogate keys and redesigning an entire domain model.

An option to permit the old behavior should be added or individual columns of a many-to-one should be permitted to be read-only.

fredericDelaporte commented 4 years ago

May you provide a test case demonstrating the issue? It will help checking what can/should be done for supporting your case. We avoid as much as possible breaking previously supported use cases. There should be a way to adjust the mapping. A test case helps a lot making sure we look for the right case which would be no more map-able for you.

fredericDelaporte commented 4 years ago

I have made a test case, see a074830 commit. It does not involve any natural-id, which I believe are not actually related to the trouble you report. The trouble seems to me bound to using composite ids with some of their columns shared for many relations. Is this test case demonstrating the issue you are reporting?

deAtog commented 4 years ago

Natural ids are composite keys. Surrogate keys are single column row identifiers that replace the natural id or composite key. The example you provided demonstrates the issue I'm experiencing. The only difference in my case is that the "Id1" column would also be part of the composite key of the Parent class. In your example one of your many-to-one relationships must update the "Id1" column, whereas none of my many-to-one's should as it is part of the composite key of the Parent class.

fredericDelaporte commented 4 years ago

Then it seems to me you can fix the mapping by declaring as formulas the columns which have to be handled as read-only in the association mapping. See 173c928c. It seems to me adding readonly or update="false"/insert="false" settings on association columns would be a bit overkill, since declaring them as formula achieves the desired effect. Can you confirm this?

deAtog commented 4 years ago

Switching to using formula fixes the issue. While this works, it feels a little dirty as formula is considered to be arbitrary SQL and not handled the same way column names are.