-2

I have the following function, which returns the next available client ID from the Client table:

CREATE OR REPLACE FUNCTION getNextClientID RETURN INT AS
ctr INT;

BEGIN
SELECT MAX(NUM) INTO ctr FROM Client;

IF SQL%NOTFOUND THEN
    RETURN 1;
ELSIF SQL%FOUND THEN
    -- RETURN SQL%ROWCOUNT;
    RAISE_APPLICATION_ERROR(-20010, 'ROWS FOUND!');
    -- RETURN ctr + 1;
END IF; 

END;

But when calling this function,

BEGIN 
DBMS_OUTPUT.PUT_LINE(getNextClientID());

END;

I get the following result:

execution

which I found a bit odd, since the Client table contains no data:

Client table

Also, if I comment out RAISE_APPLICATION_ERROR(-20010, 'ROWS FOUND!'); & log the value of SQL%ROWCOUNT to the console, I get 1 as a result.

On the other hand, when changing

SELECT MAX(NUM) INTO ctr FROM Client;

to

SELECT NUM INTO ctr FROM Client;

The execution went as expected. What is the reason behind this behavior ?

Mohammed Aouf Zouag
  • 17,042
  • 4
  • 41
  • 67

4 Answers4

3

Aggregate functions will always return a result:

All aggregate functions except COUNT(*), GROUPING, and GROUPING_ID ignore nulls. You can use the NVL function in the argument to an aggregate function to substitute a value for a null. COUNT and REGR_COUNT never return null, but return either a number or zero. For all the remaining aggregate functions, if the data set contains no rows, or contains only rows with nulls as arguments to the aggregate function, then the function returns null.

You can change your query to:

SELECT COALESCE(MAX(num), 1) INTO ctr FROM Client;

and remove the conditionals altogether. Be careful about concurrency issues though if you do not use SELECT FOR UPDATE.

Glenn
  • 8,932
  • 2
  • 41
  • 54
  • Please get out of the habit of using `NVL()`. It's inherently dangerous as it implicitly converts the second parameter to the datatype of the first parameter, [which can cause problems](http://sqlfiddle.com/#!4/9eecb7d/14962). Use `COALESCE()` instead, this'll return exactly the same result but raise an error if you're accidentally implicitly converting datatypes. `COALESCE()` is also the ANSI standard, so makes your code more portable. – Ben Jan 01 '16 at 23:13
2

Query with any aggregate function and without GROUP BY clause always returns 1 row. If you want no_data_found exception on empty table, add GROUP BY clause or remove max:

SQL> create table t (id number, client_id number);

Table created.

SQL> select nvl(max(id), 0) from t;

NVL(MAX(ID),0)
--------------
         0

SQL> select nvl(max(id), 0) from t group by client_id;

no rows selected

Usually queries like yours (with max and without group by) are used to avoid no_data_found.

Dmitriy
  • 5,525
  • 12
  • 25
  • 38
  • I appreciate your help :) – Mohammed Aouf Zouag Jan 01 '16 at 22:28
  • Please get out of the habit of using `NVL()`. It's inherently dangerous as it implicitly converts the second parameter to the datatype of the first parameter, [which can cause problems](http://sqlfiddle.com/#!4/9eecb7d/14962). Use `COALESCE()` instead, this'll return exactly the same result but raise an error if you're accidentally implicitly converting datatypes. `COALESCE()` is also the ANSI standard, so makes your code more portable. – Ben Jan 01 '16 at 23:13
  • @Ben I added `nvl` just to show that query without `group by` return 1 row (otherwise query return empty string, and in SQL*Plus it could be confused with no rows in result). – Dmitriy Jan 01 '16 at 23:18
1

Agregate functions like MAX will always return a row. It will return one row with a null value if no row is found.

By the way SELECT NUM INTO ctr FROM Client; will raise an exception where there's more than one row in the table.

You should instead check whether or not ctr is null.

Tulains Córdova
  • 2,559
  • 2
  • 20
  • 33
1

Others have already explained the reason why your code isn't "working", so I'm not going to be doing that.

You seem to be instituting an identity column of some description yourself, probably in order to support a surrogate key. Doing this yourself is dangerous and could cause large issues in your application.

You don't need to implement identity columns yourself. From Oracle 12c onwards Oracle has native support for identity columns, these are implemented using sequences, which are available in 12c and previous versions.

A sequence is a database object that is guaranteed to provide a new, unique, number when called, no matter the number of concurrent sessions requesting values. Your current approach is extremely vulnerable to collision when used by multiple sessions. Imagine 2 sessions simultaneously finding the largest value in the table; they then both add one to this value and try to write this new value back. Only one can be correct.

See How to create id with AUTO_INCREMENT on Oracle?

Basically, if you use a sequence then you don't need any of this code.


As a secondary note your statement at the top is incorrect:

I have the following function, which returns the next available client ID from the Client table

Your function returns the maximum ID + 1. If there's a gap in the IDs, i.e. 1, 2, 3, 5 then the "missing" number (4 in this case) will not be returned. A gap can occur for any number of reasons (deletion of a row for example) and does not have a negative impact on your database in any way at all - don't worry about them.

Community
  • 1
  • 1
Ben
  • 51,770
  • 36
  • 127
  • 149
  • Thank you @Ben for your response. +1 for your ***multiple sessions*** note, I was unaware of that. Concerning *identity columns* & *sequences*, I knew about them (I checked the link you attached before asking this question), & I found out as you said, that that *identity columns* are available in the **12c** version **only**. Plus, this is work for school, so I can't really use neither of them... concerning the function behavior, I am aware of the potential gaps, hoping I could fix that using a **trigger** on the table that will update the `id`s on each insertion or deletion. – Mohammed Aouf Zouag Jan 01 '16 at 23:00
  • Please don't add a trigger to update the table @Sparta, I mentioned it so that you can know that it just doesn't matter. If you update the table you'll have to update the entire table every time you delete a row, which'll make your code extremely inefficient. Sequences are available prior to 12c and I've linked to 3 different ways of using them. I would get in the habit of doing the correct thing now. If your next project is to extend this database then you're going to run into problems immediately. Sequences are the simple, easy, option requiring a lot less code and less chances of mistakes. – Ben Jan 01 '16 at 23:05
  • I appreciate your concerns, & I shall consider them & keep them in mind. **I've already learned a lot from you, Thanks !** PS: Welcome to Morocco, where you can't try anything new or useful unless it's listed in the curriculum :p – Mohammed Aouf Zouag Jan 01 '16 at 23:10