0

Please, advice me on below procedure. below query returns null on filename_ but not sure how to get this filename_ properly.

PROCEDURE pickup( app_id IN VARCHAR2, Interval IN NUMBER, filename_ OUT VARCHAR2, status_ OUT VARCHAR2) IS
    BEGIN
        SELECT filename, status INTO filename_, status_
            FROM (SELECT filename, status, CUSTPROFID, FILESIZE, AMP_FILE_NAME FROM INBOUND_UNCOMPLETED_PROCESS WHERE (status = 'error' or status = 'retry') 
                AND application_id IS NULL AND CREATEDAT < sysdate - 1/(24*60)  AND (LAST_UPDATEDAT IS NULL OR LAST_UPDATEDAT < sysdate - Interval/(24*60))
                order by LAST_UPDATEDAT NULLS FIRST)
            WHERE ROWNUM < 2 FOR UPDATE NOWAIT;
        UPDATE INBOUND SET application_id = app_id WHERE filename = filename_;
        COMMIT;
    EXCEPTION
         WHEN NO_DATA_FOUND THEN
            filename_ := NO_DATA_FOUND;
        WHEN OTHERS THEN
        filename_ := NULL;
    END pickup;

Below query returns filename_ and status_ but I would need to add ORDER BY LAST_UPDATEDAT on the select query.

PROCEDURE pickup( app_id IN VARCHAR2, Interval IN NUMBER, filename_ OUT VARCHAR2, status_ OUT VARCHAR2) IS
    BEGIN
        SELECT filename, status INTO filename_, status_
            FROM INBOUND WHERE (status = 'error' or status = 'retry') 
            AND application_id IS NULL AND CREATEDAT < sysdate - 1/(24*60)  AND (LAST_UPDATEDAT IS NULL OR LAST_UPDATEDAT < sysdate - Interval/(24*60))
            AND ROWNUM < 2 FOR UPDATE NOWAIT;
        UPDATE INBOUND SET application_id = app_id WHERE filename = filename_;
        COMMIT;
    EXCEPTION
         WHEN NO_DATA_FOUND THEN
            filename_ := NO_DATA_FOUND;
        WHEN OTHERS THEN
        filename_ := NULL;
    END pickup;

Thank you!

Julia
  • 133
  • 11

1 Answers1

0

There are two errors in the procedure.

  1. In your NO_DATA_FOUND exception handler, the line:

    filename_ := NO_DATA_FOUND;
    

    is not valid as NO_DATA_FOUND is a built-in exception and not a string. You need to change this to something like:

    filename_ := 'Not found';
    
  2. When you fix this then it the SELECT statement raises the exception:

    ORA-02014: cannot select FOR UPDATE from view with DISTINCT, GROUP BY
    

    This is because you can't filter by ROWNUM (or use the modern FETCH FIRST ROW ONLY syntax) alongside FOR UPDATE. This issue is addressed in this answer.

    The exception is then caught by the OTHERS handler and NULL is returned.

It is bad practice to catch OTHERS. As you have found there can be many issues and catching OTHERS masks them and makes it difficult to find where the problem lies. If you need to catch exceptions then make sure you are specific about which ones need catching and then any other that gets raised is an error and should cause your code to fail and this gives you the opportunity to debug.

Similarly, using COMMIT should not be used a procedure as it prevents you grouping procedures together and issuing a ROLLBACK command to revert multiple changes at once. Instead, the block calling the procedure should COMMIT the values once all changes have been made.

The solution is to do something like this:

CREATE PACKAGE pkg_name IS
PROCEDURE pickup(
  app_id    IN  INBOUND.APPLICATION_ID%TYPE,
  Interval  IN  NUMBER,
  filename_ OUT INBOUND.FILENAME%TYPE,
  status_   OUT INBOUND.STATUS%TYPE
);
END;
/

CREATE PACKAGE BODY pkg_name IS

PROCEDURE pickup(
  app_id    IN  INBOUND.APPLICATION_ID%TYPE,
  Interval  IN  NUMBER,
  filename_ OUT INBOUND.FILENAME%TYPE,
  status_   OUT INBOUND.STATUS%TYPE
) IS
BEGIN
  SELECT filename, status
  INTO filename_, status_
  FROM INBOUND
  WHERE ROWID IN (
    SELECT ROWID
    FROM   INBOUND
    WHERE  status IN ( 'error', 'retry') 
    AND application_id IS NULL
    AND CREATEDAT < sysdate - 1/(24*60)
    AND (LAST_UPDATEDAT IS NULL OR LAST_UPDATEDAT < sysdate - Interval/(24*60))
    ORDER BY LAST_UPDATEDAT NULLS FIRST
    FETCH FIRST ROW ONLY
  )
  FOR UPDATE NOWAIT;

  UPDATE INBOUND SET application_id = app_id WHERE filename = filename_;
EXCEPTION
  WHEN NO_DATA_FOUND THEN
    filename_ := 'Not found.';
END pickup;
END;
/

Which, for the sample data:

CREATE TABLE inbound (
  application_id VARCHAR2(20),
  filename       VARCHAR2(20),
  status         VARCHAR2(20),
  CREATEDAT      DATE,
  LAST_UPDATEDAT DATE
);

INSERT INTO inbound ( filename, status, CREATEDAT, LAST_UPDATEDAT )
SELECT 'abc', 'error', SYSDATE - INTERVAL '3' HOUR, NULL FROM DUAL UNION ALL
SELECT 'def', 'retry', SYSDATE - INTERVAL '2' HOUR, SYSDATE FROM DUAL;

Then:

DECLARE
  fn INBOUND.FILENAME%TYPE;
  st INBOUND.STATUS%TYPE;
BEGIN
  pkg_name.pickup(
    app_id    => 'E42',
    interval  => 0,
    filename_ => fn,
    status_   => st
  );
  COMMIT;
  DBMS_OUTPUT.PUT_LINE( fn || ', ' || st );
END;
/

Outputs:

abc, error

and:

SELECT * FROM inbound;

outputs:

APPLICATION_ID | FILENAME | STATUS | CREATEDAT | LAST_UPDATEDAT
:------------- | :------- | :----- | :-------- | :-------------
E42            | abc      | error  | 21-OCT-20 | null          
null           | def      | retry  | 21-OCT-20 | 21-OCT-20     

db<>fiddle here

MT0
  • 143,790
  • 11
  • 59
  • 117
  • Thanks MT0! I appreciate all your detailed comment! It works like a charm! I would love to start using FETCH FIRST ROW ONLY!!! – Julia Oct 21 '20 at 21:38