Function doesn't return value...but does not throw error

[TimGoff]

Hello,

I use the following function in a query. It doesn't return any result in the field - but also doesn't return an error.

Can you see what's wrong?

Thanks in advance
Tim

create or replace package body UKTV_TOOLS_PKG is

FUNCTION Get_Ready_For_TX_Flag(v_prog_media_id NUMBER)
RETURN VARCHAR2
IS
v_ready_for_tx NUMBER(9);

BEGIN

/**
Codesc values:
4280000 = Ready for TX
4280001 = Not ready for TX
If v_ready_for_tx = 4280001, one or more parts are not ready for tx
**/

SELECT DISTINCT READY_FOR_TX_CODE
INTO
v_ready_for_tx
FROM PROG_MEDIA_SEGMENT
WHERE PROG_MEDIA_ID = v_prog_media_id
AND READY_FOR_TX_CODE = 4280001;


IF v_ready_for_tx = 4280001 THEN
RETURN 'NOT READY FOR TX';
ELSE
RETURN 'READY FOR TX';
END IF;

END Get_Ready_For_TX_Flag;

end UKTV_TOOLS_PKG;

 

[BJCooperIT]

Before the END statement try adding:

exception when others then return(sqlerrm);

to have it return the error message. I suspect you are getting a NO_DATA_FOUND on your select.

^{Beware of false knowledge; it is more dangerous than ignorance. ~George Bernard Shaw}
Consultant Developer/Analyst Oracle, Forms, Reports & PL/SQL (Windows)
My website: Emu Products Plus

 

[Thargy]

Tim,

BJ has made a valid point, but I think that there are a few other problems with you code.

First off, you are using an implicit cursor, so you must be expecting a single value. As always with implicits, you must error trap no_data_found and too_many_rows - I agree with BJ.

However, your sql is a dead give away. Look at it,

SELECT DISTINCT READY_FOR_TX_CODE !!!!

If you are using an implicit cursor you must be expecting a single value. The use of distinct means that you've got several values and are whittling them down somehow. This is fundamentally wrong, and I'm not surprised you're getting bad results. Put sufficient criteria on the sql to guarantee only one value.

I must also pull you up on the use of "magic numbers" in your code. You have rightly chosen to put the function in a package (excellent move) but have then not bothered to define package constants in the specification, which can soft code you magic numbers, and remove them from your code.

If I had code reviewed your posting, it would definitely have failed. You are also making a mistake (IMHO) with the name of your function and its return value. functions always get or return a value, so why put that in its name. Procedures do things, but not functions.

The logic of your pl is also flawed. In the where clause you specify that the ready_to_transmit number must be such that it is ready to transmit. This elegantly guarantees you only find what you want, but is functionally useless.

You then go on to check to see if it's ready to tx or not - why? Your pl guarantees that you only return values that are ok.

Please please please post some clarification, table definitions and sample data. I'm struggling to help here.

As a first attempt, I have the following for you:-

CREATE OR REPLACE package UKTV_TOOLS_PKG is

c_ready_to_transmit     constant number := 4280000;
c_not_ready_to_transmit constant number := 4280001;

FUNCTION Ready_to_transmit(v_prog_media_id in NUMBER)
  RETURN BOOLEAN;
	

end UKTV_TOOLS_PKG;

Note that the use of constants eliminates magic numbers.

create or replace package body UKTV_TOOLS_PKG is

FUNCTION ready_to_transmit(v_prog_media_id in NUMBER)
  RETURN boolean

IS 

v_ready_for_tx   NUMBER(9);

BEGIN  
 
    SELECT ready_for_tx_code
      INTO v_ready_for_tx
      FROM PROG_MEDIA_SEGMENT
     WHERE PROG_MEDIA_ID = v_prog_media_id 
       AND READY_FOR_TX_CODE = c_not_ready_to_transmit;

   
return (v_ready_for_tx = c_ready_to_transmit);

exception

--Note that by handling exceptions like this, you simultaneously deal
--with no_data_found and too_many_rows, and the function fails safe,
--by returning it's nto ok to transmit.
when others then
	 return false;

END ready_to_transmit;

end UKTV_TOOLS_PKG;

I hope this starts to point you in the right direction

Regards

Tharg

Grinding away at things Oracular

 

[TimGoff]

Tharg,

Many thanks for all your comments, I've read through a few times and I've learnt alot from what you've said.

The reason I wrote the code with SELECT DISINCT is that

SELECT DISTINCT READY_FOR_TX_CODE
INTO
v_ready_for_tx
FROM PROG_MEDIA_SEGMENT
WHERE PROG_MEDIA_ID = v_prog_media_id
AND READY_FOR_TX_CODE = 4280001;

..may return 0 - 10 rows. Ready for tx will only ever be 3 values, 4280001, 4280000 or NULL.

I spoke to a friend at the weekend, and on his recommendation rewrote it to:


SELECT COUNT(*) READY_FOR_TX_CODE
INTO
v_ready_for_tx
FROM PROG_MEDIA_SEGMENT
WHERE PROG_MEDIA_ID = v_prog_media_id
AND READY_FOR_TX_CODE = c_not_ready_for_Tx;


IF v_ready_for_tx > 0 THEN
RETURN 'NOT READY FOR TX';
ELSE
RETURN 'READY FOR TX';
END IF;

Cheers
Tim