Looping

510477

I need to create a set of child values (empty records) when a master is created. They are pay rate variations dependent on three variables. Is the following trigger logic using nested loops correct?

CREATE OR REPLACE TRIGGER  "BI_LOAD_RATES" 
  before insert on "LOAD_RATES"
  for each row
  --variable declarations
  actv    INTEGER;
  actv_dt DATE;
  end_dt  DATE;
  rt_id   NUMBER;
  shft    VARCHAR2(2);
  prem    VARCHAR2(2);
  lvl     INTEGER;
  cursor c_shft is SELECT 'D' from DUAL UNION SELECT 'N' FROM DUAL;
  cursor c_prem is SELECT 'B' from DUAL UNION SELECT 'P' FROM DUAL;
begin
  if :NEW."LOAD_RATE_ID" is null then
    select "SEQ_LOAD_RATE_ID".nextval into rt_id from dual;
    :NEW."LOAD_RATE_ID"   := rt_id;
  end if;
  actv     :=  :NEW.ACTIVE;
  actv_dt  :=  :NEW.EFFECTIVE_DATE;
  end_dt   :=  :NEW.END_DATE
  for shft in c_shft loop
    for prem in c_prem loop
      FOR lvl in 1..4 LOOP
        INSERT INTO LOAD_PAY(ID, LOAD_RATE_ID, PAY_AMT, EXP_LVL, SHIFT, PREMIUM, ACTIVE, ACTIVE_DT, END_DT)
          VALUES (SEQ_LOAD_PAY.nextval, rt_id, 0, lvl, shft, prem, actv, SYSDATE, actv_dt, end_dt);
      END LOOP;
    end loop;
  end loop;
end;
/

Unknown

That I don't know, as you don't describe the logic.
It will be included in my new course 'How do I turn Oracle into a 3GL by processing records instead of sets'
This short fragment is an excellent example of unscalable pl/sql aka 'Slow-by-slow programming.
Even if the design also looks like an exercise in denormalization you could and should have used BULK INSERT.

-------------
Sybrand Bakker
Senior Oracle DBA

ttt

Hi !

I have found ( and try to correct ) some errors

CREATE OR REPLACE TRIGGER  "BI_LOAD_RATES" 
  before insert on "LOAD_RATES"
  for each row
DECLARE -- *missing*
  --variable declarations
  actv    INTEGER;
  actv_dt DATE;
  end_dt  DATE;
  rt_id   NUMBER;
  shft    VARCHAR2(2);
  prem    VARCHAR2(2);
  lvl     INTEGER;
  cursor c_shft is SELECT 'D' a from DUAL UNION SELECT 'N' FROM DUAL; -- *use alias for later reference*
  cursor c_prem is SELECT 'B' a from DUAL UNION SELECT 'P' FROM DUAL; -- *use alias for later reference*
begin
  if :NEW."LOAD_RATE_ID" is null then -- maybe it's better to avoid double_quotes -- not clear 
    select "SEQ_LOAD_RATE_ID".nextval into rt_id from dual;
    :NEW."LOAD_RATE_ID"   := rt_id;
  end if;
  actv     :=  :NEW.ACTIVE;
  actv_dt  :=  :NEW.EFFECTIVE_DATE;
  end_dt   :=  :NEW.END_DATE
  for shft in c_shft loop
    for prem in c_prem loop
      FOR lvl in 1..4 LOOP
        INSERT INTO LOAD_PAY(ID, LOAD_RATE_ID, PAY_AMT, EXP_LVL, SHIFT, PREMIUM, ACTIVE, ACTIVE_DT, END_DT)
          VALUES (SEQ_LOAD_PAY.nextval, rt_id, 0, lvl, shft.a, prem.a, actv, SYSDATE, actv_dt, end_dt); -- *use aliases* 
      END LOOP;
    end loop;
  end loop;
end;
/ 

I haven't tested the code ..

T

Frank Kulash

Hi,

