Skip to content

Loading…

DDC-567: Foreign Key to Unique Field Update Failure #5073

Open
doctrinebot opened this Issue · 10 comments

1 participant

@doctrinebot

Jira issue originally created by user mridgway:

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.

@doctrinebot

Comment created by romanb:

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.

@doctrinebot

Comment created by mridgway:

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

@doctrinebot

Comment created by romanb:

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

@doctrinebot

Comment created by mridgway:

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

Thanks.

@doctrinebot

Comment created by mridgway:

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

@doctrinebot

Comment created by romanb:

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

@doctrinebot

Comment created by mridgway:

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.

@doctrinebot

Comment created by romanb:

Lowering priority.

@doctrinebot

Comment created by dready:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.