Mimetis / Dotmim.Sync

A brand new database synchronization framework, multi platform, multi databases, developed on top of .Net Standard 2.0. https://dotmimsync.readthedocs.io/
MIT License
899 stars 194 forks source link

Forbidden Primary Key Column Names #987

Open SvdSinner opened 1 year ago

SvdSinner commented 1 year ago

Inside of the BaseOrchestrator.Schema.cs file there are 5 prohibited column names that will make DMS unable to sync a table if they are used as a primary key column of said table. Currently, the column names (update_scope_id, timestamp, timestamp_bigint, sync_row_is_tombstone, last_change_datetime) are general names that have potential to conflict with real-world database column names. (One of my databases I would like to sync has a table with a timestamp column as part of a multi-part primary key, and can't be synced because of it.)

Three possible solutions:

  1. Change these excluded columns (and of course the reason they need to be excluded) to use DMS specific names. (E.G. dms_update_scope_id, dms_timestamp, dms_timestamp_bigint, dms_sync_row_is_tombstone, dms_last_change_datetime) This would dramatically reduce the chance of them ever conflicting with a column name in a real world database.
  2. Develop a documented workaround for this situations. (Untested idea: Possibly create a new computed column that is computed as the value of the column and use it as the primary key instead of the original column)
  3. Enhance the code to allow these column names.
VagueGit commented 1 year ago

I agree. I think the same principle should apply to triggers. DMS creates trigger names that could easily conflict with existing trigger names eg [table-name]_deletetrigger would be less likely to conflict with existing triggers if the naming convention were DMS[table-name]_delete_trigger

Mimetis commented 1 year ago

I agree. I think the same principle should apply to triggers. DMS creates trigger names that could easily conflict with existing trigger names eg [table-name]_deletetrigger would be less likely to conflict with existing triggers if the naming convention were DMS[table-name]_delete_trigger

You just have to specify a trigger prefix or suffix option : https://dotmimsync.readthedocs.io/Configuration.html#syncsetup

Change these excluded columns (and of course the reason they need to be excluded) to use DMS specific names. (E.G. dms_update_scope_id, dms_timestamp, dms_timestamp_bigint, dms_sync_row_is_tombstone, dms_last_change_datetime) This would dramatically reduce the chance of them ever conflicting with a column name in a real world database.

Indeed, but it will break the compatibility with previous version. Of course, it's possible to automatically migrate to new names, but it's difficult to handle from DMS. But feasible.

Develop a documented workaround for this situations. (Untested idea: Possibly create a new computed column that is computed as the value of the column and use it as the primary key instead of the original column)

No workaround

Enhance the code to allow these column names.

Yes, in a future version, I guess it could be part of SyncOptions (like ScopeInfoTableName we already have)