PL/SQL error question
I am trying to write a stored procedure that inserts a row into an employee table. If the department does not exist, that department needs to be inserted into the departments table. I have the following code:
drop table employees;
drop table departments;
create table departments(
dept varchar2(30),
dept_number number,
dept_city varchar2(30),
CO开发者_开发问答NSTRAINT pk_dept PRIMARY KEY(dept)
);
create table employees(
dept varchar2(30),
employee_name varchar2(40),
employee_id number,
CONSTRAINT pk_id PRIMARY KEY(employee_id),
CONSTRAINT fk_dept FOREIGN KEY (dept) REFERENCES departments(dept)
);
CREATE OR REPLACE PROCEDURE employeeadd(
a_dept IN VARCHAR2,
a_employee_name IN VARCHAR2,
a_employee_id IN NUMBER)
as
li_count NUMBER;
BEGIN
sp_check_dept(a_dept, li_count);
if li_count = 0 then
INSERT INTO departments (dept) values (a_dept);
return;
end if;
INSERT INTO employee values (a_dept, a_employee_name, a_employee_id);
end;
/
create or replace procedure sp_check_dept(a_dept IN NUMBER,
a_count OUT NUMBER)
as
begin
select count(*)
into a_count
from departments
where dept_number = a_dept;
end;
/
When I run my execute statement as execute employeeadd('marketing', 'john', 10); I get the following errors. I can't seem to figure out how to get past the errors and/or write this correctly:
ORA-06502: PL/SQL: numeric or value error: character to number conversion error ORA-06512: at "employeeadd", line 8 ORA-06512: at line 1
Why is li_count declared outside the BEGIN...END block? Do you need to assign it before sending it as an argument to sp_check_dept()?
Edit: Just saw your followup comment: sp_check_dept is expecting a number as its first parameter; you have declared a_dept as VARCHAR.
sp_check_dept
takes a department number an input parameter (a NUMBER) and returns a count as an ouput parameter. employeeadd
is passing a department name (a VARCHAR2) as the first parameter to sp_check_dept
. There are a couple of ways to fix this. In general, you'll want a more consistent method of naming parameters to make it easier to identify these problems.
Option 1: Use the department name for both functions
create or replace procedure sp_check_dept(p_dept_name IN departments.dept%type,
p_count OUT NUMBER)
as
begin
select count(*)
into p_count
from departments
where dept = p_dept_name;
end;
/
CREATE OR REPLACE PROCEDURE employeeadd(
p_dept_name IN departments.dept%type,
p_employee_name IN employees.employee_name%type,
p_employee_id IN employees.employee_id%type)
as
li_count NUMBER;
BEGIN
sp_check_dept(p_dept_name, li_count);
if li_count = 0 then
INSERT INTO departments (dept)
VALUES (p_dept_name);
end if;
INSERT INTO employee(dept, employee_name, employee_id)
VALUES (p_dept, p_employee_name, p_employee_id);
end;
/
Option 2: Convert the department name in employeeAdd
to the department number before passing it to sp_check_dept
create or replace procedure sp_check_dept(p_dept_number IN departments.dept_number%type,
p_count OUT NUMBER)
as
begin
select count(*)
into p_count
from departments
where dept_number = p_dept_number;
end;
/
CREATE OR REPLACE FUNCTION get_dept_number( p_dept_name IN departments.dept%tyep )
RETURN departments.dept_number%type
IS
l_dept_number departments.dept_number%type;
BEGIN
SELECT dept_number
INTO l_dept_number
FROM departments
WHERE dept = p_dept_name;
RETURN l_dept_number
END;
/
CREATE OR REPLACE PROCEDURE employeeadd(
p_dept_name IN departments.dept%type,
p_employee_name IN employees.employee_name%type,
p_employee_id IN employees.employee_id%type)
as
li_count NUMBER;
BEGIN
sp_check_dept( get_dept_number(p_dept_name), li_count);
if li_count = 0 then
INSERT INTO departments (dept)
VALUES (p_dept_name);
end if;
INSERT INTO employee(dept, employee_name, employee_id)
VALUES (p_dept, p_employee_name, p_employee_id);
end;
/
A couple of other observations
- I removed the
RETURN
statement from your IF statement inemployeeAdd
. You almost certainly do not want to exit the procedure after inserting a row into theDEPARTMENTS
table before inserting the row into theEMPLOYEE
table. - Your table definition used the plural
EMPLOYEES
. Your procedure used the singularEMPLOYEE
. I did not correct that because I wasn't sure whether the DDL you posted was incorrect or whether the procedure you posted was incorrect. - It would, in general, make far more sense for
sp_check_dept
to be implemented as a function that returned the count rather than as a procedure with an OUT parameter. If a piece of code simply exists to return data to the caller, it should be declared as a function. - From a data model standpoint, the column name
DEPT
isn't particularly good. It would be far more appropriate to use something likeDEPARTMENT_NAME
that conveys what the column actually represents. - From a data model standpoint, having the VARCHAR2 column
DEPT
(even if it is renamed toDEPARTMENT_NAME
) as the primary key ofDEPARTMENTS
and the foreign key inEMPLOYEES
does not make much sense. The primary key should be immutable. However the name of the department will change over time. It would make far more sense for theDEPARTMENT_NUMBER
to be the primary key and for theDEPARTMENT_NAME
to simply be marked as unique. That will make it far easier when the Marketing department gets renamed Advertising in the future because you won't have to chase down all the child tables to update them. - You should pick a naming convention for procedures and stick with that. I would prefer
check_dept
andadd_employee
(verb followed by subject, underscores separating words, no prefix). But if you wantedsp_check_dept
andsp_add_employee
orcheckDept
andaddEmployee
or evensp_dept_check
andsp_employee_add
that would be fine. But you'll drive yourself, and the other developers, crazy if there is no pattern to your procedure naming conventions.
2 possibilities I can see: 1. the employee table has columns in a different order than your insert statement and it's trying to convert dept or name to the id 2. the value set into li_count isn't a number so it's trying to convert the return value to a number and giving you the error
精彩评论