blarman74 wrote:
I need to create a set of child values (empty records) when a master is created. They are pay rate variations dependent on three variables. Is the following trigger logic using nested loops correct?

It's more efficient to do that without any loops at all, something like this:

...     INSERT INTO LOAD_PAY (ID, LOAD_RATE_ID, PAY_AMT, EXP_LVL, SHIFT, PREMIUM, ACTIVE, ACTIVE_DT, END_DT)
	WITH  c_shft    AS
	(
		SELECT	'D'	AS shft		FROM dual	UNION ALL
		SELECT	'N'	AS shft		FROM dual
	)
	,	c_prem	AS
	(
		SELECT	'B'	AS prem		FROM dual	UNION ALL
		SELECT	'P'	AS prem		FROM dual
	)
	,	c_lvl	AS
	(
		SELECT	LEVEL	AS lvl
		FROM	dual
		CONNECT BY	LEVEL <= 4
	)
	SELECT	SEQ_LOAD_PAY.nextval,	-- ID
		rt_id,			-- LOAD_RATE_ID
		0,			-- PAY_AMT
		l.lvl,			-- EXP_LVL
		s.shft,			-- SHIFT
		p.prem,			-- PREMIUM
		:NEW.active,		-- ACTIVE
		SYSDATE,		-- ACTIVE_DT
		:NEW.end_date		-- END_DT
	FROM		c_shft	s
	CROSS JOIN	c_prem	p
	CROSS JOIN	c_lvl	l
	;

There's no point in using a local variable that's just a copy of something in :NEW (but it's not an error, so do it if you want to).
I can see a reson for the local variable rt_id, but not for actv or end_dt.
By the way, you're very smart not to use the exact same name for local variables and columns (e.g., variable end_dt corresponding to column end_date).

CREATE OR REPLACE TRIGGER  "BI_LOAD_RATES" 
before insert on "LOAD_RATES"
...
if :NEW."LOAD_RATE_ID" is null then
select "SEQ_LOAD_RATE_ID".nextval into rt_id from dual;
:NEW."LOAD_RATE_ID"   := rt_id;
...

You don't need any of those double-quotes.

Edited by: Frank Kulash on Mar 29, 2010 4:45 PM

510477

Thanks. That's pretty much what I was after. Basically, I need to leave this open ended for each loop section so that I can add values where needed, thus the desire to avoid a hard-coded loop even though it may be more efficient. This isn't going to be happening very often (1 record/week or so), so I am willing to trade speed for the flexibility.

As for the double-quotes, I took the existing trigger and exported it as text then added the loops. I haven't had a problem with them, but I usually remove them when I hard code a trigger.

Here's the amended version:

CREATE OR REPLACE TRIGGER  BI_LOAD_RATES 
  before insert on LOAD_RATES
  for each row
declare
  --variable declarations
  actv    INTEGER;
  actv_dt DATE;
  end_dt  DATE;
  rt_id   NUMBER;
  lvl     INTEGER;
begin
  if :NEW.LOAD_RATE_ID is null then
    select SEQ_LOAD_RATE_ID.nextval into rt_id from dual;
    :NEW.LOAD_RATE_ID   := rt_id;
  end if;
  actv     :=  :NEW.ACTIVE;
  actv_dt  :=  :NEW.EFFECTIVE_DATE;
  end_dt   :=  :NEW.END_DATE;
  for shft in (SELECT 'D' s from DUAL UNION SELECT 'N' s FROM DUAL) loop
    for prem in (SELECT 'B' p from DUAL UNION SELECT 'P' p FROM DUAL) loop
      FOR lvl in 1..4 LOOP
        INSERT INTO LOAD_PAY(ID, LOAD_RATE_ID, PAY_AMT, EXP_LVL, SHIFT, PREMIUM, ACTIVE, ACTIVE_DT, END_DT)
          VALUES (SEQ_LOAD_PAY.nextval, rt_id, 0, lvl, shft.s, prem.p, actv, actv_dt, end_dt);
      END LOOP;
    end loop;
  end loop;
end;
/

This one works.