Get Started
- CodeAnt AI
- Setup
- Control Center
- Pull Request Review
- IDE
- Compliance
- Anti-Patterns
- Code Governance
- Infrastructure Security Database
- Application Security Database
Plsql
Jump statements, such as RETURN and CONTINUE let you change the default flow of program execution, but jump statements that direct the control flow to the original direction are just a waste of keystrokes.
CREATE PROCEDURE print_numbers AS
BEGIN
FOR i in 1..4 LOOP
DBMS_OUTPUT.PUT_LINE(i);
CONTINUE; -- Noncompliant
END LOOP;
RETURN; -- Noncompliant
END;
Naming custom exceptions the same as predefined ones, while technically acceptable, is not a good practice.
SET SERVEROUTPUT ON
DECLARE
no_data_found EXCEPTION; -- Noncompliant, overrides an Oracle predefined exception
d VARCHAR2(1);
BEGIN
SELECT dummy INTO d FROM DUAL WHERE dummy = 'Y'; -- Will raise STANDARD.NO_DATA_FOUND
EXCEPTION
WHEN NO_DATA_FOUND THEN
DBMS_OUTPUT.PUT_LINE('No data found!'); -- Won't be executed, as NO_DATA_FOUND was overriden, confusing!
WHEN OTHERS THEN
DBMS_OUTPUT.PUT_LINE('Unknown error!'); -- *Will* be executed
END;
/
Multi-line comments are more readable when each line is aligned using the ”*” character. At most one violation is created for each comment
/*
this line is not aligned and ugly Non-Compliant
no violation is created on this line, even though is it also bad
*/
/* this is Compliant */
The FETCH … INTO statement is inefficient when used in a loop (where many records are expected). It leads to many context-switches between the SQL and PL/SQL engines. Instead, the FETCH … BULK COLLECT INTO statement will issue the SQL requests in bulk, minimizing context switches.
SET SERVEROUTPUT ON
CREATE TABLE largeTable AS SELECT ROWNUM AS id FROM all_objects;
SET TIMING ON
DECLARE
x PLS_INTEGER;
CURSOR largeCursor IS SELECT ROWNUM FROM largeTable;
largeTableRowId BINARY_INTEGER;
BEGIN
OPEN largeCursor;
x := 0;
LOOP
FETCH largeCursor INTO largeTableRowId; -- Noncompliant
EXIT WHEN largeCursor%NOTFOUND;
x := x + largeTableRowId;
END LOOP;
DBMS_OUTPUT.PUT_LINE('Sum of rownums using alternative 1: ' || x);
CLOSE largeCursor;
END;
/
SET TIMING OFF
DECLARE
r largeTable%ROWTYPE;
CURSOR myCursor IS SELECT * FROM largeTable;
BEGIN
OPEN myCursor;
FETCH myCursor INTO r; -- Compliant, outside of a loop
CLOSE myCursor;
END;
/
DROP TABLE largeTable;
DECODE is an old function that has been replaced by the easier to understand and more common CASE. Unlike DECODE, CASE may also be used directly within PL/SQL.
SET SERVEROUTPUT ON
DECLARE
operand CHAR(1) := 'B';
l_result PLS_INTEGER;
BEGIN
-- Noncompliant
SELECT DECODE(operand, 'A', 1
, 'B', 2
, 'C', 3
, 'D', 4
, 'E', 5
, 'F', 6
, 7)
INTO l_result
FROM dual;
DBMS_OUTPUT.PUT_LINE('l_result = ' || l_result); -- 2
END;
/
For better readability, column aliases should be used with the AS keyword. If it is missing, it could be misread as another column being selected.
DECLARE
result DUAL.dummy%TYPE;
BEGIN
SELECT
dummy d -- Non-Compliant - could be misread as selecting both "dummy" and a column "d"
INTO
result
FROM
DUAL;
END;
/
Having a bunch of standalone functions or procedures reduces maintainability because it becomes harder to find them and to see how they are related. Instead, they should be logically grouped into meaningful packages.
CREATE PROCEDURE show_name(name VARCHAR2) AS -- Non-Compliant
BEGIN
DBMS_OUTPUT.PUT_LINE('Name: ' || name);
END;
/
DROP PROCEDURE show_name;
Functions with OUT parameters are complex to understand. Indeed, it is impossible to tell, just by looking at the function call, whether an argument is a input or output. Moreover, functions with OUT parameters cannot be called from SQL. It is better to either break such functions up into smaller ones, which each return a single value, or to return several values at once, by combining them in a collection, record, type, or table row.
SET SERVEROUTPUT ON
CREATE TABLE employee(
firstName VARCHAR2(42),
name VARCHAR2(42),
phone VARCHAR2(42)
);
INSERT INTO employee VALUES ('John', 'Smith', '+1');
DECLARE
firstName VARCHAR2(42);
name VARCHAR2(42);
phone VARCHAR2(42);
-- This DOES NOT return the employee name
FUNCTION getEmployeeInfos(firstName OUT VARCHAR2, phone OUT VARCHAR2) RETURN VARCHAR2 AS -- Non-Compliant, confusing
name VARCHAR2(42);
BEGIN
SELECT firstName, name, phone INTO firstName, name, phone FROM employee;
RETURN name;
END;
BEGIN
name := getEmployeeInfos(firstName, phone);
DBMS_OUTPUT.PUT_LINE('firstName: ' || firstName);
DBMS_OUTPUT.PUT_LINE('name: ' || name);
DBMS_OUTPUT.PUT_LINE('phone: ' || phone);
END;
/
Labeled loops are useful, especially when the code is badly indented, to match the begin and end of each loop. When loops are nested, labeling them can improve the code’s readability. This rule detects nested loops which do not have a start label.
BEGIN
LOOP
LOOP -- Noncompliant, this nested loop is not labeled
EXIT;
END LOOP;
EXIT;
END LOOP;
FOR i IN 1..10 LOOP
WHILE true LOOP -- Noncompliant, this nested loop has no start label
EXIT;
END LOOP nestedLoopLabel1;
EXIT;
END LOOP;
WHILE true LOOP
<<nestedLoopLabel2>>
LOOP -- Compliant, but better with an end label
EXIT;
END LOOP;
EXIT;
END LOOP;
END;
/
FORMS_DDL command, like every DDL statements, is performing an implicit COMMIT. It should be used only if there is no pending transaction otherwise this transaction is automatically committed without updating the Form statuses. Also, the potentially acquired locks are lost in case of this implicit COMMIT.
”FORMS_DDL(‘COMMIT’)” and “FORMS_DDL(‘ROLLBACK’)” should be used with care and most of the time, “COMMIT_FORM” or “ROLLBACK_FORM” should be preferred.
Check the Oracle Forms documentation for more details.
FORMS_DDL('COMMIT');
Labeled blocks are useful, especially when the code is badly indented, to match the begin and end of each block. This rule verifies that block start and end labels match, when both are specified.
BEGIN
NULL;
END; -- Compliant, no labels at all
/
<<myBlockLabel1>>
BEGIN
NULL;
END; -- Compliant, only starting label
/
BEGIN
NULL;
END myBlockLabel2; -- Compliant, only ending label
/
<<myBlockLabel3>>
BEGIN
NULL;
END myBlockLabel4; -- Noncompliant, labels mismatch
/
<<myBlockLabel6>>
<<myBlockLabel6>>
BEGIN
NULL;
END myBlockLabel6; -- Noncompliant, several starting labels
/
WHERE clause conditions that reinforce or contradict the definitions of their columns are useless; they are always either unconditionally true or unconditionally false. For instance, there’s no point in including AND column IS NOT NULL if the column is defined as non-null.
Noteworthy
This rule raises issues only when a Data Dictionary is provided during the analysis. See https://docs.sonarqube.org/latest/analysis/languages/plsql/
CREATE TABLE product
(id INT,
name VARCHAR(6) NOT NULL,
mfg_name VARCHAR(6),
mfg_id INT
...
SELECT name, price
FROM product
WHERE name is not null -- Noncompliant; always true. This column is NOT NULL
AND mfg_name = 'Too long name' -- Noncompliant; always false. This column can contain only 6 characters
NATURAL JOIN is a type of equi-join which implicitly compares all identically-named columns of the two tables. While this a feature which may seem convenient at first, it becomes hard to maintain over time.
Consider an EMPLOYEE table with the columns FULL_NAME, and DEPT_ID, and a DEPARTMENT table with the columns DEPT_ID, and NAME. A natural join between those tables will join on the DEPT_ID column, which is the only identically-named column.
But, if a new NAME column is later added to the EMPLOYEE table, then the join will be done on both DEPT_ID and NAME. Natural joins make simple changes such as adding a column complicated and are therefore better avoided.
BEGIN
SELECT *
INTO employeeArray
FROM employee
NATURAL JOIN departement; -- Non-Compliant, the join predicate is implicit
END;
/
Shared coding conventions allow teams to collaborate efficiently. This rule checks that object attribute names match the provided regular expression.
CREATE TYPE my_type AS OBJECT(
foo__bar INTEGER -- Non-Compliant
);
/
DROP TYPE my_type;
Procedures, unlike functions, do not return values. The RETURN statement therefore, when used within a procedure, is used to prematurely end the procedure. However, having multiple exit points (i.e. more than the END of the procedure itself), increases the complexity of the procedure and makes it harder to understand and debug.
SET SERVEROUTPUT ON
DECLARE
PROCEDURE prcoedureWithReturn AS
BEGIN
RETURN; -- Noncompliant
DBMS_OUTPUT.PUT_LINE('prcoedureWithReturn called'); -- This is actually unreachable
END;
BEGIN
prcoedureWithReturn;
END;
/
When a single primitive is tested against three or more values in an IF, ELSIF chain, it should be converted to a CASE instead for greater readability.
DECLARE
x PLS_INTEGER := 0;
BEGIN
IF x = 0 THEN -- Noncompliant
DBMS_OUTPUT.PUT_LINE('x = 0');
ELSIF x = 1 THEN
DBMS_OUTPUT.PUT_LINE('x = 1');
ELSE
DBMS_OUTPUT.PUT_LINE('x > 1');
END IF;
END;
/
The `TO_NUMBER function is converting the value of BINARY_FLOAT, BINARY_DOUBLE, CHAR, VARCHAR2, NCHAR, or NVARCHAR2 datatype to a value of NUMBER datatype.
Without providing the format of the input, TO_NUMBER will consider the provided value is compliant with the default format. Relying on a default configuration is a source of error because it creates a dependency between the code and the configuration of the ORACLE server where this code is deployed.
The behaviour of the TO_NUMBER` function will certainly not be the expected one if the configuration of the ORACLE server is changing.
IF ( TO_NUMBER(p_string) >= 0 and TO_NUMBER(p_string) <= TO_NUMBER('50.00') ) THEN
RETURN 1;
ELSE
RETURN 0;
END IF;
Shared coding conventions allow teams to collaborate efficiently. This rule checks that type names match the provided regular expression.
DECLARE
TYPE Collection_type_ IS VARRAY(42) OF PLS_INTEGER; -- Noncompliant
BEGIN
NULL;
END;
/
Since Oracle 11.2, RELIES_ON has been deprecated because the dependencies of result cache-enabled functions are automatically computed.
CREATE OR REPLACE FUNCTION foo RETURN PLS_INTEGER RESULT_CACHE RELIES_ON(DUAL) AS -- Noncompliant
BEGIN
RETURN 0;
END;
/
DROP FUNCTION foo;
NVL was introduced by Oracle Database during the 80’s. COALESCE is a more modern function part of ANSI-92 standard than can replace NVL. Use of COALESCE should be preferred to NVL for performance reason if the two parameters provided have the same type. COALESCE is running faster than NVL thanks to its short-circuit evaluation of the parameters.
In order to avoid “ORA-00932: inconsistent datatypes” error, double-check the two arguments of the NVL function have the same type before switching to COALESCE.
SELECT employee_uuid, NVL(TO_CHAR(bonus_pct), 'not this year') "Bonus" FROM employees
EXECUTE IMMEDIATE is easier to use and understand than the DBMS_SQL package’s procedures. It should therefore be preferred, when possible.
SET SERVEROUTPUT ON
CREATE TABLE myTable(
foo VARCHAR2(42)
);
CREATE PROCEDURE drop_table(tableName VARCHAR2) AS
cursorIdentifier INTEGER;
BEGIN
cursorIdentifier := DBMS_SQL.OPEN_CURSOR; -- Compliant; this is not a procedure call
DBMS_SQL.PARSE(cursorIdentifier, 'DROP TABLE ' || tableName, DBMS_SQL.NATIVE); -- Noncompliant
DBMS_SQL.CLOSE_CURSOR(cursorIdentifier); -- Noncompliant
DBMS_OUTPUT.PUT_LINE('Table ' || tableName || ' dropped.');
EXCEPTION
WHEN OTHERS THEN
DBMS_SQL.CLOSE_CURSOR(cursorIdentifier); -- Noncompliant
END;
/
BEGIN
drop_table('myTable');
END;
/
DROP PROCEDURE drop_table;
The DML events clause of a trigger is not meant to be used with multiple `OF conditions. When it is, only the last one will actually be taken into account, without any error message being produced. This can lead to counter-intuitive code.
Only the UPDATE event should have an OF` condition, and there should be at most one occurence of it.
CREATE OR REPLACE TRIGGER myTrigger
BEFORE UPDATE OF firstName OR UPDATE OF lastName -- Noncompliant - will *only* be triggered on updates of lastName!
ON myTable
FOR EACH ROW
BEGIN
NULL;
END;
/
FOR and WHILE loops are structured control flow statements.
A FOR loop will iterate once for each element in the range, and the WHILE iterates for as long as a condition holds.
However, inserting an EXIT statement within the loop breaks this structure, reducing the code’s readability and making it harder to debug.
SET SERVEROUTPUT ON
DECLARE
TYPE myCollectionType IS VARRAY(10) OF VARCHAR2(42);
myCollection myCollectionType := myCollectionType('Foo', 'Bar', NULL, 'Baz', 'Qux');
i PLS_INTEGER;
BEGIN
i := 1;
WHILE i <= myCollection.LAST LOOP
EXIT WHEN myCollection(i) IS NULL; -- Noncompliant, breaks the structure of the WHILE
DBMS_OUTPUT.PUT_LINE('Got: ' || myCollection(i));
i := i + 1;
END LOOP;
The use of %TYPE and %ROWTYPE is an easy way to make sure that a variable’s type always matches the type of the relevant column in an existing table. If the column type changes, then the variable changes with it.
CREATE FUNCTION func1(acc_no IN NUMBER)
RETURN NUMBER
IS total NUMBER(11,2); -- Noncompliant
BEGIN
SELECT order_total
INTO total -- total is assigned here: it could be defined as orders.order_total%TYPE
FROM orders
WHERE customer_id = acc_no;
RETURN(total);
END;
A loop with at most one iteration is equivalent to the use of an `IF statement to conditionally execute one piece of code. No developer expects to find such usage of a loop statement. If the initial intention of the author was really to conditionally execute one piece of code, an IF statement should be used in place.
At worst that was not the initial intention of the author and so the body of the loop should be fixed to use the nested RETURN, EXIT, RAISE or GOTO` statements in a more appropriate way.
LOOP
counter := counter + 1;
dbms_output.put_line(counter);
EXIT; -- Noncompliant
END LOOP;
The `TO_DATE and TO_TIMESTAMP functions are converting char of CHAR, VARCHAR2, NCHAR, or NVARCHAR2 datatype to respectively a value of DATE or TIMESTAMP datatype.
Without providing the format of the input char, TO_DATE will consider the provided char is compliant with the default date format. Relying on a default configuration is a source of error because it creates a dependency between the code and the configuration of the ORACLE server where this code is deployed.
According to the Oracle’s documentation “the default date format is determined implicitly by the NLS_TERRITORY initialization parameter or can be set explicitly by the NLS_DATE_FORMAT parameter.”. It means the behaviour of the TO_DATE` function will certainly not be the expected one if the configuration of the ORACLE server is changing.
SELECT TO_DATE( 'January 15, 2018, 11:00 A.M.')
FROM DUAL;
`SELECT queries that return too many columns may be complex or difficult to maintain.
This rule identifies queries that SELECT` more than the specified number of columns.
BEGIN
SELECT id, name, firstname, gender, height, weight, age -- Noncompliant
INTO peopleArray
FROM people
WHERE age > 60;
END;
/
The output of DBMS_OUTPUT.PUT_LINE is not always visible, for example when SERVEROUTPUT is set to OFF. Moreover, there is no standardized way to specify the importance of the message. It is better to use a logging mechanism instead.
SET SERVEROUTPUT ON
BEGIN
DBMS_OUTPUT.PUT_LINE('An error occured'); -- Noncompliant
END;
/
Constants must be immediately initialized at declaration. They cannot be reassigned any value after the declaration, as they are constant. This rule prevents PLS-00322 exceptions from being raised at runtime.
The following code snippet illustrates this rule:
DECLARE
foo CONSTANT PLS_INTEGER NULL; -- Noncompliant PLS-00322
bar CONSTANT PLS_INTEGER NOT NULL; -- Noncompliant PLS-00322
aa CONSTANT PLS_INTEGER; -- Noncompliant
BEGIN
NULL;
END;
/
cursor%NOTFOUND is clearer and more readable than NOT cursor%FOUND, and is preferred.
SET SERVEROUTPUT ON
DECLARE
CURSOR c IS SELECT DUMMY FROM DUAL;
x VARCHAR2(1);
BEGIN
OPEN c;
FETCH c INTO x;
IF NOT c%FOUND THEN -- Noncompliant
DBMS_OUTPUT.PUT_LINE('uh?');
ELSE
DBMS_OUTPUT.PUT_LINE('all good: ' || x);
END IF;
CLOSE c;
END;
/
Some types cannot be constrained, and attempting to do so results in the exception PLS-00566: type name ”…” cannot be constrained being raised.
DECLARE
foo BLOB(42); -- Noncompliant - raises PLS-00566: type name "BLOB" cannot be constrained
BEGIN
NULL;
END;
/
Each function and procedure should be documented with a comment either just before or right after the IS or AS keyword it to explain its goal and how it works.
CREATE FUNCTION my_function RETURN PLS_INTEGER AS
BEGIN
RETURN 42;
END;
/
CREATE PACKAGE my_package AS
PROCEDURE my_procedure;
FUNCTION my_function RETURN PLS_INTEGER;
END my_package;
/
String data types, such as VARCHAR2 or NVARCHAR2 require a size constraint. Omitting the size results in the exception PLS-00215: String length constraints must be in range (1 .. 32767) being raised.
DECLARE
foo VARCHAR2; -- Noncompliant - raises PLS-00215
BEGIN
NULL;
END;
/
Complex chains of IF, ELSIF and ELSE statements should be replaced by the more readable CASE one. A complex IF statement has either several ELSIF clauses, or both an ELSIF and an ELSE clause.
DECLARE
x PLS_INTEGER := 0;
BEGIN
IF x = 0 THEN -- Noncompliant
DBMS_OUTPUT.PUT_LINE('x = 0');
ELSIF x = 1 THEN
DBMS_OUTPUT.PUT_LINE('x = 1');
ELSIF x = 2 THEN
DBMS_OUTPUT.PUT_LINE('x = 2');
END IF;
END;
/
DECLARE
x PLS_INTEGER := 0;
y PLS_INTEGER := 0;
BEGIN
IF x = 0 THEN -- Noncompliant
DBMS_OUTPUT.PUT_LINE('x = 0, y = ?');
ELSIF y = 1 THEN
DBMS_OUTPUT.PUT_LINE('x != 0, y = 1');
ELSE
DBMS_OUTPUT.PUT_LINE('x != 0, y != 1');
END IF;
END;
/
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all record field names match the provided regular rexpression.
DECLARE
TYPE my_type IS RECORD(
foo__bar PLS_INTEGER -- Non-Compliant
);
BEGIN
NULL;
END;
/
Developers should use the FROM … OUTER JOIN syntax rather than the Oracle join operator (). The reason is that outer join queries that use are subject to several restrictions which do not apply to the FROM … OUTER JOIN syntax. For instance, a WHERE condition containing the + operator cannot be combined with another condition using the OR logical operator.
BEGIN
-- Noncompliant
SELECT *
INTO employeesArray
FROM employee, department
WHERE employee.DepartmentID = department.ID(+);
END;
/
Before trapping all possible exceptions, it is best to try to trap the specific ones and try to recover from those.
SET SERVEROUTPUT ON
CREATE TABLE hitCounter
(
page VARCHAR2(42),
hits NUMBER,
CONSTRAINT pk PRIMARY KEY (page)
);
CREATE PROCEDURE hitPage(pageIn VARCHAR2) AS
BEGIN
INSERT INTO hitCounter VALUES (pageIn, 1);
EXCEPTION -- Noncompliant, the only exception handler is WHEN OTHERS
WHEN OTHERS THEN
IF SQLCODE = -1 THEN
UPDATE hitCounter SET hits = hits + 1 WHERE page = pageIn;
ELSE
DBMS_OUTPUT.PUT_LINE('An unknown error occured!');
END IF;
END;
/
BEGIN
hitPage('index.html');
hitPage('index.html');
END;
/
SELECT * FROM hitCounter;
DROP PROCEDURE hitPage;
DROP TABLE hitCounter;
Full outer joins aren’t in common use, and as a result many developers don’t really understand them. Therefore, each use of this language feature should be reviewed.
BEGIN
SELECT *
BULK COLLECT INTO result
FROM DUAL d1
FULL OUTER JOIN DUAL d2 ON d1.dummy != d2.dummy; -- Noncompliant
END;
/
SYNCHRONIZE was introduced by Oracle as a recovery mechanism in case there is a loss of synchronization between what is in memory and what is displayed. It works but it comes with a price; each call to SYNCHRONIZE generates a network round-trip between the server and the Forms client. Most of the time it should be avoided or used with caution as explicitly stated in the Oracle Forms documentation.
SYNCHRONIZE;
Procedures which don’t accept parameters are likely to either not be reused that often or to depend on global variables instead. Refactoring those procedures to take parameters will make them both more flexible and reusable.
SET SERVEROUTPUT ON
DECLARE
name VARCHAR2(42) := 'John';
PROCEDURE print_name; -- Noncompliant
PROCEDURE print_name AS -- Noncompliant
BEGIN
DBMS_OUTPUT.PUT_LINE('Name: ' || name);
END;
BEGIN
print_name;
END;
/
Labeled loops are useful, especially when the code is badly indented, to match the begin and end of each loop. However, those labels, if used, must appear on the same line as the “END” keyword in order to avoid any confusion. Indeed, the label might otherwise be seen as a procedure call.
SET SERVEROUTPUT ON
DECLARE
PROCEDURE foo AS
BEGIN
DBMS_OUTPUT.PUT_LINE('foo was called!');
END;
BEGIN
LOOP
EXIT;
END LOOP -- The semicolon was forgotten
foo; -- Noncompliant, This is interpreted as a label of the previous FOR loop, not as a procedure call to foo!
END;
/
Any insert which omits a value for a NOT NULL column in a database table will be automatically rejected by the database unless a default value has been specified for the column.
Noteworthy
This rule raises issues only when a Data Dictionary is provided during the analysis. See https://docs.sonarqube.org/latest/analysis/languages/plsql/
INSERT INTO MY_TABLE -- Noncompliant; N2 value omitted
(
N1
)
VALUES
(
1
)
Ensure that every possible exception is caught by using a WHEN OTHERS clause.
SET SERVEROUTPUT ON
DECLARE
result PLS_INTEGER;
custom_exception EXCEPTION;
BEGIN
result := 42 / 0; -- "Unexpected" division by 0
RAISE custom_exception;
EXCEPTION -- Non-Compliant
WHEN custom_exception THEN
DBMS_OUTPUT.PUT_LINE ('custom_exception: ' || DBMS_UTILITY.FORMAT_ERROR_STACK);
END;
/
`UNION is significantly less performant compared to UNION ALL because it removes duplicated entries and run an internal DISTINCT to achieve this.
UNION ALL is not removing duplicates and returns all the results from the queries. It performs faster in most of the cases compared to UNION. Nevertheless, the quantity of data returned by UNION ALL can be significantly more important than with UNION. On slow network, the performance gain to use UNION ALL instead of UNION can be challenged by the time lost to transfer more data than with UNION`.
-- if you care about not having duplicated entries, then UNION is the good choice
SELECT EMAIL FROM EMPLOYEES UNION SELECT EMAIL FROM CUSTOMERS
Labeled blocks are useful, especially when the code is badly indented, to match the begin and end of each block. This check detects labeled blocks which are missing an ending label.
<<myBlockLabel1>>
BEGIN
NULL;
END; -- Noncompliant; this labeled loop has no ending label
/
BEGIN
NULL; -- Compliant; not a labeled block
END;
/
By default, the parameter mode is IN. However, specifying it explicitly makes the code easier to read.
SET SERVEROUTPUT ON
DECLARE
PROCEDURE printName(name VARCHAR2) AS -- Noncompliant; relies on default mode
BEGIN
DBMS_OUTPUT.PUT_LINE('name: ' || name);
END;
BEGIN
printName('Foo');
END;
/
The use of GOTO in general is arguable. However, when used within loops, GOTO statements are even more evil, and they can often be replaced by other constructs.
DECLARE
i PLS_INTEGER := 0;
BEGIN
LOOP
IF i = 3 THEN
GOTO loopEnd; -- Noncompliant
END IF;
DBMS_OUTPUT.PUT_LINE('i = ' || i);
i := i + 1;
END LOOP;
<<loopEnd>>
DBMS_OUTPUT.PUT_LINE('Loop end');
END;
/
Using the same value on either side of a binary operator is almost always a mistake. In the case of logical operators, it is either a copy/paste error and therefore a bug, or it is simply wasted code, and should be simplified.
This rule ignores operators +, * and ||, and expressions: 1=1, 1<>1, 1!=1, 1~=1 and 1^=1.
SELECT code
FROM Person
WHERE first_name IS NULL OR first_name IS NULL; -- Noncompliant
SELECT * FROM Users
INNER JOIN Clients ON Clients.id = Clients.id; -- Noncompliant
The LONG and LONG RAW datatypes are deprecated and Oracle recommends to migrate them to the LOB datatypes CLOB, NCLOB or BLOB.
CREATE TABLE images(
data LONG RAW
);
A CASE and a chain of IF/ELSIF statements is evaluated from top to bottom. At most, only one branch will be executed: the first one with a condition that evaluates to true.
Therefore, duplicating a condition automatically leads to dead code. Usually, this is due to a copy/paste error. At best, it’s simply dead code and at worst, it’s a bug that is likely to induce further bugs as the code is maintained, and obviously it could lead to unexpected behavior.
IF param == 1 THEN
x := 'A';
ELSIF param == 2 THEN
x := 'B';
ELSIF param == 1 THEN -- Noncompliant, for sure this is a bug
x := 'C';
END IF;
result := CASE param
WHEN 1 THEN 'A'
WHEN 2 THEN 'B'
WHEN 1 THEN 'C' -- Noncompliant
ELSE 'D'
END;
When creating a function, procedure, package, package body, type, type body, trigger or library, it is a good practice replace the existing one to avoid errors.
CREATE FUNCTION my_function RETURN PLS_INTEGER AS -- Noncompliant
BEGIN
RETURN 42;
END;
/
Some Oracle packages contain powerful SYS-owned functions that can be used to perform malicious operations. For instance, DBMS_SYS_SQL.PARSE_AS_USER can be used to execute a statement as another user.
Most programs do not need those functions and this rule helps identify them in order to prevent security risks.
DECLARE
c INTEGER;
sqltext VARCHAR2(100) := 'ALTER USER system IDENTIFIED BY hacker'; -- Might be injected by the user
BEGIN
c := SYS.DBMS_SYS_SQL.OPEN_CURSOR(); -- Noncompliant
-- Will change 'system' user's password to 'hacker'
SYS.DBMS_SYS_SQL.PARSE_AS_USER(c, sqltext, DBMS_SQL.NATIVE, UID); -- Non-Compliant
SYS.DBMS_SYS_SQL.CLOSE_CURSOR(c); -- Noncompliant
END;
/
Anchored types, i.e. those specified using either %TYPE or %ROWTYPE, cannot be constrained. Trying to do so results in the exception PLS-00573: cannot constrain scale, precision, or range of an anchored type being raised.
DECLARE
foo DUAL.DUMMY%TYPE(42); -- Non-Compliant - raises PLS-00573
BEGIN
NULL;
END;
/
Each function and procedure parameter name must match a given regular expression.
DECLARE
FUNCTION myfunction2(parameter_ PLS_INTEGER) RETURN PLS_INTEGER; -- Noncompliant
PROCEDURE myprocedure2(parameter_ PLS_INTEGER); -- Noncompliant
FUNCTION myfunction2(parameter_ PLS_INTEGER) RETURN PLS_INTEGER AS -- Noncompliant
BEGIN
RETURN 42;
END;
PROCEDURE myprocedure2(parameter_ PLS_INTEGER) AS -- Noncompliant
BEGIN
NULL;
END;
BEGIN
NULL;
END;
/
Shared coding conventions allow teams to collaborate efficiently. This rule checks that cursor parameters match the provided regular expression.
CREATE TABLE employee(
name VARCHAR2(42)
);
DECLARE
CURSOR mycursor2(Employee-name-parameter_ VARCHAR2) RETURN employee%ROWTYPE; -- Noncompliant
CURSOR mycursor2(Employee-name-parameter_ VARCHAR2) RETURN employee%ROWTYPE IS SELECT * FROM employee WHERE name = Employee-name-parameter_; -- Noncompliant
BEGIN
NULL;
END;
/
DROP TABLE employee;
Pipelined functions offers the ability to create programmatically generated tables.
One of the benefits of such functions is that they reduce memory consumption as results are not all kept in memory before being returned.
Instead of relying on `RETURN, PIPE ROW must be used to return the results, one row at a time.
Trying to return an expression from a pipelined function raises PLS-00633: RETURN statement in a pipelined function cannot contain an expression`
CREATE OR REPLACE TYPE myScalarType AS OBJECT
(
dummy VARCHAR2(42)
)
/
CREATE OR REPLACE TYPE myTableType AS TABLE OF myScalarType;
/
CREATE OR REPLACE FUNCTION foo RETURN myTableType PIPELINED AS -- Noncompliant, should contain at least one PIPE ROW
result myTableType := myTableType();
BEGIN
FOR i IN 1 .. 3 LOOP
result.EXTEND;
result(i) := myScalarType('Dummy ' || i);
END LOOP;
RETURN result; -- Noncompliant, will raise PLS-00633
END;
/
SELECT * FROM TABLE(foo());
DROP FUNCTION foo;
DROP TYPE myTableType;
DROP TYPE myScalarType;
New lines and control characters can be injected in the source code by bad manipulations. Control characters aren’t visible to maintainers, so whether or not they are actually wanted should be double-checked. Note that this rule can optionally also report violations on literals containing the tabulation character.
SET SERVEROUTPUT ON
BEGIN
/* Non-Compliant */ DBMS_OUTPUT.PUT_LINE('Hello
world!');
DBMS_OUTPUT.PUT_LINE('Hello'); -- Compliant, this is preferred
DBMS_OUTPUT.PUT_LINE('world!');
END;
/
Jump statements (`EXIT, CONTINUE, RETURN, RAISE, and RAISE_APPLICATION_ERROR), move control flow out of the current code block. So any statements that come after a jump are dead code.
This rule detects for statements that follow:
EXIT without a WHEN
CONTINUE without a WHEN
RETURN
RAISE
RAISE_APPLICATION_ERROR`
SET SERVEROUTPUT ON
BEGIN
LOOP
DBMS_OUTPUT.PUT_LINE('This will be printed out');
EXIT;
DBMS_OUTPUT.PUT_LINE('This will NEVER be printed out'); -- Non-Compliant
END LOOP;
END;
/
Labeled loops are useful, especially when the code is badly indented, to match the begin and end of each loop. Within a labeled loop, the code’s maintainability is increased by explicitly providing the loop’s label in every EXIT statement. Indeed, if a nested loop is added afterwards, it is clear which loop has to be exited.
BEGIN
<<myLoopLabel1>>
LOOP
EXIT; -- Noncompliant, the loop label is missing
END LOOP myLoopLabel1;
LOOP
EXIT; -- Compliant, this EXIT is not in a labeled loop
END LOOP;
END;
/
ASC or DESC should be specified for every column of an ORDER BY clause to improve readability.
BEGIN
SELECT col1, col2, col3
BULK COLLECT INTO result
FROM my_table
ORDER BY
col1 ASC,
col2, -- Noncompliant - ASC or DESC should be specified
col3 DESC;
END;
/
CREATE_TIMER is generating network traffic each time the timer is fired. It’s probably totally fine for a timer being executed every hour but generally, this is used to provide clocks components that are going to generate network traffic every second or more.
It is recommended by Oracle to examine timers and replace them with JavaBeans.
BEGIN
timer := CREATE_TIMER('foo', 1000, REPEAR)
ENDl
Shadowing all exceptions with NULL statements indicates that no error handling has been done for a given block of code. This is a common bad-practice and only the non-relevant exceptions should be ignored (and a comment is welcome in such cases).
BEGIN
SELECT value
INTO :hits
FROM hitCounter
WHERE pageIn = 'Sample';
EXCEPTION -- Noncompliant
WHEN OTHERS THEN
NULL;
END;
BEGIN
SELECT value
INTO :hits
FROM hitCounter
WHERE pageIn = 'Sample';
EXCEPTION -- Noncompliant
WHEN TOO_MANY_ROWS THEN
NULL;
WHEN OTHERS THEN
NULL;
END;
Declaring a `NUMBER variable without any precision wastes memory because Oracle supports up to 38 decimal digits by default (or the maximum supported by your system, whichever is less). If you don’t need that large a value, you should specify whatever matches your needs. This will save memory and provide extra integrity checking on input.
This rule also applies to some NUMBER subtypes as well: NUMERIC, DEC, and DECIMAL`.
DECLARE
var1 NUMBER; -- Noncompliant
var2 NUMERIC; -- Noncompliant
BEGIN
NULL;
END;
/
Labeled loops are useful, especially when the code is badly indented, to match the begin and end of each loop. This rule verifies that loop start and end labels match, when both are specified.
BEGIN
LOOP
EXIT;
END LOOP; -- Compliant, this loop has no label at all
<<myLoopLabel1>>
LOOP
EXIT;
END LOOP; -- Compliant, this loop only has a start label
LOOP
EXIT;
END LOOP myLoopLabel2; -- Compliant, this loop only has an end label
<<myLoopLabel4>>
LOOP
EXIT;
END LOOP myLoopLabel5; -- Noncompliant, label mismatch
<<myLoopLabel6>>
<<myLoopLabel7>>
LOOP
EXIT;
END LOOP myLoopLabel7; -- Noncompliant, several start labels mismatch
END;
/
The EXIT WHEN syntax can exit a loop depending on a condition. It should be preferred to the more verbose and error-prone IF … THEN EXIT; END IF; syntax.
SET SERVEROUTPUT ON
DECLARE
i PLS_INTEGER;
BEGIN
i := 0;
LOOP
IF i > 10 THEN -- Noncompliant
EXIT;
END IF;
DBMS_OUTPUT.PUT_LINE('i = ' || i);
i := i + 1;
END LOOP;
END;
/
This rule applies whenever an `IF statement is followed by one or
more ELSEIF statements; the final ELSEIF should be followed by an ELSE statement.
The requirement for a final ELSE statement is defensive programming.
The ELSE statement should either take appropriate action or contain
a suitable comment as to why no action is taken. This is consistent with the
requirement to have a final ELSE clause in a CASE`
statement.
IF my_variable = 0 THEN
do_something;
ELSIF my_variable = 1 THEN
do_something_else;
END IF;
Not every data type supports the RANGE or scale constraints. Using these constraints on incompatible types results in an PLS-00572: improper constraint form used exception being raised.
DECLARE
foo INTEGER RANGE 0 .. 42; -- Non-Compliant - raises PLS-00572 as NUMBER does not support the RANGE constraint
BEGIN
NULL;
END;
/
Assigning default values to variables in an initialization section is a good practice as it increases the readability of the code: when starting reading a block, one will know exactly which variables have default values.
DECLARE
var1 integer;
var2 integer;
var3 integer;
BEGIN
var1 := 1; -- Noncompliant
var2 := 2; -- Noncompliant
var3 := var1 + var2;
END;
/
When multiple tables are involved in a query, using table aliases helps to make it more understandable and keeps it short.
BEGIN
SELECT
name,
firstname,
location
INTO employeesArray
FROM employee -- Noncompliant - should be aliased
INNER JOIN department -- Noncompliant - should be aliased
ON employee.DepartmentID = department.ID;
END;
/
ORACLE 11g introduced the `SIMPLE_INTEGER data type, which is a sub-type of PLS_INTEGER, and covers the same range. There are three main differences between the two types:
SIMPLE_INTEGER is always NOT NULL. So when the value of the declared variable is never going to be null, you can declare it as SIMPLE_INTEGER.
You will never face a numeric overflow using SIMPLE_INTEGER because this data type wraps around without giving any error.
The SIMPLE_INTEGER data type gives a major performance boost over PLS_INTEGER when the code is compiled in “NATIVE” mode, because arithmetic operations on SIMPLE_INTEGER` type are performed directly at the hardware level.
DECLARE
v1 PLS_INTEGER; -- Noncompliant
v2 VARCHAR2(10);
BEGIN
NULL;
END;
/
The repetition of a prefix operator (+, -, or NOT) is usually a typo. The second operator invalidates the first one:
IF NOT ( NOT foo = 5 ) THEN -- Noncompliant: equivalent to "foo = 5"
value := ++1; -- Noncompliant: equivalent to "+1"
END IF;
Oracle’s ROWNUM is a pseudo column that numbers the rows in a result set. Unfortunately, it numbers the rows in the set before ordering is applied. So combining the two in the same query won’t get you the results you expect. Instead, you should move your selection and ordering into a subquery, and use ROWNUM only on the outer query.
SELECT fname, lname, deptId
FROM employee
WHERE rownum <= 10
ORDER BY salary -- Noncompliant
Labeled loops are useful, especially when the code is badly indented, to match the begin and end of each loop. This rule raises an issue when the end of a labeled loop is unlabeled.
BEGIN
<<myLoopLabel1>>
LOOP
EXIT;
END LOOP; -- Noncompliant; this labeled loop has no ending label
LOOP
EXIT;
END LOOP; -- Compliant; not a labeled loop
END;
/
Using the same name for multiple purposes reduces the understandability of the code and might eventually lead to bugs.
This rule verifies that no name is reused in an inner scope.
<<foo>> -- Compliant
DECLARE
a CONSTANT PLS_INTEGER := 0;
BEGIN
<<foo>> -- Non-Compliant
DECLARE
b CONSTANT PLS_INTEGER := 42;
BEGIN
DBMS_OUTPUT.PUT_LINE('x = ' || foo.b); -- Confusing
END;
END;
/
When data structures (scalar variables, collections, cursors) are declared in the package specification (not within any specific program), they can be referenced directly by any program running in a session with EXECUTE rights to the package.
Instead, declare all package-level data in the package body and provide getter and setter functions in the package specification. Developers can then access the data using these methods and will automatically follow all rules you set upon data modification.
By doing so you can guarantee data integrity, change your data structure implementation, and also track access to those data structures.
-- Package specification
CREATE PACKAGE employee AS
name VARCHAR2(42); -- Non-Compliant
END employee;
/
DROP PACKAGE employee;
SQL is an extremely powerful and hard to master language. It may be tempting to emulate SQL joins in PL/SQL using nested cursor loops, but those are not optimized by Oracle at all. In fact, they lead to numerous context switches between the SQL and PL/SQL engines, and those switches have a highly negative impact on performance. It is therefore much better to replace nested PL/SQL cursor loops with native SQL joins.
SET SERVEROUTPUT ON
CREATE TABLE countriesTable(
countryName VARCHAR2(42)
);
CREATE TABLE citiesTable(
cityName VARCHAR2(42)
);
INSERT INTO countriesTable VALUES('India');
INSERT INTO countriesTable VALUES('Switzerland');
INSERT INTO countriesTable VALUES('United States');
INSERT INTO citiesTable VALUES('Berne');
INSERT INTO citiesTable VALUES('Delhi');
INSERT INTO citiesTable VALUES('Bangalore');
INSERT INTO citiesTable VALUES('New York');
BEGIN
FOR countryRecord IN (SELECT countryName FROM countriesTable) LOOP
FOR cityRecord IN (SELECT cityName FROM citiesTable) LOOP -- Non-Compliant
DBMS_OUTPUT.PUT_LINE('Country: ' || countryRecord.countryName || ', City: ' || cityRecord.cityName);
END LOOP;
END LOOP;
END;
/
DROP TABLE citiesTable;
DROP TABLE countriesTable;
An EXCEPTION WHEN … THEN clause that only rethrows the caught exception has the same effect as omitting the EXCEPTION clause altogether and letting it bubble up automatically.
BEGIN
SELECT 1/0;
EXCEPTION
WHEN ZERO_DIVIDE THEN
RAISE; -- Noncompliant
WHEN OTHERS THEN
RAISE; -- Noncompliant
END;
Consistently using aliases for column names is useful for several reasons. The main one is that the code is independant from potential database modifications - when a column has been renamed to comply with standards for instance. Another reason is to remove ambiguity when querying several tables that may have equivalent column names.
BEGIN
SELECT
emp.name, -- Noncompliant - should be aliased
dpt.name -- Noncompliant - should be aliased
INTO employeesArray
FROM employee emp INNER JOIN department dpt
ON emp.DepartmentID = dpt.ID;
END;
/
For better readability, and to prevent the PLS-00312: a positional parameter association may not follow a named association exception from being raised, do not mix named and positional argument invocations.
SET SERVEROUTPUT ON
DECLARE
PROCEDURE sub(op1 PLS_INTEGER, op2 PLS_INTEGER) AS
BEGIN
DBMS_OUTPUT.PUT_LINE('Sub = ' || (op1 - op2));
END;
BEGIN
sub(10, op2 => 2); -- Noncompliant
sub(op1 => 10, 2); -- Noncompliant - raises PLS-00312: a positional parameter association may not follow a named association
END;
/
At most one declaration of a variable in a given scope is allowed in PL/SQL. The PLS-00371 error will be raised at runtime when attempting to reference a variable declared more than once.
SET SERVEROUTPUT ON
DECLARE
foo VARCHAR2(42) := 'foo';
foo VARCHAR2(42) := 'bar'; -- Non-Compliant
BEGIN
DBMS_OUTPUT.PUT_LINE(foo); -- Raises PLS-00371: at most one declaration for 'FOO' is permitted
END;
/
The result_cache Oracle hint can vastly improve performance, but it comes at the cost of extra memory consumption, so one should double-check that the gain in performance is significant, and avoid overusing this feature in general.
SELECT /*+ result_cache */ * FROM DUAL; -- Noncompliant
Compound triggers were introduced to ease the implementation of multiple triggers which need to work in cooperation.
Typically, a `FOR EACH ROW trigger accumulates facts, and an AFTER STATEMENT trigger performs the actual changes.
The compound trigger can hold a state common to all the triggers it defines, thereby removing the need to use package variables. This approach is sometimes the only possible one, as when avoiding a mutating table ORA-04091` error, or it can be used to get better performance.
However, there is no point in defining a compound trigger which contains only a single trigger, since there is no state to be shared. In such cases, a simple trigger should be used instead.
CREATE OR REPLACE TRIGGER my_trigger -- Noncompliant; defines a single trigger
FOR INSERT ON my_table
COMPOUND TRIGGER
AFTER EACH ROW IS
BEGIN
DBMS_OUTPUT.PUT_LINE('New row inserted!');
END AFTER EACH ROW;
END;
/
All reserved words should be written using the same case to ensure consistency in the code.
This rule checks that reserved words are all in lower case.
begin
null;
NULL; -- Noncompliant
end;
/
It is a best practice to have variable declarations outside of the loop. Additionally, declaring variables inside a loop is slightly less efficient because memory management is then performed with each iteration of the loop.
BEGIN
FOR i IN 1..10
LOOP
DECLARE
variableX NUMBER:= 10;
BEGIN
variableX:= variableX+i;
dbms_output.put_line(variableX);
END;
END LOOP;
END;
`%TYPE and %ROWTYPE seem like an easy way to make sure that a variable’s type always matches the type of the relevant column in an existing table. If the column type changes, then the variable changes with it.
However, Oracle Forms compiled against a procedure using either of these two symbols won’t get the benefit of that flexibility. Instead, at compile time, the relevant type is looked up from the underlying database and used in the form. If the column type changes later or the form is running against a database with different length semantics, attempting to use the form results in an “ORA-04062: Signature of package has been changed” error on the package in question. And the form needs to be recompiled on exactly the same database environment where it will run to avoid the error.
Note that %TYPE and %ROWTYPE` can be used in a package’s private procedures and functions, private package variables, and local variables without issue, but not in the package specification.
CREATE OR REPLACE PACKAGE PACK IS
TYPE mytype IS RECORD (
var1 mytable.mycolumn%TYPE -- Noncompliant
);
FUNCTION MY_FUNC(param1 IN mytable.mycolumn%TYPE) RETURN VARCHAR2; -- Noncompliant
FUNCTION MY_FUNC2(param1 IN mytable%ROWTYPE) RETURN VARCHAR2; -- Noncompliant
END;
Labeled blocks are useful, especially when the code is badly indented, to help maintainers match the beginning and ending of each block. When blocks are nested, labeling them can improve the code’s readability. This rule detects nested block which do not have a start label.
BEGIN -- Compliant, this is not a nested block
NULL;
END;
/
BEGIN
BEGIN -- Noncompliant; this nested block has no label
NULL;
END;
END;
/
BEGIN
BEGIN -- Noncompliant; this nested block has only an end label
NULL;
END myBlockLabel1;
<<myBlockLabel2>> -- Compliant
BEGIN
NULL;
END;
END;
/
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all cursor names match the provided regular expression.
CREATE TABLE employee(
name VARCHAR2(42)
);
DECLARE
CURSOR myCursor_ RETURN employee%ROWTYPE; -- Noncompliant
CURSOR myCursor_ RETURN employee%ROWTYPE IS SELECT * FROM employee; -- Noncompliant
BEGIN
NULL;
END;
/
DROP TABLE employee;
Calling COMMIT or ROLLBACK from within a trigger will lead to an ORA-04092 exception, unless the trigger has its own autonomous transaction.
SET SERVEROUTPUT ON
CREATE TABLE accounts(
balance NUMBER
);
INSERT INTO accounts VALUES(0);
CREATE TABLE log(
message VARCHAR2(100)
);
CREATE TRIGGER beforeLogger
BEFORE UPDATE ON accounts
FOR EACH ROW
BEGIN
INSERT INTO log VALUES('Attempt to update the value from ' || :OLD.balance || ' to ' || :NEW.balance);
COMMIT; -- Noncompliant, will fail with a ORA-04092
END;
/
-- We want to be able to log any attempt to update the "accounts" table
BEGIN
UPDATE accounts SET balance = 100;
ROLLBACK; -- Ultimately, this update is rolled back, however we still want to log it
END;
/
SELECT * FROM log;
DROP TRIGGER beforeLogger;
DROP TABLE log;
DROP TABLE accounts;
Declaring a variable with the NOT NULL constraint incurs a small performance cost - while this constraint may not really be required. Using such a constraint should be avoided.
DECLARE
counter PLS_INTEGER NOT NULL := 0; -- Noncompliant
BEGIN
NULL;
END;
/
A FETCH … BULK COLLECT INTO without a LIMIT clause will load all the records returned by the cursor at once. This may lead to memory exhaustion. Instead, it is better to process the records in chunks using the LIMIT clause.
SET SERVEROUTPUT ON
-- Fetches all records at once, requiring lots of memory
DECLARE
TYPE largeTableRowArrayType IS TABLE OF largeTable%ROWTYPE;
largeTableRowArray largeTableRowArrayType;
CURSOR myCursor IS SELECT * FROM largeTable;
BEGIN
OPEN myCursor;
FETCH myCursor BULK COLLECT INTO largeTableRowArray; -- Non-compliant
DBMS_OUTPUT.PUT_LINE('Alternative 1: ' || largeTableRowArray.COUNT || ' records');
CLOSE myCursor;
END;
/
Since Oracle 10g, DBMS_UTILITY.FORMAT_ERROR_BACKTRACE is available to get an exception’s stack trace, i.e. files and lines that lead up to the exception. When combined with DBMS_UTILITY.FORMAT_ERROR_STACK, which contains the exception error code and message, developers are able quickly identify defects.
This rule verifies that whenever either is used in an exception handler, the other is used as well.
BEGIN
RAISE_APPLICATION_ERROR(-20000, 'This is an error example');
EXCEPTION
WHEN OTHERS THEN -- Noncompliant; only FORMAT_ERROR_STACK is used
DBMS_OUTPUT.PUT(DBMS_UTILITY.FORMAT_ERROR_STACK); -- "ORA-20000: This is an error example"
DBMS_OUTPUT.PUT_LINE('');
END;
/
A GOTO statement is an unstructured change in the control flow. They should be avoided and replaced by structured constructs.
SET SERVEROUTPUT ON
DECLARE
i PLS_INTEGER := 42;
BEGIN
IF i < 0 THEN
GOTO negative; -- Noncompliant
END IF;
DBMS_OUTPUT.PUT_LINE('positive');
goto cleanup; -- Noncompliant
<<negative>>
DBMS_OUTPUT.PUT_LINE('negative!');
<<cleanup>>
NULL;
END;
/
Float data types, such as FLOAT, DOUBLE PRECISION, and REAL cannot have a scale constraint. Trying to specify a scale results in the exception PLS-00510: Float cannot have scale being raised.
DECLARE
foo FLOAT(10, 3); -- Noncompliant - raises PLS-00510
BEGIN
NULL;
END;
/
Shared coding conventions allow teams to collaborate efficiently. This rule checks that reserved words are written in upper case.
BEGIN
null; -- Noncompliant
END;
/
When the FORALL statement is used without the SAVE EXCEPTIONS clause and an exception is raised by a DML query, the whole operation is rolled back and the exception goes unhandled. Instead of relying on this default behavior, it is better to always use the SAVE EXCEPTIONS clause and explicitly handle exceptions in a ORA-24381 handler.
CREATE TABLE my_table(
id NUMBER(10) NOT NULL
);
DECLARE
TYPE my_table_id_type IS TABLE OF my_table.id%TYPE;
my_table_ids my_table_id_type := my_table_id_type();
BEGIN
FOR i IN 1 .. 10 LOOP
my_table_ids.EXTEND;
my_table_ids(my_table_ids.LAST) := i;
END LOOP;
-- Cause the failure
my_table_ids(10) := NULL;
FORALL i IN my_table_ids.FIRST .. my_table_ids.LAST -- Noncompliant
INSERT INTO my_table
VALUES (my_table_ids(i));
END;
/
SELECT COUNT(*) FROM my_table;
DROP TABLE my_table;
Be careful about your use of Oracle-specific data types like `ROWID and UROWID. They might offer a slight improvement in performance over other means of identifying a single row (primary key or unique index value), but that is by no means guaranteed.
On the other hand, the use of ROWID or UROWID` means that your SQL statement will not be portable to other SQL databases. Further, many developers are not familiar with these data types, which can make the code harder to maintain.
SET SERVEROUTPUT ON
DECLARE
id rowid; -- Non-Compliant
universeId urowid; -- Non-Compliant
BEGIN
SELECT rowid INTO id FROM DUAL;
SELECT rowid INTO universeId FROM DUAL;
DBMS_OUTPUT.PUT_LINE('id = ' || id);
DBMS_OUTPUT.PUT_LINE('universe id = ' || universeId);
END;
/
Weak `REF CURSOR types are harder to work with than ones with a return type. Indeed, the compiler’s type-checker is unable to make some verifications, which are then delayed till runtime.
When the use of weak REF CURSOR is required, it is best to use the SYS_REFCURSOR built-in type instead of defining a new one.
This rule’s sysRefCursorAllowed parameter can be used to control whether or not the usage of SYS_REFCURSOR` is allowed.
DECLARE
TYPE dualCursorType IS REF CURSOR; -- Noncompliant
dualCursor dualCursorType;
otherCursor SYS_REFCURSOR; -- Compliant or non-compliant, depending on the "sysRefCursorAllowed" parameter
BEGIN
otherCursor := dualCursor; -- Works
END;
/
Since the purpose of the EXECUTE IMMEDIATE statement is to execute dynamic SQL queries - which by definition can contain unexpected errors - properly handling exceptions becomes critical. Therefore, care should be taken to trap all possible exceptions.
DECLARE
result VARCHAR2(42);
column VARCHAR2(42);
BEGIN
column := 'DUMMY_2';
EXECUTE IMMEDIATE 'SELECT ' || column || ' FROM DUAL' INTO result; -- Non-Compliant
END;
/
Trying to assign a large character value to a smaller variable or column will raise an error.
create table persons (id number, name varchar2(4));
insert into persons (id, name) values (1, 'Alice'); -- Noncompliant, raises ORA-12899
create or replace procedure sp1
is
foo varchar2(2);
begin
select name into foo from persons where id = 1; -- Noncompliant, may raise ORA-06502
end;
The CASE statement should be used only to clearly define some new branches in the control flow. As soon as a WHEN clause contains too many statements this highly decreases the readability of the overall control flow statement. In such case, the content of WHEN clause should be extracted in a dedicated function.
CASE my_variable
WHEN 0 THEN -- 6 lines till next WHEN
procedure1;
procedure2;
procedure3;
procedure4;
procedure5;
WHEN 1 THEN
-- ...
END CASE;
From the Oracle docs:
DECLARE
howmany NUMBER;
num_tables NUMBER;
BEGIN
-- Begin processing
SELECT COUNT(*) INTO howmany
FROM USER_OBJECTS
WHERE OBJECT_TYPE = 'TABLE'; -- Check number of tables
num_tables := howmany; -- Compute some other value
END;
/
Having all branches of a CASE or IF/ELSIF chain with the same implementation indicates a problem.
In the following code:
IF param = 1 THEN
result := 'A';
ELSIF param = 2 THEN
result := 'A';
ELSE
result := 'A';
END IF;
result := CASE param
WHEN 1 THEN 'A'
WHEN 2 THEN 'A'
ELSE 'A'
END;
Explicit variable initializations with null values are superfluous, since unassigned variables are implicitly initialized to null.
SET SERVEROUTPUT ON
DECLARE
foo PLS_INTEGER := NULL; -- Noncompliant, the null assignation is superfluous
bar VARCHAR2(100) := ''; -- Noncompliant, the null assignation is superfluous
correctInitializedString VARCHAR2(100) := 'Hello world!';
BEGIN
IF foo IS NULL THEN
DBMS_OUTPUT.PUT_LINE('foo is NULL');
ELSE
DBMS_OUTPUT.PUT_LINE('foo is NOT NULL');
END IF;
END;
/
Labeled blocks are useful to help maintainers match-up the beginning and ending of each section of code, especially when that code is badly indented. However, if used, those labels must appear on the same line as the “END” keyword in order to avoid confusion. Otherwise, the label might be misread by maintainers as a procedure call.
SET SERVEROUTPUT ON
DECLARE
PROCEDURE foo AS
BEGIN
DBMS_OUTPUT.PUT_LINE('foo was called!');
END;
BEGIN
BEGIN
NULL;
END -- Semicolon was forgotten?
foo; -- Noncompliant; looks like a procedure call, but is actually END block label
<<myBlockLabel>>
BEGIN
NULL;
END
myBlockLabel; -- Noncompliant
END;
/
Currently, VARCHAR and VARCHAR2 are identical data types. But to prevent future changes in behavior, Oracle recommends the use of VARCHAR2.
DECLARE
var VARCHAR(42); -- Noncompliant
BEGIN
NULL;
END;
/
NCHAR and NVARCHAR2 lengths must be given in characters, not bytes. This is partly because a single character may occupy more than a single byte in memory. Specify the field length in bytes, and theoretically your value could overrun the field, but instead Oracle simply refuses to run the code. Specify it in characters, and Oracle will allocate the appropriate number of bytes to store the requested number of characters. Trying to specify the length semantics in bytes will result in the PLS-00639: NCHAR/NVARCHAR2 cannot be byte length semantics exception being raised.
DECLARE
foo NCHAR(42 BYTE); -- Noncompliant - raises PLS-00639
BEGIN
NULL;
END;
/
Always having a RETURN as the last statement in a function is a good practice as it prevents the ORA-06503 PL/SQL: Function returned without value error.
CREATE FUNCTION incorrectFunction RETURN PLS_INTEGER IS -- Non-Compliant
BEGIN
NULL; -- This function was expected to return a PLS_INTEGER, but did not. Will lead to ORA-06503
END;
/
BEGIN
DBMS_OUTPUT.PUT_LINE('Ret = ' || incorrectFunction2); -- ORA-06503 PL/SQL: Function returned without value
END;
/
DROP FUNCTION incorrectFunction;
Centralizing the definitions of custom exceptions comes with two major benefits:
The duplication of the exceptions declarations and
PRAGMA EXCEPTION_INITis avoidedThe risk of associating multiple different exceptions to the same number is reduced
SET SERVEROUTPUT ON
DECLARE
user_not_found EXCEPTION;
PRAGMA EXCEPTION_INIT(user_not_found, -20000); -- Noncompliant, user_not_found is bound to -20000
BEGIN
NULL;
END;
/
DECLARE
user_not_found EXCEPTION;
PRAGMA EXCEPTION_INIT(user_not_found, -20000); -- Noncompliant, user_not_found is again bound to -20000, duplication
BEGIN
NULL;
END;
/
DECLARE
wrong_password EXCEPTION;
PRAGMA EXCEPTION_INIT(wrong_password, -20000); -- Noncompliant, wrong_password is bound to -20000, conflicting with user_not_found
BEGIN
NULL;
END;
/
Using a `NUMBER to store an integer is less performant than using a PLS_INTEGER. PLS_INTEGERs require less storage than NUMBERs, and benefit from the use of hardware math, as opposed to the library math required for NUMBERs. Even more performant is the SIMPLE_INTEGER subtype of PLS_INTEGER. However, changing to either of these types is only appropriate under certain circumstances.
PLS_INTEGER is only a candidate for NUMBER with a scale of up to 9.
SIMPLE_INTEGER has the same size limitation, in addition to it’s NOT NULL constraint and lack of overflow checking.
This rule raises an issue when a NUMBER` is declared with a scale of 9 or less.
DECLARE
son NUMBER(1); -- Noncompliant
rumbo NUMBER(9); -- Noncompliant
conga Number(10); -- Ignored; falls outside the PLS_INTEGER range
compalsa PLS_INTEGER;
Simple loops, of the form LOOP … END LOOP, behave by default as infinite ones, since they do not have a loop condition. They can often be replaced by other, safer, loop constructs.
SET SERVEROUTPUT ON
DECLARE
i PLS_INTEGER;
BEGIN
i := 1;
LOOP -- Noncompliant, an infinite loop by default and therefore dangerous
DBMS_OUTPUT.PUT_LINE('First loop i: ' || i);
i := i + 1;
EXIT WHEN i > 10;
END LOOP;
END;
/
Labels are useful to match the begin and end of each PACKAGE, PROCEDURE or FUNCTION, especially when the code is badly indented or too much nested.
This rule raised an issue when the END statement of a PACKAGE, PROCEDURE or FUNCTION is having no label matching the name of the corresponding “begin” statement.
CREATE OR REPLACE PACKAGE BODY cust_sal AS
PROCEDURE find_sal(c_id customers.id%TYPE) IS
c_sal customers.salary%TYPE;
BEGIN
SELECT salary INTO c_sal
FROM customers
WHERE id = c_id;
dbms_output.put_line('Salary: '|| c_sal);
END; -- Noncompliant; not a PROCEDURE
END; -- Noncompliant; not a PACKAGE
/
`UNION is a convenient syntax to combine the results of two or more SQL statements because it helps you cut a complex problem into multiple simple SQL statements. But when it comes to execution, using UNION is debatable.
First, it may be possible to fuse two simple SQL statements into a bigger one that will run faster. Second, UNION is significantly less performant compared to UNION ALL because it removes duplicated entries and runS an internal DISTINCT to achieve this.
UNION ALL does not remove duplicates and returns all the results from the queries. It performs faster in most cases compared to UNION. Nevertheless, the quantity of data returned by UNION ALL can be significantly larger than with UNION. On a slow network, the performance gain of using UNION ALL instead of UNION can be negated by the time lost in the larger data transfer.
This rule raises an issue on each UNION. It’s up to the developer to challenge its use and see if there is a better way to rewrite without UNION`.
-- case #1
SELECT EMAIL FROM EMPLOYEES WHERE COUNTRY = 'FR'
UNION -- Noncompliant
SELECT EMAIL FROM EMPLOYEES WHERE COUNTRY = 'CH'
-- case #2
-- if you care about not having duplicated entries, then UNION is the good choice
SELECT EMAIL FROM EMPLOYEES
UNION -- Noncompliant
SELECT EMAIL FROM CUSTOMERS
IF statements with conditions that are always false have the effect of making blocks of code non-functional. This can be useful during debugging, but should not be checked in. IF statements with conditions that are always true are completely redundant, and make the code less readable. In either case, unconditional IF statements should be removed.
IF TRUE THEN
do_something;
END IF;
IF FALSE THEN
do_something_else;
END IF;
Jumping back to a previous statement using GOTO is a way to reimplement loops, which PL/SQL already provides in much more readable forms.
SET SERVEROUTPUT ON
DECLARE
result PLS_INTEGER := 0;
counter PLS_INTEGER := 1;
BEGIN
<<loop>>
result := result + counter;
counter := counter + 1;
IF counter <= 9 THEN
GOTO loop; -- Noncompliant
END IF;
DBMS_OUTPUT.PUT_LINE('Sum from 1 to 9 is ' || result); -- Displays 1 + 2 + ... + 8 + 9 = 45
END;
/
RAISE_APPLICATION_ERROR may only be called with an error code from -20,000 to -20,999, which is the range reserved for application errors. When called with another value, Oracle raises the exception: ORA-21000: error number argument to raise_application_error of 0 is out of range.
BEGIN
RAISE_APPLICATION_ERROR(0, 'This is an application error'); -- Non-Compliant - raises ORA-21000
END;
/
The deprecated MLSLABEL datatype is still supported only for backwards compatibility with Trusted Oracle, and since Oracle8, the only valid value it can hold is NULL. Thus, the usage of this type should be progressively removed.
DECLARE
foo MLSLABEL; -- Noncompliant
BEGIN
NULL;
END;
/
For fixed-length values, a `CHAR field occupies the same amount of disk space as a VARCHAR2 field, but for variable-length values CHAR fields use more storage space and make searching more difficult by right-padding values with whitespaces. Therefore VARCHAR2 fields are preferred. Similarly, NCHAR should be replaced by NVARCHAR2.
Note that for 1-character fields, CHAR is naturally equivalent to VARCHAR2`, but the latter is still preferred for consistency.
DECLARE
var1 CHAR; -- Noncompliant
var2 CHAR(42); -- Noncompliant
var3 NCHAR; -- Noncompliant
var4 NCHAR(42); -- Noncompliant
BEGIN
NULL;
END;
/
Using the same name for multiple purposes reduces the understandability of the code and might eventually lead to bugs.
This rule verifies that no label is reused in an inner scope.
<<foo>>
DECLARE
a CONSTANT PLS_INTEGER := 0;
BEGIN
<<foo>> -- Noncompliant
DECLARE
b CONSTANT PLS_INTEGER := 42;
BEGIN
DBMS_OUTPUT.PUT_LINE('x = ' || foo.b); -- Confusing
END;
END;
/
A CROSS JOIN query will return all records where each row from the first table is combined with each row from the second table. This means that such a query returns the Cartesian product of the sets of rows from the joined tables, which is why it is also know as “Cartesian product query”.
Such a query can return a huge amount of data, and therefore should be used only with great caution and only when really needed.
BEGIN
-- Standard ANSI syntax
SELECT *
INTO employeeArray
FROM employee CROSS JOIN department; -- Noncompliant; explicit cross join
END;
/
BEGIN
-- Old syntax
SELECT *
INTO employeeArray
FROM employee, department; -- Noncompliant; also a cross join
END;
/
Because RESULT_CACHE-enabled functions increase memory consumption, one should double-check that the gain in performances is significant, and avoid over-using this feature in general.
CREATE FUNCTION myFastFunction RETURN PLS_INTEGER RESULT_CACHE AS -- Noncompliant
BEGIN
RETURN 42;
END;
/
DROP FUNCTION myFastFunction;
Hard-coding bounds in FOR loops is a bad practice, just as magic numbers in general are. Often, those magic bounds can be replaced by dynamic values. If that is not possible, replacing the literal number with a constant is still better.
SET SERVEROUTPUT ON
DECLARE
TYPE myCollectionType IS VARRAY(3) OF VARCHAR2(42);
myCollection myCollectionType := myCollectionType('David', 'John', 'Richard');
BEGIN
FOR i IN 2 .. 3 -- Noncompliant; magic numbers used for the loop bounds
LOOP
DBMS_OUTPUT.PUT_LINE('name = ' || myCollection(i));
END LOOP;
FOR i IN 2 .. myCollection.LAST -- Noncompliant, better but still magic
LOOP
DBMS_OUTPUT.PUT_LINE('name = ' || myCollection(i));
END LOOP;
END;
/
Oracle supports at most 1000 items in a SQL query’s IN clause. When more items are given, the exception ORA-01795 maximum number of expressions in a list is 1000 is raised. Thus, IN clauses are not as scalable as joins.
BEGIN
SELECT *
INTO result
FROM my_table
WHERE col1 IN (1, 2, 3, ..., 1001); -- Noncompliant - raises ORA-01795
END;
/
The performance of DML queries in loops can be improved by placing them in a FORALL statement. This way, queries will be sent in bulk, minimizing the number of context switches between PL/SQL and SQL.
SET SERVEROUTPUT ON
CREATE TABLE largeTable(
foo VARCHAR2(42)
);
BEGIN
FOR i IN 1 .. 100000 LOOP
INSERT INTO largeTable VALUES('bar' || i); -- Non-compliant
END LOOP;
END;
/
SET TIMING ON
DECLARE
TYPE largeTableRowArrayType IS TABLE OF largeTable%ROWTYPE;
largeTableRowArray largeTableRowArrayType;
BEGIN
SELECT * BULK COLLECT INTO largeTableRowArray FROM largeTable;
EXECUTE IMMEDIATE 'TRUNCATE TABLE largeTable';
FOR i IN largeTableRowArray.FIRST .. largeTableRowArray.LAST LOOP
INSERT INTO largeTable (foo) VALUES (largeTableRowArray(i).foo); -- Non-compliant
END LOOP;
END;
/
SET TIMING OFF
DROP TABLE largeTable;
Unused cursor parameters are misleading. Whatever the values passed to such parameters, the behavior will be the same.
cursor c_list_emp(pp_country varchar2, pp_status varchar2) is -- Noncompliant pp_status is not used
select e.employee_code,
p.first_name,
p.last_name,
e.country
from persons p,
join employee_list e on e.person_id = p.person_id
where e.country = pp_country;
In a CREATE PACKAGE BODY, the purpose of the initialization section is to set the initial values of the package’s global variables. It is therefore surprising to find a RETURN statement there, as all its following statements will be unreachable.
SET SERVEROUTPUT ON
CREATE OR REPLACE PACKAGE foo AS
FUNCTION getBar RETURN PLS_INTEGER;
bar PLS_INTEGER;
END;
/
CREATE OR REPLACE PACKAGE BODY foo AS
FUNCTION getBar RETURN PLS_INTEGER AS
BEGIN
RETURN bar; -- Compliant
END;
BEGIN
bar := 42;
DBMS_OUTPUT.PUT_LINE('package loaded');
RETURN; -- Noncompliant
DBMS_OUTPUT.PUT_LINE('this is unreachable code');
END;
/
DROP PACKAGE BODY foo;
DROP PACKAGE foo;
The `FOR loop at first seems like a convenient way of iterating over the elements of a collection, but doing so will raise a VALUE_ERROR exception if the collection is empty. Looping instead from 1 to COUNT doesn’t work either if the collection is sparse; that leads to a ORA-01403: no data found error.
Instead, a WHILE` loop should be used.
DECLARE
TYPE fooType IS TABLE OF VARCHAR2(42);
foo fooType := new fooType('Strawberry', 'Apple', 'Banana');
BEGIN
foo.DELETE(2); -- The collection is now sparse
FOR i IN 1 .. foo.COUNT -- Noncompliant - leads to ORA-01403: no data found
LOOP
DBMS_OUTPUT.PUT_LINE(i || ' = ' || foo(i));
END LOOP;
END;
/
The requirement for a final ELSE clause is defensive programming. The CASE statement should always provide a value.
CASE grade -- Noncompliant, can raise a CASE_NOT_FOUND exception.
WHEN 'A' THEN DBMS_OUTPUT.PUT_LINE('Excellent');
WHEN 'B' THEN DBMS_OUTPUT.PUT_LINE('Very Good');
END CASE;
Quoted identifiers are confusing to many programmers, as they look similar to string literals. Moreover, for maximum portability, identifiers should be self-descriptive and should not contain accents. Quoted identifiers can contain any character, which can be confusing.
SET SERVEROUTPUT ON
DECLARE
"x + y" PLS_INTEGER := 0; -- Noncompliant, quoted identifiers are confusing
x PLS_INTEGER := 40;
y PLS_INTEGER := 2;
"hello" VARCHAR2(42) := 'world'; -- Noncompliant
BEGIN
DBMS_OUTPUT.PUT_LINE("x + y"); -- Noncompliant, displays 0
DBMS_OUTPUT.PUT_LINE("hello"); -- Noncompliant, confusing, displays "world" and not "hello"
END;
/
Using FORALL i IN x.first … x.last or FORALL i IN 1 … x.count might fail when indexed collections are sparse as Oracle tries to access non-existent element(s) of x. FORALL i IN INDICES OF x syntax will always work including sparse collections. Thus using FORALL i IN INDICES OF x should be preferred as it makes code more robust and easier to review.
FORALL i IN 1 .. l_tab.COUNT -- Non-Compliant
INSERT INTO forall_test VALUES l_tab(i);
FORALL i IN l_tab.first .. l_tab.last -- Non-Compliant
INSERT INTO forall_test VALUES l_tab(i);
It is needlessly complex to invert the result of a boolean comparison. The opposite comparison should be made instead.
IF NOT x <> y THEN -- Noncompliant
-- ...
END IF;
Trailing whitespaces bring no information, they may generate noise when comparing different versions of the same file, and they can create bugs when they appear after a \ marking a line continuation. They should be systematically removed.
An automated code formatter allows to completely avoid this family of issues and should be used wherever possible.
DECLARE
-- The following line has many trailing whitespaces
foo VARCHAR2(42) := 'a
b';
BEGIN
-- Will misleadingly show 3, counting only the characters 'a', 'b', and the line terminator, but none of the trailing whitespaces
DBMS_OUTPUT.PUT_LINE(LENGTH(foo));
END;
/
Putting multiple statements on a single line lowers the code readability and makes debugging the code more complex.
Unresolved directive in <stdin> - include::{noncompliant}[]
Write one statement per line to improve readability.
Unresolved directive in <stdin> - include::{compliant}[]
BEGIN
DBMS_OUTPUT.PUT_LINE('Hello '); DBMS_OUTPUT.PUT_LINE('World'); -- Noncompliant
END;
/
When a SQL query is joining n tables (with n>=2), it is expected to have join conditions defined to determine on which columns these n tables should be joined. At minimum, for n joined tables, the SQL query should contain (n-1) join conditions involving all the joined table to avoid a full cartesian product between the rows of the n tables.
Not doing so will imply that too many rows will be returned. If this is not the case and unless this has been done on purpose, the SQL query should be reviewed and missing conditions should be added or useless tables should be removed from the SQL query.
This rule is raising no issue when the SQL query is involving CROSS JOIN, NATURAL JOIN statements.
SELECT c.id, c.name, o.id, o.item_id, o.item_quantity
FROM ORDERS o, CUSTOMERS c; -- Noncompliant; no JOIN condition at all
SELECT c.id, c.name, o.id, o.item_id, o.item_quantity
FROM ORDERS o
JOIN CUSTOMERS c ON o.customer_id = o.id; -- Noncompliant; no condition related to CUSTOMERS
SELECT f.name, d.title, l.*
FROM FOLDERS f, DOCUMENTS d, DOC_LINES l -- Noncompliant; missing at least one condition related to DOC_LINES
WHERE f.id = d.folder_id;
When handling a caught exception, the original exception’s message and stack trace should be logged or passed forward.
SET SERVEROUTPUT ON
DECLARE
d VARCHAR2(1);
BEGIN
SELECT dummy INTO d FROM DUAL WHERE dummy = 'Y'; -- Will raise NO_DATA_FOUND
DBMS_OUTPUT.PUT_LINE('d = ' || d);
EXCEPTION
WHEN NO_DATA_FOUND THEN -- Noncompliant, did we really want to mask this exception?
NULL;
END;
/
Allowing an application to dynamically change the structure of a database at runtime is very dangerous because the application can become unstable under unexpected conditions. Best practices dictate that applications only manipulate data.
CREATE TABLE my_table(
my_column INTEGER
);
Magic numbers make the code more complex to understand as it requires the reader to have knowledge about the global context to understand the number itself. Their usage may seem obvious when writing the code, but it may not be the case for another developer or later once the context faded away. -1, 0, and 1 are not considered magic numbers.
BEGIN
FOR l_counter IN 1..5 -- Noncompliant, 5 is a magic number
LOOP
...
END LOOP;
END;
A boolean literal can be represented in two different ways: {true} or {false}. They can be combined with logical operators ({ops}) to produce logical expressions that represent truth values. However, comparing a boolean literal to a variable or expression that evaluates to a boolean value is unnecessary and can make the code harder to read and understand. The more complex a boolean expression is, the harder it will be for developers to understand its meaning and expected behavior, and it will favour the introduction of new bugs.
SET SERVEROUTPUT ON
DECLARE
foo BOOLEAN := TRUE;
BEGIN
IF foo = FALSE THEN -- Noncompliant
DBMS_OUTPUT.PUT_LINE('foo = false!');
ELSIF foo = TRUE THEN -- Noncompliant
DBMS_OUTPUT.PUT_LINE('foo = true!');
END IF;
END;
/
When exceptions occur, it is usually a bad idea to simply ignore them. Instead, it is better to handle them properly, or at least to log them.
SET SERVEROUTPUT ON
DECLARE
d VARCHAR2(1);
BEGIN
SELECT dummy INTO d FROM DUAL WHERE dummy = 'Y'; -- Will raise NO_DATA_FOUND
DBMS_OUTPUT.PUT_LINE('d = ' || d);
EXCEPTION
WHEN NO_DATA_FOUND THEN -- Noncompliant, did we really want to mask this exception?
NULL;
END;
/
SQL queries that use EXISTS subqueries are inefficient because the subquery is re-run for every row in the outer query’s table. There are more efficient ways to write most queries, ways that do not use the EXISTS condition.
SELECT e.name
FROM employee e
WHERE EXISTS (SELECT * FROM department d WHERE e.department_id = d.id AND d.name = 'Marketing');
Nested code - blocks of code inside blocks of code - is eventually necessary, but increases complexity. This is why keeping the code as flat as possible, by avoiding unnecessary nesting, is considered a good practice.
Merging if statements when possible will decrease the nesting of the code and improve its readability.
IF something THEN
IF something_else THEN -- Noncompliant
-- ...
END IF;
END IF;
Each source file should start with a header stating file ownership and the license which must be used to distribute the application.
This rule must be fed with the header text that is expected at the beginning of every file.
--
-- SonarQube, open source software quality management tool.
-- Copyright (C) 2008-2018 SonarSource
-- mailto:contact AT sonarsource DOT com
--
-- SonarQube is free software; you can redistribute it and/or
-- modify it under the terms of the GNU Lesser General Public
-- License as published by the Free Software Foundation; either
-- version 3 of the License, or (at your option) any later version.
--
-- SonarQube is distributed in the hope that it will be useful,
-- but WITHOUT ANY WARRANTY; without even the implied warranty of
-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
-- Lesser General Public License for more details.
--
-- You should have received a copy of the GNU Lesser General Public License
-- along with this program; if not, write to the Free Software Foundation,
-- Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
--
The use of parentheses, even those not required to enforce a desired order of operations, can clarify the intent behind a piece of code. However, redundant pairs of parentheses could be misleading and should be removed.
x := (y / 2 + 1); -- Compliant even if the parentheses are ignored
IF (x > 0) AND ((x+y > 0)) THEN -- Noncompliant
-- ...
END IF;
UPDATE and DELETE statements should contain WHERE clauses to keep the modification of records under control. Otherwise unexpected data loss could result.
DECLARE
maxAge PLS_INTEGER := 60;
BEGIN
UPDATE employee SET status = 'retired'; -- Noncompliant - the WHERE was forgotten
END;
/
A typical code smell known as unused function parameters refers to parameters declared in a function but not used anywhere within the function’s body. While this might seem harmless at first glance, it can lead to confusion and potential errors in your code. Disregarding the values passed to such parameters, the function’s behavior will be the same, but the programmer’s intention won’t be clearly expressed anymore. Therefore, removing function parameters that are not being utilized is considered best practice.
CREATE PROCEDURE say_hello(name VARCHAR2) AS -- Noncompliant; name is not used
BEGIN
DBMS_OUTPUT.PUT_LINE('Hello World');
END;
/
Shared naming conventions improve readability and allow teams to collaborate efficiently. This rule checks that all package names match a provided regular expression.
CREATE OR REPLACE PACKAGE invalid_package_ AS
PROCEDURE display_message;
END invalid_package_;
There is no reason to re-assign a variable to itself. Either this statement is redundant and should be removed, or the re-assignment is a mistake and some other value or variable was intended for the assignment instead.
UPDATE person
SET name = name;
Executing code dynamically is security-sensitive. It has led in the past to the following vulnerabilities:
Some APIs enable the execution of dynamic code by providing it as strings at runtime. These APIs might be useful in some very specific meta-programming use-cases. However most of the time their use is frowned upon because they also increase the risk of maliciously Injected Code. Such attacks can either run on the server or in the client (example: XSS attack) and have a huge impact on an application’s security.
This rule marks for review each occurrence of such dynamic code execution. This rule does not detect code injections. It only highlights the use of APIs which should be used sparingly and very carefully.
CREATE OR REPLACE PROCEDURE ckpwd_bind (p_user IN VARCHAR2, p_pass IN VARCHAR2)
IS
v_query VARCHAR2(100);
v_output NUMBER;
BEGIN
v_query :=
q'{SELECT COUNT(*) FROM user_pwd WHERE username = :1 AND password = :2}';
EXECUTE IMMEDIATE v_query
INTO v_output
USING p_user, p_pass;
END;
Duplicated string literals make the process of refactoring complex and error-prone, as any change would need to be propagated on all occurrences.
BEGIN
prepare('action1');
execute('action1');
release('action1');
END;
/
SELECT * should be avoided because it releases control of the returned columns and could therefore lead to errors and potentially to performance issues.
DECLARE
myvar CHAR;
BEGIN
SELECT * INTO myvar FROM DUAL; -- Noncompliant
END;
/
According to the US National Institute of Standards and Technology (NIST), the Data Encryption Standard (DES) is no longer considered secure:
For similar reasons, RC2 should also be avoided.
PLS_INTEGER := DBMS_CRYPTO.ENCRYPT_DES
+ DBMS_CRYPTO.CHAIN_CBC
+ DBMS_CRYPTO.PAD_PKCS5;
Marking a parameter for output means that callers will expect its value to be updated with a result from the execution of the procedure. Failing to update the parameter before the procedure returns is surely an error.
CREATE OR REPLACE PROCEDURE greet(
name IN VARCHAR2,
greeting OUT VARCHAR2) -- Noncompliant
AS
message VARCHAR2(45);
BEGIN
SELECT 'Hello ' || RTRIM(name) INTO message FROM DUAL;
END;
The MD5 algorithm and its successor, SHA-1, are no longer considered secure, because it is too easy to create hash collisions with them. That is, it takes too little computational effort to come up with a different input that produces the same MD5 or SHA-1 hash, and using the new, same-hash value gives an attacker the same access as if he had the originally-hashed value. This applies as well to the other Message-Digest algorithms: MD2, MD4, MD6, HAVAL-128, HMAC-MD5, DSA (which uses SHA-1), RIPEMD, RIPEMD-128, RIPEMD-160, HMACRIPEMD160.
Consider using safer alternatives, such as SHA-256, SHA-512 or SHA-3.
DBMS_CRYPTO.Hash(str, HASH_MD4);
DBMS_CRYPTO.Hash(str, HASH_MD5);
DBMS_CRYPTO.Hash(str, HASH_SH1);
This rule verifies that single-line comments are not located at the ends of lines of code. The main idea behind this rule is that in order to be really readable, trailing comments would have to be properly written and formatted (correct alignment, no interference with the visual structure of the code, not too long to be visible) but most often, automatic code formatters would not handle this correctly: the code would end up less readable. Comments are far better placed on the previous empty line of code, where they will always be visible and properly formatted.
a := b + c; -- This is a trailing comment that can be very very long
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all function names match a provided regular expression.
CREATE FUNCTION my_function_ RETURN PLS_INTEGER AS -- Noncompliant
BEGIN
RETURN 42;
END;
/
Overriding a variable declared in an outer scope can strongly impact the readability, and therefore the maintainability, of a piece of code. Further, it could lead maintainers to introduce bugs because they think they’re using one variable but are really using another.
SET SERVEROUTPUT ON
DECLARE
foo VARCHAR2(42) := 'foo';
BEGIN
DECLARE
foo VARCHAR2(42) := 'bar'; -- Noncompliant - this variable hides the one above and should be renamed
BEGIN
DBMS_OUTPUT.PUT_LINE(foo); -- Displays "bar", which is confusing
END;
DBMS_OUTPUT.PUT_LINE(foo); -- Displays "foo"
END;
/
Tables without primary keys are largely unusable in a relational database because they cannot be joined to. A primary key should be specified at table creation to guarantee that all its records have primary key values.
CREATE TABLE employee
(
employee_id INTEGER NOT NULL,
first_name VARCHAR2(42) NOT NULL,
last_name VARCHAR2(42) NOT NULL
);
Return of boolean literal statements wrapped into if-then-else ones should be simplified.
IF expression THEN
RETURN TRUE;
ELSE
RETURN FALSE;
END IF;
Even though the ORDER BY clause supports using column numbers, doing so makes the code difficult to read and maintain. Therefore the use of column names is preferred.
BEGIN
SELECT col2, col3
BULK COLLECT INTO result
FROM my_table
ORDER BY
1 ASC; -- Noncompliant - if col1 is added to the selected fields, this may break
END;
/
An unused local variable is a variable that has been declared but is not used anywhere in the block of code where it is defined. It is dead code, contributing to unnecessary complexity and leading to confusion when reading the code. Therefore, it should be removed from your code to maintain clarity and efficiency.
DECLARE
seconds INTEGER := 0; -- Noncompliant - seconds is unused
hours INTEGER := 2;
BEGIN
minutes := hours * 60;
DBMS_OUTPUT.PUT_LINE('Number of minutes: ' || minutes);
END;
Dead stores refer to assignments made to local variables that are subsequently never used or immediately overwritten. Such assignments are unnecessary and don’t contribute to the functionality or clarity of the code. They may even negatively impact performance. Removing them enhances code cleanliness and readability. Even if the unnecessary operations do not do any harm in terms of the program’s correctness, they are - at best - a waste of computing resources.
declare
my_user VARCHAR2(30);
my_date VARCHAR2(30);
begin
my_user := user();
my_date := sysdate(); -- Noncompliant, the value of my_date is never read
dbms_output.put_line('User:' || my_user || ', date: ' || my_user);
end;
A naming convention in software development is a set of guidelines for naming code elements like variables, functions, and classes. {identifier_capital_plural} hold the meaning of the written code. Their names should be meaningful and follow a consistent and easily recognizable pattern. Adhering to a consistent naming convention helps to make the code more readable and understandable, which makes it easier to maintain and debug. It also ensures consistency in the code, especially when multiple developers are working on the same project.
This rule checks that {identifier} names match a provided regular expression.
DECLARE
badVariable_ PLS_INTEGER; -- Non-Compliant
BEGIN
NULL;
END;
/
Variables should not be shadowed
SET SERVEROUTPUT ON
DECLARE
foo VARCHAR2(42) := 'foo';
BEGIN
DECLARE
foo VARCHAR2(42) := 'bar'; -- Noncompliant - this variable hides the one above and should be renamed
BEGIN
DBMS_OUTPUT.PUT_LINE(foo); -- Displays "bar", which is confusing
END;
DBMS_OUTPUT.PUT_LINE(foo); -- Displays "foo"
END;
/
Having two branches in an IF/ELSIF chain with the same implementation is at best duplicate code, and at worst a coding error.
IF param = 1 THEN
sort_order := 0;
column := 'LastName';
ELSIF param = 2 THEN
sort_order := 0;
column := 'LastName'; -- Noncompliant
ELSE
sort_order := 1;
column := 'FirstName';
END IF;
PL/SQL does not support nested C-style (
/* … */) comments.