Use of Cursor in Trigger causing too many row inserts

Gurus,

I have a trigger utilizing a cursor which is inserting too many rows. In the following trigger I'm inserting rows into the GROUP_CALC_TRIGGER2 table from data in the GROUP_CAL_TRIGGER table. I'm getting way too many records after the trigger executes.

Example:
GROUP_CAL_TRIGGER  = 432 rows
GROUP_CAL_TRIGGER = 46872 rows

Why is it inserting so many duplicate records??

create or replace 
TRIGGER UPDATE_ITEM_ART1_SCO2 before
  INSERT OR
  UPDATE ON SCO 
  FOR EACH ROW 
  DECLARE 
  scokey number;
  element_key number;
  VARIANT_KEY NUMBER;
  VARIANT_KEY2 NUMBER;
  ARTICLE_FIELD_1 VARCHAR2(2000);
  CALC_NAME VARCHAR2(2000);
  art_fld_hold varchar2(2000);
  

 
[b]CURSOR C1 IS 
SELECT DISTINCT VARIANT, PROBEZC||'('||ART_FLD1||')' FROM GROUP_CALC_TRIGGER2
WHERE ART_KEY LIKE '%,%';  [/b]

BEGIN

      SELECT DISTINCT :NEW.SCOOSZKEYI, PROKEYI , OSZVARKEYI,
      SCRBEZC, LISTAGG(ARTANRC,',') WITHIN GROUP (
    ORDER BY ARTANRC) INTO SCOKEY, ELEMENT_KEY, VARIANT_KEY,
      Calc_name, article_field_1
    FROM SCC,
      SCR,
      OSZ,
      SSZ,
      ART,
      AEZ,
      PRO
    WHERE PROKEYI       = AEZPROKEYI
    AND ARTKEYI         = AEZARTKEYI
    AND :NEW.SCOOSZKEYI = OSZKEYI
    AND AEZARTKEYI     = :NEW.SCOOBJKEYI
    AND OSZSCCKEYI      = SCCKEYI
    AND SSZSCCKEYI      = SCCKEYI
    AND SSZSCRKEYI      = SCRKEYI
    AND OSZVARKEYI      = AEZKAVKEYI
    AND OSZVARKEYI      = ARTKAVKEYI
    AND OSZVARKEYI      = PROKAVKEYI
    and prokavkeyi = artkavkeyi
    AND OSZOBJKEYI      = PROKEYI
    AND OSZTYPS         = 170
    AND SCRBEZC NOT LIKE '%!%'
    GROUP BY :NEW.SCOOSZKEYI,PROKEYI, OSZVARKEYI,
      scrbezc;
      
   
[b]INSERT INTO GROUP_CAL_TRIGGER (ART_KEY,PROBEZC,ART_FLD1,VARIANT) 
values (:new.scoobjkeyi, calc_name,article_field_1,variant_key);

 
insert into GROUP_CALC_TRIGGER2
SELECT DISTINCT VARIANT, (DBMS_LOB.SUBSTR(WM_CONCAT(ART_KEY))), PROBEZC, 
(DBMS_LOB.SUBSTR(WM_CONCAT(ART_FLD1))) 
FROM GROUP_CAL_TRIGGER
GROUP BY PROBEZC, VARIANT;[/b]

OPEN C1;
  LOOP
    FETCH C1  INTO VARIANT_KEY2, ART_FLD_HOLD;
    
    
UPDATE ART
    SET ARTVF1C    =  ART_FLD_HOLD
    WHERE ARTKAVKEYI = VARIANT_KEY2 
    and artkeyi = :new.SCOOBJKEYI; 

EXIT
  WHEN C1%NOTFOUND;
  end loop;

  close C1;    

  EXCEPTION
  WHEN NO_DATA_FOUND THEN
    ARTICLE_FIELD_1 := ARTICLE_FIELD_1;
    VARIANT_KEY     := VARIANT_KEY;
 
  END;

Open in new window

xbox360dpAsked:
Who is Participating?

