How to fix this error?

jdev1

Hello all,

I am working in pl/sql and my task is Create a procedure to display up to 30 employees on the screen. Display the employees sorted by department id and

employee id in ascending order. Do not show more than 10 employees from one department even if there are more

employees there. Use two nested loops – one for all departments and another for employees in each department. The

result should be displayed as follows:

#1: Whalen (#1 in dept 10)

#2: Hartstein (#1 in dept 20)

#3: Fay (#2 in dept 20)

#4: Raphaely (#1 in dept 30)

...

#26: Baer (#1 in dept 70)

#27: Russell (#1 in dept 80)

#28: Partners (#2 in dept 80)

#29: Errazuriz (#3 in dept 80)

#30: Cambrault (#4 in dept 80). But I am getting error in loop:

Alex Nuijten

Where employee_id = department_id? (line 21)

Are you sure about that condition? So, probably your collection is empty.

jdev1

I tried to check if array is null with this

And after testing I got this statement: Not null as output.

jdev1

Oh I forgot to write from departments table also. I then try with this.

mathguy

1. If you want us to help you, you should help us as well. Everything you posted is fine, just as it should be, with one major exception. It is often helpful to us to be able to work with your code. For that, we must be able to copy it from here and paste it in our editor. We can't do that with images - it is much more helpful to post your code as plain text (copy from your editor, paste here; select the code, and use the small icon/link on the left to format as code).
2. Several issues in your existing code. The WHERE clause: your associative array WILL be empty (not null), because you want all the rows where the employee id equals the department id. There are no such rows in the EMPLOYEE table. In a later answer (which I didn't read yet) you seem to say that that's not the case. It is; you must be making additional mistakes in your tests. Keep in mind though that for associative arrays you never test for "null" - the array exists as soon as you declare it. You must check for "empty".
3. Your instructor showed explicitly the output you must get. That shows the LAST name for each employee; why are you using FIRST_NAME everywhere?
4. I assume you are building your code one step at a time, which is fine. I don't see any attempt to limit the number of employees shown in the output (either to "no more than 10 from a single department" or "no more than 30 total"). I assume you were planning to do that next, so I won't show you how to do that; it is best if you continue work on your own, and ask more questions as they arise. But, I was curious here: are you allowed to write all the necessary conditions in the SELECT statement (so that the PL/SQL piece is just an unnecessary wrapper around the SELECT statement - which you must write only as practice for your class), or are you required to collect ALL the records from the base table into your associative array, and build all the "limiting" logic in the procedure (in PL/SQL)? If this is homework in a PL/SQL class, it would make more sense to build all the logic in PL/SQL (even though in real life that is a bad practice). Do you know what your instructor had in mind about this?

jdev1

Thank you for the answer, my pleasure. I added limit to 30 employees but I don't imagine about comparing when limiting employees working in department, count not bigger than 10 in same dept. Also I am publishing my updated code here:

create or replace procedure emps is

TYPE

em

IS RECORD (

employee_id employees.employee_id%type,

last_name employees.first_name%type,

department_id departments.department_id%type

);

type empl is table of em index by pls_integer;

employee empl;

begin

select employee_id, last_name, department_id bulk collect into employee

from employees

order by department_id, employee_id;

--IF employee IS NULL THEN

 --  dbms_output.Put_line('Null associative array');

 -- ELSE

 --  dbms_output.Put_line('Not null');

    

--end if;

for i in employee.first..30

loop

 dbms_output.put_line(employee(i).employee_id || employee(i).last_name || i|| employee(i).department_id);

 --- dbms_output.put_line('asd');

end loop;

end emps;

jdev1

Updated code:

 

create or replace procedure emps is

 

 

TYPE

em

IS RECORD (

employee_id employees.employee_id%type,

last_name employees.first_name%type,

department_id departments.department_id%type

);

type empl is table of em index by pls_integer;

employee empl;

begin

select employee_id, last_name, department_id bulk collect into employee

from employees

order by department_id, employee_id;

--IF employee IS NULL THEN

 --  dbms_output.Put_line('Null associative array');

 -- ELSE

 --  dbms_output.Put_line('Not null');

    

--end if;

<<outer>>

for i in employee.first..10

loop

 <<inner>>

 for j in employee.first..30

 loop

 dbms_output.put_line(employee(i).employee_id || employee(i).last_name || i|| employee(i).department_id);

 exit outer when i =30;

 exit when j=10;

end loop;

 --- dbms_output.put_line('asd');

end loop;

end emps;

James Su

select *

bulk collect into employee

from (

select employee_id, last_name, department_id

from (

select employee_id, last_name, department_id,row_number() over(partition by department_id order by employee_id) rn

from employees

)

where rn<=10

order by department_id, employee_id

)

where rownum<=30;

jdev1

Ok nice, thanks. But I have to use data structure as array and use nested loops

mathguy

1. Did you try to format your code "as code", as I suggested? If not, why not? If yes, what problem did you run into?
2. Change the data type of the LAST_NAME component of your record to ...LAST_NAME%TYPE (you changed everything else, but you left FIRST_NAME%TYPE in the declaration; perhaps the first and the last name columns have the same data type, but that still doesn't look right).
3. I don't really know how to do this with a loop nested within another loop. I am able to do it with a single loop, as I show below; perhaps that will give you some ideas. I coded exactly what I would do with pen on paper (without the help of a computer), step by step. I need two counters: one for how many records I selected from a given department, changing it back to 1 whenever I move to a new department; and another counter for the total count of records I selected for output, exiting the (only) loop when this global counter reaches 30. Like this:

create or replace procedure emps is
  type em is record (
    employee_id   hr.employees.employee_id%type,
    last_name     hr.employees.last_name%type,
    department_id hr.employees.department_id%type
  );
  type empl is table of em index by pls_integer;
  employee empl;
  cnt_dept  pls_integer;
  cnt_total pls_integer;
  d_id      hr.employees.department_id%type;
begin
  select employee_id, last_name, department_id bulk collect into employee
  from   hr.employees
  order  by department_id, employee_id;
  cnt_total := 0; 
  for i in employee.first .. employee.last
  loop
    exit when cnt_total = 30;
    cnt_dept := case when employee(i).department_id = d_id then cnt_dept + 1 else 1 end;
    if cnt_dept <= 10 then
      dbms_output.put_line(employee(i).employee_id || ' ' || employee(i).last_name ||
                                                      ' ' || employee(i).department_id);
      cnt_total := cnt_total + 1;
    end if;
    d_id := employee(i).department_id;
  end loop;
end;
/

James Su

https://community.oracle.com/tech/developers/discussion/comment/16782574#Comment_16782574

What is the reason to fetch all the data into an array while you only need 30 rows? This is a waste of PGA.