propelorm / Propel3

High performance data-mapper ORM with optional active-record traits for RAD and modern PHP 7.2+
MIT License
250 stars 35 forks source link

Primary and foreign keys #59

Closed cristianoc72 closed 6 years ago

cristianoc72 commented 7 years ago

While fixing test suite, I encountered a lot of problems, when we have a primary key (expecially composite) which is also a foreign key (e.g. see https://github.com/propelorm/Propel3/blob/master/tests/Fixtures/bookstore/schema.xml#L106). Imho, the problem is that those primary keys are marked as "implementation detail".

Which is the better solution? Avoid to mark a primary key as "implementation detail" or leave everything as is? Both solutions require some workarounds.

marcj commented 7 years ago

I added implementation detail to mark a column as a database implementation detail, which has nothing todo with a PHP object. For example to store relation a extra field author_id is necessary because SQL databases force us to do so as they store relations through extra fields. Other databases don't. In the PHP world, when I deal with plain old php objects I really usually don't care. I just set setAuthor(), without knowing what the underlying database is needing to actually store it. I marked fields generated by foreign keys per default as implementation detail, so we don't have them at every object as actual field. Instead we read the PK of foreign's object during the persisting stage. That's the story behind implementation detail.

I can imagine that we should do the same for 1-1 relations (where PK = FK in SQL terms). However, it could get tricky if we have auto-increment PK, since we need to reserve the auto-increment value beforehand (like we currently do) and use it in both objects (relation dependant creates the PK, owner side needs to have a PK without auto values as it simply copies the value from the dependant). Composite PK as FK is actually the same. We just make sure auto-increment is filled (if one has AI enabled like before) and read the other PKs from the foreign object in local FK. Should actually already work I guess. Can you point me to some missing cases?

So, workflow User 1:1 Author (has <relation>)

  1. persist ($author, $user)
  2. resolve dependency tree to see which needs to be saved first (already working)
  3. When $user is in SQL persister class (which should come first as it's the dependant of $author), we know it has two PKs. 3.1. One is AI, so don't do anything as this is already done 3.2. One is a FK. Fetch the object from the class field and read its values. (restriction, other objects PK can not be auto-increment when it's a 1:1 sibling). Set it values to $user's PK. This should already be done in EntityMap::buildSqlBulkInsertPart 3.3. Save $user
  4. When $author is in SQL persister class, we know its a 1:1 sibling. 4.1. Read all FKs from $author and use it as PK fields. Should already be done in buildSqlBulkInsertPart 4.2. Save $author

Did I miss something?

cristianoc72 commented 7 years ago

Ok, I understand now. I had to restart some work. I leave this issue opened, to ask something else, if needed. Thank you!