Forum Stats

  • 3,722,538 Users
  • 2,244,331 Discussions
  • 7,849,911 Comments

Discussions

Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

Issues with procedure!

user782973-Oracle
user782973-Oracle Member Posts: 67
edited January 2015 in SQL & PL/SQL

Hi, Can someone help me understand whats wrong with my procedure.

CREATE OR REPLACE procedure VALID_PROC( START_DATE IN DATE, END_DATE IN DATE)
IS


cursor c1 is 
SELECT * FROM EMP
WHERE HIREDATE BETWEEN TO_DATE('01-01-85','DD-MM-YY') AND TO_DATE('01-01-80','DD-MM-YY');
c_row c1%rowtype;
BEGIN
OPEN C1;
fetch c1 into c_row;
if c1%notfound then 
endif;


INSERT INTO emp_back(empno,ename,job,mgr,hiredate,sal,comm,deptno)
VALUES(c_row.empno,c_row.ename,c_row.job,c_row.hiredate,c_row.sal,c_row.comm,c_row.deptno);


commit;
close c1;
END;
/
Tagged:
Boneist

Answers

  • Frank Kulash
    Frank Kulash Member, Moderator Posts: 40,186 Red Diamond
    edited January 2015

    Hi,

    That depends on what "wrong" means to you.  What is the procedure supposed to do?  What is it doing differently?

    Whenever you have a question, please post a little sample data (CREATE TABLE and INSERT statements, relevant columns only) for all the tables involved, and the exact results you want from that data, so that the people who want to help you can re-create the problem and test their ideas.  (If you're using the Oracle-supplied scott.emp table, you don't need to post CREATE TABLE and INSERT statements for it; just make it clear what you're doing.)

    If you're asking about a DML operation, such as UPDATE, then the INSERT statements you post should show what the tables are like before the DML, and the results will be the contents of the changed table after the DML.

    Explain, using specific examples, how you get those results from that data.

    Always say what version of Oracle you're using (e.g. 11.2.0.2.0).

    See the forum FAQ: 

    You're using 2-digit years; that's always a mistake,

  • ascheffer
    ascheffer Member Posts: 1,882 Gold Trophy

    Try to spot the difference with this:

    CREATE OR REPLACE procedure VALID_PROC( START_DATE IN DATE, END_DATE IN DATE)

    is

    begin

      insert into emp_back(empno,ename,job,mgr,hiredate,sal,comm,deptno)

      select c_row.empno,c_row.ename,c_row.job,c_row.hiredate,c_row.sal,c_row.comm,c_row.deptno

      from emp c_row

      where hiredate between start_date and end_date;

    end;

    It might give you a clue to what's wrong with your procedure

  • John Stegeman
    John Stegeman Member Posts: 24,269 Blue Diamond
    edited January 2015

    ooh - exclamation point in the title, must be important.

    First, before I even read your procedure - why do you think there is something wrong with it? Did you get an error? If so, why didn't you tell us?

    Secondly, there are lot's of problems...

    1). Why are you using 2 digits to represent a year?

    2). You just blindly skip over the case if there is no record found

    3). You don't use the parameters you passed in

    4). Committing inside a stored procedure is often not the right thing to do

    5). Using PL/SQL when you didn't need to

    6). Not processing all of the rows in the cursor (assuming that was your intent)

    There may be more, but those jumped right to mind

  • user782973-Oracle
    user782973-Oracle Member Posts: 67
    edited January 2015

    Hi John, I do get a lot of errors. First i tried using dynamic values which didn't work and hence tried static dates.

    CREATE OR REPLACE procedure VALID_PROC( START_DATE IN DATE, END_DATE IN DATE)
    IS
    cursor c1 is 
    SELECT * FROM EMP
    WHERE HIREDATE BETWEEN TO_DATE(START_DATE,'DD-MM-YY') AND TO_DATE(END_DATE,'DD-MM-YY');
    c_row c1%rowtype;
    BEGIN
    OPEN C1;
    fetch c1 into c_row;
    if c1%notfound then 
    endif;
    INSERT INTO emp_back(empno,ename,job,mgr,hiredate,sal,comm,deptno)
    VALUES(c_row.empno,c_row.ename,c_row.job,c_row.hiredate,c_row.sal,c_row.comm,c_row.deptno);
    commit;
    close c1;
    END;
    /
    

    And i tried

    select TO_DATE('01-01-80','DD-MM-YY') from dual;
    

    which gives me date. So not sure what you meant by "2 digits to represent a year".

  • user782973-Oracle
    user782973-Oracle Member Posts: 67

    Hi Frank, My apologies, will surely follow instructions. I was trying to create a procedure which inserts values based on 2 dates. I'm using oracle scott.emp data.

  • user782973-Oracle
    user782973-Oracle Member Posts: 67
    edited January 2015

    Thanks Ascheffer! That helps. However the validation fails and still gives errors when i try and complile.

  • ascheffer
    ascheffer Member Posts: 1,882 Gold Trophy
    edited January 2015
    • select TO_DATE('01-01-80','DD-MM-YY') from dual; 

    will give a date in 2080. Probably not what you want

  • Frank Kulash
    Frank Kulash Member, Moderator Posts: 40,186 Red Diamond

    Hi,

    user782973-Oracle wrote:
    
    Thanks Ascheffer! That helps. However the validation fails and still gives errors when i try and complile.
    

    Don't you think it would be helpful to say what the errors are?

  • ascheffer
    ascheffer Member Posts: 1,882 Gold Trophy
    edited January 2015

    No need, I'm very good at guessing:

    endif should be end if;

    PLS-00103: Encountered the symbol ";" when expecting one of the following:

       if
    06550. 00000 -  "line %s, column %s:\n%s"
    *Cause:    Usually a PL/SQL compilation error.
    *Action:

    And if you fixed that you wil get

    PLS-00103: Encountered the symbol "END" when expecting one of the following:

       begin case declare exit for goto if loop mod null pragma
       raise return select update while with <een ID>
       <een scheidingsteken-ID tussen dubbele aanhalingstekens>
       <een bindvariabele> << close current delete fetch lock insert
       open rollback savepoint set sql execute commit forall merge
       pipe
    06550. 00000 -  "line %s, column %s:\n%s"
    *Cause:    Usually a PL/SQL compilation error.
    *Action:

    because you forgot to put a statement after your if clause:

  • Frank Kulash
    Frank Kulash Member, Moderator Posts: 40,186 Red Diamond
    edited January 2015
    user782973-Oracle wrote:
    
    ...
    And i tried
    
    
    
    
    1. select TO_DATE('01-01-80','DD-MM-YY') from dual; 
    select TO_DATE('01-01-80','DD-MM-YY') from dual;
    
    which gives me date. So not sure what you meant by "2 digits to represent a year".
    
    
    
    

    It means using a string like '01-01-80', where only 2 digits (e.g. '8' and '0') are supposed to indicate the year.

    The trouble with 2-digit years is that you can easily get confused by 2 different years (in different centuries) having the same last 2 digits.  For example, the table might have data from 1980, but your procedure might be looking for data from 2080.

  • user782973-Oracle
    user782973-Oracle Member Posts: 67

    Thanks Frank! Whats the right way of finding difference between two dates?

  • Solomon Yakobson
    Solomon Yakobson Member Posts: 18,195 Black Diamond
    edited January 2015

    The right way is to use 4 digit year, use RR format or better use date literals. For example:

    DATE '1980-01-01'


    And BETWEEN works left to right, therefore start value must be less or equal to end value, therefore


    WHERE HIREDATE BETWEEN TO_DATE('01-01-85','DD-MM-YY') AND TO_DATE('01-01-80','DD-MM-YY'); 


    will never work. Use:

    WHERE HIREDATE BETWEEN DATE '1980-01-01' AND DATE '1985-01-01'; 


    And keep in mind, the above includes January 1, 1985. Somehow I have a feeling you want:

    WHERE HIREDATE BETWEEN DATE '1980-01-01' AND DATE '1984-12-31';


    SY.

  • Solomon Yakobson
    Solomon Yakobson Member Posts: 18,195 Black Diamond

    And one more thing. Why do you have literals in where clause? It sounds you should use procedure parameters:

    WHERE HIREDATE BETWEEN START_DATE AND END_DATE;

    SY.

  • James Su
    James Su Member Posts: 979 Silver Trophy

    There's nothing between line 12 and 13. You need to have some code between IF and END IF

    if c1%notfound then  

    endif; 

    BTW why don't you do a simple INERT...SELECT ?

  • 2842297
    2842297 Member Posts: 3
    edited January 2015

    Hi,

    This will solve your problem

    ------------------------------------

    CREATE OR REPLACE procedure VALID_PROC( START_DATE IN DATE, END_DATE IN DATE) 

    IS 

    cursor c1 is  

    SELECT * FROM EMP 

    WHERE HIREDATE BETWEEN TO_DATE(START_DATE,'DD-MM-YYYY') AND TO_DATE(END_DATE,'DD-MM-YYYY'); 

    c_row c1%rowtype; 

    BEGIN 

    OPEN C1;

    loop 

    fetch c1 into c_row; 

    exit when  c1%notfound;    

    INSERT INTO emp_back(empno,ename,job,mgr,hiredate,sal,comm,deptno) 

    VALUES(c_row.empno,c_row.ename,c_row.job,c_row.mgr,c_row.hiredate,c_row.sal,c_row.comm,c_row.deptno); 

    end loop;

    close c1; 

    END; 

    --------------------------

    mistakes : missed MGR value

                    didn't use loop

    when Procedure/Function return compilation error...please try 

               SHOW ERROR PROCEDURE procedure_name;

    it will give cause of error.

    Thanks

    Rajesh

  • BluShadow
    BluShadow Member, Moderator Posts: 40,870 Red Diamond
    user782973-Oracle wrote:
    
    Hi, Can someone help me understand whats wrong with my procedure.
    
    
    
    1. CREATE OR REPLACE procedure VALID_PROC( START_DATE IN DATE, END_DATE IN DATE) 
    2. IS 
    3.  
    4.  
    5. cursor c1 is 
    6. SELECT * FROM EMP 
    7. WHERE HIREDATE BETWEEN TO_DATE('01-01-85','DD-MM-YY') AND TO_DATE('01-01-80','DD-MM-YY'); 
    8. c_row c1%rowtype; 
    9. BEGIN 
    10. OPEN C1; 
    11. fetch c1 into c_row; 
    12. if c1%notfound then 
    13. endif; 
    14.  
    15.  
    16. INSERT INTO emp_back(empno,ename,job,mgr,hiredate,sal,comm,deptno) 
    17. VALUES(c_row.empno,c_row.ename,c_row.job,c_row.hiredate,c_row.sal,c_row.comm,c_row.deptno); 
    18.  
    19.  
    20. commit; 
    21. close c1; 
    22. END; 
    CREATE OR REPLACE procedure VALID_PROC( START_DATE IN DATE, END_DATE IN DATE)
    IS
    
    
    cursor c1 is 
    SELECT * FROM EMP
    WHERE HIREDATE BETWEEN TO_DATE('01-01-85','DD-MM-YY') AND TO_DATE('01-01-80','DD-MM-YY');
    c_row c1%rowtype;
    BEGIN
    OPEN C1;
    fetch c1 into c_row;
    if c1%notfound then 
    endif;
    
    
    INSERT INTO emp_back(empno,ename,job,mgr,hiredate,sal,comm,deptno)
    VALUES(c_row.empno,c_row.ename,c_row.job,c_row.hiredate,c_row.sal,c_row.comm,c_row.deptno);
    
    
    commit;
    close c1;
    END;
    /
    

    What's right with it is the question.... the answer being... "not much".

    The whole thing can be simplified to just:

    create or replace procedure valid_proc(start_date in date, end_date in date) as
    begin
      insert into emp_back(empno, ename, job, mgr, hiredate, sal, comm, deptno)
        select empno, ename, job, mgr, hiredate, sal, comm, deptno
        from  emp
        where  hiredate between start_date and end_date;
      commit; -- if appropriate to business/transaction logic
    end; 
    / 
    
    Boneist
This discussion has been closed.