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

Enh: Add Global ID #6718

Draft
wants to merge 14 commits into
base: develop
Choose a base branch
from
Draft

Conversation

martin-rueegg
Copy link
Contributor

@martin-rueegg martin-rueegg commented Dec 10, 2023

  • Add class_map table
  • Add global_id table
  • Update ContentContainer, User, Space, ContentContainerActiveRecord

What kind of change does this PR introduce?

  • Feature
  • Refactor

Does this PR introduce a breaking change?

  • Not yet

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • It's submitted to the develop branch, not the master branch if no hotfix
  • When resolving a specific issue, it's referenced in the PR's description (e.g. Fix #xxx[,#xxx], where "xxx" is the Github issue number)
  • All tests are passing
  • New/updated tests are included
  • Changelog was modified

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

@martin-rueegg martin-rueegg force-pushed the enh/add-global_id-1 branch 2 times, most recently from 1607cfe to 4b39bf5 Compare December 10, 2023 15:49
@martin-rueegg
Copy link
Contributor Author

@luke- there is a theoretical question that comes along with the introduction of GID that is independent of the review of this PR or implementing code:

  • Adding a GID to a specific table poses some additional complications:
    • if the GID becomes the primary key, the model::findOne($id) will no longer work, but model::findOne($gid) would, instead.
    • if the ID remains the primary key, the referencing tables must not store the GID, which in turn kinda limits the purpose of the GID
  • Replacing the ID with the GID would be the better approach in terms of compatibility, usability of the id.
    • However, it would require to update the current number with new unique GID. This in turn would require that all referencing tables have a foreign key set to the current id field of the table being referenced and that this FK has ON UPDATE set to CASCADE.
    • Then the migration script can easily update update the current id with a new gid value (without name change)
    • The old id would be stored in old_id for existing tables so that plugin or system settings can be updated after the migration.

I would recommend the second approach.

Would you see such a requirement (that plugins create updating-FKs before the update of the core version) as feasible? Maybe the core updater would have to check the maximum required versions of all enabled modules before an update is possible.

The update process for any table that will become an GlobalActiveRecordInterface (referencing the global_id table) would then look something like this:

  1. core and modules should create an FK to the source.field to the target.id that is going to be converted
    (announced in the ChangeLog and Migration guide)
  2. if it's not already the case, the target.id field is updated to meet the field specification of global_id.gid field:
    1. all FKs pointing to that target.id field will be dropped (memorizing source table and field names)
    2. the target.id field specification will be updated (unsigned integer (11) not null) and auto increment dropped.
    3. all source.fields of the previously dropped FKs will be updated to the same new field specification
    4. and a new FK generated with ON UPDATE CASCADE
  3. the id value will be updated to the new GID generated by the newly created global_id record (thanks to CASCADE all values in the referencing tables will also be updated)
  4. the old target.id value will be stored in target.old_id for future reference. This field will not be filled for new records.
  5. optional: modules who had no FK and update later, will have to update their referencing field with the value from id (based on old_id) and then create an FK
    • functionality for this could be provided by a Migration method
  6. further migrations steps would update settings (maybe using a special migration event that modules can register for?
  7. cache will be automatically cleared

(I hope I was able express the challenge in an understandable way.)

@luke-
Copy link
Contributor

luke- commented Dec 14, 2023

@martin-rueegg Thank you for your work here. I need some time to dig into the topic.

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

Successfully merging this pull request may close these issues.

None yet

2 participants