New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ResultSet\Simple delete method returns true when a constraint violation occurs if the model has a virtual FK defined #11133

Closed
GavinMcL opened this Issue Nov 13, 2015 · 4 comments

Comments

Projects
None yet
4 participants
@GavinMcL

GavinMcL commented Nov 13, 2015

Using Robots demo as the example;

If I have a Robots model, which is related to a Parts model via RobotsParts, and Robots has this relationship map;

$this->hasMany('id', 'RobotsParts', 'robots_id', ['alias' => 'RobotsParts']);

if I do

$robots = Robots::find();
$robots->delete();

I correctly get a PDOException

PDOException: SQLSTATE[23000]: Integrity constraint violation: 
1451 Cannot delete or update a parent row: a foreign key constraint fails (`robots`.`robots_parts`, CONSTRAINT `robots_parts_ibfk_1` FOREIGN KEY (`robots_id`) REFERENCES `robots` (`id`))

however
if I alter the Robots relationship map to define a virtual foreign key with a friendlier message, eg

$this->hasMany('id', 'RobotsParts', 'robots_id', [
    'alias' => 'RobotsParts', 
    'foreignKey' => ['message' => 'robot is associated to a part']
]);

then re-run

$robots = Robots::find();
$robots->delete();

$robots->delete() returns true, even though $robots->getMessages() has the FK message I defined. The actual database records are still correctly in place.

I would expect $robots->delete() to return false if any of the contained records fail to delete, in the same way as calling

$robot = Robots::findFirst();
$robot->delete();

will return false with the same FK violation.

@GavinMcL

This comment has been minimized.

Show comment
Hide comment
@GavinMcL

GavinMcL Nov 16, 2015

I've created 2 files in gist which demonstrate the behaviour I'm getting.

https://gist.github.com/GavinMcL/4d58e9ee2182ede3a7da

GavinMcL commented Nov 16, 2015

I've created 2 files in gist which demonstrate the behaviour I'm getting.

https://gist.github.com/GavinMcL/4d58e9ee2182ede3a7da

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag

Jurigag Nov 25, 2015

Member

What happens if you add , 'foreignKey' => true also in RobotsParts relations ? Also try maybe to add it like this:

'foreignKey' => [
    'message' => 'test message',
    'action'  => Relation::NO_ACTION // it should be already be default
]
Member

Jurigag commented Nov 25, 2015

What happens if you add , 'foreignKey' => true also in RobotsParts relations ? Also try maybe to add it like this:

'foreignKey' => [
    'message' => 'test message',
    'action'  => Relation::NO_ACTION // it should be already be default
]
@GavinMcL

This comment has been minimized.

Show comment
Hide comment
@GavinMcL

GavinMcL Jan 6, 2016

Hi sorry for the delay in responding, got diverted onto different project.

I've added foreignKey into RobotsParts, makes no difference

Adding foreignKey with a message makes no difference, but at least shows it is the fk in the Robots class that is being triggered

Adding 'action' => Relation::NO_ACTION seems to have the same effect as NOT declaring the relations with foreignKey - the original behaviour happens where the underlying PDO Exception is thrown, which is what I was trying avoid in the first place.

GavinMcL commented Jan 6, 2016

Hi sorry for the delay in responding, got diverted onto different project.

I've added foreignKey into RobotsParts, makes no difference

Adding foreignKey with a message makes no difference, but at least shows it is the fk in the Robots class that is being triggered

Adding 'action' => Relation::NO_ACTION seems to have the same effect as NOT declaring the relations with foreignKey - the original behaviour happens where the underlying PDO Exception is thrown, which is what I was trying avoid in the first place.

@stale

This comment has been minimized.

Show comment
Hide comment
@stale

stale bot Apr 16, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

stale bot commented Apr 16, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

@stale stale bot added the stale label Apr 16, 2018

@sergeyklay sergeyklay closed this Apr 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment