Why is my PL/SQL procedure so slow?

Question

I have this sub-procedure in a fairly large program, and it is taking hundreds of times longer than comparable sub-procedures. Is there any way I could improve efficiency here?

None of this procedure's sub-procedures are absurdly slow, so I am sure that it has to do with the structure of this one.

This program looks through a table containing hierarchies of doctors. If it finds a matching root doctor, it adds to that root. Otherwise, it tries to find a matching not-root doctor. I think this might be the source of the inefficiency; the fact that it's opening two very similar cursors and looping through both quite often.

Additional info:

UNIQUE_GOOD_MATCH is, on average, taking about four seconds. UNIQUE_PHYSICIAN has roughly 200k records, and is indexed by first and last names. It doesn't seem to me that it should be taking this long, especially when other procedures are completing at a fraction of this time.

create or replace PROCEDURE UNIQUE_GOOD_MATCH 
(
  PHYSICIANID IN NUMBER 
, SOURCETABLE IN VARCHAR2 
, FNAME IN VARCHAR2 
, LNAME IN VARCHAR2 
, POSTALCODE IN VARCHAR2 
, FSA IN VARCHAR2
, END_PROC OUT NUMBER 
) AS 

CURSOR all_fitting_roots is
SELECT DISTINCT ROOT_ID FROM
  (
  SELECT UNIQUE_ID, ROOT_ID, FIRST_NAME, LAST_NAME, POSTAL_CODE, FSA
  FROM
    (SELECT p.*, CONNECT_BY_ROOT UNIQUE_ID AS ROOT_ID
    FROM UNIQUE_PHYSICIAN p
    START WITH PARENT_PHYS_ID IS NULL 
    CONNECT BY PRIOR UNIQUE_ID = PARENT_PHYS_ID)
  WHERE UNIQUE_ID = ROOT_ID
  AND FIRST_NAME = FNAME
  AND LAST_NAME = LNAME
  AND POSTAL_CODE = POSTALCODE
);

CURSOR all_fitting_nonroots is
SELECT DISTINCT UNIQUE_ID FROM
  (
  SELECT UNIQUE_ID, ROOT_ID, FIRST_NAME, LAST_NAME, POSTAL_CODE, FSA
  FROM
    (SELECT p.*, CONNECT_BY_ROOT UNIQUE_ID AS ROOT_ID
    FROM UNIQUE_PHYSICIAN p
    START WITH PARENT_PHYS_ID IS NULL 
    CONNECT BY PRIOR UNIQUE_ID = PARENT_PHYS_ID)
  WHERE UNIQUE_ID <> ROOT_ID
  AND FIRST_NAME = FNAME
  AND LAST_NAME = LNAME
  AND POSTAL_CODE = POSTALCODE
);


FITTING NUMBER;
FITTING_NR NUMBER;

PARENT_UID NUMBER;

too_many_roots EXCEPTION;

  timeStart  TIMESTAMP;
  timeEnd    TIMESTAMP;
  timeMS NUMBER;


BEGIN

timeStart  := SYSTIMESTAMP;

IF FNAME = 'NULLNULLNULL' OR LNAME = 'NULLNULLNULL' OR POSTALCODE = 'NULLNULLNULL'
THEN
END_PROC := 0;

 timeEnd  := SYSTIMESTAMP;
timeMS := ((extract(hour from timeEnd) - extract(hour from timeStart)) * 3600000) + ((extract(minute from timeEnd) - extract(minute from timeStart)) * 60000) +
((extract(second from timeEnd) - extract(second from timeStart)) * 1000);

PROGRESS_RECORD(0, timeMS, 'UNIQUE_GOOD_MATCH');

RETURN;
END IF;

FITTING := 0;
FITTING_NR := 0;
for root in all_fitting_roots
LOOP
  FITTING := FITTING + 1;
END LOOP;
IF FITTING > 1
  THEN

timeEnd  := SYSTIMESTAMP;
timeMS := ((extract(hour from timeEnd) - extract(hour from timeStart)) * 3600000) + ((extract(minute from timeEnd) - extract(minute from timeStart)) * 60000) +
((extract(second from timeEnd) - extract(second from timeStart)) * 1000);

PROGRESS_RECORD(0, timeMS, 'UNIQUE_GOOD_MATCH');

  RAISE too_many_roots;