[Product update] Infrastructure Analysis Tool is now available with Business Accounts.Learn More

x
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

slightwv (䄆 Netminder) Commented:
First:  You are using LISTAGG so why use the undocumented WM_CONCAT elsewhere in the same code?


My guess to the problem is here:
insert into GROUP_CALC_TRIGGER2
SELECT DISTINCT ...


For every row affected by the DML on SCO, you insert into GROUP_CALC_TRIGGER then do the above insert into GROUP_CALC_TRIGGER2 selecting EVERYTHING from GROUP_CALC_TRIGGER.

There is no where clause...
0
xbox360dpAuthor Commented:
Slighwv,

Why would I need a where clause? If GROUP_CALC_TRIGGER has 300 records then I expect 300 records in GROUP_CALC_TRIGGER2. It seems to be looping over and over causing duplicate records.


Listagg vs wm_concat ... no reason really.
0
flow01Commented:
You need a where clause :

example when starting with 0 records in both  GROUP_CALC_TRIGGER  and  GROUP_CALC_TRIGGER2
first trigger execution
   insert 1 record in GROUP_CALC_TRIGGER        >  total 1
   insert 1  record in GROUP_CALC_TRIGGER2     >  total 1
second trigger execution
   insert 1 record in GROUP_CALC_TRIGGER        >   total 2
   insert 2 records in GROUP_CALC_TRIGGER2   because there are 2 records in GROUP_CALC_TRIGGER  (unless all key values in GROUP_CALC_TRIGGER are the same)  >  total 3  
etc, etc,  ....
0
Big Business Goals? Which KPIs Will Help You

The most successful MSPs rely on metrics – known as key performance indicators (KPIs) – for making informed decisions that help their businesses thrive, rather than just survive. This eBook provides an overview of the most important KPIs used by top MSPs.

slightwv (䄆 Netminder) Commented:
What flow01 said.

First time through the loop: 1 row each.
Second time:  2 rows in TRIGGER, three rows in TRIGGER2 (The one before and the two new ones from TRIGGER)
Third time:  3 rows in TRIGGER, six rows in TRIGGER2 (The three from before and the 3 rows currently in TRIGGER)
etc...

So you need to selectively insert into TRIGGER2 or clean it out before new rows are added.

Personally, I'm not sure there is a need for the two intermediate tables and the cursor but you know your system better than we do.
0
xbox360dpAuthor Commented:
Ok. What would the where clause look like?
0
xbox360dpAuthor Commented:
Or what this the best way to clear out the rows before new rows are inserted?
0
slightwv (䄆 Netminder) Commented:
>>Ok. What would the where clause look like?

I don't know.  I don't know your system, data or what you are trying to accomplish.

The only requirements you posted is you are inserting a lot of data into what I believe is a temporary table on insert or update on some other table.  Then updating yet another table.

>>Or what this the best way to clear out the rows before new rows are inserted?

I prefer truncate table or from your new question:  GLOBAL TEMPORARY TABLES.

It all depends on the problem you are trying to solve.  We can only provide information based on what you tell us.
0
xbox360dpAuthor Commented:
Slightw,

I'm trying to figure out the best option for deleting data from the two temp tables in my trigger (GROUP_CALC_TRIGGER, GROUP_CALC_TRIGGER2).

If Global Temporary Tables is the best option. How would I incorporate them in my current trigger?
0
slightwv (䄆 Netminder) Commented:
>>If Global Temporary Tables is the best option. How would I incorporate them in my current trigger?

A Global Temporary Table is created once and continually used.

Data in them is automatically removed when the session ends.  They can also be set up to delete or preserve data in them, in the current session, on commit.

The online docs talks all about them.

>>I'm trying to figure out the best option for deleting data from the two temp tables in my trigger

Delete or truncate.

Are you sure you need the temp tables?

Say you insert two rows into SCO.

row 1 will insert data into GROUP_CALC_TRIGGER and GROUP_CALC_TRIGGER2.  then go into a cursor and do a lot more work.

then do all that work again for row2.

