pl/sql DELETE is deleting all rows instead of selected rows

Question

I have the trigger:

create or replace
TRIGGER JACKET_DELETE 
BEFORE DELETE ON JACKET 
FOR EACH ROW 
BEGIN
  DELETE FROM PORT
  WHERE EXISTS
    (SELECT * FROM port LEFT JOIN device on port.fkdevice = device.pkid
     where port.fkjacket = :old.pkid
     and device.fkdevice_type = 1);

  UPDATE PORT
  set port.fkjacket = null, port.fkport = null
  WHERE EXISTS
(SELECT port.fkjacket, port.fkport FROM port LEFT JOIN device on port.fkdevice = device.pkid
     where port.fkjacket = :old.pkid
     and device.fkdevice_type <> 1);
END;

For some reason, when the where in the delete matches, it deletes the WHOLE port table! I thought my SQL was correct, but obviously it's not, and I can't see what's wrong with it. Can anyone see the issue that is making it do this?

When the update matches, everything works as expected.

table structure: port links to device, jacket, and port

score 8 · Accepted Answer · answered Jun 08 '11 at 22:05

8

Your DELETE is referencing the PORT table twice. To clarify, let's first modify the statement to include table aliases:

  DELETE FROM PORT p1
  WHERE EXISTS
    (SELECT * FROM port p2 LEFT JOIN device on p2.fkdevice = device.pkid
     where p2.fkjacket = :old.pkid
     and device.fkdevice_type = 1);

Notice that the subquery is not correlated to p1. In other words, the result of this subquery will be identical for every row in PORT that is being considered for deletion. So you're either going to delete all rows or no rows.

(It's also odd that you use a LEFT JOIN when you have a non-join predicate on the outer table. But that's at worst an efficiency problem and more likely just confusing to anyone reading your code.)

I believe that what you want is:

DELETE FROM PORT
WHERE fkjacket = :old.pkid
AND EXISTS
  (SELECT NULL FROM device
   WHERE device.pkid = port.fkdevice
     AND device.fkdevice_type=1);

And the UPDATE seems to have the same issue; even if it's currently giving you expected results, I bet that's just luck due to the data you're testing with. I think it can be simplified to:

UPDATE PORT
  set port.fkjacket = null, port.fkport = null
  WHERE port.fkjacket = :old.pkid
    AND EXISTS
    (SELECT NULL FROM device
       WHERE port.fkdevice = device.pkid
       AND device.fkdevice_type <> 1);

Note that an EXISTS operator doesn't care what if any columns are returned by its subquery; just whether rows are returned at all.

answered Jun 08 '11 at 22:05

Dave Costa

47,262
8
56
72

is this another option, or are you saying @DCookie 's answer won't work (I am in the middle of testing it to find out for myself) – AsherMaximum Jun 08 '11 at 22:09
I wasn't sure whether DCookie's answer would work. Looks like you've decided it does. I have some concerns about it though. Does everything in PORT with the same value for FKJACKET also have the same value for FKDEVICE? If not, it seems that the subquery there could give you false positives. – Dave Costa Jun 08 '11 at 22:44
"Note that an EXISTS operator doesn't care what if any columns are returned by its subquery; just whether rows are returned at all." - So that's why the `select null` is used? I'm guessing that's faster than selecting any columns? – AsherMaximum Jun 08 '11 at 23:14
It's probably not really faster, as Oracle can essentially eliminate column references if they're not necessary. But my habit is to use SELECT NULL to make clear that no specific data is being selected, and I thought it worth pointing out since in your code you select multiple columns in the subquery. – Dave Costa Jun 08 '11 at 23:18
yeah, in `update` I thought I needed to select the columns that `set` was updating, and `*` in the `delete` because ... well, not sure my logic on doing that. I wasn't aware that `null` was an option for select. – AsherMaximum Jun 08 '11 at 23:38

DCookie · Answer 2 · 2011-06-08T22:07:02.593

5

Your DELETE is deleting everything when the fkjacket field matches :old.pkid, because you haven't restricted the delete on anything else. If the EXISTS clause returns a row, then everything goes.

Change this to something like:

  DELETE FROM PORT
  WHERE fkjacket IN
    (SELECT port.fkjacket FROM port LEFT JOIN device on port.fkdevice = device.pkid
     where port.fkjacket = :old.pkid
     and device.fkdevice_type = 1);

This will delete all rows in the port table where fkjacket is in the list of fkjacket values returned in the select list.

Are you sure your update is working correctly? Seems to me you should be getting the same sort of behavior with it - all rows updated.

EDIT:

Since your update is failing the same way, I suggest changing it to:

  UPDATE PORT
  SET port.fkjacket = null, port.fkport = null
  WHERE fkjacket IN
       (SELECT port.fkjacket 
          FROM port LEFT JOIN device on port.fkdevice = device.pkid
         WHERE port.fkjacket = :old.pkid
           AND device.fkdevice_type <> 1);

This will update all rows in the port table where fkjacket is in the list of fkjacket values returned in the select list.

edited Jun 08 '11 at 22:07

answered Jun 08 '11 at 21:28

DCookie

42,630
11
83
92

yes, I did more thorough testing, and you are correct, update updates all rows as well. To fix that would be the same modification? – AsherMaximum Jun 08 '11 at 21:52
Yes. You must restrict the update to particular values of fkjacket, if I understand your code correctly. – DCookie Jun 08 '11 at 21:54
particular values of fkjacket being the pkid of the row that is being deleted, and the value of device.fkdevice_type – AsherMaximum Jun 08 '11 at 21:56
Say you have two ports, both linked to the same jacket but to different devices; and of those devices, one is type 1 and the other is not. I believe the DELETE given here would delete both rows from PORT, not just the one that is linked to the device of type 1. – Dave Costa Jun 08 '11 at 22:45
@Dave Actually, hold on, my database is a little large, looks like the results migh still be there – AsherMaximum Jun 08 '11 at 23:04

pl/sql DELETE is deleting all rows instead of selected rows

2 Answers2