Sunday, April 27, 2008

Fine tuning

Mike is doing some work on an application that started life as SQL Server. Now that it has been converted to Oracle, there are one or two bits that could still do with a little fine tuning.

One particular procedure seemed to take rather a lot of time, and several developers had tried to get better performance out of it without much success. Here it is:

CREATE OR REPLACE PROCEDURE getupdatedrunids
    ( p_customer_id  IN VARCHAR2 DEFAULT NULL
    , pc_results_out IN OUT SYS_REFCURSOR )
AS
    v_fetch_status     INTEGER := 0;
    v_sql_status       INTEGER;
    v_fetch_status1    INTEGER := 0;
    v_sql_status1      INTEGER;
    v_event_id         VARCHAR2(50);
    v_runid            NUMBER(10, 0);
    v_count_run_conns  INTEGER := 0;
    v_temp_runs        INTEGER;

BEGIN
    DELETE FROM temp_runs;

    DELETE FROM temp_run_connections;

    INSERT INTO temp_runs
         ( run_id )
    SELECT DISTINCT i.run_id
    FROM   event_status i
         , run_status    b
    WHERE  i.run_id = b.run_id
    AND    i.event_id IN
           ( SELECT DISTINCT i.event_id
             FROM   event_status i
                  , run_status   b
             WHERE  i.run_id = b.run_id
             AND    b.customer_id = p_customer_id
             GROUP  BY i.event_id
             HAVING COUNT(i.run_id) > 1)
             AND    b.customer_id = p_customer_id;

    BEGIN
        v_temp_runs := 0;
        SELECT COUNT(*)
        INTO   v_temp_runs
        FROM   dual
        WHERE  EXISTS
               ( SELECT *
                 FROM   temp_runs );
    END;

    IF v_temp_runs > 0 THEN
    
        DECLARE
            CURSOR c_runs IS
                SELECT DISTINCT run_id
                FROM   temp_runs;
        BEGIN
            OPEN c_runs;
            FETCH c_runs INTO v_runid;

            IF c_runs%NOTFOUND
            THEN
                v_sql_status1   := 2;
                v_fetch_status1 := -1;
            ELSE
                v_sql_status1   := 0;
                v_fetch_status1 := 0;
            END IF;
        
            WHILE v_fetch_status1 = 0
            LOOP
                BEGIN
                    DECLARE
                        CURSOR cust_incidents_cs IS
                        
                            SELECT DISTINCT i.event_id
                            FROM   event_status i
                            WHERE  i.run_id = v_runid
                            AND    i.rejected = 0;
                    BEGIN
                        OPEN cust_incidents_cs;
                    
                        FETCH cust_incidents_cs INTO v_event_id;

                        IF cust_incidents_cs%NOTFOUND
                        THEN
                            v_sql_status   := 2;
                            v_fetch_status := -1;
                        ELSE
                            v_sql_status   := 0;
                            v_fetch_status := 0;
                        END IF;

                        <<i_loop1>>
                        WHILE v_fetch_status = 0
                        LOOP
                            BEGIN
                                INSERT INTO temp_run_connections
                                     ( run_id
                                     , connectedids)
                                SELECT DISTINCT v_runid
                                     , i.run_id AS connectedids
                                FROM   event_status i
                                WHERE  i.run_id < v_runid
                                AND    i.event_id = v_event_id
                                AND    i.rejected = 0
                                AND    i.run_id IN
                                       ( SELECT DISTINCT run_id
                                         FROM   temp_runs );

                                <<fetchnext>>
                                FETCH cust_incidents_cs INTO v_event_id;

                                IF cust_incidents_cs%NOTFOUND
                                THEN
                                    v_sql_status   := 2;
                                    v_fetch_status := -1;
                                ELSE
                                    v_sql_status   := 0;
                                    v_fetch_status := 0;
                                END IF;
                            END;
                        END LOOP;

                        CLOSE cust_incidents_cs;
                    END;
                
                    FETCH c_runs INTO v_runid;

                    IF c_runs%NOTFOUND
                    THEN
                        v_sql_status1   := 2;
                        v_fetch_status1 := -1;
                    ELSE
                        v_sql_status1   := 0;
                        v_fetch_status1 := 0;
                    END IF;
                END;
            END LOOP;
            CLOSE c_runs;
        END;
    END IF;

    SELECT COUNT(*)
    INTO   v_count_run_conns
    FROM   temp_run_connections;

    IF v_count_run_conns > 0
    THEN
        OPEN pc_results_out FOR
            SELECT DISTINCT run_id, connectedids
            FROM   temp_run_connections;
    END IF;

END getupdatedrunids;

His final version was much faster. See if you can spot the difference:

CREATE OR REPLACE PROCEDURE getupdatedrunids
    ( p_customer_id  IN VARCHAR2 DEFAULT NULL
    , pc_results_out IN OUT SYS_REFCURSOR )
AS
BEGIN
    OPEN pc_results_out FOR
        SELECT DISTINCT e2.run_id, ic.run_id AS connectedids
        FROM   run_status r1
             , run_status r2
             , event_status e1
             , event_status e2
        WHERE  r1.customer_id = p_customer_id
        AND    r2.customer_id = r1.customer_id
        AND    e1.run_id = r2.run_id
        AND    e1.rejected = 0
        AND    e2.run_id = r1.run_id
        AND    e2.event_id = e1.event_id
        AND    e2.run_id > e1.run_id
        AND    e2.rejected = 0
        ORDER BY 1, 2;

END getupdatedrunids;

7 comments:

Noons said...

and of course, running a simple SQL cursor with that statement would be completely impossible!
LOL!

Patrick Wolf said...

What I have heard, SQL Server developer like/have to use temporary tables, which is completely nonsense in Oracle.

That's about 1 to 1 transfer of code without looking at the target platform...

Patrick

Stew said...

William,

Thanks for sharing, this is great stuff!

I work on a vendor application that was ported from Sybase to Oracle, where they never bothered to get rid of all the Sybase trash, which is very similar to what you're showing here.

Yikes!

Anonymous said...

I worked with Oracle migration workbench and it produces similar results... They did the same work that a silly translator can do!

Soon... we will be replaced by a very small script... performance will s*ck too, but caffeine budget will be lower.

Niall said...

how bizarre

the second example should be the more complicated since you can do the same in sqlserver - only without the ref cursor.

ah well.

Nilesh Jethwa said...

I have worked on Oracle almost all my life, only recently getting my hands dirty with SQL Server, MySQL and other databases and slowly begin to realize that some of the things that Oracle provides had been taken for granted until you see and work with non-oracle databases.


regards
Nilesh
Dashboards

Anonymous said...

My best guess will go for this beautiful condition.

AND e1.run_id = r2.run_id
AND e2.run_id = r1.run_id
AND e2.run_id > e1.run_id

Having said that original block I am not able to figure out how the original code will go past the first run_id

Regards