Seems inefficient.
0
xbox360dpAuthor Commented:
I completely agree that my approach is inefficient. BUT ... the customer needs the data displayed in a column a particular way.

UPDATE ART
    SET ARTVF1C    =  ART_FLD_HOLD
    WHERE ARTKAVKEYI = VARIANT_KEY2
    and artkeyi = :new.SCOOBJKEYI;

ARTVF1C = Sum of Products(116704,300255)

Values 116704, 300255 are two different rows of data. Since I'm using a row level trigger I'm unable to concatenate the these values.

I wind up getting: Sum of Products(300255),Sum of Products(116704) for their respective rows. I simply couldn't figure a way to get what I needed without using temp tables.
0
slightwv (䄆 Netminder) Commented:
>>ARTVF1C = Sum of Products(116704,300255)

I really don't see where you SUM anything.

>>Since I'm using a row level trigger

Then why use a row level trigger?

Why not a table level trigger?
drop table tab1 purge;
create table tab1(col1 char(1), col2 number);


drop table tab2 purge;
create table tab2(col1 char(1), tab1_col2_sum number);
insert into tab2 values('a',0);
insert into tab2 values('b',0);
commit;

create or replace trigger tab1_trig
after insert on tab1
begin

	update tab2 t1a set tab1_col2_sum = (select sum(col2) from tab1 t1b where t1b.col1=t1a.col1);

end;
/


show errors

insert into tab1 values('a',1);
insert into tab1 values('a',1);
insert into tab1 values('a',1);

insert into tab1 values('b',1);

select * from tab2;

Open in new window



The downside is that you'll recompute all sums for all rows in every DML statement.

This might not work for you when you scale it up.
0
xbox360dpAuthor Commented:
slightwv,

I'm not actually looking for the SUM ... Sum of Products is just a name. It could be Price Range ... etc etc

I need to concatenate two different rows of data through the trigger.

Example:

Key|Value
1234 999
4567 888

Result:

XXX(999,888)
0
slightwv (䄆 Netminder) Commented:
Will a table level trigger not work?

>>I need to concatenate two different rows of data through the trigger.

Then why do you need the two temp tables?

It seems like you can generate the values without them but I don't fully understand the requirements.

If you could post a small test case complete with tables, sample data and expected results we can probably post working code.
0
xbox360dpAuthor Commented:
slightwv,

Here is a perfect example:

SQL>   CREATE TABLE "CMKAT"."ART_TEST" (        "ARTKEYI" NUMBER, "ARTBEZC" VARH
AR2(30 CHAR),"ARTVF1C" VARCHAR2(30 CHAR));

Table created.

SQL> CREATE TABLE "CMKAT"."PRO_TEST" (  "PROKEYI" NUMBER, "PROBEZC" VARCHAR2(30
CHAR), "PROVF1C" VARCHAR2(30 CHAR)) ;

Table created.

