Doctrine 2 - ORM

Foreign Key to Unique Field Update Failure

Details

  • Type: New Feature New Feature
  • Status: Reopened Reopened
  • Priority: Trivial Trivial
  • Resolution: Unresolved
  • Affects Version/s: 2.0-BETA2
  • Fix Version/s: None
  • Component/s: ORM
  • Security Level: All
  • Labels:
    None

Description

I am getting an error: 'Notice: Undefined index: sysname in ./libraries/Doctrine/ORM/Persisters/BasicEntityPersister.php on line 434' when I try to flush a change to a property that references a unique field on another object.

From poking around in the _prepareUpdateData function, it seems that it only allows you to use identifier fields: 

$newValId = $uow->getEntityIdentifier($newVal);

..

$result[$owningTable][$sourceColumn] = $newValId[$targetClass->fieldNames[$targetColumn]];

I'll see if I can get a test case for this set up.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Roman S. Borschel added a comment -

Hi. That is right. Foreign keys (join columns) must point to primary keys, not arbitrary other columns, whether they're unique or not, Doctrine does not know.

In other words, joinColumn must always refer to an identifier/pk. I'm not sure but I think anything else would be a pretty strange relational model, too, but there may be usecases we have not yet encountered.

I'm afraid this will not be possible and would be very hard to implement. Of course if somebody has a patch we happily accept it (after reviewing).

Leaving this open in the case somebody wants to work on it.

Show
Roman S. Borschel added a comment - Hi. That is right. Foreign keys (join columns) must point to primary keys, not arbitrary other columns, whether they're unique or not, Doctrine does not know. In other words, joinColumn must always refer to an identifier/pk. I'm not sure but I think anything else would be a pretty strange relational model, too, but there may be usecases we have not yet encountered. I'm afraid this will not be possible and would be very hard to implement. Of course if somebody has a patch we happily accept it (after reviewing). Leaving this open in the case somebody wants to work on it.
Hide
Permalink
Michael Ridgway added a comment -

A minimal test case. Removing the first flush produces the same error, so this seems to be a bug on inserts as well.

Show
Michael Ridgway added a comment - A minimal test case. Removing the first flush produces the same error, so this seems to be a bug on inserts as well.
Hide
Permalink
Roman S. Borschel added a comment -

Its not really a bug but rather a new feature This was not intended to work so far.

Show
Roman S. Borschel added a comment - Its not really a bug but rather a new feature This was not intended to work so far.
Hide
Permalink
Michael Ridgway added a comment -

Ah, ok. Maybe it didn't work before. I don't know where I got the idea that it did.

Thanks.

Show
Michael Ridgway added a comment - Ah, ok. Maybe it didn't work before. I don't know where I got the idea that it did. Thanks.
Hide
Permalink
Michael Ridgway added a comment -

Oops, closed it before I noticed you said you wanted to leave it open.

Show
Michael Ridgway added a comment - Oops, closed it before I noticed you said you wanted to leave it open.
Hide
Permalink
Roman S. Borschel added a comment -

Thanks for the testcase though, it is useful. In your concrete example, is it not an option to make the sysname the @Id ?

Show
Roman S. Borschel added a comment - Thanks for the testcase though, it is useful. In your concrete example, is it not an option to make the sysname the @Id ?
Hide
Permalink
Michael Ridgway added a comment -

Yes. That is definitely the way it should be done in this case. I can't really think of a case to have a reference to a unique key while still having an Id on it, except when you're working with an existing, poorly designed database (which is our case).

The reason I assumed this was possible is that the references actually work for lazy loading, but as soon as you start changing the references it throws this error.

Show
Michael Ridgway added a comment - Yes. That is definitely the way it should be done in this case. I can't really think of a case to have a reference to a unique key while still having an Id on it, except when you're working with an existing, poorly designed database (which is our case). The reason I assumed this was possible is that the references actually work for lazy loading, but as soon as you start changing the references it throws this error.
Hide
Permalink
Roman S. Borschel added a comment -

Lowering priority.

Show
Roman S. Borschel added a comment - Lowering priority.
Hide
Permalink
Thomas Subera added a comment -

although we would also need this i would suggest adding an error message if the associated column is not found in $newValId. (class BasicEntityPersister.php _prepareUpdateData)

otherwise the field is populated with null leaving the developer debugging an hour :-/

thx

Show
Thomas Subera added a comment - although we would also need this i would suggest adding an error message if the associated column is not found in $newValId. (class BasicEntityPersister.php _prepareUpdateData) otherwise the field is populated with null leaving the developer debugging an hour :-/ thx

People

  • Assignee:
    Roman S. Borschel
    Reporter:
    Michael Ridgway
Vote (2)
Watch (2)

Dates

  • Created:
    Updated: