Use of Cursor in Trigger causing too many row inserts

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...

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.

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,  ....

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.

Ok. What would the where clause look like?

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

>>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.

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?

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.

>>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;

Select allOpen 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.

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)

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.

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

Select allOpen 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;

Select allOpen 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

Select allOpen in new window

What I want is the following:

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

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; 

Select allOpen 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; 
/ 

Select allOpen 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; 

Select allOpen in new window

Using your sample and that code, I got the results you posted.

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.

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.

slightwv,

This works great! Could help me incorporate this logic into the trigger in my original question?

>>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.

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.

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.