ELSIF FITTING = 1
  THEN
  OPEN all_fitting_roots;
  FETCH all_fitting_roots INTO PARENT_UID;
  INSERT INTO UNIQUE_PHYSICIAN(SOURCE_ID, PARENT_PHYS_ID, SOURCE_TABLE, CONFIDENCE_IS_PARENT, CONFIDENCE_IS_ROOT, FIRST_NAME, LAST_NAME, POSTAL_CODE, FSA,
  NEEDS_VALIDATION)
  VALUES
  (PHYSICIANID, PARENT_UID, SOURCETABLE, 1, 1, FNAME, LNAME, POSTALCODE, FSA, 0);
  END_PROC := 1;
  CLOSE all_fitting_roots;

   timeEnd  := SYSTIMESTAMP;
timeMS := ((extract(hour from timeEnd) - extract(hour from timeStart)) * 3600000) + ((extract(minute from timeEnd) - extract(minute from timeStart)) * 60000) +
((extract(second from timeEnd) - extract(second from timeStart)) * 1000);

PROGRESS_RECORD(0, timeMS, 'UNIQUE_GOOD_MATCH');

  RETURN;

ELSIF FITTING = 0
  THEN
  for nonroot in all_fitting_nonroots
  LOOP
  FITTING_NR := FITTING_NR + 1;
  END LOOP;
  IF FITTING_NR = 0
    THEN
    END_PROC := 0;

 timeEnd  := SYSTIMESTAMP;
timeMS := ((extract(hour from timeEnd) - extract(hour from timeStart)) * 3600000) + ((extract(minute from timeEnd) - extract(minute from timeStart)) * 60000) +
((extract(second from timeEnd) - extract(second from timeStart)) * 1000);

PROGRESS_RECORD(0, timeMS, 'UNIQUE_GOOD_MATCH');

    RETURN;

  ELSE  -- Insert under NR
    OPEN all_fitting_nonroots;
    FETCH all_fitting_nonroots INTO PARENT_UID;
    INSERT INTO UNIQUE_PHYSICIAN(SOURCE_ID, PARENT_PHYS_ID, SOURCE_TABLE, CONFIDENCE_IS_PARENT, FIRST_NAME, LAST_NAME, POSTAL_CODE, FSA,
    NEEDS_VALIDATION)
    VALUES
    (PHYSICIANID, PARENT_UID, SOURCETABLE, 1, FNAME, LNAME, POSTALCODE, FSA, 0);
    END_PROC := 1;
    CLOSE all_fitting_nonroots;

timeEnd  := SYSTIMESTAMP;
timeMS := ((extract(hour from timeEnd) - extract(hour from timeStart)) * 3600000) + ((extract(minute from timeEnd) - extract(minute from timeStart)) * 60000) +
((extract(second from timeEnd) - extract(second from timeStart)) * 1000);

PROGRESS_RECORD(0, timeMS, 'UNIQUE_GOOD_MATCH');

    RETURN;

  END IF;
END IF;

timeEnd  := SYSTIMESTAMP;
timeMS := ((extract(hour from timeEnd) - extract(hour from timeStart)) * 3600000) + ((extract(minute from timeEnd) - extract(minute from timeStart)) * 60000) +
((extract(second from timeEnd) - extract(second from timeStart)) * 1000);

PROGRESS_RECORD(0, timeMS, 'UNIQUE_GOOD_MATCH');

EXCEPTION
  WHEN too_many_roots THEN
      DBMS_OUTPUT.PUT_LINE('LOGIC FLOW ERROR IN UNIQUE_GOOD_MATCH FOR ');
      DBMS_OUTPUT.PUT_LINE(PHYSICIANID);




END UNIQUE_GOOD_MATCH;

Answer 1

One ad hoc improvement:

poor:

for root in all_fitting_roots
LOOP
  FITTING := FITTING + 1;
END LOOP;

better:

SELECT COUNT(DISTINCT ROOT_ID)
into FITTING 
FROM
  (
  SELECT UNIQUE_ID, ROOT_ID, FIRST_NAME, LAST_NAME, POSTAL_CODE, FSA
  FROM
    (SELECT p.*, CONNECT_BY_ROOT UNIQUE_ID AS ROOT_ID
    FROM UNIQUE_PHYSICIAN p
    START WITH PARENT_PHYS_ID IS NULL 
    CONNECT BY PRIOR UNIQUE_ID = PARENT_PHYS_ID)
  WHERE UNIQUE_ID = ROOT_ID
  AND FIRST_NAME = FNAME
  AND LAST_NAME = LNAME
  AND POSTAL_CODE = POSTALCODE
);

or

FITTING := 0;
OPEN all_fitting_roots;
FETCH all_fitting_roots INTO PARENT_UID;
IF all_fitting_roots%FOUND THEN
   FITTING := 1;
   FETCH all_fitting_roots INTO PARENT_UID;
   IF all_fitting_roots%FOUND THEN
      FITTING := 2;
   END IF;
   -- no more fetches are needed for further processing
END IF;
CLOSE all_fitting_roots;