SQL>   CREATE TABLE "CMKAT"."LOOKUP" (  "LOOKUP_ARTKEYI" NUMBER, "LOOKUP_PROKEYI
" NUMBER);

Table created.

SQL> Insert into ART_TEST (ARTKEYI,ARTBEZC,ARTVF1C) values (3001,'pants',null);

1 row created.

SQL> Insert into ART_TEST (ARTKEYI,ARTBEZC,ARTVF1C) values (3002,'socks',null);

1 row created.

SQL> Insert into PRO_TEST (PROKEYI,PROBEZC,PROVF1C) values (4001,'outfit',null);

1 row created.

SQL> Insert into LOOKUP(LOOKUP_ARTKEYI,LOOKUP_PROKEYI) values (3001,4001);

1 row created.

SQL> Insert into LOOKUP(LOOKUP_ARTKEYI,LOOKUP_PROKEYI) values (3002,4001);

1 row created.

SQL> commit;

Commit complete.

SQL> SELECT LISTAGG(ARTBEZC,',') WITHIN GROUP (
ORDER BY ARTKEYI) CON,
  PROBEZC GRP
FROM ART_TEST,
  PRO_TEST,
  LOOKUP
WHERE ART_TEST.ARTKEYI = LOOKUP.LOOKUP_ARTKEYI
AND PRO_TEST.PROKEYI   = LOOKUP.LOOKUP_PROKEYI
GROUP BY PROBEZC;

CON                    GRP
----------------        --------
pants,socks       outfit

Open in new window


CREATE OR REPLACE TRIGGER Trigg_Test before
  INSERT OR
  UPDATE ON ART_TEST FOR EACH ROW 
  DECLARE 
  ARTICLE_NAME VARCHAR2(100);
  ELEMENT_NAME VARCHAR2(100);
  BEGIN
    SELECT LISTAGG(:NEW.ARTBEZC,',') WITHIN GROUP (
    ORDER BY :new.ARTKEYI) CON,
      PROBEZC GRP
    INTO article_name,
      element_name
    FROM
      PRO_TEST,
      LOOKUP
    WHERE :new.ARTKEYI = LOOKUP.LOOKUP_ARTKEYI
    AND PRO_TEST.PROKEYI   = LOOKUP.LOOKUP_PROKEYI
    GROUP BY PROBEZC;
    INSERT INTO TEST_TABLE_FOR_TRIGGER VALUES
      (ARTICLE_NAME,ELEMENT_NAME);
  END;

Open in new window


SQL> UPDATE ART_TEST SET ARTVF1C = 'TEST'
where artkeyi in (3001,3002);

2 rows updated.

SQL> commit;

Commit complete.

SQL> SELECT * FROM TEST_TABLE_FOR_TRIGGER;

ART_CONCAT     ELEMENT
-------                     -------  
pants                   outfit
socks		     outfit

Open in new window


What I want is the following:

ART_CONCAT     ELEMENT
-------                     -------  
pants,socks        outfit
0
johnsoneSenior Oracle DBACommented:
Based on the sample provided, it looks to me like you are overcomplicating this.  All you really need is a list of the rows updated in a temporary table.  Since I am assuming that more than one session could be updating the data, ideally you need a global temporary table.  Using your sample, I would create it this way:
CREATE global TEMPORARY TABLE test_table_for_trigger 
  ( 
     artkeyi NUMBER 
  ) 
ON COMMIT DELETE ROWS; 

Open in new window

Then the trigger becomes simply:
CREATE OR replace TRIGGER trigg_test 
  BEFORE INSERT OR UPDATE ON art_test 
  FOR EACH ROW 
BEGIN 
    INSERT INTO test_table_for_trigger 
    VALUES      (:new.artkeyi); 
END; 
/ 

Open in new window

Then once you do the update that you provided in the sample, query the results with this:
SELECT Listagg(art_test.artbezc, ',') 
         within GROUP ( ORDER BY art_test.artkeyi) CON, 
       pro_test.probezc                            GRP 
FROM   art_test, 
       pro_test, 
       lookup, 
       test_table_for_trigger 
WHERE  art_test.artkeyi = lookup.lookup_artkeyi 
       AND pro_test.prokeyi = lookup.lookup_prokeyi 
       AND test_table_for_trigger.artkeyi = lookup.lookup_artkeyi 
GROUP  BY pro_test.probezc; 

Open in new window

Using your sample and that code, I got the results you posted.
0
xbox360dpAuthor Commented:
johnsone,

I don't see how this will work given the example that I gave you. I values must already be concatenated in the column.

So that when I query the table I get the  correct results.
0
slightwv (䄆 Netminder) Commented:
What happens if you issue two updates back to back without a commit or rollback in between?

...
UPDATE ART_TEST SET ARTVF1C = 'TEST' where artkeyi in (3001,3002);
UPDATE ART_TEST SET ARTVF1C = 'TEST' where artkeyi in (3001,3002);
commit;
...

If you expect:
pants,pants,socks,socks        outfit

I think I have a solution.

If you expect something different, then there is a flaw in the requirement.
0
slightwv (䄆 Netminder) Commented:
Here is what I came up with.
/*
drop TABLE "ART_TEST" purge;
drop TABLE "PRO_TEST" purge;
drop TABLE "LOOKUP" purge;
drop table TEST_TABLE_FOR_TRIGGER purge;
drop table mygtt purge;
*/

CREATE TABLE "ART_TEST" (        "ARTKEYI" NUMBER, "ARTBEZC" VARCHAR2(30 CHAR),"ARTVF1C" VARCHAR2(30 CHAR));
CREATE TABLE "PRO_TEST" (  "PROKEYI" NUMBER, "PROBEZC" VARCHAR2(30 CHAR), "PROVF1C" VARCHAR2(30 CHAR)) ;
CREATE TABLE "LOOKUP" (  "LOOKUP_ARTKEYI" NUMBER, "LOOKUP_PROKEYI" NUMBER);

create global temporary table mygtt(tmpvalue varchar2(30)) on commit delete rows; 

create table TEST_TABLE_FOR_TRIGGER (ART_CONCAT varchar2(50),     ELEMENT varchar2(30));

Insert into ART_TEST (ARTKEYI,ARTBEZC,ARTVF1C) values (3001,'pants',null);
Insert into ART_TEST (ARTKEYI,ARTBEZC,ARTVF1C) values (3002,'socks',null);

Insert into PRO_TEST (PROKEYI,PROBEZC,PROVF1C) values (4001,'outfit',null);

Insert into LOOKUP(LOOKUP_ARTKEYI,LOOKUP_PROKEYI) values (3001,4001);
Insert into LOOKUP(LOOKUP_ARTKEYI,LOOKUP_PROKEYI) values (3002,4001);
commit;

CREATE OR REPLACE TRIGGER Trigg_Test before
  INSERT OR
  UPDATE ON ART_TEST FOR EACH ROW 
  DECLARE 
  v_ARTICLE_NAME VARCHAR2(100);
  v_ELEMENT_NAME VARCHAR2(100);
  BEGIN
  	insert into mygtt values(:new.artbezc);

	select listagg(tmpvalue,',') within group (order by tmpvalue) into v_article_name from mygtt;

	select probezc into v_element_name
    FROM
      PRO_TEST,
      LOOKUP
    WHERE :new.ARTKEYI = LOOKUP.LOOKUP_ARTKEYI
    AND PRO_TEST.PROKEYI   = LOOKUP.LOOKUP_PROKEYI
    ;

  	delete from test_table_for_trigger where element=v_element_name;

    INSERT INTO TEST_TABLE_FOR_TRIGGER VALUES
      (v_ARTICLE_NAME,v_ELEMENT_NAME);
END;
/

show errors

UPDATE ART_TEST SET ARTVF1C = 'TEST' where artkeyi in (3001,3002);
commit;
UPDATE ART_TEST SET ARTVF1C = 'TEST' where artkeyi in (3001,3002);
commit;

SELECT * FROM TEST_TABLE_FOR_TRIGGER;

Open in new window

0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
xbox360dpAuthor Commented:
slightwv,

This works great! Could help me incorporate this logic into the trigger in my original question?
0
slightwv (䄆 Netminder) Commented:
>>Could help me incorporate this logic into the trigger in my original question?

Not really.  Since I don't have the tables or values to test with, anything I would post would be a guess at best.

What I can do is say that you don't need the cursor or two temp tables.

Just create the global temporary table in place of your two temp tables.

Then replace the insert into test_table_for_trigger in my code with the UPDATE in your original trigger.
0
slightwv (䄆 Netminder) Commented:
You also probably don't need the:
        delete from test_table_for_trigger where element=v_element_name;


Since you are updating a single column, just keep updating...  The final update should have the correct string.

I did the delete because your test case did an insert so I had one row for every trigger fire.
0
johnsoneSenior Oracle DBACommented:
For what I posted, why does the table need to have the concatenated values in it?  You either query it to concatenate the values at that point, or create a view that does it for you.
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Oracle Database

From novice to tech pro — start learning today.