Fix issue #1388: broken model-only relationships #1389
Conversation
| $noExceptionThrown = false; | ||
| } | ||
|
|
||
| $this->assertTrue($noExceptionThrown); |
There was a problem hiding this comment.
When an exception is thrown this test is automatically marked as failed, so no need for that assertTrue :)
| @@ -791,7 +791,7 @@ public function setupReferrers($throwErrors = false) | |||
| $foreignPrimaryKeys = $foreignTable->getPrimaryKey(); | |||
| // check all keys are referenced in foreign key | |||
| foreach ($foreignPrimaryKeys as $foreignPrimaryKey) { | |||
| if (!$foreignPrimaryKey->hasReferrer($foreignKey) && $throwErrors) { | |||
| if (!$foreignPrimaryKey->hasReferrer($foreignKey) && !$foreignKey->isSkipSql() && $throwErrors) { | |||
There was a problem hiding this comment.
well, generating SQL has nothing to do with checking for valid foreign keys. So this check is a misuse of the skip-sql attribute to allow models generation with invalid foreign keys.
There was a problem hiding this comment.
Propel used to allow flexible models tailored for application's needs. This checking for foreign keys merely ties hands. For example, if referent table has composite key, it appears to be impossible to join it by a column outside the key.
Please also consider fully justified and common scenario: i18n tables having primary key(id, locale). Of course in many cases other tables would refer to i18n tables only by id. Another similar common scenario is versioning of entities.
Maybe it's ok to be strict and thus guarantee scheme validity between different RDBMS and to guide amateurs.
But model only relationships feature shouldn't be broken. It doesn't work now as described in docs and my pull request fixes this regression.
No description provided.