Spacialist icon indicating copy to clipboard operation
Spacialist copied to clipboard

Change Primary Key 'thesaurus_url' to id's

Open bmitzkus opened this issue 7 years ago • 3 comments

Currently we reference thesaurus concepts with url's as foreign keys. Since there is no practical reason to do that and it is inconsistent with the rest of our database, I would suggest switching from url references to id's. ToDo's for that:

  • [ ] drop the url foreign key constraint
  • [ ] migrate the existing thesaurus databases to the new reference system
  • [ ] create the id foreign key constraint

bmitzkus avatar Nov 09 '17 14:11 bmitzkus

Additional note: We added Eloquent Relationships on the orm branch and some of the relations are impossible to create due to the url reference. Since Eloquent is very useful I agree to switching back to ID (:roll_eyes:) as it was in the beginning of the project.

v1r0x avatar Nov 09 '17 15:11 v1r0x

We had very good reasons to switch to URLs. On issue with an ID based system would occur if one exports and re-imports an existing vocabulary and gets new IDs for the very same concepts. Switching to URLs was implemented to avoid exactly such cases!

I'm strongly against switching back to IDs unless absolute necessary!

@Hvitgar what would be the benefit of IDs beyond consistency? Performance?

dirkseidensticker avatar Nov 10 '17 08:11 dirkseidensticker

But is this really a use-case? Is there a case where a user wants to remove the whole voc and re-import it?

Switching back is not necessary, but as @Hvitgar said it would be more consistent. And as I said, we couldn't create some of the relations due to this, which is a pity, because relations are a great concept of Eloquent. In addition the code would be much cleaner and easier to read. Relations help us amongst others to replace JOIN statements with a simple property. Here's an example:

// Current implementation
Context::join('sources', 'contexts.id', '=', 'sources.context_id)->havingRaw('COUNT(sources.context_id) > 5')->get();

// Completely Eloquent based
Context::has('sources', '>', 5)->get();

Yes, performance is another reason for IDs. Not sure if the impact is even noticeable, but (in general) a integer comparison is faster than a string comparison. For complete different strings such as "house" and "car" the comparison if "house" equals "car" is very fast, since it returns false after checking the first char. For the strings "house" and "houses" it has to compare 5 chars before returning false (if we ignore that the strings aren't of the same length). Our URLs are much longer than these example strings and most of the strings are equal (http://spacialist....de/-).

v1r0x avatar Nov 10 '17 10:11 v1r0x