diff --git a/docs/1-introduction/introduction.md b/docs/1-introduction/introduction.md index 87010b71..620e62d5 100644 --- a/docs/1-introduction/introduction.md +++ b/docs/1-introduction/introduction.md @@ -78,7 +78,7 @@ There are basically two types of standards. 2. Controversial - Almost every rule/guildeline falls into this category. An example of this category is [3 space indention](../../3-coding-style/coding-style/#rules). - Why not 2 or 4 or even 8? Why not use tabs? You can argue in favor of all these options. In most cases it does not really matter which option you choose. Being consistent is more important. In this case it will make the code easier to read. + Almost every rule/guideline falls into this category. An example of this category is [3 space indention](../../3-coding-style/coding-style/#rules). - Why not 2 or 4 or even 8? Why not use tabs? You can argue in favor of all these options. In most cases it does not really matter which option you choose. Being consistent is more important. In this case it will make the code easier to read. For very controversial rules, we have started to include the reasoning either as a footnote or directly in the text. diff --git a/docs/2-naming-conventions/naming-conventions.md b/docs/2-naming-conventions/naming-conventions.md index 73551c71..a3627d01 100644 --- a/docs/2-naming-conventions/naming-conventions.md +++ b/docs/2-naming-conventions/naming-conventions.md @@ -8,8 +8,8 @@ 4. Avoid long abbreviations. Abbreviations should be shorter than 5 characters. 5. Any abbreviations must be widely known and accepted. 6. Create a glossary with all accepted abbreviations. -7. Never use ORACLE keywords as names. A list of ORACLEs keywords may be found in the dictionary view `V$RESERVED_WORDS`. -8. Avoid adding redundant or meaningless prefixes and suffixes to identifiers.
Example: `CREATE TABLE emp_table`. +7. Never use ORACLE keywords as names. A list of ORACLEs keywords may be found in the dictionary view `v$reserved_words`. +8. Avoid adding redundant or meaningless prefixes and suffixes to identifiers.
Example: `create table emp_table`. 9. Always use one spoken language (e.g. English, German, French) for all objects in your application. 10. Always use the same names for elements with the same meaning. diff --git a/docs/3-coding-style/coding-style.md b/docs/3-coding-style/coding-style.md index 866afedc..c6306ce3 100644 --- a/docs/3-coding-style/coding-style.md +++ b/docs/3-coding-style/coding-style.md @@ -6,10 +6,10 @@ Rule | Description :--: | ----------- -1 | Keywords are written uppercase, names are written in lowercase. -2 | 3 space indention[^2]. +1 | Keywords and names are written in lowercase[^2]. +2 | 3 space indention[^3]. 3 | One command per line. -4 | Keywords `LOOP`, `ELSE`, `ELSIF`, `END IF`, `WHEN` on a new line. +4 | Keywords `loop`, `else`, `elsif`, `end if`, `when` on a new line. 5 | Commas in front of separated elements. 6 | Call parameters aligned, operators aligned, values aligned. 7 | SQL keywords are right aligned within a SQL command. @@ -18,34 +18,34 @@ Rule | Description ### Example -``` -PROCEDURE set_salary(in_employee_id IN employees.employee_id%TYPE) IS - CURSOR c_employees(p_employee_id IN employees.employee_id%TYPE) IS - SELECT last_name +``` sql +procedure set_salary(in_employee_id in employees.employee_id%type) is + cursor c_employees(p_employee_id in employees.employee_id%type) is + select last_name ,first_name ,salary - FROM employees - WHERE employee_id = p_employee_id - ORDER BY last_name + from employees + where employee_id = p_employee_id + order by last_name ,first_name; - r_employee c_employees%ROWTYPE; - l_new_salary employees.salary%TYPE; -BEGIN - OPEN c_employees(p_employee_id => in_employee_id); - FETCH c_employees INTO r_employee; - CLOSE c_employees; + r_employee c_employees%rowtype; + l_new_salary employees.salary%type; +begin + open c_employees(p_employee_id => in_employee_id); + fetch c_employees into r_employee; + close c_employees; new_salary (in_employee_id => in_employee_id ,out_salary => l_new_salary); -- Check whether salary has changed - IF r_employee.salary <> l_new_salary THEN - UPDATE employees - SET salary = l_new_salary - WHERE employee_id = in_employee_id; - END IF; -END set_salary; + if r_employee.salary <> l_new_salary then + update employees + set salary = l_new_salary + where employee_id = in_employee_id; + end if; +end set_salary; ``` ## Code Commenting @@ -70,7 +70,7 @@ Tag | Meaning | Example This is an example using the documentation capabilities of SQL Developer. -``` +``` sql /** Check whether we passed a valid sql name @@ -80,15 +80,21 @@ Check whether we passed a valid sql name Call Example:
-   SELECT TVDAssert.valid_sql_name('TEST') from dual;
-   SELECT TVDAssert.valid_sql_name('123') from dual
+   select TVDAssert.valid_sql_name('TEST') from dual;
+   select TVDAssert.valid_sql_name('123') from dual
 
*/ ``` [^2]: + It used to be good practice to use uppercase keywords and lowercase names to help visualize code structure. + But practically all editors support more or less advanced color highlighting of code, similar to the examples in these guidelines. + Hence as of version 4.0 we are now recommending all lowercase, as this is easier and faster for the brain to process. + You may choose to prefer the old rule - however, it is important to always be consistent, like for example keywords always in uppercase and names always in lowercase. + +[^3]: Tabs are not used because the indentation depends on the editor configuration. - We want to ensure that the code looks the same, indepenent of the editor used. + We want to ensure that the code looks the same, independent of the editor used. Hence, no tabs. But why not use 8 spaces? That's the traditional value for a tab. When writing a package function the code in the body has an indentation of 3. That's 24 characters as a starting point for the code. We think it's too much. diff --git a/docs/4-language-usage/1-general/g-1010.md b/docs/4-language-usage/1-general/g-1010.md index c92a2f11..ea506286 100644 --- a/docs/4-language-usage/1-general/g-1010.md +++ b/docs/4-language-usage/1-general/g-1010.md @@ -9,32 +9,32 @@ It's a good alternative for comments to indicate the start and end of a named pr ## Example (bad) -``` -BEGIN - BEGIN - NULL; - END; - - BEGIN - NULL; - END; -END; +``` sql +begin + begin + null; + end; + + begin + null; + end; +end; / ``` ## Example (good) -``` -BEGIN +``` sql +begin <> - BEGIN - NULL; - END prepare_data; + begin + null; + end prepare_data; <> - BEGIN - NULL; - END process_data; -END good; + begin + null; + end process_data; +end good; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/1-general/g-1020.md b/docs/4-language-usage/1-general/g-1020.md index 4041caab..8b817353 100644 --- a/docs/4-language-usage/1-general/g-1020.md +++ b/docs/4-language-usage/1-general/g-1020.md @@ -8,84 +8,84 @@ Use a label directly in front of loops and nested anonymous blocks: * To give a name to that portion of code and thereby self-document what it is doing. -* So that you can repeat that name with the END statement of that block or loop. +* So that you can repeat that name with the `end` statement of that block or loop. ## Example (bad) -``` -DECLARE - i INTEGER; - co_min_value CONSTANT INTEGER := 1; - co_max_value CONSTANT INTEGER := 10; - co_increment CONSTANT INTEGER := 1; -BEGIN +``` sql +declare + i integer; + co_min_value constant integer := 1; + co_max_value constant integer := 10; + co_increment constant integer := 1; +begin <> - BEGIN - NULL; - END; + begin + null; + end; <> - BEGIN - NULL; - END; + begin + null; + end; i := co_min_value; <> - WHILE (i <= co_max_value) - LOOP + while (i <= co_max_value) + loop i := i + co_increment; - END LOOP; + end loop; <> - LOOP - EXIT basic_loop; - END LOOP; + loop + exit basic_loop; + end loop; <> - FOR i IN co_min_value..co_max_value - LOOP + for i in co_min_value..co_max_value + loop sys.dbms_output.put_line(i); - END LOOP; -END; + end loop; +end; / ``` ## Example (good) -``` -DECLARE - i INTEGER; - co_min_value CONSTANT INTEGER := 1; - co_max_value CONSTANT INTEGER := 10; - co_increment CONSTANT INTEGER := 1; -BEGIN +``` sql +declare + i integer; + co_min_value constant integer := 1; + co_max_value constant integer := 10; + co_increment constant integer := 1; +begin <> - BEGIN - NULL; - END prepare_data; + begin + null; + end prepare_data; <> - BEGIN - NULL; - END process_data; + begin + null; + end process_data; i := co_min_value; <> - WHILE (i <= co_max_value) - LOOP + while (i <= co_max_value) + loop i := i + co_increment; - END LOOP while_loop; + end loop while_loop; <> - LOOP - EXIT basic_loop; - END LOOP basic_loop; + loop + exit basic_loop; + end loop basic_loop; <> - FOR i IN co_min_value..co_max_value - LOOP + for i in co_min_value..co_max_value + loop sys.dbms_output.put_line(i); - END LOOP for_loop; -END; + end loop for_loop; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/1-general/g-1030.md b/docs/4-language-usage/1-general/g-1030.md index 07f6d643..367983cd 100644 --- a/docs/4-language-usage/1-general/g-1030.md +++ b/docs/4-language-usage/1-general/g-1030.md @@ -9,45 +9,45 @@ Unused variables decrease the maintainability and readability of your code. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE my_proc IS - l_last_name employees.last_name%TYPE; - l_first_name employees.first_name%TYPE; - co_department_id CONSTANT departments.department_id%TYPE := 10; - e_good EXCEPTION; - BEGIN - SELECT e.last_name - INTO l_last_name - FROM employees e - WHERE e.department_id = co_department_id; - EXCEPTION - WHEN no_data_found THEN NULL; -- handle_no_data_found; - WHEN too_many_rows THEN null; -- handle_too_many_rows; - END my_proc; -END my_package; +``` sql +create or replace package body my_package is + procedure my_proc is + l_last_name employees.last_name%type; + l_first_name employees.first_name%type; + co_department_id constant departments.department_id%type := 10; + e_good exception; + begin + select e.last_name + into l_last_name + from employees e + where e.department_id = co_department_id; + exception + when no_data_found then null; -- handle_no_data_found; + when too_many_rows then null; -- handle_too_many_rows; + end my_proc; +end my_package; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE my_proc IS - l_last_name employees.last_name%TYPE; - co_department_id CONSTANT departments.department_id%TYPE := 10; - e_good EXCEPTION; - BEGIN - SELECT e.last_name - INTO l_last_name - FROM employees e - WHERE e.department_id = co_department_id; - - RAISE e_good; - EXCEPTION - WHEN no_data_found THEN NULL; -- handle_no_data_found; - WHEN too_many_rows THEN null; -- handle_too_many_rows; - END my_proc; -END my_package; +``` sql +create or replace package body my_package is + procedure my_proc is + l_last_name employees.last_name%type; + co_department_id constant departments.department_id%type := 10; + e_good exception; + begin + select e.last_name + into l_last_name + from employees e + where e.department_id = co_department_id; + + raise e_good; + exception + when no_data_found then null; -- handle_no_data_found; + when too_many_rows then null; -- handle_too_many_rows; + end my_proc; +end my_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/1-general/g-1040.md b/docs/4-language-usage/1-general/g-1040.md index bff00b2a..c2fb5189 100644 --- a/docs/4-language-usage/1-general/g-1040.md +++ b/docs/4-language-usage/1-general/g-1040.md @@ -9,66 +9,66 @@ Any part of your code, which is no longer used or cannot be reached, should be e ## Example (bad) -``` -DECLARE - co_dept_purchasing CONSTANT departments.department_id%TYPE := 30; -BEGIN - IF 2=3 THEN - NULL; -- some dead code here - END IF; +``` sql +declare + co_dept_purchasing constant departments.department_id%type := 30; +begin + if 2=3 then + null; -- some dead code here + end if; - NULL; -- some enabled code here + null; -- some enabled code here <> - LOOP - EXIT my_loop; - NULL; -- some dead code here - END LOOP my_loop; + loop + exit my_loop; + null; -- some dead code here + end loop my_loop; - NULL; -- some other enabled code here + null; -- some other enabled code here - CASE - WHEN 1 = 1 AND 'x' = 'y' THEN - NULL; -- some dead code here - ELSE - NULL; -- some further enabled code here - END CASE; + case + when 1 = 1 and 'x' = 'y' then + null; -- some dead code here + else + null; -- some further enabled code here + end case; <> - FOR r_emp IN (SELECT last_name - FROM employees - WHERE department_id = co_dept_purchasing - OR commission_pct IS NOT NULL - AND 5=6) - -- "OR commission_pct IS NOT NULL" is dead code - LOOP - SYS.dbms_output.put_line(r_emp.last_name); - END LOOP my_loop2; + for r_emp in (select last_name + from employees + where department_id = co_dept_purchasing + or commission_pct is not null + and 5=6) + -- "or commission_pct is not null" is dead code + loop + sys.dbms_output.put_line(r_emp.last_name); + end loop my_loop2; - RETURN; - NULL; -- some dead code here -END; + return; + null; -- some dead code here +end; / ``` ## Example (good) -``` -DECLARE - co_dept_admin CONSTANT dept.deptno%TYPE := 10; -BEGIN - NULL; -- some enabled code here - NULL; -- some other enabled code here - NULL; -- some further enabled code here +``` sql +declare + co_dept_admin constant dept.deptno%type := 10; +begin + null; -- some enabled code here + null; -- some other enabled code here + null; -- some further enabled code here <> - FOR r_emp IN (SELECT last_name - FROM employees - WHERE department_id = co_dept_admin - OR commission_pct IS NOT NULL) - LOOP + for r_emp in (select last_name + from employees + where department_id = co_dept_admin + or commission_pct is not null) + loop sys.dbms_output.put_line(r_emp.last_name); - END LOOP my_loop2; -END; + end loop my_loop2; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/1-general/g-1050.md b/docs/4-language-usage/1-general/g-1050.md index 60f6776b..f2964c19 100644 --- a/docs/4-language-usage/1-general/g-1050.md +++ b/docs/4-language-usage/1-general/g-1050.md @@ -9,53 +9,55 @@ Literals are often used more than once in your code. Having them defined as a co All constants should be collated in just one package used as a library. If these constants should be used in SQL too it is good practice to write a deterministic package function for every constant. +In specific situations this rule could lead to an extreme plethora of constants, for example if you use Logger like `logger.append_param(p_params =>l_params, p_name => 'p_param1_todo', p_val => p_param1_todo);`, where the value for `p_name` always should be the name of the variable that is passed to `p_val`. For such cases it would be overkill to add constants for every single variable name you are logging, so if you use Logger or similar, consider making that an exception to the rule, just document exactly which exceptions you will allow and stick to them. + ## Example (bad) -``` -DECLARE - l_job employees.job_id%TYPE; -BEGIN - SELECT e.job_id - INTO l_job - FROM employees e - WHERE e.manager_id IS NULL; +``` sql +declare + l_job employees.job_id%type; +begin + select e.job_id + into l_job + from employees e + where e.manager_id is null; - IF l_job = 'AD_PRES' THEN - NULL; - END IF; -EXCEPTION - WHEN NO_DATA_FOUND THEN - NULL; -- handle_no_data_found; - WHEN TOO_MANY_ROWS THEN - NULL; -- handle_too_many_rows; -END; + if l_job = 'AD_PRES' then + null; + end if; +exception + when no_data_found then + null; -- handle_no_data_found; + when too_many_rows then + null; -- handle_too_many_rows; +end; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE constants_up IS - co_president CONSTANT employees.job_id%TYPE := 'AD_PRES'; -END constants_up; +``` sql +create or replace package constants_up is + co_president constant employees.job_id%type := 'AD_PRES'; +end constants_up; / -DECLARE - l_job employees.job_id%TYPE; -BEGIN - SELECT e.job_id - INTO l_job - FROM employees e - WHERE e.manager_id IS NULL; - - IF l_job = constants_up.co_president THEN - NULL; - END IF; -EXCEPTION - WHEN NO_DATA_FOUND THEN - NULL; -- handle_no_data_found; - WHEN TOO_MANY_ROWS THEN - NULL; -- handle_too_many_rows; -END; +declare + l_job employees.job_id%type; +begin + select e.job_id + into l_job + from employees e + where e.manager_id is null; + + if l_job = constants_up.co_president then + null; + end if; +exception + when no_data_found then + null; -- handle_no_data_found; + when too_many_rows then + null; -- handle_too_many_rows; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/1-general/g-1060.md b/docs/4-language-usage/1-general/g-1060.md index 1633cc8a..e31713fe 100644 --- a/docs/4-language-usage/1-general/g-1060.md +++ b/docs/4-language-usage/1-general/g-1060.md @@ -5,38 +5,38 @@ ## Reason -It is an extremely dangerous practice to store ROWIDs in a table, except for some very limited scenarios of runtime duration. Any manually explicit or system generated implicit table reorganization will reassign the row's ROWID and break the data consistency. +It is an extremely dangerous practice to store `rowid`'s in a table, except for some very limited scenarios of runtime duration. Any manually explicit or system generated implicit table reorganization will reassign the row's `rowid` and break the data consistency. -Instead of using ROWID for later reference to the original row one should use the primary key column(s). +Instead of using `rowid` for later reference to the original row one should use the primary key column(s). ## Example (bad) -``` -BEGIN - INSERT INTO employees_log (employee_id +``` sql +begin + insert into employees_log (employee_id ,last_name ,first_name ,rid) - SELECT employee_id + select employee_id ,last_name ,first_name - ,ROWID - FROM employees; -END; + ,rowid + from employees; +end; / ``` ## Example (good) -``` -BEGIN - INSERT INTO employees_log (employee_id +``` sql +begin + insert into employees_log (employee_id ,last_name ,first_name) - SELECT employee_id + select employee_id ,last_name ,first_name - FROM employees; -END; + from employees; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/1-general/g-1070.md b/docs/4-language-usage/1-general/g-1070.md index 079284ab..871b075e 100644 --- a/docs/4-language-usage/1-general/g-1070.md +++ b/docs/4-language-usage/1-general/g-1070.md @@ -9,24 +9,24 @@ Having an end-of-comment within a block comment will end that block-comment. Thi ## Example (bad) -``` -BEGIN +``` sql +begin /* comment one -- nested comment two */ - NULL; + null; -- comment three /* nested comment four */ - NULL; -END; + null; +end; / ``` ## Example (good) -``` -BEGIN +``` sql +begin /* comment one, comment two */ - NULL; + null; -- comment three, comment four - NULL; -END; + null; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/1-general/g-2110.md b/docs/4-language-usage/2-variables-and-types/1-general/g-2110.md index 169b6a36..c9fd55ce 100644 --- a/docs/4-language-usage/2-variables-and-types/1-general/g-2110.md +++ b/docs/4-language-usage/2-variables-and-types/1-general/g-2110.md @@ -5,44 +5,44 @@ ## Reason -Changing the size of the database column last_name in the employees table from `VARCHAR2(20)` to `VARCHAR2(30)` will result in an error within your code whenever a value larger than the hard coded size is read from the table. This can be avoided using anchored declarations. +Changing the size of the database column last_name in the employees table from `varchar2(20)` to `varchar2(30)` will result in an error within your code whenever a value larger than the hard coded size is read from the table. This can be avoided using anchored declarations. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE my_proc IS - l_last_name VARCHAR2(20 CHAR); - co_first_row CONSTANT INTEGER := 1; - BEGIN - SELECT e.last_name - INTO l_last_name - FROM employees e - WHERE rownum = co_first_row; - EXCEPTION - WHEN no_data_found THEN NULL; -- handle no_data_found - WHEN too_many_rows THEN NULL; -- handle too_many_rows (impossible) - END my_proc; -END my_package; +``` sql +create or replace package body my_package is + procedure my_proc is + l_last_name varchar2(20 char); + co_first_row constant integer := 1; + begin + select e.last_name + into l_last_name + from employees e + where rownum = co_first_row; + exception + when no_data_found then null; -- handle no_data_found + when too_many_rows then null; -- handle too_many_rows (impossible) + end my_proc; +end my_package; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE my_proc IS - l_last_name employees.last_name%TYPE; - co_first_row CONSTANT INTEGER := 1; - BEGIN - SELECT e.last_name - INTO l_last_name - FROM employees e - WHERE rownum = co_first_row; - EXCEPTION - WHEN no_data_found THEN NULL; -- handle no_data_found - WHEN too_many_rows THEN NULL; -- handle too_many_rows (impossible) - END my_proc; -END my_package; +``` sql +create or replace package body my_package is + procedure my_proc is + l_last_name employees.last_name%type; + co_first_row constant integer := 1; + begin + select e.last_name + into l_last_name + from employees e + where rownum = co_first_row; + exception + when no_data_found then null; -- handle no_data_found + when too_many_rows then null; -- handle too_many_rows (impossible) + end my_proc; +end my_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/1-general/g-2120.md b/docs/4-language-usage/2-variables-and-types/1-general/g-2120.md index 9e20211f..1ee905be 100644 --- a/docs/4-language-usage/2-variables-and-types/1-general/g-2120.md +++ b/docs/4-language-usage/2-variables-and-types/1-general/g-2120.md @@ -11,32 +11,32 @@ A single location could be either a type specification package or the database ( ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE my_proc IS - SUBTYPE big_string_type IS VARCHAR2(1000 CHAR); +``` sql +create or replace package body my_package is + procedure my_proc is + subtype big_string_type is varchar2(1000 char); l_note big_string_type; - BEGIN + begin l_note := some_function(); - END my_proc; -END my_package; + end my_proc; +end my_package; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE types_up IS - SUBTYPE big_string_type IS VARCHAR2(1000 CHAR); -END types_up; +``` sql +create or replace package types_up is + subtype big_string_type is varchar2(1000 char); +end types_up; / -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE my_proc IS +create or replace package body my_package is + procedure my_proc is l_note types_up.big_string_type; - BEGIN + begin l_note := some_function(); - END my_proc; -END my_package; + end my_proc; +end my_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/1-general/g-2130.md b/docs/4-language-usage/2-variables-and-types/1-general/g-2130.md index fae6ca7d..93b39c9f 100644 --- a/docs/4-language-usage/2-variables-and-types/1-general/g-2130.md +++ b/docs/4-language-usage/2-variables-and-types/1-general/g-2130.md @@ -14,37 +14,37 @@ Your code will be easier to read as the usage of a variable/constant may be deri Type | Usage ------------------ | ----- `ora_name_type` | Object corresponding to the ORACLE naming conventions (table, variable, column, package, etc.). -max_vc2_type | String variable with maximal VARCHAR2 size. +`max_vc2_type` | String variable with maximal VARCHAR2 size. `array_index_type` | Best fitting data type for array navigation. `id_type` | Data type used for all primary key (id) columns. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE my_proc IS - l_note VARCHAR2(1000 CHAR); - BEGIN +``` sql +create or replace package body my_package is + procedure my_proc is + l_note varchar2(1000 char); + begin l_note := some_function(); - END my_proc; -END my_package; + end my_proc; +end my_package; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE types_up IS - SUBTYPE big_string_type IS VARCHAR2(1000 CHAR); -END types_up; +``` sql +create or replace package types_up is + subtype big_string_type is varchar2(1000 char); +end types_up; / -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE my_proc IS +create or replace package body my_package is + procedure my_proc is l_note types_up.big_string_type; - BEGIN + begin l_note := some_function(); - END my_proc; -END my_package; + end my_proc; +end my_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/1-general/g-2135.md b/docs/4-language-usage/2-variables-and-types/1-general/g-2135.md new file mode 100644 index 00000000..07431350 --- /dev/null +++ b/docs/4-language-usage/2-variables-and-types/1-general/g-2135.md @@ -0,0 +1,56 @@ +# G-2135: Avoid assigning values to local variables that are not used by a subsequent statement. + +!!! warning "Major" + Efficiency, Maintainability, Testability + +## Reason + +Expending resources calculating and assigning values to a local variable and never use the value subsequently is at best a waste, at worst indicative of a mistake that leads to a bug. + +## Example (bad) + +``` sql +create or replace package body my_package is + procedure my_proc is + co_employee_id constant employees.employee_id%type := 1042; + l_last_name employees.last_name%type; + l_message varchar2(100 char); + begin + select emp.last_name + into l_last_name + from employees emp + where emp.employee_id = co_employee_id; + + l_message := 'Hello, ' || l_last_name; + exception + when no_data_found then null; -- handle_no_data_found; + when too_many_rows then null; -- handle_too_many_rows; + end my_proc; +end my_package; +/ +``` + +## Example (good) + +``` sql +create or replace package body my_package is + procedure my_proc is + co_employee_id constant employees.employee_id%type := 1042; + l_last_name employees.last_name%type; + l_message varchar2(100 char); + begin + select emp.last_name + into l_last_name + from employees emp + where emp.employee_id = co_employee_id; + + l_message := 'Hello, ' || l_last_name; + + message_api.send_message(l_message); + exception + when no_data_found then null; -- handle_no_data_found; + when too_many_rows then null; -- handle_too_many_rows; + end my_proc; +end my_package; +/ +``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/1-general/g-2140.md b/docs/4-language-usage/2-variables-and-types/1-general/g-2140.md index 06a795de..dfff463f 100644 --- a/docs/4-language-usage/2-variables-and-types/1-general/g-2140.md +++ b/docs/4-language-usage/2-variables-and-types/1-general/g-2140.md @@ -5,26 +5,26 @@ ## Reason -Variables are initialized to NULL by default. +Variables are initialized to `null` by default. ## Example (bad) -``` -DECLARE - l_note big_string_type := NULL; -BEGIN +``` sql +declare + l_note big_string_type := null; +begin sys.dbms_output.put_line(l_note); -END; +end; / ``` ## Example (good) -``` -DECLARE +``` sql +declare l_note big_string_type; -BEGIN +begin sys.dbms_output.put_line(l_note); -END; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/1-general/g-2150.md b/docs/4-language-usage/2-variables-and-types/1-general/g-2150.md index b66d53f1..fa7ee9f5 100644 --- a/docs/4-language-usage/2-variables-and-types/1-general/g-2150.md +++ b/docs/4-language-usage/2-variables-and-types/1-general/g-2150.md @@ -5,30 +5,30 @@ ## Reason -The NULL value can cause confusion both from the standpoint of code review and code execution. You must always use the `IS NULL` or `IS NOT NULL` syntax when you need to check if a value is or is not `NULL`. +The `null` value can cause confusion both from the standpoint of code review and code execution. You must always use the `is null` or `is not null` syntax when you need to check if a value is or is not `null`. ## Example (bad) -``` -DECLARE - l_value INTEGER; -BEGIN - IF l_value = NULL THEN - NULL; - END IF; -END; +``` sql +declare + l_value integer; +begin + if l_value = null then + null; + end if; +end; / ``` ## Example (good) -``` -DECLARE - l_value INTEGER; -BEGIN - IF l_value IS NULL THEN - NULL; - END IF; -END; +``` sql +declare + l_value integer; +begin + if l_value is null then + null; + end if; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/1-general/g-2160.md b/docs/4-language-usage/2-variables-and-types/1-general/g-2160.md index aa2681a1..87e73ada 100644 --- a/docs/4-language-usage/2-variables-and-types/1-general/g-2160.md +++ b/docs/4-language-usage/2-variables-and-types/1-general/g-2160.md @@ -9,34 +9,34 @@ If your initialization fails, you will not be able to handle the error in your e ## Example (bad) -``` -DECLARE - co_department_id CONSTANT INTEGER := 100; - l_department_name departments.department_name%TYPE := +``` sql +declare + co_department_id constant integer := 100; + l_department_name departments.department_name%type := department_api.name_by_id(in_id => co_department_id); -BEGIN +begin sys.dbms_output.put_line(l_department_name); -END; +end; / ``` ## Example (good) -``` -DECLARE - co_department_id CONSTANT INTEGER := 100; - co_unkown_name CONSTANT departments.department_name%TYPE := 'unknown'; - l_department_name departments.department_name%TYPE; -BEGIN +``` sql +declare + co_department_id constant integer := 100; + co_unkown_name constant departments.department_name%type := 'unknown'; + l_department_name departments.department_name%type; +begin <> - BEGIN + begin l_department_name := department_api.name_by_id(in_id => co_department_id); - EXCEPTION - WHEN value_error THEN + exception + when value_error then l_department_name := co_unkown_name; - END init; + end init; sys.dbms_output.put_line(l_department_name); -END; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/1-general/g-2170.md b/docs/4-language-usage/2-variables-and-types/1-general/g-2170.md index aa521801..17a8b8ec 100644 --- a/docs/4-language-usage/2-variables-and-types/1-general/g-2170.md +++ b/docs/4-language-usage/2-variables-and-types/1-general/g-2170.md @@ -9,44 +9,44 @@ The readability of your code will be higher when you do not overload variables. ## Example (bad) -``` -BEGIN +``` sql +begin <
> - DECLARE - co_main CONSTANT user_objects.object_name%TYPE := 'test_main'; - co_sub CONSTANT user_objects.object_name%TYPE := 'test_sub'; - co_sep CONSTANT user_objects.object_name%TYPE := ' - '; - l_variable user_objects.object_name%TYPE := co_main; - BEGIN + declare + co_main constant user_objects.object_name%type := 'test_main'; + co_sub constant user_objects.object_name%type := 'test_sub'; + co_sep constant user_objects.object_name%type := ' - '; + l_variable user_objects.object_name%type := co_main; + begin <> - DECLARE - l_variable user_objects.object_name%TYPE := co_sub; - BEGIN + declare + l_variable user_objects.object_name%type := co_sub; + begin sys.dbms_output.put_line(l_variable || co_sep || main.l_variable); - END sub; - END main; -END; + end sub; + end main; +end; / ``` ## Example (good) -``` -BEGIN +``` sql +begin <
> - DECLARE - co_main CONSTANT user_objects.object_name%TYPE := 'test_main'; - co_sub CONSTANT user_objects.object_name%TYPE := 'test_sub'; - co_sep CONSTANT user_objects.object_name%TYPE := ' - '; - l_main_variable user_objects.object_name%TYPE := co_main; - BEGIN + declare + co_main constant user_objects.object_name%type := 'test_main'; + co_sub constant user_objects.object_name%type := 'test_sub'; + co_sep constant user_objects.object_name%type := ' - '; + l_main_variable user_objects.object_name%type := co_main; + begin <> - DECLARE - l_sub_variable user_objects.object_name%TYPE := co_sub; - BEGIN + declare + l_sub_variable user_objects.object_name%type := co_sub; + begin sys.dbms_output.put_line(l_sub_variable || co_sep || l_main_variable); - END sub; - END main; -END; + end sub; + end main; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/1-general/g-2180.md b/docs/4-language-usage/2-variables-and-types/1-general/g-2180.md index 46a61a2d..ab49ae16 100644 --- a/docs/4-language-usage/2-variables-and-types/1-general/g-2180.md +++ b/docs/4-language-usage/2-variables-and-types/1-general/g-2180.md @@ -9,32 +9,32 @@ Quoted identifiers make your code hard to read and maintain. ## Example (bad) -``` -DECLARE - "sal+comm" INTEGER; - "my constant" CONSTANT INTEGER := 1; - "my exception" EXCEPTION; -BEGIN +``` sql +declare + "sal+comm" integer; + "my constant" constant integer := 1; + "my exception" exception; +begin "sal+comm" := "my constant"; -EXCEPTION - WHEN "my exception" THEN - NULL; -END; +exception + when "my exception" then + null; +end; / ``` ## Example (good) -``` -DECLARE - l_sal_comm INTEGER; - co_my_constant CONSTANT INTEGER := 1; - e_my_exception EXCEPTION; -BEGIN +``` sql +declare + l_sal_comm integer; + co_my_constant constant integer := 1; + e_my_exception exception; +begin l_sal_comm := co_my_constant; -EXCEPTION - WHEN e_my_exception THEN - NULL; -END; +exception + when e_my_exception then + null; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/1-general/g-2185.md b/docs/4-language-usage/2-variables-and-types/1-general/g-2185.md index dbef6c1d..6798e7b6 100644 --- a/docs/4-language-usage/2-variables-and-types/1-general/g-2185.md +++ b/docs/4-language-usage/2-variables-and-types/1-general/g-2185.md @@ -9,32 +9,32 @@ You should ensure that the name you have chosen well defines its purpose and usa ## Example (bad) -``` -DECLARE - i INTEGER; - c CONSTANT INTEGER := 1; - e EXCEPTION; -BEGIN +``` sql +declare + i integer; + c constant integer := 1; + e exception; +begin i := c; -EXCEPTION - WHEN e THEN - NULL; -END; +exception + when e then + null; +end; / ``` ## Example (good) -``` -DECLARE - l_sal_comm INTEGER; - co_my_constant CONSTANT INTEGER := 1; - e_my_exception EXCEPTION; -BEGIN +``` sql +declare + l_sal_comm integer; + co_my_constant constant integer := 1; + e_my_exception exception; +begin l_sal_comm := co_my_constant; -EXCEPTION - WHEN e_my_exception THEN - NULL; -END; +exception + when e_my_exception then + null; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/1-general/g-2190.md b/docs/4-language-usage/2-variables-and-types/1-general/g-2190.md index c1eecb18..5bec89df 100644 --- a/docs/4-language-usage/2-variables-and-types/1-general/g-2190.md +++ b/docs/4-language-usage/2-variables-and-types/1-general/g-2190.md @@ -5,34 +5,34 @@ ## Reason -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. +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. -Use of `ROWID` or `UROWID` means that your SQL statement will not be portable to other SQL databases. Many developers are also not familiar with these data types, which can make the code harder to maintain. +Use of `rowid` or `urowid` means that your SQL statement will not be portable to other SQL databases. Many developers are also not familiar with these data types, which can make the code harder to maintain. ## Example (bad) -``` -DECLARE - l_department_name departments.department_name%TYPE; - l_rowid ROWID; -BEGIN - UPDATE departments - SET department_name = l_department_name - WHERE ROWID = l_rowid; -END; +``` sql +declare + l_department_name departments.department_name%type; + l_rowid rowid; +begin + update departments + set department_name = l_department_name + where rowid = l_rowid; +end; / ``` ## Example (good) -``` -DECLARE - l_department_name departments.department_name%TYPE; - l_department_id departments.department_id%TYPE; -BEGIN - UPDATE departments - SET department_name = l_department_name - WHERE department_id = l_department_id; -END; +``` sql +declare + l_department_name departments.department_name%type; + l_department_id departments.department_id%type; +begin + update departments + set department_name = l_department_name + where department_id = l_department_id; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/2-numeric-data-types/g-2210.md b/docs/4-language-usage/2-variables-and-types/2-numeric-data-types/g-2210.md index 9d6da7e9..6903bcbc 100644 --- a/docs/4-language-usage/2-variables-and-types/2-numeric-data-types/g-2210.md +++ b/docs/4-language-usage/2-variables-and-types/2-numeric-data-types/g-2210.md @@ -5,32 +5,32 @@ ## Reason -If you do not specify precision `NUMBER` is defaulted to 38 or the maximum supported by your system, whichever is less. You may well need all this precision, but if you know you do not, you should specify whatever matches your needs. +If you do not specify precision `number` is defaulted to 38 or the maximum supported by your system, whichever is less. You may well need all this precision, but if you know you do not, you should specify whatever matches your needs. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY constants_up IS - co_small_increase CONSTANT NUMBER := 0.1; - - FUNCTION small_increase RETURN NUMBER IS - BEGIN - RETURN co_small_increase; - END small_increase; -END constants_up; +``` sql +create or replace package body constants_up is + co_small_increase constant number := 0.1; + + function small_increase return number is + begin + return co_small_increase; + end small_increase; +end constants_up; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY constants_up IS - co_small_increase CONSTANT NUMBER(5,1) := 0.1; - - FUNCTION small_increase RETURN NUMBER IS - BEGIN - RETURN co_small_increase; - END small_increase; -END constants_up; +``` sql +create or replace package body constants_up is + co_small_increase constant number(5,1) := 0.1; + + function small_increase return number is + begin + return co_small_increase; + end small_increase; +end constants_up; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/2-numeric-data-types/g-2220.md b/docs/4-language-usage/2-variables-and-types/2-numeric-data-types/g-2220.md index ae436201..2b388cfc 100644 --- a/docs/4-language-usage/2-variables-and-types/2-numeric-data-types/g-2220.md +++ b/docs/4-language-usage/2-variables-and-types/2-numeric-data-types/g-2220.md @@ -5,37 +5,37 @@ ## Reason -`PLS_INTEGER` having a length of -2,147,483,648 to 2,147,483,647, on a 32bit system. +`pls_integer` having a length of -2,147,483,648 to 2,147,483,647, on a 32bit system. -There are many reasons to use `PLS_INTEGER` instead of `NUMBER`: +There are many reasons to use `pls_integer` instead of `number`: -* `PLS_INTEGER` uses less memory -* `PLS_INTEGER` uses machine arithmetic, which is up to three times faster than library arithmetic, which is used by `NUMBER`. +* `pls_integer` uses less memory +* `pls_integer` uses machine arithmetic, which is up to three times faster than library arithmetic, which is used by `number`. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY constants_up IS - co_big_increase CONSTANT NUMBER(5,0) := 1; +``` sql +create or replace package body constants_up is + co_big_increase constant number(5,0) := 1; - FUNCTION big_increase RETURN NUMBER IS - BEGIN - RETURN co_big_increase; - END big_increase; -END constants_up; + function big_increase return number is + begin + return co_big_increase; + end big_increase; +end constants_up; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY constants_up IS - co_big_increase CONSTANT PLS_INTEGER := 1; +``` sql +create or replace package body constants_up is + co_big_increase constant pls_integer := 1; - FUNCTION big_increase RETURN PLS_INTEGER IS - BEGIN - RETURN co_big_increase; - END big_increase; -END constants_up; + function big_increase return pls_integer is + begin + return co_big_increase; + end big_increase; +end constants_up; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/2-numeric-data-types/g-2230.md b/docs/4-language-usage/2-variables-and-types/2-numeric-data-types/g-2230.md index 8789b00f..f5dad5af 100644 --- a/docs/4-language-usage/2-variables-and-types/2-numeric-data-types/g-2230.md +++ b/docs/4-language-usage/2-variables-and-types/2-numeric-data-types/g-2230.md @@ -9,35 +9,35 @@ ORACLE 11g or later ## Reason -`SIMPLE_INTEGER` does no checks on numeric overflow, which results in better performance compared to the other numeric datatypes. +`simple_integer` does no checks on numeric overflow, which results in better performance compared to the other numeric datatypes. -With ORACLE 11g, the new data type `SIMPLE_INTEGER` has been introduced. It is a sub-type of `PLS_INTEGER` and covers the same range. The basic difference is that `SIMPLE_INTEGER` is always `NOT NULL`. When the value of the declared variable is never going to be null then you can declare it as `SIMPLE_INTEGER`. Another major difference is that you will never face a numeric overflow using `SIMPLE_INTEGER` as this data type wraps around without giving any error. `SIMPLE_INTEGER` data type gives major performance boost over `PLS_INTEGER` when code is compiled in `NATIVE` mode, because arithmetic operations on SIMPLE_INTEGER type are performed directly at the hardware level. +With ORACLE 11g, the new data type `simple_integer` has been introduced. It is a sub-type of `pls_integer` and covers the same range. The basic difference is that `simple_integer` is always `not null`. When the value of the declared variable is never going to be null then you can declare it as `simple_integer`. Another major difference is that you will never face a numeric overflow using `simple_integer` as this data type wraps around without giving any error. `simple_integer` data type gives major performance boost over `pls_integer` when code is compiled in `native` mode, because arithmetic operations on `simple_integer` type are performed directly at the hardware level. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY constants_up IS - co_big_increase CONSTANT NUMBER(5,0) := 1; +``` sql +create or replace package body constants_up is + co_big_increase constant number(5,0) := 1; - FUNCTION big_increase RETURN NUMBER IS - BEGIN - RETURN co_big_increase; - END big_increase; -END constants_up; + function big_increase return number is + begin + return co_big_increase; + end big_increase; +end constants_up; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY constants_up IS - co_big_increase CONSTANT SIMPLE_INTEGER := 1; +``` sql +create or replace package body constants_up is + co_big_increase constant simple_integer := 1; - FUNCTION big_increase RETURN SIMPLE_INTEGER IS - BEGIN - RETURN co_big_increase; - END big_increase; -END constants_up; + function big_increase return simple_integer is + begin + return co_big_increase; + end big_increase; +end constants_up; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2310.md b/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2310.md index c1def68b..456801b2 100644 --- a/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2310.md +++ b/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2310.md @@ -5,24 +5,24 @@ ## Reason -`CHAR` is a fixed length data type, which should only be used when appropriate. `CHAR` columns/variables are always filled to its specified lengths; this may lead to unwanted side effects and undesired results. +`char` is a fixed length data type, which should only be used when appropriate. `char` columns/variables are always filled to its specified lengths; this may lead to unwanted side effects and undesired results. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE types_up -IS - SUBTYPE description_type IS CHAR(200); -END types_up; +``` sql +create or replace package types_up +is + subtype description_type is char(200); +end types_up; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE types_up -IS - SUBTYPE description_type IS VARCHAR2(200 CHAR); -END types_up; +``` sql +create or replace package types_up +is + subtype description_type is varchar2(200 char); +end types_up; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2320.md b/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2320.md index 1151beb6..960ba9a7 100644 --- a/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2320.md +++ b/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2320.md @@ -1,26 +1,26 @@ -# G-2320: Avoid using VARCHAR data type. +# G-2320: Never use VARCHAR data type. !!! warning "Major" Portability ## Reason -Do not use the `VARCHAR` data type. Use the `VARCHAR2` data type instead. Although the `VARCHAR` data type is currently synonymous with `VARCHAR2`, the `VARCHAR` data type is scheduled to be redefined as a separate data type used for variable-length character strings compared with different comparison semantics. +Do not use the `varchar` data type. Use the `varchar2` data type instead. Although the `varchar` data type is currently synonymous with `varchar2`, the `varchar` data type is scheduled to be redefined as a separate data type used for variable-length character strings compared with different comparison semantics. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE types_up IS - SUBTYPE description_type IS VARCHAR(200); -END types_up; +``` sql +create or replace package types_up is + subtype description_type is varchar(200); +end types_up; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE types_up IS - SUBTYPE description_type IS VARCHAR2(200 CHAR); -END types_up; +``` sql +create or replace package types_up is + subtype description_type is varchar2(200 char); +end types_up; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2330.md b/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2330.md index da1c9e6e..b18e5e05 100644 --- a/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2330.md +++ b/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2330.md @@ -5,31 +5,31 @@ ## Reason -Today zero-length strings and `NULL` are currently handled identical by ORACLE. There is no guarantee that this will still be the case in future releases, therefore if you mean `NULL` use `NULL`. +Today zero-length strings and `null` are currently handled identical by ORACLE. There is no guarantee that this will still be the case in future releases, therefore if you mean `null` use `null`. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY constants_up IS - co_null_string CONSTANT VARCHAR2(1) := ''; +``` sql +create or replace package body constants_up is + co_null_string constant varchar2(1) := ''; - FUNCTION null_string RETURN VARCHAR2 IS - BEGIN - RETURN co_null_string; - END null_string; -END constants_up; + function null_string return varchar2 is + begin + return co_null_string; + end null_string; +end constants_up; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY constants_up IS +``` sql +create or replace package body constants_up is - FUNCTION empty_string RETURN VARCHAR2 IS - BEGIN - RETURN NULL; - END empty_string; -END constants_up; + function empty_string return varchar2 is + begin + return null; + end empty_string; +end constants_up; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2340.md b/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2340.md index 9f76ea2c..487eef4f 100644 --- a/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2340.md +++ b/docs/4-language-usage/2-variables-and-types/3-character-data-types/g-2340.md @@ -5,24 +5,24 @@ ## Reason -Changes to the `NLS_LENGTH_SEMANTIC` will only be picked up by your code after a recompilation. +Changes to the `nls_length_semantic` will only be picked up by your code after a recompilation. -In a multibyte environment a `VARCHAR2(10)` definition may not necessarily hold 10 characters, when multibyte characters a part of the value that should be stored unless the definition was done using the char semantic. +In a multibyte environment a `varchar2(10)` definition may not necessarily hold 10 characters when multibyte characters are part of the value that should be stored, unless the definition was done using the `char` semantic. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE types_up IS - SUBTYPE description_type IS VARCHAR2(200); -END types_up; +``` sql +create or replace package types_up is + subtype description_type is varchar2(200); +end types_up; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE types_up IS - SUBTYPE description_type IS VARCHAR2(200 CHAR); -END types_up; +``` sql +create or replace package types_up is + subtype description_type is varchar2(200 char); +end types_up; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/4-boolean-data-types/g-2410.md b/docs/4-language-usage/2-variables-and-types/4-boolean-data-types/g-2410.md index 3cd67a88..601d0506 100644 --- a/docs/4-language-usage/2-variables-and-types/4-boolean-data-types/g-2410.md +++ b/docs/4-language-usage/2-variables-and-types/4-boolean-data-types/g-2410.md @@ -5,52 +5,52 @@ ## Reason -The use of TRUE and FALSE clarifies that this is a boolean value and makes the code easier to read. +The use of `true` and `false` clarifies that this is a boolean value and makes the code easier to read. ## Example (bad) -``` -DECLARE - co_newFile CONSTANT PLS_INTEGER := 1000; - co_oldFile CONSTANT PLS_INTEGER := 500; - l_bigger PLS_INTEGER; -BEGIN - IF co_newFile < co_oldFile THEN +``` sql +declare + co_newfile constant pls_integer := 1000; + co_oldfile constant pls_integer := 500; + l_bigger pls_integer; +begin + if co_newfile < co_oldfile then l_bigger := constants_up.co_numeric_true; - ELSE + else l_bigger := constants_up.co_numeric_false; - END IF; -END; + end if; +end; / ``` ## Example (better) -``` -DECLARE - co_newFile CONSTANT PLS_INTEGER := 1000; - co_oldFile CONSTANT PLS_INTEGER := 500; - l_bigger BOOLEAN; -BEGIN - IF co_newFile < co_oldFile THEN - l_bigger := TRUE; - ELSE - l_bigger := FALSE; - END IF; -END; +``` sql +declare + co_newfile constant pls_integer := 1000; + co_oldfile constant pls_integer := 500; + l_bigger boolean; +begin + if co_newfile < co_oldfile then + l_bigger := true; + else + l_bigger := false; + end if; +end; / ``` ## Example (good) -``` -DECLARE - co_newFile CONSTANT PLS_INTEGER := 1000; - co_oldFile CONSTANT PLS_INTEGER := 500; - l_bigger BOOLEAN; -BEGIN - l_bigger := NVL(co_newFile < co_oldFile,FALSE); -END; +``` sql +declare + co_newfile constant pls_integer := 1000; + co_oldfile constant pls_integer := 500; + l_bigger boolean; +begin + l_bigger := nvl(co_newfile < co_oldfile, false); +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/2-variables-and-types/5-large-objects/g-2510.md b/docs/4-language-usage/2-variables-and-types/5-large-objects/g-2510.md index bcb47e67..ced84793 100644 --- a/docs/4-language-usage/2-variables-and-types/5-large-objects/g-2510.md +++ b/docs/4-language-usage/2-variables-and-types/5-large-objects/g-2510.md @@ -5,46 +5,46 @@ ## Reason -`LONG` and `LONG RAW` data types have been deprecated by ORACLE since version 8i - support might be discontinued in future ORACLE releases. +`long` and `long raw` data types have been deprecated by ORACLE since version 8i - support might be discontinued in future ORACLE releases. -There are many constraints to LONG datatypes in comparison to the LOB types. +There are many constraints to `long` datatypes in comparison to the `lob` types. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE example_package IS - g_long LONG; - g_raw LONG RAW; +``` sql +create or replace package example_package is + g_long long; + g_raw long raw; - PROCEDURE do_something; -END example_package; + procedure do_something; +end example_package; / -CREATE OR REPLACE PACKAGE BODY example_package IS - PROCEDURE do_something IS - BEGIN - NULL; - END do_something; -END example_package; +create or replace package body example_package is + procedure do_something is + begin + null; + end do_something; +end example_package; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE example_package IS - PROCEDURE do_something; -END example_package; +``` sql +create or replace package example_package is + procedure do_something; +end example_package; / -CREATE OR REPLACE PACKAGE BODY example_package IS - g_long CLOB; - g_raw BLOB; +create or replace package body example_package is + g_long clob; + g_raw blob; - PROCEDURE do_something IS - BEGIN - NULL; - END do_something; -END example_package; + procedure do_something is + begin + null; + end do_something; +end example_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/3-dml-and-sql/1-general/g-3110.md b/docs/4-language-usage/3-dml-and-sql/1-general/g-3110.md index ae345b1e..f433592b 100644 --- a/docs/4-language-usage/3-dml-and-sql/1-general/g-3110.md +++ b/docs/4-language-usage/3-dml-and-sql/1-general/g-3110.md @@ -9,9 +9,9 @@ Data structures often change. Having the target columns in your insert statement ## Example (bad) -``` -INSERT INTO departments - VALUES (departments_seq.nextval +``` sql +insert into departments + values (departments_seq.nextval ,'Support' ,100 ,10); @@ -19,12 +19,12 @@ INSERT INTO departments ## Example (good) -``` -INSERT INTO departments (department_id +``` sql +insert into departments (department_id ,department_name ,manager_id ,location_id) - VALUES (departments_seq.nextval + values (departments_seq.nextval ,'Support' ,100 ,10); diff --git a/docs/4-language-usage/3-dml-and-sql/1-general/g-3120.md b/docs/4-language-usage/3-dml-and-sql/1-general/g-3120.md index 8efb9f6a..5658c191 100644 --- a/docs/4-language-usage/3-dml-and-sql/1-general/g-3120.md +++ b/docs/4-language-usage/3-dml-and-sql/1-general/g-3120.md @@ -11,61 +11,61 @@ Especially when using subqueries the omission of table aliases may end in unexpe ## Example (bad) -``` -SELECT last_name +``` sql +select last_name ,first_name ,department_name - FROM employees - JOIN departments USING (department_id) - WHERE EXTRACT(MONTH FROM hire_date) = EXTRACT(MONTH FROM SYSDATE); + from employees + join departments using (department_id) + where extract(month from hire_date) = extract(month from sysdate); ``` ## Example (better) -``` -SELECT e.last_name +``` sql +select e.last_name ,e.first_name ,d.department_name - FROM employees e - JOIN departments d ON (e.department_id = d.department_id) - WHERE EXTRACT(MONTH FROM e.hire_date) = EXTRACT(MONTH FROM SYSDATE); + from employees e + join departments d on (e.department_id = d.department_id) + where extract(month from e.hire_date) = extract(month from sysdate); ``` ## Example (good) Using meaningful aliases improves the readability of your code. -``` -SELECT emp.last_name +``` sql +select emp.last_name ,emp.first_name ,dept.department_name - FROM employees emp - JOIN departments dept ON (emp.department_id = dept.department_id) - WHERE EXTRACT(MONTH FROM emp.hire_date) = EXTRACT(MONTH FROM SYSDATE); + from employees emp + join departments dept on (emp.department_id = dept.department_id) + where extract(month from emp.hire_date) = extract(month from sysdate); ``` ## Example Subquery (bad) If the `jobs` table has no `employee_id` column and `employees` has one this query will not raise an error but return all rows of the `employees` table as a subquery is allowed to access columns of all its parent tables - this construct is known as correlated subquery. -``` -SELECT last_name +``` sql +select last_name ,first_name - FROM employees - WHERE employee_id IN (SELECT employee_id - FROM jobs - WHERE job_title like '%Manager%'); + from employees + where employee_id in (select employee_id + from jobs + where job_title like '%Manager%'); ``` ## Example Subquery (good) If the `jobs` table has no `employee_id` column this query will return an error due to the directive (given by adding the table alias to the column) to read the `employee_id` column from the `jobs` table. -``` -SELECT emp.last_name +``` sql +select emp.last_name ,emp.first_name - FROM employees emp - WHERE emp.employee_id IN (SELECT j.employee_id - FROM jobs j - WHERE j.job_title like '%Manager%'); + from employees emp + where emp.employee_id in (select j.employee_id + from jobs j + where j.job_title like '%Manager%'); ``` \ No newline at end of file diff --git a/docs/4-language-usage/3-dml-and-sql/1-general/g-3130.md b/docs/4-language-usage/3-dml-and-sql/1-general/g-3130.md index beb605fd..1109a675 100644 --- a/docs/4-language-usage/3-dml-and-sql/1-general/g-3130.md +++ b/docs/4-language-usage/3-dml-and-sql/1-general/g-3130.md @@ -9,25 +9,25 @@ ANSI SQL-92 join syntax supports the full outer join. A further advantage of the ## Example (bad) -``` -SELECT e.employee_id +``` sql +select e.employee_id ,e.last_name ,e.first_name ,d.department_name - FROM employees e + from employees e ,departments d - WHERE e.department_id = d.department_id - AND EXTRACT(MONTH FROM e.hire_date) = EXTRACT(MONTH FROM SYSDATE); + where e.department_id = d.department_id + and extract(month from e.hire_date) = extract(month from sysdate); ``` ## Example (good) -``` -SELECT emp.employee_id +``` sql +select emp.employee_id ,emp.last_name ,emp.first_name ,dept.department_name - FROM employees emp - JOIN departments dept ON dept.department_id = emp.department_id - WHERE EXTRACT(MONTH FROM emp.hire_date) = EXTRACT(MONTH FROM SYSDATE); + from employees emp + join departments dept on dept.department_id = emp.department_id + where extract(month from emp.hire_date) = extract(month from sysdate); ``` \ No newline at end of file diff --git a/docs/4-language-usage/3-dml-and-sql/1-general/g-3140.md b/docs/4-language-usage/3-dml-and-sql/1-general/g-3140.md index 1116bfac..c9d67629 100644 --- a/docs/4-language-usage/3-dml-and-sql/1-general/g-3140.md +++ b/docs/4-language-usage/3-dml-and-sql/1-general/g-3140.md @@ -9,46 +9,46 @@ Using cursor-anchored records as targets for your cursors results enables the po ## Example (bad) -``` -DECLARE - CURSOR c_employees IS - SELECT employee_id, first_name, last_name - FROM employees; - l_employee_id employees.employee_id%TYPE; - l_first_name employees.first_name%TYPE; - l_last_name employees.last_name%TYPE; -BEGIN - OPEN c_employees; - FETCH c_employees INTO l_employee_id, l_first_name, l_last_name; +``` sql +declare + cursor c_employees is + select employee_id, first_name, last_name + from employees; + l_employee_id employees.employee_id%type; + l_first_name employees.first_name%type; + l_last_name employees.last_name%type; +begin + open c_employees; + fetch c_employees into l_employee_id, l_first_name, l_last_name; <> - WHILE c_employees%FOUND - LOOP + while c_employees%found + loop -- do something with the data - FETCH c_employees INTO l_employee_id, l_first_name, l_last_name; - END LOOP process_employees; - CLOSE c_employees; -END; + fetch c_employees into l_employee_id, l_first_name, l_last_name; + end loop process_employees; + close c_employees; +end; / ``` ## Example (good) -``` -DECLARE - CURSOR c_employees IS - SELECT employee_id, first_name, last_name - FROM employees; - r_employee c_employees%ROWTYPE; -BEGIN - OPEN c_employees; - FETCH c_employees INTO r_employee; +``` sql +declare + cursor c_employees is + select employee_id, first_name, last_name + from employees; + r_employee c_employees%rowtype; +begin + open c_employees; + fetch c_employees into r_employee; <> - WHILE c_employees%FOUND - LOOP + while c_employees%found + loop -- do something with the data - FETCH c_employees INTO r_employee; - END LOOP process_employees; - CLOSE c_employees; -END; + fetch c_employees into r_employee; + end loop process_employees; + close c_employees; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/3-dml-and-sql/1-general/g-3150.md b/docs/4-language-usage/3-dml-and-sql/1-general/g-3150.md index b2693e61..7b57b020 100644 --- a/docs/4-language-usage/3-dml-and-sql/1-general/g-3150.md +++ b/docs/4-language-usage/3-dml-and-sql/1-general/g-3150.md @@ -13,38 +13,38 @@ An identity column is a surrogate key by design – there is no reason why we sh ## Example (bad) -``` -CREATE TABLE locations ( - location_id NUMBER(10) NOT NULL - ,location_name VARCHAR2(60 CHAR) NOT NULL - ,city VARCHAR2(30 CHAR) NOT NULL - ,CONSTRAINT locations_pk PRIMARY KEY (location_id) +``` sql +create table locations ( + location_id number(10) not null + ,location_name varchar2(60 char) not null + ,city varchar2(30 char) not null + ,constraint locations_pk primary key (location_id) ) / -CREATE SEQUENCE location_seq START WITH 1 CACHE 20 +create sequence location_seq start with 1 cache 20 / -CREATE OR REPLACE TRIGGER location_br_i - BEFORE INSERT ON LOCATIONS - FOR EACH ROW -BEGIN +create or replace trigger location_br_i + before insert on locations + for each row +begin :new.location_id := location_seq.nextval; -END; +end; / ``` ## Example (good) -``` -CREATE TABLE locations ( - location_id NUMBER(10) GENERATED ALWAYS AS IDENTITY - ,location_name VARCHAR2(60 CHAR) NOT NULL - ,city VARCHAR2(30 CHAR) NOT NULL - ,CONSTRAINT locations_pk PRIMARY KEY (location_id)) +``` sql +create table locations ( + location_id number(10) generated always as identity + ,location_name varchar2(60 char) not null + ,city varchar2(30 char) not null + ,constraint locations_pk primary key (location_id)) / ``` -`GENERATED ALWAYS AS IDENTITY` ensures that the `location_id` is populated by a sequence. It is not possible to override the behavior in the application. +`generated always as identity` ensures that the `location_id` is populated by a sequence. It is not possible to override the behavior in the application. -However, if you use a framework that produces an `INSERT` statement including the surrogate key column, and you cannot change this behavior, then you have to use the `GENERATED BY DEFAULT ON NULL AS IDENTITY` option. This has the downside that the application may pass a value, which might lead to an immediate or delayed `ORA-00001: unique constraint violated` error. +However, if you use a framework that produces an `insert` statement including the surrogate key column, and you cannot change this behavior, then you have to use the `generated by default on null as identity` option. This has the downside that the application may pass a value, which might lead to an immediate or delayed `ORA-00001: unique constraint violated` error. diff --git a/docs/4-language-usage/3-dml-and-sql/1-general/g-3160.md b/docs/4-language-usage/3-dml-and-sql/1-general/g-3160.md index 9ef90043..9d67962c 100644 --- a/docs/4-language-usage/3-dml-and-sql/1-general/g-3160.md +++ b/docs/4-language-usage/3-dml-and-sql/1-general/g-3160.md @@ -9,53 +9,53 @@ ORACLE 12c ## Reason -In contrast to visible columns, invisible columns are not part of a record defined using `%ROWTYPE` construct. This is helpful as a virtual column may not be programmatically populated. If your virtual column is visible you have to manually define the record types used in API packages to be able to exclude them from being part of the record definition. +In contrast to visible columns, invisible columns are not part of a record defined using `%rowtype` construct. This is helpful as a virtual column may not be programmatically populated. If your virtual column is visible you have to manually define the record types used in API packages to be able to exclude them from being part of the record definition. -Invisible columns may be accessed by explicitly adding them to the column list in a SELECT statement. +Invisible columns may be accessed by explicitly adding them to the column list in a `select` statement. ## Example (bad) -``` -ALTER TABLE employees - ADD total_salary GENERATED ALWAYS AS (salary + NVL(commission_pct,0) * salary) +``` sql +alter table employees + add total_salary generated always as (salary + nvl(commission_pct,0) * salary) / -DECLARE - r_employee employees%ROWTYPE; - l_id employees.employee_id%TYPE := 107; -BEGIN +declare + r_employee employees%rowtype; + l_id employees.employee_id%type := 107; +begin r_employee := employee_api.employee_by_id(l_id); r_employee.salary := r_employee.salary * constants_up.small_increase(); - UPDATE employees - SET ROW = r_employee - WHERE employee_id = l_id; -END; + update employees + set row = r_employee + where employee_id = l_id; +end; / Error report - -ORA-54017: UPDATE operation disallowed ON virtual COLUMNS +ORA-54017: update operation disallowed on virtual columns ORA-06512: at line 9 ``` ## Example (good) -``` -ALTER TABLE employees - ADD total_salary INVISIBLE GENERATED ALWAYS AS - (salary + NVL(commission_pct,0) * salary) +``` sql +alter table employees + add total_salary invisible generated always as + (salary + nvl(commission_pct,0) * salary) / -DECLARE - r_employee employees%ROWTYPE; - co_id CONSTANT employees.employee_id%TYPE := 107; -BEGIN +declare + r_employee employees%rowtype; + co_id constant employees.employee_id%type := 107; +begin r_employee := employee_api.employee_by_id(co_id); r_employee.salary := r_employee.salary * constants_up.small_increase(); - UPDATE employees - SET ROW = r_employee - WHERE employee_id = co_id; -END; + update employees + set row = r_employee + where employee_id = co_id; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/3-dml-and-sql/1-general/g-3170.md b/docs/4-language-usage/3-dml-and-sql/1-general/g-3170.md index ed519fad..073633d6 100644 --- a/docs/4-language-usage/3-dml-and-sql/1-general/g-3170.md +++ b/docs/4-language-usage/3-dml-and-sql/1-general/g-3170.md @@ -9,20 +9,20 @@ ORACLE 12c ## Reason -Default values have been nullifiable until ORACLE 12c. Meaning any tool sending null as a value for a column having a default value bypassed the default value. Starting with ORACLE 12c default definitions may have an `ON NULL` definition in addition, which will assign the default value in case of a null value too. +Default values have been nullifiable until ORACLE 12c. Meaning any tool sending null as a value for a column having a default value bypassed the default value. Starting with ORACLE 12c default definitions may have an `on null` definition in addition, which will assign the default value in case of a `null` value too. ## Example (bad) -``` -CREATE TABLE null_test ( - test_case NUMBER(2) NOT NULL - ,column_defaulted VARCHAR2(10 CHAR) DEFAULT 'Default') +``` sql +create table null_test ( + test_case number(2) not null + ,column_defaulted varchar2(10 char) default 'Default') / -INSERT INTO null_test(test_case, column_defaulted) VALUES (1,'Value'); -INSERT INTO null_test(test_case, column_defaulted) VALUES (2,DEFAULT); -INSERT INTO null_test(test_case, column_defaulted) VALUES (3,NULL); +insert into null_test(test_case, column_defaulted) values (1,'Value'); +insert into null_test(test_case, column_defaulted) values (2,default); +insert into null_test(test_case, column_defaulted) values (3,null); -SELECT * FROM null_test; +select * from null_test; TEST_CASE COLUMN_DEF --------- ----------- @@ -33,16 +33,16 @@ TEST_CASE COLUMN_DEF ## Example (good) -``` -CREATE TABLE null_test ( - test_case NUMBER(2) NOT NULL - ,column_defaulted VARCHAR2(10 CHAR) DEFAULT ON NULL 'Default') +``` sql +create table null_test ( + test_case number(2) not null + ,column_defaulted varchar2(10 char) default on null 'Default') / -INSERT INTO null_test(test_case, column_defaulted) VALUES (1,'Value'); -INSERT INTO null_test(test_case, column_defaulted) VALUES (2,DEFAULT); -INSERT INTO null_test(test_case, column_defaulted) VALUES (3,NULL); +insert into null_test(test_case, column_defaulted) values (1,'Value'); +insert into null_test(test_case, column_defaulted) values (2,default); +insert into null_test(test_case, column_defaulted) values (3,null); -SELECT * FROM null_test; +select * from null_test; TEST_CASE COLUMN_DEF ---------- ---------- diff --git a/docs/4-language-usage/3-dml-and-sql/1-general/g-3180.md b/docs/4-language-usage/3-dml-and-sql/1-general/g-3180.md index 90cbd105..eb407028 100644 --- a/docs/4-language-usage/3-dml-and-sql/1-general/g-3180.md +++ b/docs/4-language-usage/3-dml-and-sql/1-general/g-3180.md @@ -5,28 +5,28 @@ ## Reason -If you change your select list afterwards the ORDER BY will still work but order your rows differently, when not changing the positional number. Furthermore, it is not comfortable to the readers of the code, if they have to count the columns in the SELECT list to know the way the result is ordered. +If you change your `select` list afterwards the `order by` will still work but order your rows differently, when not changing the positional number. Furthermore, it is not comfortable to the readers of the code, if they have to count the columns in the `select` list to know the way the result is ordered. ## Example (bad) -``` -SELECT UPPER(first_name) +``` sql +select upper(first_name) ,last_name ,salary ,hire_date - FROM employees - ORDER BY 4,1,3; + from employees + order by 4,1,3; ``` ## Example (good) -``` -SELECT upper(first_name) AS first_name +``` sql +select upper(first_name) as first_name ,last_name ,salary ,hire_date - FROM employees - ORDER BY hire_date + from employees + order by hire_date ,first_name ,salary; ``` \ No newline at end of file diff --git a/docs/4-language-usage/3-dml-and-sql/1-general/g-3190.md b/docs/4-language-usage/3-dml-and-sql/1-general/g-3190.md index aa4cad34..8f30f962 100644 --- a/docs/4-language-usage/3-dml-and-sql/1-general/g-3190.md +++ b/docs/4-language-usage/3-dml-and-sql/1-general/g-3190.md @@ -5,31 +5,31 @@ ## Reason -A natural join joins tables on equally named columns. This may comfortably fit on first sight, but adding logging columns to a table (changed_by, changed_date) will result in inappropriate join conditions. +A `natural join` joins tables on equally named columns. This may comfortably fit on first sight, but adding logging columns to a table (`changed_by`, `changed_date`) will result in inappropriate join conditions. ## Example (bad) -``` -SELECT department_name +``` sql +select department_name ,last_name ,first_name - FROM employees NATURAL JOIN departments - ORDER BY department_name + from employees natural join departments + order by department_name ,last_name; DEPARTMENT_NAME LAST_NAME FIRST_NAME ------------------------------ ------------------------- -------------------- Accounting Gietz William Executive De Haan Lex -… +... -ALTER TABLE departments ADD modified_at DATE DEFAULT ON NULL SYSDATE; -ALTER TABLE employees ADD modified_at DATE DEFAULT ON NULL SYSDATE; +alter table departments add modified_at date default on null sysdate; +alter table employees add modified_at date default on null sysdate; -SELECT department_name +select department_name ,last_name ,first_name - FROM employees NATURAL JOIN departments - ORDER BY department_name + from employees natural join departments + order by department_name ,last_name; No data found @@ -37,18 +37,18 @@ No data found ## Example (good) -``` -SELECT d.department_name +``` sql +select d.department_name ,e.last_name ,e.first_name - FROM employees e - JOIN departments d ON (e.department_id = d.department_id) - ORDER BY d.department_name + from employees e + join departments d on (e.department_id = d.department_id) + order by d.department_name ,e.last_name; DEPARTMENT_NAME LAST_NAME FIRST_NAME ------------------------------ ------------------------- -------------------- Accounting Gietz William Executive De Haan Lex -… +... ``` \ No newline at end of file diff --git a/docs/4-language-usage/3-dml-and-sql/1-general/g-3195.md b/docs/4-language-usage/3-dml-and-sql/1-general/g-3195.md new file mode 100644 index 00000000..174391f4 --- /dev/null +++ b/docs/4-language-usage/3-dml-and-sql/1-general/g-3195.md @@ -0,0 +1,35 @@ +# G-3195: Always use wildcards in a LIKE clause. + +!!! tip "Minor" + Maintainability + +## Reason + +Using `like` without at least one wildcard (`%` or `_`) is unclear to a maintainer whether a wildcard is forgotten or it is meant as equality test. A common antipattern is also to forget that an underscore is a wildcard, so using `like` instead of equal can return unwanted rows. If the `char` datatype is involved, there is also the danger of `like` not using blank padded comparison where equal will. Depending on use case, you should either remember at least one wildcard or use normal equality operator. + +## Example (bad) + +``` sql +select e.employee_id + ,e.last_name + from employees e + where e.last_name like 'Smith'; +``` + +## Example (good) + +``` sql +select e.employee_id + ,e.last_name + from employees e + where e.last_name like 'Smith%'; +``` + +## Example (good) + +``` sql +select e.employee_id + ,e.last_name + from employees e + where e.last_name = 'Smith'; +``` \ No newline at end of file diff --git a/docs/4-language-usage/3-dml-and-sql/2-bulk-operations/g-3210.md b/docs/4-language-usage/3-dml-and-sql/2-bulk-operations/g-3210.md index 197f7d59..5b4f2aca 100644 --- a/docs/4-language-usage/3-dml-and-sql/2-bulk-operations/g-3210.md +++ b/docs/4-language-usage/3-dml-and-sql/2-bulk-operations/g-3210.md @@ -11,42 +11,42 @@ Context switches between PL/SQL and SQL are extremely costly. BULK Operations re ## Example (bad) -``` -DECLARE +``` sql +declare t_employee_ids employee_api.t_employee_ids_type; - co_increase CONSTANT employees.salary%type := 0.1; - co_department_id CONSTANT departments.department_id%TYPE := 10; -BEGIN + co_increase constant employees.salary%type := 0.1; + co_department_id constant departments.department_id%type := 10; +begin t_employee_ids := employee_api.employee_ids_by_department( id_in => co_department_id ); <> - FOR i IN 1..t_employee_ids.COUNT() - LOOP - UPDATE employees - SET salary = salary + (salary * co_increase) - WHERE employee_id = t_employee_ids(i); - END LOOP process_employees; -END; + for i in 1..t_employee_ids.count() + loop + update employees + set salary = salary + (salary * co_increase) + where employee_id = t_employee_ids(i); + end loop process_employees; +end; / ``` ## Example (good) -``` -DECLARE +``` sql +declare t_employee_ids employee_api.t_employee_ids_type; - co_increase CONSTANT employees.salary%type := 0.1; - co_department_id CONSTANT departments.department_id%TYPE := 10; -BEGIN + co_increase constant employees.salary%type := 0.1; + co_department_id constant departments.department_id%type := 10; +begin t_employee_ids := employee_api.employee_ids_by_department( id_in => co_department_id ); <> - FORALL i IN 1..t_employee_ids.COUNT() - UPDATE employees - SET salary = salary + (salary * co_increase) - WHERE employee_id = t_employee_ids(i); -END; + forall i in 1..t_employee_ids.count() + update employees + set salary = salary + (salary * co_increase) + where employee_id = t_employee_ids(i); +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/3-dml-and-sql/3-transaction-control/.pages b/docs/4-language-usage/3-dml-and-sql/3-transaction-control/.pages new file mode 100644 index 00000000..dce6bfdf --- /dev/null +++ b/docs/4-language-usage/3-dml-and-sql/3-transaction-control/.pages @@ -0,0 +1 @@ +title: Transaction Control \ No newline at end of file diff --git a/docs/4-language-usage/3-dml-and-sql/3-transaction-control/g-3310.md b/docs/4-language-usage/3-dml-and-sql/3-transaction-control/g-3310.md new file mode 100644 index 00000000..90b05d16 --- /dev/null +++ b/docs/4-language-usage/3-dml-and-sql/3-transaction-control/g-3310.md @@ -0,0 +1,66 @@ +# G-3310: Never commit within a cursor loop. + +!!! danger "Critical" + Efficiency, Reliability + +## Reason + +Doing frequent commits within a cursor loop (all types of loops over cursors, whether implicit cursor for loop or loop with explicit fetch from cursor or cursor variable) risks not being able to complete due to ORA-01555, gives bad performance, and risks that the work is left in an unknown half-finished state and cannot be restarted. + +* If the work belongs together (an atomic transaction) the `commit` should be moved to after the loop. Or even better if the logic can be rewritten to a single DML statement on all relevant rows instead of a loop, committing after the single statement. +* If each loop iteration is a self-contained atomic transaction, consider instead to populate a collection of transactions to be done (taking restartability into account by collection population), loop over that collection (instead of looping over a cursor) and call a procedure (that contains the transaction logic and the `commit`) in the loop (see also [G-3320](../../../../4-language-usage/3-dml-and-sql/3-transaction-control/g-3320)). + + +## Example (bad) + +``` sql +declare + l_counter integer := 0; + l_discount discount.percentage%type; +begin + for r_order in ( + select o.order_id, o.customer_id + from orders o + where o.order_status = 'New' + ) loop + l_discount := sales_api.calculate_discount(p_customer_id => r_order.customer_id); + + update order_lines ol + set ol.discount = l_discount + where ol.order_id = r_order.order_id; + + l_counter := l_counter + 1; + if l_counter = 100 then + commit; + l_counter := 0; + end if; + end loop; + if l_counter > 0 then + commit; + end if; +end; +/ +``` + +## Example (good) + +``` sql +declare + l_discount discount.percentage%type; +begin + for r_order in ( + select o.order_id, o.customer_id + from orders o + where o.order_status = 'New' + ) loop + l_discount := sales_api.calculate_discount(p_customer_id => r_order.customer_id); + + update order_lines ol + set ol.discount = l_discount + where ol.order_id = r_order.order_id; + end loop; + + commit; +end; +/ +``` \ No newline at end of file diff --git a/docs/4-language-usage/3-dml-and-sql/3-transaction-control/g-3320.md b/docs/4-language-usage/3-dml-and-sql/3-transaction-control/g-3320.md new file mode 100644 index 00000000..dcaea96a --- /dev/null +++ b/docs/4-language-usage/3-dml-and-sql/3-transaction-control/g-3320.md @@ -0,0 +1,51 @@ +# G-3320: Try to move transactions within a non-cursor loop into procedures. + +!!! warning "Major" + Maintainability, Reusability, Testability + +## Reason + +Commit inside a non-cursor loop (other loop types than loops over cursors - see also [G-3310](../../../../4-language-usage/3-dml-and-sql/3-transaction-control/g-3310)) is either a self-contained atomic transaction, or it is a chunk (with suitable restartability handling) of very large data manipulations. In either case encapsulating the transaction in a procedure is good modularity, enabling reuse and testing of a single call. + +## Example (bad) + +``` sql +begin + for l_counter in 1..5 loop + insert into headers (id, text) values (l_counter, 'Number '||l_counter); + + insert into lines (header_id, line_no, text) + select l_counter, rownum, 'Line '||rownum + from dual + connect by level <= 3; + + commit; + end loop; +end; +/ +``` + +## Example (good) + +``` sql +declare + procedure create_rows ( + p_header_id in headers.id%type + ) is + begin + insert into headers (id, text) values (p_header_id, 'Number '||p_header_id); + + insert into lines (header_id, line_no, text) + select p_header_id, rownum, 'Line '||rownum + from dual + connect by level <= 3; + + commit; + end; +begin + for l_counter in 1..5 loop + create_rows(l_counter); + end loop; +end; +/ +``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/1-cursor/g-4110.md b/docs/4-language-usage/4-control-structures/1-cursor/g-4110.md index 9125e433..552666c4 100644 --- a/docs/4-language-usage/4-control-structures/1-cursor/g-4110.md +++ b/docs/4-language-usage/4-control-structures/1-cursor/g-4110.md @@ -9,50 +9,50 @@ The readability of your code will be higher when you avoid negative sentences. ## Example (bad) -``` -DECLARE - CURSOR c_employees IS - SELECT last_name +``` sql +declare + cursor c_employees is + select last_name ,first_name - FROM employees - WHERE commission_pct IS NOT NULL; + from employees + where commission_pct is not null; - r_employee c_employees%ROWTYPE; -BEGIN - OPEN c_employees; + r_employee c_employees%rowtype; +begin + open c_employees; <> - LOOP - FETCH c_employees INTO r_employee; - EXIT read_employees WHEN NOT c_employees%FOUND; - END LOOP read_employees; + loop + fetch c_employees into r_employee; + exit read_employees when not c_employees%found; + end loop read_employees; - CLOSE c_employees; -END; + close c_employees; +end; / ``` ## Example (good) -``` -DECLARE - CURSOR c_employees IS - SELECT last_name +``` sql +declare + cursor c_employees is + select last_name ,first_name - FROM employees - WHERE commission_pct IS NOT NULL; + from employees + where commission_pct is not null; - r_employee c_employees%ROWTYPE; -BEGIN - OPEN c_employees; + r_employee c_employees%rowtype; +begin + open c_employees; <> - LOOP - FETCH c_employees INTO r_employee; - EXIT read_employees WHEN c_employees%NOTFOUND; - END LOOP read_employees; + loop + fetch c_employees into r_employee; + exit read_employees when c_employees%notfound; + end loop read_employees; - CLOSE c_employees; -END; + close c_employees; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/1-cursor/g-4120.md b/docs/4-language-usage/4-control-structures/1-cursor/g-4120.md index 46fd7c6a..0bbe94c1 100644 --- a/docs/4-language-usage/4-control-structures/1-cursor/g-4120.md +++ b/docs/4-language-usage/4-control-structures/1-cursor/g-4120.md @@ -5,39 +5,39 @@ ## Reason -`%NOTFOUND` is set to `TRUE` as soon as less than the number of rows defined by the `LIMIT` clause has been read. +`%notfound` is set to `true` as soon as less than the number of rows defined by the `limit` clause has been read. ## Example (bad) -The employees table holds 107 rows. The example below will only show 100 rows as the cursor attribute `NOTFOUND` is set to true as soon as the number of rows to be fetched defined by the limit clause is not fulfilled anymore. +The employees table holds 107 rows. The example below will only show 100 rows as the cursor attribute `notfound` is set to true as soon as the number of rows to be fetched defined by the `limit` clause is not fulfilled anymore. -``` -DECLARE - CURSOR c_employees IS - SELECT * - FROM employees - ORDER BY employee_id; +``` sql +declare + cursor c_employees is + select * + from employees + order by employee_id; - TYPE t_employees_type IS TABLE OF c_employees%ROWTYPE; + type t_employees_type is table of c_employees%rowtype; t_employees t_employees_type; - co_bulk_size CONSTANT SIMPLE_INTEGER := 10; -BEGIN - OPEN c_employees; + co_bulk_size constant simple_integer := 10; +begin + open c_employees; <> - LOOP - FETCH c_employees BULK COLLECT INTO t_employees LIMIT co_bulk_size; - EXIT process_employees WHEN c_employees%NOTFOUND; + loop + fetch c_employees bulk collect into t_employees limit co_bulk_size; + exit process_employees when c_employees%notfound; <> - FOR i IN 1..t_employees.COUNT() - LOOP + for i in 1..t_employees.count() + loop sys.dbms_output.put_line(t_employees(i).last_name); - END LOOP display_employees; - END LOOP process_employees; + end loop display_employees; + end loop process_employees; - CLOSE c_employees; -END; + close c_employees; +end; / ``` @@ -45,32 +45,32 @@ END; This example will show all 107 rows but execute one fetch too much (12 instead of 11). -``` -DECLARE - CURSOR c_employees IS - SELECT * - FROM employees - ORDER BY employee_id; +``` sql +declare + cursor c_employees is + select * + from employees + order by employee_id; - TYPE t_employees_type IS TABLE OF c_employees%ROWTYPE; + type t_employees_type is table of c_employees%rowtype; t_employees t_employees_type; - co_bulk_size CONSTANT SIMPLE_INTEGER := 10; -BEGIN - OPEN c_employees; + co_bulk_size constant simple_integer := 10; +begin + open c_employees; <> - LOOP - FETCH c_employees BULK COLLECT INTO t_employees LIMIT co_bulk_size; - EXIT process_employees WHEN t_employees.COUNT() = 0; + loop + fetch c_employees bulk collect into t_employees limit co_bulk_size; + exit process_employees when t_employees.count() = 0; <> - FOR i IN 1..t_employees.COUNT() - LOOP + for i in 1..t_employees.count() + loop sys.dbms_output.put_line(t_employees(i).last_name); - END LOOP display_employees; - END LOOP process_employees; + end loop display_employees; + end loop process_employees; - CLOSE c_employees; -END; + close c_employees; +end; / ``` @@ -78,31 +78,31 @@ END; This example does the trick (11 fetches only to process all rows) -``` -DECLARE - CURSOR c_employees IS - SELECT * - FROM employees - ORDER BY employee_id; +``` sql +declare + cursor c_employees is + select * + from employees + order by employee_id; - TYPE t_employees_type IS TABLE OF c_employees%ROWTYPE; + type t_employees_type is table of c_employees%rowtype; t_employees t_employees_type; - co_bulk_size CONSTANT SIMPLE_INTEGER := 10; -BEGIN - OPEN c_employees; + co_bulk_size constant simple_integer := 10; +begin + open c_employees; <> - LOOP - FETCH c_employees BULK COLLECT INTO t_employees LIMIT co_bulk_size; + loop + fetch c_employees bulk collect into t_employees limit co_bulk_size; <> - FOR i IN 1..t_employees.COUNT() - LOOP + for i in 1..t_employees.count() + loop sys.dbms_output.put_line(t_employees(i).last_name); - END LOOP display_employees; - EXIT process_employees WHEN t_employees.COUNT() <> co_bulk_size; - END LOOP process_employees; + end loop display_employees; + exit process_employees when t_employees.count() <> co_bulk_size; + end loop process_employees; - CLOSE c_employees; -END; + close c_employees; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/1-cursor/g-4130.md b/docs/4-language-usage/4-control-structures/1-cursor/g-4130.md index f2f00079..e6b22fda 100644 --- a/docs/4-language-usage/4-control-structures/1-cursor/g-4130.md +++ b/docs/4-language-usage/4-control-structures/1-cursor/g-4130.md @@ -5,46 +5,46 @@ ## Reason -Any cursors left open can consume additional memory space (i.e. SGA) within the database instance, potentially in both the shared and private SQL pools. Furthermore, failure to explicitly close cursors may also cause the owning session to exceed its maximum limit of open cursors (as specified by the `OPEN_CURSORS` database initialization parameter), potentially resulting in the Oracle error of “ORA-01000: maximum open cursors exceeded”. +Any cursors left open can consume additional memory space (i.e. SGA) within the database instance, potentially in both the shared and private SQL pools. Furthermore, failure to explicitly close cursors may also cause the owning session to exceed its maximum limit of open cursors (as specified by the `open_cursors` database initialization parameter), potentially resulting in the Oracle error of “ORA-01000: maximum open cursors exceeded”. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY employee_api AS - FUNCTION department_salary (in_dept_id IN departments.department_id%TYPE) - RETURN NUMBER IS - CURSOR c_department_salary(p_dept_id IN departments.department_id%TYPE) IS - SELECT sum(salary) AS sum_salary - FROM employees - WHERE department_id = p_dept_id; +``` sql +create or replace package body employee_api as + function department_salary (in_dept_id in departments.department_id%type) + return number is + cursor c_department_salary(p_dept_id in departments.department_id%type) is + select sum(salary) as sum_salary + from employees + where department_id = p_dept_id; r_department_salary c_department_salary%rowtype; - BEGIN - OPEN c_department_salary(p_dept_id => in_dept_id); - FETCH c_department_salary INTO r_department_salary; + begin + open c_department_salary(p_dept_id => in_dept_id); + fetch c_department_salary into r_department_salary; - RETURN r_department_salary.sum_salary; - END department_salary; -END employee_api; + return r_department_salary.sum_salary; + end department_salary; +end employee_api; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY employee_api AS - FUNCTION department_salary (in_dept_id IN departments.department_id%TYPE) - RETURN NUMBER IS - CURSOR c_department_salary(p_dept_id IN departments.department_id%TYPE) IS - SELECT SUM(salary) AS sum_salary - FROM employees - WHERE department_id = p_dept_id; +``` sql +create or replace package body employee_api as + function department_salary (in_dept_id in departments.department_id%type) + return number is + cursor c_department_salary(p_dept_id in departments.department_id%type) is + select sum(salary) as sum_salary + from employees + where department_id = p_dept_id; r_department_salary c_department_salary%rowtype; - BEGIN - OPEN c_department_salary(p_dept_id => in_dept_id); - FETCH c_department_salary INTO r_department_salary; - CLOSE c_department_salary; - RETURN r_department_salary.sum_salary; - END department_salary; -END employee_api; + begin + open c_department_salary(p_dept_id => in_dept_id); + fetch c_department_salary into r_department_salary; + close c_department_salary; + return r_department_salary.sum_salary; + end department_salary; +end employee_api; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/1-cursor/g-4140.md b/docs/4-language-usage/4-control-structures/1-cursor/g-4140.md index ca44c36b..f296e212 100644 --- a/docs/4-language-usage/4-control-structures/1-cursor/g-4140.md +++ b/docs/4-language-usage/4-control-structures/1-cursor/g-4140.md @@ -5,70 +5,70 @@ ## Reason -Oracle provides a variety of cursor attributes (like `%FOUND` and `%ROWCOUNT`) that can be used to obtain information about the status of a cursor, either implicit or explicit. +Oracle provides a variety of cursor attributes (like `%found` and `%rowcount`) that can be used to obtain information about the status of a cursor, either implicit or explicit. You should avoid inserting any statements between the cursor operation and the use of an attribute against that cursor. Interposing such a statement can affect the value returned by the attribute, thereby potentially corrupting the logic of your program. -In the following example, a procedure call is inserted between the DELETE statement and a check for the value of `SQL%ROWCOUNT`, which returns the number of rows modified by that last SQL statement executed in the session. If this procedure includes a `COMMIT` / `ROLLBACK` or another implicit cursor the value of `SQL%ROWCOUNT` is affected. +In the following example, a procedure call is inserted between the `delete` statement and a check for the value of `sql%rowcount`, which returns the number of rows modified by that last SQL statement executed in the session. If this procedure includes a `commit` / `rollback` or another implicit cursor the value of `sql%rowcount` is affected. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY employee_api AS - co_one CONSTANT SIMPLE_INTEGER := 1; +``` sql +create or replace package body employee_api as + co_one constant simple_integer := 1; - PROCEDURE process_dept(in_dept_id IN departments.department_id%TYPE) IS - BEGIN - NULL; - END process_dept; + procedure process_dept(in_dept_id in departments.department_id%type) is + begin + null; + end process_dept; - PROCEDURE remove_employee (in_employee_id IN employees.employee_id%TYPE) IS - l_dept_id employees.department_id%TYPE; - BEGIN - DELETE FROM employees - WHERE employee_id = in_employee_id - RETURNING department_id INTO l_dept_id; + procedure remove_employee (in_employee_id in employees.employee_id%type) is + l_dept_id employees.department_id%type; + begin + delete from employees + where employee_id = in_employee_id + returning department_id into l_dept_id; process_dept(in_dept_id => l_dept_id); - IF SQL%ROWCOUNT > co_one THEN + if sql%rowcount > co_one then -- too many rows deleted. - ROLLBACK; - END IF; - END remove_employee; -END employee_api; + rollback; + end if; + end remove_employee; +end employee_api; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY employee_api AS - co_one CONSTANT SIMPLE_INTEGER := 1; +``` sql +create or replace package body employee_api as + co_one constant simple_integer := 1; - PROCEDURE process_dept(in_dept_id IN departments.department_id%TYPE) IS - BEGIN - NULL; - END process_dept; + procedure process_dept(in_dept_id in departments.department_id%type) is + begin + null; + end process_dept; - PROCEDURE remove_employee (in_employee_id IN employees.employee_id%TYPE) IS - l_dept_id employees.department_id%TYPE; - l_deleted_emps SIMPLE_INTEGER; - BEGIN - DELETE FROM employees - WHERE employee_id = in_employee_id - RETURNING department_id INTO l_dept_id; + procedure remove_employee (in_employee_id in employees.employee_id%type) is + l_dept_id employees.department_id%type; + l_deleted_emps simple_integer; + begin + delete from employees + where employee_id = in_employee_id + returning department_id into l_dept_id; - l_deleted_emps := SQL%ROWCOUNT; + l_deleted_emps := sql%rowcount; process_dept(in_dept_id => l_dept_id); - IF l_deleted_emps > co_one THEN + if l_deleted_emps > co_one then -- too many rows deleted. - ROLLBACK; - END IF; - END remove_employee; -END employee_api; + rollback; + end if; + end remove_employee; +end employee_api; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4210.md b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4210.md index 7c49c60a..1e193548 100644 --- a/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4210.md +++ b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4210.md @@ -5,40 +5,40 @@ ## Reason -`IF` statements containing multiple `ELSIF` tend to become complex quickly. +`if` statements containing multiple `elsif` tend to become complex quickly. ## Example (bad) -``` -DECLARE - l_color VARCHAR2(7 CHAR); -BEGIN - IF l_color = constants_up.co_red THEN +``` sql +declare + l_color varchar2(7 char); +begin + if l_color = constants_up.co_red then my_package.do_red(); - ELSIF l_color = constants_up.co_blue THEN + elsif l_color = constants_up.co_blue then my_package.do_blue(); - ELSIF l_color = constants_up.co_black THEN + elsif l_color = constants_up.co_black then my_package.do_black(); - END IF; -END; + end if; +end; / ``` ## Example (good) -``` -DECLARE +``` sql +declare l_color types_up.color_code_type; -BEGIN - CASE l_color - WHEN constants_up.co_red THEN +begin + case l_color + when constants_up.co_red then my_package.do_red(); - WHEN constants_up.co_blue THEN + when constants_up.co_blue then my_package.do_blue(); - WHEN constants_up.co_black THEN + when constants_up.co_black then my_package.do_black(); - ELSE NULL; - END CASE; -END; + else null; + end case; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4220.md b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4220.md index b8af0619..3640ccf6 100644 --- a/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4220.md +++ b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4220.md @@ -5,26 +5,48 @@ ## Reason -`DECODE` is an ORACLE specific function hard to understand and restricted to SQL only. The “newer” `CASE` function is much more common has a better readability and may be used within PL/SQL too. +`decode` is an ORACLE specific function hard to understand and restricted to SQL only. The “newer” `case` function is much more common, has a better readability and may be used within PL/SQL too. Be careful that `decode` can handle `null` values, which the simple `case` cannot - for such cases you must use the searched `case` and `is null` instead. ## Example (bad) -``` -SELECT DECODE(dummy, 'X', 1 +``` sql +select decode(dummy, 'X', 1 , 'Y', 2 , 'Z', 3 , 0) - FROM dual; + from dual; ``` ## Example (good) +``` sql +select case dummy + when 'X' then 1 + when 'Y' then 2 + when 'Z' then 3 + else 0 + end + from dual; +``` + +## Example (bad) + +``` sql +select decode(dummy, 'X', 1 + , 'Y', 2 + , null, -1 + , 0) + from dual; ``` -SELECT CASE dummy - WHEN 'X' THEN 1 - WHEN 'Y' THEN 2 - WHEN 'Z' THEN 3 - ELSE 0 - END - FROM dual; + +## Example (good) + +``` sql +select case + when dummy = 'X' then 1 + when dummy = 'Y' then 2 + when dummy is null then -1 + else 0 + end + from dual; ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4230.md b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4230.md index 37c92070..416f751f 100644 --- a/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4230.md +++ b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4230.md @@ -5,21 +5,21 @@ ## Reason -The `NVL` function always evaluates both parameters before deciding which one to use. This can be harmful if parameter 2 is either a function call or a select statement, as it will be executed regardless of whether parameter 1 contains a NULL value or not. +The `nvl` function always evaluates both parameters before deciding which one to use. This can be harmful if parameter 2 is either a function call or a select statement, as it will be executed regardless of whether parameter 1 contains a `null` value or not. -The `COALESCE` function does not have this drawback. +The `coalesce` function does not have this drawback. ## Example (bad) -``` -SELECT NVL(dummy, my_package.expensive_null(value_in => dummy)) - FROM dual; +``` sql +select nvl(dummy, my_package.expensive_null(value_in => dummy)) + from dual; ``` ## Example (good) -``` -SELECT COALESCE(dummy, my_package.expensive_null(value_in => dummy)) - FROM dual; +``` sql +select coalesce(dummy, my_package.expensive_null(value_in => dummy)) + from dual; ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4240.md b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4240.md index f3b67c07..c51ab5f0 100644 --- a/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4240.md +++ b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4240.md @@ -5,24 +5,24 @@ ## Reason -The `NVL2` function always evaluates all parameters before deciding which one to use. This can be harmful, if parameter 2 or 3 is either a function call or a select statement, as they will be executed regardless of whether parameter 1 contains a `NULL` value or not. +The `nvl2` function always evaluates all parameters before deciding which one to use. This can be harmful, if parameter 2 or 3 is either a function call or a select statement, as they will be executed regardless of whether parameter 1 contains a `null` value or not. ## Example (bad) -``` -SELECT NVL2(dummy, my_package.expensive_nn(value_in => dummy), +``` sql +select nvl2(dummy, my_package.expensive_nn(value_in => dummy), my_package.expensive_null(value_in => dummy)) - FROM dual; + from dual; ``` ## Example (good) -``` -SELECT CASE - WHEN dummy IS NULL THEN +``` sql +select case + when dummy is null then my_package.expensive_null(value_in => dummy) - ELSE + else my_package.expensive_nn(value_in => dummy) - END -FROM dual; + end +from dual; ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4250.md b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4250.md new file mode 100644 index 00000000..f170c708 --- /dev/null +++ b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4250.md @@ -0,0 +1,46 @@ +# G-4250: Avoid using identical conditions in different branches of the same IF or CASE statement. + +!!! warning "Major" + Maintainability, Reliability, Testability + +## Reason + +Conditions are evaluated top to bottom in branches of a `case` statement or chain of `if`/`elsif` statements. The first condition to evaluate as true leads to that branch being executed, the rest will never execute. Having an identical duplicated condition in another branch will never be reached and will be dead code. + +## Example (bad) + +``` sql +declare + l_color types_up.color_code_type; +begin + case l_color + when constants_up.co_red then + my_package.do_red(); + when constants_up.co_blue then + my_package.do_blue(); + when constants_up.co_red then -- never reached + my_package.do_black(); -- dead code + else null; + end case; +end; +/ +``` + +## Example (good) + +``` sql +declare + l_color types_up.color_code_type; +begin + case l_color + when constants_up.co_red then + my_package.do_red(); + when constants_up.co_blue then + my_package.do_blue(); + when constants_up.co_black then + my_package.do_black(); + else null; + end case; +end; +/ +``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4260.md b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4260.md new file mode 100644 index 00000000..2ed6b4dc --- /dev/null +++ b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4260.md @@ -0,0 +1,34 @@ +# G-4260: Avoid inverting boolean conditions. + +!!! tip "Minor" + Maintainability, Testability + +## Reason + +It is more readable to use the opposite comparison operator instead of inverting the comparison with `not`. + +## Example (bad) + +``` sql +declare + l_color varchar2(7 char); +begin + if not l_color != constants_up.co_red then + my_package.do_red(); + end if; +end; +/ +``` + +## Example (good) + +``` sql +declare + l_color types_up.color_code_type; +begin + if l_color = constants_up.co_red then + my_package.do_red(); + end if; +end; +/ +``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4270.md b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4270.md new file mode 100644 index 00000000..55f0f3c6 --- /dev/null +++ b/docs/4-language-usage/4-control-structures/2-case-if-decode-nvl-nvl2-coalesce/g-4270.md @@ -0,0 +1,38 @@ +# G-4270: Avoid comparing boolean values to boolean literals. + +!!! tip "Minor" + Maintainability, Testability + +## Reason + +It is more readable to simply use the boolean value as a condition itself, rather than use a comparison condition comparing the boolean value to the literals `true` or `false`. + +## Example (bad) + +``` sql +declare + l_string varchar2(10 char) := '42'; + l_is_valid boolean; +begin + l_is_valid := my_package.is_valid_number(l_string); + if l_is_valid = true then + my_package.convert_number(l_string); + end if; +end; +/ +``` + +## Example (good) + +``` sql +declare + l_string varchar2(10 char) := '42'; + l_is_valid boolean; +begin + l_is_valid := my_package.is_valid_number(l_string); + if l_is_valid then + my_package.convert_number(l_string); + end if; +end; +/ +``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/3-flow-control/g-4310.md b/docs/4-language-usage/4-control-structures/3-flow-control/g-4310.md index 4a7a1dcc..11e5d062 100644 --- a/docs/4-language-usage/4-control-structures/3-flow-control/g-4310.md +++ b/docs/4-language-usage/4-control-structures/3-flow-control/g-4310.md @@ -13,98 +13,98 @@ ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE password_check (in_password IN VARCHAR2) IS - co_digitarray CONSTANT STRING(10 CHAR) := '0123456789'; - co_lower_bound CONSTANT SIMPLE_INTEGER := 1; - co_errno CONSTANT SIMPLE_INTEGER := -20501; - co_errmsg CONSTANT STRING(100 CHAR) := 'Password must contain a digit.'; - l_isdigit BOOLEAN := FALSE; - l_len_pw PLS_INTEGER; - l_len_array PLS_INTEGER; - BEGIN - l_len_pw := LENGTH(in_password); - l_len_array := LENGTH(co_digitarray); +``` sql +create or replace package body my_package is + procedure password_check (in_password in varchar2) is + co_digitarray constant string(10 char) := '0123456789'; + co_lower_bound constant simple_integer := 1; + co_errno constant simple_integer := -20501; + co_errmsg constant string(100 char) := 'Password must contain a digit.'; + l_isdigit boolean := false; + l_len_pw pls_integer; + l_len_array pls_integer; + begin + l_len_pw := length(in_password); + l_len_array := length(co_digitarray); <> - FOR i IN co_lower_bound .. l_len_array - LOOP + for i in co_lower_bound .. l_len_array + loop <> - FOR j IN co_lower_bound .. l_len_pw - LOOP - IF SUBSTR(in_password, j, 1) = SUBSTR(co_digitarray, i, 1) THEN - l_isdigit := TRUE; - GOTO check_other_things; - END IF; - END LOOP check_pw_char; - END LOOP check_digit; + for j in co_lower_bound .. l_len_pw + loop + if substr(in_password, j, 1) = substr(co_digitarray, i, 1) then + l_isdigit := true; + goto check_other_things; + end if; + end loop check_pw_char; + end loop check_digit; <> - NULL; + null; - IF NOT l_isdigit THEN + if not l_isdigit then raise_application_error(co_errno, co_errmsg); - END IF; - END password_check; -END my_package; + end if; + end password_check; +end my_package; / ``` ## Example (better) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE password_check (in_password IN VARCHAR2) IS - co_digitarray CONSTANT STRING(10 CHAR) := '0123456789'; - co_lower_bound CONSTANT SIMPLE_INTEGER := 1; - co_errno CONSTANT SIMPLE_INTEGER := -20501; - co_errmsg CONSTANT STRING(100 CHAR) := 'Password must contain a digit.'; - l_isdigit BOOLEAN := FALSE; - l_len_pw PLS_INTEGER; - l_len_array PLS_INTEGER; - BEGIN - l_len_pw := LENGTH(in_password); - l_len_array := LENGTH(co_digitarray); +``` sql +create or replace package body my_package is + procedure password_check (in_password in varchar2) is + co_digitarray constant string(10 char) := '0123456789'; + co_lower_bound constant simple_integer := 1; + co_errno constant simple_integer := -20501; + co_errmsg constant string(100 char) := 'Password must contain a digit.'; + l_isdigit boolean := false; + l_len_pw pls_integer; + l_len_array pls_integer; + begin + l_len_pw := length(in_password); + l_len_array := length(co_digitarray); <> - FOR i IN co_lower_bound .. l_len_array - LOOP + for i in co_lower_bound .. l_len_array + loop <> - FOR j IN co_lower_bound .. l_len_pw - LOOP - IF SUBSTR(in_password, j, 1) = SUBSTR(co_digitarray, i, 1) THEN - l_isdigit := TRUE; - EXIT check_digit; -- early exit condition - END IF; - END LOOP check_pw_char; - END LOOP check_digit; + for j in co_lower_bound .. l_len_pw + loop + if substr(in_password, j, 1) = substr(co_digitarray, i, 1) then + l_isdigit := true; + exit check_digit; -- early exit condition + end if; + end loop check_pw_char; + end loop check_digit; <> - NULL; + null; - IF NOT l_isdigit THEN + if not l_isdigit then raise_application_error(co_errno, co_errmsg); - END IF; - END password_check; -END my_package; + end if; + end password_check; +end my_package; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE password_check (in_password IN VARCHAR2) IS - co_digitpattern CONSTANT STRING(10 CHAR) := '\d'; - co_errno CONSTANT SIMPLE_INTEGER := -20501; - co_errmsg CONSTANT STRING(100 CHAR) := 'Password must contain a digit.'; - BEGIN - IF NOT REGEXP_LIKE(in_password, co_digitpattern) - THEN +``` sql +create or replace package body my_package is + procedure password_check (in_password in varchar2) is + co_digitpattern constant string(10 char) := '\d'; + co_errno constant simple_integer := -20501; + co_errmsg constant string(100 char) := 'Password must contain a digit.'; + begin + if not regexp_like(in_password, co_digitpattern) + then raise_application_error(co_errno, co_errmsg); - END IF; - END password_check; -END my_package; + end if; + end password_check; +end my_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/3-flow-control/g-4320.md b/docs/4-language-usage/4-control-structures/3-flow-control/g-4320.md index fd88e11a..98936461 100644 --- a/docs/4-language-usage/4-control-structures/3-flow-control/g-4320.md +++ b/docs/4-language-usage/4-control-structures/3-flow-control/g-4320.md @@ -9,69 +9,69 @@ It's a good alternative for comments to indicate the start and end of a named lo ## Example (bad) -``` -DECLARE - i INTEGER; - co_min_value CONSTANT SIMPLE_INTEGER := 1; - co_max_value CONSTANT SIMPLE_INTEGER := 10; - co_increment CONSTANT SIMPLE_INTEGER := 1; -BEGIN +``` sql +declare + i integer; + co_min_value constant simple_integer := 1; + co_max_value constant simple_integer := 10; + co_increment constant simple_integer := 1; +begin i := co_min_value; - WHILE (i <= co_max_value) - LOOP + while (i <= co_max_value) + loop i := i + co_increment; - END LOOP; + end loop; - LOOP - EXIT; - END LOOP; + loop + exit; + end loop; - FOR i IN co_min_value..co_max_value - LOOP + for i in co_min_value..co_max_value + loop sys.dbms_output.put_line(i); - END LOOP; + end loop; - FOR r_employee IN (SELECT last_name FROM employees) - LOOP + for r_employee in (select last_name from employees) + loop sys.dbms_output.put_line(r_employee.last_name); - END LOOP; -END; + end loop; +end; / ``` ## Example (good) -``` -DECLARE - i INTEGER; - co_min_value CONSTANT SIMPLE_INTEGER := 1; - co_max_value CONSTANT SIMPLE_INTEGER := 10; - co_increment CONSTANT SIMPLE_INTEGER := 1; -BEGIN +``` sql +declare + i integer; + co_min_value constant simple_integer := 1; + co_max_value constant simple_integer := 10; + co_increment constant simple_integer := 1; +begin i := co_min_value; <> - WHILE (i <= co_max_value) - LOOP + while (i <= co_max_value) + loop i := i + co_increment; - END LOOP while_loop; + end loop while_loop; <> - LOOP - EXIT basic_loop; - END LOOP basic_loop; + loop + exit basic_loop; + end loop basic_loop; <> - FOR i IN co_min_value..co_max_value - LOOP + for i in co_min_value..co_max_value + loop sys.dbms_output.put_line(i); - END LOOP for_loop; + end loop for_loop; <> - FOR r_employee IN (SELECT last_name - FROM employees) - LOOP + for r_employee in (select last_name + from employees) + loop sys.dbms_output.put_line(r_employee.last_name); - END LOOP process_employees; -END; + end loop process_employees; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/3-flow-control/g-4330.md b/docs/4-language-usage/4-control-structures/3-flow-control/g-4330.md index 85fba4af..9fd8c841 100644 --- a/docs/4-language-usage/4-control-structures/3-flow-control/g-4330.md +++ b/docs/4-language-usage/4-control-structures/3-flow-control/g-4330.md @@ -7,45 +7,45 @@ It is easier for the reader to see, that the complete data set is processed. Using SQL to define the data to be processed is easier to maintain and typically faster than using conditional processing within the loop. -Since an `EXIT` statement is similar to a `GOTO` statement, +Since an `exit` statement is similar to a `goto` statement, it should be avoided, whenever possible. ## Example (bad) -``` -DECLARE - CURSOR c_employees IS - SELECT employee_id, last_name - FROM employees; - r_employee c_employees%ROWTYPE; -BEGIN - OPEN c_employees; +``` sql +declare + cursor c_employees is + select employee_id, last_name + from employees; + r_employee c_employees%rowtype; +begin + open c_employees; <> - LOOP - FETCH c_employees INTO r_employee; - EXIT read_employees WHEN c_employees%NOTFOUND; + loop + fetch c_employees into r_employee; + exit read_employees when c_employees%notfound; sys.dbms_output.put_line(r_employee.last_name); - END LOOP read_employees; + end loop read_employees; - CLOSE c_employees; -END; + close c_employees; +end; / ``` ## Example (good) -``` -DECLARE - CURSOR c_employees IS - SELECT employee_id, last_name - FROM employees; -BEGIN +``` sql +declare + cursor c_employees is + select employee_id, last_name + from employees; +begin <> - FOR r_employee IN c_employees - LOOP + for r_employee in c_employees + loop sys.dbms_output.put_line(r_employee.last_name); - END LOOP read_employees; -END; + end loop read_employees; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/3-flow-control/g-4340.md b/docs/4-language-usage/4-control-structures/3-flow-control/g-4340.md index 243ddd59..01ad3cd7 100644 --- a/docs/4-language-usage/4-control-structures/3-flow-control/g-4340.md +++ b/docs/4-language-usage/4-control-structures/3-flow-control/g-4340.md @@ -7,50 +7,50 @@ It is easier for the reader to see, that the complete array is processed. -Since an `EXIT` statement is similar to a `GOTO` statement, +Since an `exit` statement is similar to a `goto` statement, it should be avoided, whenever possible. ## Example (bad) -``` -DECLARE - TYPE t_employee_type IS VARRAY(10) OF employees.employee_id%TYPE; +``` sql +declare + type t_employee_type is varray(10) of employees.employee_id%type; t_employees t_employee_type; - co_himuro CONSTANT INTEGER := 118; - co_livingston CONSTANT INTEGER := 177; - co_min_value CONSTANT SIMPLE_INTEGER := 1; - co_increment CONSTANT SIMPLE_INTEGER := 1; - i PLS_INTEGER; -BEGIN + co_himuro constant integer := 118; + co_livingston constant integer := 177; + co_min_value constant simple_integer := 1; + co_increment constant simple_integer := 1; + i pls_integer; +begin t_employees := t_employee_type(co_himuro, co_livingston); i := co_min_value; <> - LOOP - EXIT process_employees WHEN i > t_employees.COUNT(); + loop + exit process_employees when i > t_employees.count(); sys.dbms_output.put_line(t_employees(i)); i := i + co_increment; - END LOOP process_employees; -END; + end loop process_employees; +end; / ``` ## Example (good) -``` -DECLARE - TYPE t_employee_type IS VARRAY(10) OF employees.employee_id%TYPE; +``` sql +declare + type t_employee_type is varray(10) of employees.employee_id%type; t_employees t_employee_type; - co_himuro CONSTANT INTEGER := 118; - co_livingston CONSTANT INTEGER := 177; -BEGIN + co_himuro constant integer := 118; + co_livingston constant integer := 177; +begin t_employees := t_employee_type(co_himuro, co_livingston); <> - FOR i IN 1..t_employees.COUNT() - LOOP + for i in 1..t_employees.count() + loop sys.dbms_output.put_line(t_employees(i)); - END LOOP process_employees; -END; + end loop process_employees; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/3-flow-control/g-4350.md b/docs/4-language-usage/4-control-structures/3-flow-control/g-4350.md index bf7427d6..a2c96c44 100644 --- a/docs/4-language-usage/4-control-structures/3-flow-control/g-4350.md +++ b/docs/4-language-usage/4-control-structures/3-flow-control/g-4350.md @@ -5,21 +5,21 @@ ## Reason -Doing so will not raise a `VALUE_ERROR` if the array you are looping through is empty. If you want to use `FIRST()..LAST()` you need to check the array for emptiness beforehand to avoid the raise of `VALUE_ERROR`. +Doing so will not raise a `value_error` if the array you are looping through is empty. If you want to use `first()..last()` you need to check the array for emptiness beforehand to avoid the raise of `value_error`. ## Example (bad) -``` -DECLARE - TYPE t_employee_type IS TABLE OF employees.employee_id%TYPE; +``` sql +declare + type t_employee_type is table of employees.employee_id%type; t_employees t_employee_type := t_employee_type(); -BEGIN +begin <> - FOR i IN t_employees.FIRST()..t_employees.LAST() - LOOP + for i in t_employees.first()..t_employees.last() + loop sys.dbms_output.put_line(t_employees(i)); -- some processing - END LOOP process_employees; -END; + end loop process_employees; +end; / ``` @@ -27,36 +27,36 @@ END; Raise an unitialized collection error if `t_employees` is not initialized. -``` -DECLARE - TYPE t_employee_type IS TABLE OF employees.employee_id%TYPE; +``` sql +declare + type t_employee_type is table of employees.employee_id%type; t_employees t_employee_type := t_employee_type(); -BEGIN +begin <> - FOR i IN 1..t_employees.COUNT() - LOOP + for i in 1..t_employees.count() + loop sys.dbms_output.put_line(t_employees(i)); -- some processing - END LOOP process_employees; -END; + end loop process_employees; +end; / ``` ## Example (good) -Raises neither an error nor checking whether the array is empty. `t_employees.COUNT()` always returns a `NUMBER` (unless the array is not initialized). If the array is empty `COUNT()` returns 0 and therefore the loop will not be entered. +Raises neither an error nor checking whether the array is empty. `t_employees.count()` always returns a `number` (unless the array is not initialized). If the array is empty `count()` returns 0 and therefore the loop will not be entered. -``` -DECLARE - TYPE t_employee_type IS TABLE OF employees.employee_id%TYPE; +``` sql +declare + type t_employee_type is table of employees.employee_id%type; t_employees t_employee_type := t_employee_type(); -BEGIN - IF t_employees IS NOT NULL THEN +begin + if t_employees is not null then <> - FOR i IN 1..t_employees.COUNT() - LOOP + for i in 1..t_employees.count() + loop sys.dbms_output.put_line(t_employees(i)); -- some processing - END LOOP process_employees; - END IF; -END; + end loop process_employees; + end if; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/3-flow-control/g-4360.md b/docs/4-language-usage/4-control-structures/3-flow-control/g-4360.md index 7f9f682f..c0aa1bc6 100644 --- a/docs/4-language-usage/4-control-structures/3-flow-control/g-4360.md +++ b/docs/4-language-usage/4-control-structures/3-flow-control/g-4360.md @@ -5,56 +5,56 @@ ## Reason -When a loose array is processed using a NUMERIC `FOR LOOP` we have to check with all iterations whether the element exist to avoid a `NO_DATA_FOUND` exception. In addition, the number of iterations is not driven by the number of elements in the array but by the number of the lowest/highest element. The more gaps we have, the more superfluous iterations will be done. +When a loose (also called sparse) array is processed using a *numeric* `for loop` we have to check with all iterations whether the element exist to avoid a `no_data_found` exception. In addition, the number of iterations is not driven by the number of elements in the array but by the number of the lowest/highest element. The more gaps we have, the more superfluous iterations will be done. ## Example (bad) -``` -DECLARE -- raises no_data_found when processing 2nd record - TYPE t_employee_type IS TABLE OF employees.employee_id%TYPE; +``` sql +declare -- raises no_data_found when processing 2nd record + type t_employee_type is table of employees.employee_id%type; t_employees t_employee_type; - co_rogers CONSTANT INTEGER := 134; - co_matos CONSTANT INTEGER := 143; - co_mcewen CONSTANT INTEGER := 158; - co_index_matos CONSTANT INTEGER := 2; -BEGIN + co_rogers constant integer := 134; + co_matos constant integer := 143; + co_mcewen constant integer := 158; + co_index_matos constant integer := 2; +begin t_employees := t_employee_type(co_rogers, co_matos, co_mcewen); - t_employees.DELETE(co_index_matos); + t_employees.delete(co_index_matos); - IF t_employees IS NOT NULL THEN + if t_employees is not null then <> - FOR i IN 1..t_employees.COUNT() - LOOP + for i in 1..t_employees.count() + loop sys.dbms_output.put_line(t_employees(i)); - END LOOP process_employees; - END IF; -END; + end loop process_employees; + end if; +end; / ``` ## Example (good) -``` -DECLARE - TYPE t_employee_type IS TABLE OF employees.employee_id%TYPE; +``` sql +declare + type t_employee_type is table of employees.employee_id%type; t_employees t_employee_type; - co_rogers CONSTANT INTEGER := 134; - co_matos CONSTANT INTEGER := 143; - co_mcewen CONSTANT INTEGER := 158; - co_index_matos CONSTANT INTEGER := 2; - l_index PLS_INTEGER; -BEGIN + co_rogers constant integer := 134; + co_matos constant integer := 143; + co_mcewen constant integer := 158; + co_index_matos constant integer := 2; + l_index pls_integer; +begin t_employees := t_employee_type(co_rogers, co_matos, co_mcewen); - t_employees.DELETE(co_index_matos); + t_employees.delete(co_index_matos); - l_index := t_employees.FIRST(); + l_index := t_employees.first(); <> - WHILE l_index IS NOT NULL - LOOP + while l_index is not null + loop sys.dbms_output.put_line(t_employees(l_index)); - l_index := t_employees.NEXT(l_index); - END LOOP process_employees; -END; + l_index := t_employees.next(l_index); + end loop process_employees; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/3-flow-control/g-4370.md b/docs/4-language-usage/4-control-structures/3-flow-control/g-4370.md index e9fa2ec7..e001ec7b 100644 --- a/docs/4-language-usage/4-control-structures/3-flow-control/g-4370.md +++ b/docs/4-language-usage/4-control-structures/3-flow-control/g-4370.md @@ -9,78 +9,78 @@ A numeric for loop as well as a while loop and a cursor for loop have defined lo ## Example (bad) -``` -DECLARE - i INTEGER; - co_min_value CONSTANT SIMPLE_INTEGER := 1; - co_max_value CONSTANT SIMPLE_INTEGER := 10; - co_increment CONSTANT SIMPLE_INTEGER := 1; -BEGIN +``` sql +declare + i integer; + co_min_value constant simple_integer := 1; + co_max_value constant simple_integer := 10; + co_increment constant simple_integer := 1; +begin i := co_min_value; <> - WHILE (i <= co_max_value) - LOOP + while (i <= co_max_value) + loop i := i + co_increment; - EXIT while_loop WHEN i > co_max_value; - END LOOP while_loop; + exit while_loop when i > co_max_value; + end loop while_loop; <> - LOOP - EXIT basic_loop; - END LOOP basic_loop; + loop + exit basic_loop; + end loop basic_loop; <> - FOR i IN co_min_value..co_max_value - LOOP - NULL; - EXIT for_loop WHEN i = co_max_value; - END LOOP for_loop; + for i in co_min_value..co_max_value + loop + null; + exit for_loop when i = co_max_value; + end loop for_loop; <> - FOR r_employee IN (SELECT last_name - FROM employees) - LOOP + for r_employee in (select last_name + from employees) + loop sys.dbms_output.put_line(r_employee.last_name); - NULL; -- some processing - EXIT process_employees; - END LOOP process_employees; -END; + null; -- some processing + exit process_employees; + end loop process_employees; +end; / ``` ## Example (good) -``` -DECLARE - i INTEGER; - co_min_value CONSTANT SIMPLE_INTEGER := 1; - co_max_value CONSTANT SIMPLE_INTEGER := 10; - co_increment CONSTANT SIMPLE_INTEGER := 1; -BEGIN +``` sql +declare + i integer; + co_min_value constant simple_integer := 1; + co_max_value constant simple_integer := 10; + co_increment constant simple_integer := 1; +begin i := co_min_value; <> - WHILE (i <= co_max_value) - LOOP + while (i <= co_max_value) + loop i := i + co_increment; - END LOOP while_loop; + end loop while_loop; <> - LOOP - EXIT basic_loop; - END LOOP basic_loop; + loop + exit basic_loop; + end loop basic_loop; <> - FOR i IN co_min_value..co_max_value - LOOP + for i in co_min_value..co_max_value + loop sys.dbms_output.put_line(i); - END LOOP for_loop; + end loop for_loop; <> - FOR r_employee IN (SELECT last_name - FROM employees) - LOOP + for r_employee in (select last_name + from employees) + loop sys.dbms_output.put_line(r_employee.last_name); -- some processing - END LOOP process_employees; -END; + end loop process_employees; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/3-flow-control/g-4375.md b/docs/4-language-usage/4-control-structures/3-flow-control/g-4375.md index a348d504..20b0f304 100644 --- a/docs/4-language-usage/4-control-structures/3-flow-control/g-4375.md +++ b/docs/4-language-usage/4-control-structures/3-flow-control/g-4375.md @@ -5,42 +5,42 @@ ## Reason -If you need to use an `EXIT` statement use its full semantic to make the code easier to understand and maintain. There is simply no need for an additional IF statement. +If you need to use an `exit` statement use its full semantic to make the code easier to understand and maintain. There is simply no need for an additional `if` statement. ## Example (bad) -``` -DECLARE - co_first_year CONSTANT PLS_INTEGER := 1900; -BEGIN +``` sql +declare + co_first_year constant pls_integer := 1900; +begin <> - LOOP + loop my_package.some_processing(); - IF EXTRACT(year FROM SYSDATE) > co_first_year THEN - EXIT process_employees; - END IF; + if extract(year from sysdate) > co_first_year then + exit process_employees; + end if; my_package.some_further_processing(); - END LOOP process_employees; -END; + end loop process_employees; +end; / ``` ## Example (good) -``` -DECLARE - co_first_year CONSTANT PLS_INTEGER := 1900; -BEGIN +``` sql +declare + co_first_year constant pls_integer := 1900; +begin <> - LOOP + loop my_package.some_processing(); - EXIT process_employees WHEN EXTRACT(YEAR FROM SYSDATE) > co_first_year; + exit process_employees when extract(year from sysdate) > co_first_year; my_package.some_further_processing(); - END LOOP process_employees; -END; + end loop process_employees; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/3-flow-control/g-4380.md b/docs/4-language-usage/4-control-structures/3-flow-control/g-4380.md index 25890fef..4e02ed7f 100644 --- a/docs/4-language-usage/4-control-structures/3-flow-control/g-4380.md +++ b/docs/4-language-usage/4-control-structures/3-flow-control/g-4380.md @@ -9,62 +9,62 @@ It's a good alternative for comments, especially for nested loops to name the lo ## Example (bad) -``` -DECLARE - co_init_loop CONSTANT SIMPLE_INTEGER := 0; - co_increment CONSTANT SIMPLE_INTEGER := 1; - co_exit_value CONSTANT SIMPLE_INTEGER := 3; - co_outer_text CONSTANT types_up.short_text_type := 'Outer Loop counter is '; - co_inner_text CONSTANT types_up.short_text_type := ' Inner Loop counter is '; - l_outerlp PLS_INTEGER; - l_innerlp PLS_INTEGER; -BEGIN +``` sql +declare + co_init_loop constant simple_integer := 0; + co_increment constant simple_integer := 1; + co_exit_value constant simple_integer := 3; + co_outer_text constant types_up.short_text_type := 'Outer Loop counter is '; + co_inner_text constant types_up.short_text_type := ' Inner Loop counter is '; + l_outerlp pls_integer; + l_innerlp pls_integer; +begin l_outerlp := co_init_loop; <> - LOOP + loop l_innerlp := co_init_loop; - l_outerlp := NVL(l_outerlp,co_init_loop) + co_increment; + l_outerlp := nvl(l_outerlp,co_init_loop) + co_increment; <> - LOOP - l_innerlp := NVL(l_innerlp, co_init_loop) + co_increment; + loop + l_innerlp := nvl(l_innerlp, co_init_loop) + co_increment; sys.dbms_output.put_line(co_outer_text || l_outerlp || co_inner_text || l_innerlp); - EXIT WHEN l_innerlp = co_exit_value; - END LOOP innerloop; + exit when l_innerlp = co_exit_value; + end loop innerloop; - EXIT WHEN l_innerlp = co_exit_value; - END LOOP outerloop; -END; + exit when l_innerlp = co_exit_value; + end loop outerloop; +end; / ``` ## Example (good) -``` -DECLARE - co_init_loop CONSTANT SIMPLE_INTEGER := 0; - co_increment CONSTANT SIMPLE_INTEGER := 1; - co_exit_value CONSTANT SIMPLE_INTEGER := 3; - co_outer_text CONSTANT types_up.short_text_type := 'Outer Loop counter is '; - co_inner_text CONSTANT types_up.short_text_type := ' Inner Loop counter is '; - l_outerlp PLS_INTEGER; - l_innerlp PLS_INTEGER; -BEGIN +``` sql +declare + co_init_loop constant simple_integer := 0; + co_increment constant simple_integer := 1; + co_exit_value constant simple_integer := 3; + co_outer_text constant types_up.short_text_type := 'Outer Loop counter is '; + co_inner_text constant types_up.short_text_type := ' Inner Loop counter is '; + l_outerlp pls_integer; + l_innerlp pls_integer; +begin l_outerlp := co_init_loop; <> - LOOP + loop l_innerlp := co_init_loop; - l_outerlp := NVL(l_outerlp,co_init_loop) + co_increment; + l_outerlp := nvl(l_outerlp,co_init_loop) + co_increment; <> - LOOP - l_innerlp := NVL(l_innerlp, co_init_loop) + co_increment; + loop + l_innerlp := nvl(l_innerlp, co_init_loop) + co_increment; sys.dbms_output.put_line(co_outer_text || l_outerlp || co_inner_text || l_innerlp); - EXIT outerloop WHEN l_innerlp = co_exit_value; - END LOOP innerloop; - END LOOP outerloop; -END; + exit outerloop when l_innerlp = co_exit_value; + end loop innerloop; + end loop outerloop; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/3-flow-control/g-4385.md b/docs/4-language-usage/4-control-structures/3-flow-control/g-4385.md index cc0f50d4..a8b49a44 100644 --- a/docs/4-language-usage/4-control-structures/3-flow-control/g-4385.md +++ b/docs/4-language-usage/4-control-structures/3-flow-control/g-4385.md @@ -9,36 +9,36 @@ You might process more data than required, which leads to bad performance. ## Example (bad) -``` -DECLARE - l_employee_found BOOLEAN := FALSE; - CURSOR c_employees IS - SELECT employee_id, last_name - FROM employees; -BEGIN +``` sql +declare + l_employee_found boolean := false; + cursor c_employees is + select employee_id, last_name + from employees; +begin <> - FOR r_employee IN c_employees - LOOP - l_employee_found := TRUE; - END LOOP check_employees; -END; + for r_employee in c_employees + loop + l_employee_found := true; + end loop check_employees; +end; / ``` ## Example (good) -``` -DECLARE - l_employee_found BOOLEAN := FALSE; - CURSOR c_employees IS - SELECT employee_id, last_name - FROM employees; - r_employee c_employees%ROWTYPE; -BEGIN - OPEN c_employees; - FETCH c_employees INTO r_employee; - l_employee_found := c_employees%FOUND; - CLOSE c_employees; -END; +``` sql +declare + l_employee_found boolean := false; + cursor c_employees is + select employee_id, last_name + from employees; + r_employee c_employees%rowtype; +begin + open c_employees; + fetch c_employees into r_employee; + l_employee_found := c_employees%found; + close c_employees; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/3-flow-control/g-4390.md b/docs/4-language-usage/4-control-structures/3-flow-control/g-4390.md index 7f9c951a..cdef2b82 100644 --- a/docs/4-language-usage/4-control-structures/3-flow-control/g-4390.md +++ b/docs/4-language-usage/4-control-structures/3-flow-control/g-4390.md @@ -5,50 +5,50 @@ ## Reason -If the loop index is used for anything but traffic control inside the loop, this is one of the indicators that a numeric FOR loop is being used incorrectly. The actual body of executable statements completely ignores the loop index. When that is the case, there is a good chance that you do not need the loop at all. +If the loop index is used for anything but traffic control inside the loop, this is one of the indicators that a numeric `for` loop is being used incorrectly. The actual body of executable statements completely ignores the loop index. When that is the case, there is a good chance that you do not need the loop at all. ## Example (bad) -``` -DECLARE - l_row PLS_INTEGER; - l_value PLS_INTEGER; - co_lower_bound CONSTANT SIMPLE_INTEGER := 1; - co_upper_bound CONSTANT SIMPLE_INTEGER := 5; - co_row_incr CONSTANT SIMPLE_INTEGER := 1; - co_value_incr CONSTANT SIMPLE_INTEGER := 10; - co_delimiter CONSTANT types_up.short_text_type := ' '; - co_first_value CONSTANT SIMPLE_INTEGER := 100; -BEGIN +``` sql +declare + l_row pls_integer; + l_value pls_integer; + co_lower_bound constant simple_integer := 1; + co_upper_bound constant simple_integer := 5; + co_row_incr constant simple_integer := 1; + co_value_incr constant simple_integer := 10; + co_delimiter constant types_up.short_text_type := ' '; + co_first_value constant simple_integer := 100; +begin l_row := co_lower_bound; l_value := co_first_value; <> - FOR i IN co_lower_bound .. co_upper_bound - LOOP + for i in co_lower_bound .. co_upper_bound + loop sys.dbms_output.put_line(l_row || co_delimiter || l_value); l_row := l_row + co_row_incr; l_value := l_value + co_value_incr; - END LOOP for_loop; -END; + end loop for_loop; +end; / ``` ## Example (good) -``` -DECLARE - co_lower_bound CONSTANT SIMPLE_INTEGER := 1; - co_upper_bound CONSTANT SIMPLE_INTEGER := 5; - co_value_incr CONSTANT SIMPLE_INTEGER := 10; - co_delimiter CONSTANT types_up.short_text_type := ' '; - co_first_value CONSTANT SIMPLE_INTEGER := 100; -BEGIN +``` sql +declare + co_lower_bound constant simple_integer := 1; + co_upper_bound constant simple_integer := 5; + co_value_incr constant simple_integer := 10; + co_delimiter constant types_up.short_text_type := ' '; + co_first_value constant simple_integer := 100; +begin <> - FOR i IN co_lower_bound .. co_upper_bound - LOOP + for i in co_lower_bound .. co_upper_bound + loop sys.dbms_output.put_line(i || co_delimiter || to_char(co_first_value + i * co_value_incr)); - END LOOP for_loop; -END; + end loop for_loop; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/4-control-structures/3-flow-control/g-4395.md b/docs/4-language-usage/4-control-structures/3-flow-control/g-4395.md index 78b7e5e2..d8a952f8 100644 --- a/docs/4-language-usage/4-control-structures/3-flow-control/g-4395.md +++ b/docs/4-language-usage/4-control-structures/3-flow-control/g-4395.md @@ -5,33 +5,33 @@ ## Reason -Your `LOOP` statement uses a hard-coded value for either its upper or lower bounds. This creates a "weak link" in your program because it assumes that this value will never change. A better practice is to create a named constant (or function) and reference this named element instead of the hard-coded value. +Your `loop` statement uses a hard-coded value for either its upper or lower bounds. This creates a "weak link" in your program because it assumes that this value will never change. A better practice is to create a named constant (or function) and reference this named element instead of the hard-coded value. ## Example (bad) -``` -BEGIN +``` sql +begin <> - FOR i IN 1..5 - LOOP + for i in 1..5 + loop sys.dbms_output.put_line(i); - END LOOP for_loop; -END; + end loop for_loop; +end; / ``` ## Example (good) -``` -DECLARE - co_lower_bound CONSTANT SIMPLE_INTEGER := 1; - co_upper_bound CONSTANT SIMPLE_INTEGER := 5; -BEGIN +``` sql +declare + co_lower_bound constant simple_integer := 1; + co_upper_bound constant simple_integer := 5; +begin <> - FOR i IN co_lower_bound..co_upper_bound - LOOP + for i in co_lower_bound..co_upper_bound + loop sys.dbms_output.put_line(i); - END LOOP for_loop; -END; + end loop for_loop; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/5-exception-handling/g-5010.md b/docs/4-language-usage/5-exception-handling/g-5010.md index 3670ecef..4179ec29 100644 --- a/docs/4-language-usage/5-exception-handling/g-5010.md +++ b/docs/4-language-usage/5-exception-handling/g-5010.md @@ -17,25 +17,25 @@ This kind of framework should include ## Example (bad) -``` -BEGIN +``` sql +begin sys.dbms_output.put_line('START'); -- some processing sys.dbms_output.put_line('END'); -END; +end; / ``` ## Example (good) -``` -DECLARE +``` sql +declare -- see https://github.com/OraOpenSource/Logger l_scope logger_logs.scope%type := 'DEMO'; -BEGIN +begin logger.log('START', l_scope); -- some processing logger.log('END', l_scope); -END; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/5-exception-handling/g-5020.md b/docs/4-language-usage/5-exception-handling/g-5020.md index a3504e26..95fb4939 100644 --- a/docs/4-language-usage/5-exception-handling/g-5020.md +++ b/docs/4-language-usage/5-exception-handling/g-5020.md @@ -10,32 +10,32 @@ but it is better to use named exceptions instead, because it ensures a certain l ## Example (bad) -``` -DECLARE - co_no_data_found CONSTANT INTEGER := -1; -BEGIN +``` sql +declare + co_no_data_found constant integer := -1; +begin my_package.some_processing(); -- some code which raises an exception -EXCEPTION - WHEN TOO_MANY_ROWS THEN +exception + when too_many_rows then my_package.some_further_processing(); - WHEN OTHERS THEN - IF SQLCODE = co_no_data_found THEN - NULL; - END IF; -END; + when others then + if sqlcode = co_no_data_found then + null; + end if; +end; / ``` ## Example (good) -``` -BEGIN +``` sql +begin my_package.some_processing(); -- some code which raises an exception -EXCEPTION - WHEN TOO_MANY_ROWS THEN +exception + when too_many_rows then my_package.some_further_processing(); - WHEN NO_DATA_FOUND THEN - NULL; -- handle no_data_found -END; + when no_data_found then + null; -- handle no_data_found +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/5-exception-handling/g-5030.md b/docs/4-language-usage/5-exception-handling/g-5030.md index 7a57ca76..1df38c53 100644 --- a/docs/4-language-usage/5-exception-handling/g-5030.md +++ b/docs/4-language-usage/5-exception-handling/g-5030.md @@ -5,31 +5,31 @@ ## Reason -This is error-prone because your local declaration overrides the global declaration. While it is technically possible to use the same names, it causes confusion for others needing to read and maintain this code. Additionally, you will need to be very careful to use the prefix `STANDARD` in front of any reference that needs to use Oracle’s default exception behavior. +This is error-prone because your local declaration overrides the global declaration. While it is technically possible to use the same names, it causes confusion for others needing to read and maintain this code. Additionally, you will need to be very careful to use the prefix `standard` in front of any reference that needs to use Oracle’s default exception behavior. ## Example (bad) -Using the code below, we are not able to handle the no_data_found exception raised by the `SELECT` statement as we have overwritten that exception handler. In addition, our exception handler doesn't have an exception number assigned, which should be raised when the SELECT statement does not find any rows. +Using the code below, we are not able to handle the `no_data_found` exception raised by the `select` statement as we have overwritten that exception handler. In addition, our exception handler doesn't have an exception number assigned, which should be raised when the `select` statement does not find any rows. -``` -DECLARE - l_dummy dual.dummy%TYPE; - no_data_found EXCEPTION; - co_rownum CONSTANT SIMPLE_INTEGER := 0; - co_no_data_found CONSTANT types_up.short_text_type := 'no_data_found'; -BEGIN - SELECT dummy - INTO l_dummy - FROM dual - WHERE ROWNUM = co_rownum; +``` sql +declare + l_dummy dual.dummy%type; + no_data_found exception; + co_rownum constant simple_integer := 0; + co_no_data_found constant types_up.short_text_type := 'no_data_found'; +begin + select dummy + into l_dummy + from dual + where rownum = co_rownum; - IF l_dummy IS NULL THEN - RAISE no_data_found; - END IF; -EXCEPTION - WHEN no_data_found THEN + if l_dummy is null then + raise no_data_found; + end if; +exception + when no_data_found then sys.dbms_output.put_line(co_no_data_found); -END; +end; / Error report - @@ -43,27 +43,27 @@ ORA-06512: at line 5 ## Example (good) -``` -DECLARE - l_dummy dual.dummy%TYPE; - empty_value EXCEPTION; - co_rownum CONSTANT simple_integer := 0; - co_empty_value CONSTANT types_up.short_text_type := 'empty_value'; - co_no_data_found CONSTANT types_up.short_text_type := 'no_data_found'; -BEGIN - SELECT dummy - INTO l_dummy - FROM dual - WHERE rownum = co_rownum; +``` sql +declare + l_dummy dual.dummy%type; + empty_value exception; + co_rownum constant simple_integer := 0; + co_empty_value constant types_up.short_text_type := 'empty_value'; + co_no_data_found constant types_up.short_text_type := 'no_data_found'; +begin + select dummy + into l_dummy + from dual + where rownum = co_rownum; - IF l_dummy IS NULL THEN - RAISE empty_value; - END IF; -EXCEPTION - WHEN empty_value THEN + if l_dummy is null then + raise empty_value; + end if; +exception + when empty_value then sys.dbms_output.put_line(co_empty_value); - WHEN no_data_found THEN + when no_data_found then sys.dbms_output.put_line(co_no_data_found); -END; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/5-exception-handling/g-5040.md b/docs/4-language-usage/5-exception-handling/g-5040.md index 0d9951b5..a421d0de 100644 --- a/docs/4-language-usage/5-exception-handling/g-5040.md +++ b/docs/4-language-usage/5-exception-handling/g-5040.md @@ -5,28 +5,43 @@ ## Reason -There is not necessarily anything wrong with using `WHEN OTHERS`, but it can cause you to "lose" error information unless your handler code is relatively sophisticated. Generally, you should use `WHEN OTHERS` to grab any and every error only after you have thought about your executable section and decided that you are not able to trap any specific exceptions. If you know, on the other hand, that a certain exception might be raised, include a handler for that error. By declaring two different exception handlers, the code more clearly states what we expect to have happen and how we want to handle the errors. That makes it easier to maintain and enhance. We also avoid hard-coding error numbers in checks against `SQLCODE`. +There is not necessarily anything wrong with using `when others`, but it can cause you to "lose" error information unless your handler code is relatively sophisticated. Generally, you should use `when others` to grab any and every error only after you have thought about your executable section and decided that you are not able to trap any specific exceptions. If you know, on the other hand, that a certain exception might be raised, include a handler for that error. By declaring two different exception handlers, the code more clearly states what we expect to have happen and how we want to handle the errors. That makes it easier to maintain and enhance. We also avoid hard-coding error numbers in checks against `sqlcode`. + +When using a logging framework like Logger, consider making an exception to this rule and allow a `when others` even without other specific handlers, but *only* if the `when others` exception handler calls a logging procedure that saves the error stack (that otherwise is lost) and the last statement of the handler is `raise`. ## Example (bad) -``` -BEGIN +``` sql +begin my_package.some_processing(); -EXCEPTION - WHEN OTHERS THEN +exception + when others then my_package.some_further_processing(); -END; +end; / ``` ## Example (good) -``` -BEGIN +``` sql +begin my_package.some_processing(); -EXCEPTION - WHEN DUP_VAL_ON_INDEX THEN +exception + when dup_val_on_index then my_package.some_further_processing(); -END; +end; +/ +``` + +## Example (exception to the rule) + +``` sql +begin + my_package.some_processing(); +exception + when others then + logger.log_error('Unhandled Exception'); + raise; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/5-exception-handling/g-5050.md b/docs/4-language-usage/5-exception-handling/g-5050.md index 6d9d7531..18aec6b5 100644 --- a/docs/4-language-usage/5-exception-handling/g-5050.md +++ b/docs/4-language-usage/5-exception-handling/g-5050.md @@ -5,22 +5,22 @@ ## Reason -If you are not very organized in the way you allocate, define and use the error numbers between 20999 and 20000 (those reserved by Oracle for its user community), it is very easy to end up with conflicting usages. You should assign these error numbers to named constants and consolidate all definitions within a single package. When you call `RAISE_APPLICATION_ERROR`, you should reference these named elements and error message text stored in a table. Use your own raise procedure in place of explicit calls to `RAISE_APPLICATION_ERROR`. If you are raising a "system" exception like `NO_DATA_FOUND`, you must use RAISE. However, when you want to raise an application-specific error, you use `RAISE_APPLICATION_ERROR`. If you use the latter, you then have to provide an error number and message. This leads to unnecessary and damaging hard-coded values. A more fail-safe approach is to provide a predefined raise procedure that automatically checks the error number and determines the correct way to raise the error. +If you are not very organized in the way you allocate, define and use the error numbers between 20999 and 20000 (those reserved by Oracle for its user community), it is very easy to end up with conflicting usages. You should assign these error numbers to named constants and consolidate all definitions within a single package. When you call `raise_application_error`, you should reference these named elements and error message text stored in a table. Use your own raise procedure in place of explicit calls to `raise_application_error`. If you are raising a "system" exception like `no_data_found`, you must use `raise`. However, when you want to raise an application-specific error, you use `raise_application_error`. If you use the latter, you then have to provide an error number and message. This leads to unnecessary and damaging hard-coded values. A more fail-safe approach is to provide a predefined raise procedure that automatically checks the error number and determines the correct way to raise the error. ## Example (bad) -``` -BEGIN +``` sql +begin raise_application_error(-20501,'Invalid employee_id'); -END; +end; / ``` ## Example (good) -``` -BEGIN +``` sql +begin err_up.raise(in_error => err.co_invalid_employee_id); -END; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/5-exception-handling/g-5060.md b/docs/4-language-usage/5-exception-handling/g-5060.md index 5bf220c9..1829a8d6 100644 --- a/docs/4-language-usage/5-exception-handling/g-5060.md +++ b/docs/4-language-usage/5-exception-handling/g-5060.md @@ -9,45 +9,45 @@ This may be your intention, but you should review the code to confirm this behav If you are raising an error in a program, then you are clearly predicting a situation in which that error will occur. You should consider including a handler in your code for predictable errors, allowing for a graceful and informative failure. After all, it is much more difficult for an enclosing block to be aware of the various errors you might raise and more importantly, what should be done in response to the error. -The form that this failure takes does not necessarily need to be an exception. When writing functions, you may well decide that in the case of certain exceptions, you will want to return a value such as NULL, rather than allow an exception to propagate out of the function. +The form that this failure takes does not necessarily need to be an exception. When writing functions, you may well decide that in the case of certain exceptions, you will want to return a value such as `null`, rather than allow an exception to propagate out of the function. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY department_api IS - FUNCTION name_by_id (in_id IN departments.department_id%TYPE) - RETURN departments.department_name%TYPE IS - l_department_name departments.department_name%TYPE; - BEGIN - SELECT department_name - INTO l_department_name - FROM departments - WHERE department_id = in_id; - - RETURN l_department_name; - END name_by_id; -END department_api; +``` sql +create or replace package body department_api is + function name_by_id (in_id in departments.department_id%type) + return departments.department_name%type is + l_department_name departments.department_name%type; + begin + select department_name + into l_department_name + from departments + where department_id = in_id; + + return l_department_name; + end name_by_id; +end department_api; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY department_api IS - FUNCTION name_by_id (in_id IN departments.department_id%TYPE) - RETURN departments.department_name%TYPE IS - l_department_name departments.department_name%TYPE; - BEGIN - SELECT department_name - INTO l_department_name - FROM departments - WHERE department_id = in_id; - - RETURN l_department_name; - EXCEPTION - WHEN NO_DATA_FOUND THEN RETURN NULL; - WHEN TOO_MANY_ROWS THEN RAISE; - END name_by_id; -END department_api; +``` sql +create or replace package body department_api is + function name_by_id (in_id in departments.department_id%type) + return departments.department_name%type is + l_department_name departments.department_name%type; + begin + select department_name + into l_department_name + from departments + where department_id = in_id; + + return l_department_name; + exception + when no_data_found then return null; + when too_many_rows then raise; + end name_by_id; +end department_api; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/5-exception-handling/g-5070.md b/docs/4-language-usage/5-exception-handling/g-5070.md index 7e05cefb..31d182e3 100644 --- a/docs/4-language-usage/5-exception-handling/g-5070.md +++ b/docs/4-language-usage/5-exception-handling/g-5070.md @@ -13,20 +13,20 @@ Being as specific as possible with the errors raised will allow developers to ch ## Example (bad) -``` -BEGIN - RAISE NO_DATA_FOUND; -END; +``` sql +begin + raise no_data_found; +end; / ``` ## Example (good) -``` -DECLARE - my_exception EXCEPTION; -BEGIN - RAISE my_exception; -END; +``` sql +declare + my_exception exception; +begin + raise my_exception; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/6-dynamic-sql/g-6010.md b/docs/4-language-usage/6-dynamic-sql/g-6010.md index dc83ecfd..8c3e6313 100644 --- a/docs/4-language-usage/6-dynamic-sql/g-6010.md +++ b/docs/4-language-usage/6-dynamic-sql/g-6010.md @@ -9,24 +9,24 @@ Having the executed statement in a variable makes it easier to debug your code ( ## Example (bad) -``` -DECLARE - l_next_val employees.employee_id%TYPE; -BEGIN - EXECUTE IMMEDIATE 'select employees_seq.nextval from dual' INTO l_next_val; -END; +``` sql +declare + l_next_val employees.employee_id%type; +begin + execute immediate 'select employees_seq.nextval from dual' into l_next_val; +end; / ``` ## Example (good) -``` -DECLARE - l_next_val employees.employee_id%TYPE; - co_sql CONSTANT types_up.big_string_type := +``` sql +declare + l_next_val employees.employee_id%type; + co_sql constant types_up.big_string_type := 'select employees_seq.nextval from dual'; -BEGIN - EXECUTE IMMEDIATE co_sql INTO l_next_val; -END; +begin + execute immediate co_sql into l_next_val; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/6-dynamic-sql/g-6020.md b/docs/4-language-usage/6-dynamic-sql/g-6020.md index 8713fbf6..75a89bd8 100644 --- a/docs/4-language-usage/6-dynamic-sql/g-6020.md +++ b/docs/4-language-usage/6-dynamic-sql/g-6020.md @@ -5,48 +5,48 @@ ## Reason -When a dynamic `INSERT`, `UPDATE`, or `DELETE` statement has a `RETURNING` clause, output bind arguments can go in the `RETURNING INTO` clause or in the USING clause. +When a dynamic `insert`, `update`, or `delete` statement has a `returning` clause, output bind arguments can go in the `returning into` clause or in the `using` clause. -You should use the `RETURNING INTO` clause for values returned from a DML operation. Reserve `OUT` and `IN OUT` bind variables for dynamic PL/SQL blocks that return values in PL/SQL variables. +You should use the `returning into` clause for values returned from a DML operation. Reserve `out` and `in out` bind variables for dynamic PL/SQL blocks that return values in PL/SQL variables. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY employee_api IS - PROCEDURE upd_salary (in_employee_id IN employees.employee_id%TYPE - ,in_increase_pct IN types_up.percentage - ,out_new_salary OUT employees.salary%TYPE) - IS - co_sql_stmt CONSTANT types_up.big_string_type := ' - UPDATE employees SET salary = salary + (salary / 100 * :1) - WHERE employee_id = :2 - RETURNING salary INTO :3'; - BEGIN - EXECUTE IMMEDIATE co_sql_stmt - USING in_increase_pct, in_employee_id, OUT out_new_salary; - END upd_salary; -END employee_api; +``` sql +create or replace package body employee_api is + procedure upd_salary (in_employee_id in employees.employee_id%type + ,in_increase_pct in types_up.percentage + ,out_new_salary out employees.salary%type) + is + co_sql_stmt constant types_up.big_string_type := ' + update employees set salary = salary + (salary / 100 * :1) + where employee_id = :2 + returning salary into :3'; + begin + execute immediate co_sql_stmt + using in_increase_pct, in_employee_id, out out_new_salary; + end upd_salary; +end employee_api; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY employee_api IS - PROCEDURE upd_salary (in_employee_id IN employees.employee_id%TYPE - ,in_increase_pct IN types_up.percentage - ,out_new_salary OUT employees.salary%TYPE) - IS - co_sql_stmt CONSTANT types_up.big_string_type := - 'UPDATE employees SET salary = salary + (salary / 100 * :1) - WHERE employee_id = :2 - RETURNING salary INTO :3'; - BEGIN - EXECUTE IMMEDIATE co_sql_stmt - USING in_increase_pct, in_employee_id - RETURNING INTO out_new_salary; - END upd_salary; -END employee_api; +``` sql +create or replace package body employee_api is + procedure upd_salary (in_employee_id in employees.employee_id%type + ,in_increase_pct in types_up.percentage + ,out_new_salary out employees.salary%type) + is + co_sql_stmt constant types_up.big_string_type := + 'update employees set salary = salary + (salary / 100 * :1) + where employee_id = :2 + returning salary into :3'; + begin + execute immediate co_sql_stmt + using in_increase_pct, in_employee_id + returning into out_new_salary; + end upd_salary; +end employee_api; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/1-general/g-7110.md b/docs/4-language-usage/7-stored-objects/1-general/g-7110.md index be0db7cc..58357a4f 100644 --- a/docs/4-language-usage/7-stored-objects/1-general/g-7110.md +++ b/docs/4-language-usage/7-stored-objects/1-general/g-7110.md @@ -7,28 +7,28 @@ Named notation makes sure that changes to the signature of the called program unit do not affect your call. -This is not needed for standard functions like (`TO_CHAR`, `TO_DATE`, `NVL`, `ROUND`, etc.) but should be followed for any other stored object having more than one parameter. +This is not needed for standard functions like (`to_char`, `to_date`, `nvl`, `round`, etc.) but should be followed for any other stored object having more than one parameter. ## Example (bad) -``` -DECLARE +``` sql +declare r_employee employees%rowtype; - co_id CONSTANT employees.employee_id%type := 107; -BEGIN + co_id constant employees.employee_id%type := 107; +begin employee_api.employee_by_id(r_employee, co_id); -END; +end; / ``` ## Example (good) -``` -DECLARE +``` sql +declare r_employee employees%rowtype; - co_id CONSTANT employees.employee_id%type := 107; -BEGIN + co_id constant employees.employee_id%type := 107; +begin employee_api.employee_by_id(out_row => r_employee, in_employee_id => co_id); -END; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/1-general/g-7120.md b/docs/4-language-usage/7-stored-objects/1-general/g-7120.md index 2dc22092..6b7c45a2 100644 --- a/docs/4-language-usage/7-stored-objects/1-general/g-7120.md +++ b/docs/4-language-usage/7-stored-objects/1-general/g-7120.md @@ -9,48 +9,48 @@ It's a good alternative for comments to indicate the end of program units, espec ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY employee_api IS - FUNCTION employee_by_id (in_employee_id IN employees.employee_id%TYPE) - RETURN employees%rowtype IS +``` sql +create or replace package body employee_api is + function employee_by_id (in_employee_id in employees.employee_id%type) + return employees%rowtype is r_employee employees%rowtype; - BEGIN - SELECT * - INTO r_employee - FROM employees - WHERE employee_id = in_employee_id; + begin + select * + into r_employee + from employees + where employee_id = in_employee_id; - RETURN r_employee; - EXCEPTION - WHEN NO_DATA_FOUND THEN - NULL; - WHEN TOO_MANY_ROWS THEN - RAISE; - END; -END; + return r_employee; + exception + when no_data_found then + null; + when too_many_rows then + raise; + end; +end; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY employee_api IS - FUNCTION employee_by_id (in_employee_id IN employees.employee_id%TYPE) - RETURN employees%rowtype IS +``` sql +create or replace package body employee_api is + function employee_by_id (in_employee_id in employees.employee_id%type) + return employees%rowtype is r_employee employees%rowtype; - BEGIN - SELECT * - INTO r_employee - FROM employees - WHERE employee_id = in_employee_id; + begin + select * + into r_employee + from employees + where employee_id = in_employee_id; - RETURN r_employee; - EXCEPTION - WHEN NO_DATA_FOUND THEN - NULL; - WHEN TOO_MANY_ROWS THEN - RAISE; - END employee_by_id; -END employee_api; + return r_employee; + exception + when no_data_found then + null; + when too_many_rows then + raise; + end employee_by_id; +end employee_api; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/1-general/g-7130.md b/docs/4-language-usage/7-stored-objects/1-general/g-7130.md index c9bd6efa..e8f5ed94 100644 --- a/docs/4-language-usage/7-stored-objects/1-general/g-7130.md +++ b/docs/4-language-usage/7-stored-objects/1-general/g-7130.md @@ -11,72 +11,72 @@ This external dependency is hidden, and may cause problems in the future. You sh ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY EMPLOYEE_API IS - PROCEDURE calc_salary (in_employee_id IN employees.employee_id%TYPE) IS +``` sql +create or replace package body employee_api is + procedure calc_salary (in_employee_id in employees.employee_id%type) is r_emp employees%rowtype; - FUNCTION commission RETURN NUMBER IS - l_commission employees.salary%TYPE := 0; - BEGIN - IF r_emp.commission_pct IS NOT NULL - THEN + function commission return number is + l_commission employees.salary%type := 0; + begin + if r_emp.commission_pct is not null + then l_commission := r_emp.salary * r_emp.commission_pct; - END IF; + end if; - RETURN l_commission; - END commission; - BEGIN - SELECT * - INTO r_emp - FROM employees - WHERE employee_id = in_employee_id; + return l_commission; + end commission; + begin + select * + into r_emp + from employees + where employee_id = in_employee_id; - SYS.DBMS_OUTPUT.PUT_LINE(r_emp.salary + commission()); - EXCEPTION - WHEN NO_DATA_FOUND THEN - NULL; - WHEN TOO_MANY_ROWS THEN - NULL; - END calc_salary; -END employee_api; + sys.dbms_output.put_line(r_emp.salary + commission()); + exception + when no_data_found then + null; + when too_many_rows then + null; + end calc_salary; +end employee_api; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY EMPLOYEE_API IS - PROCEDURE calc_salary (in_employee_id IN employees.employee_id%TYPE) IS +``` sql +create or replace package body employee_api is + procedure calc_salary (in_employee_id in employees.employee_id%type) is r_emp employees%rowtype; - FUNCTION commission (in_salary IN employees.salary%TYPE - ,in_comm_pct IN employees.commission_pct%TYPE) - RETURN NUMBER IS - l_commission employees.salary%TYPE := 0; - BEGIN - IF in_comm_pct IS NOT NULL THEN + function commission (in_salary in employees.salary%type + ,in_comm_pct in employees.commission_pct%type) + return number is + l_commission employees.salary%type := 0; + begin + if in_comm_pct is not null then l_commission := in_salary * in_comm_pct; - END IF; + end if; - RETURN l_commission; - END commission; - BEGIN - SELECT * - INTO r_emp - FROM employees - WHERE employee_id = in_employee_id; + return l_commission; + end commission; + begin + select * + into r_emp + from employees + where employee_id = in_employee_id; - SYS.DBMS_OUTPUT.PUT_LINE( + sys.dbms_output.put_line( r_emp.salary + commission(in_salary => r_emp.salary ,in_comm_pct => r_emp.commission_pct) ); - EXCEPTION - WHEN NO_DATA_FOUND THEN - NULL; - WHEN TOO_MANY_ROWS THEN - NULL; - END calc_salary; -END employee_api; + exception + when no_data_found then + null; + when too_many_rows then + null; + end calc_salary; +end employee_api; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/1-general/g-7140.md b/docs/4-language-usage/7-stored-objects/1-general/g-7140.md index a8548c83..0e8e0c1f 100644 --- a/docs/4-language-usage/7-stored-objects/1-general/g-7140.md +++ b/docs/4-language-usage/7-stored-objects/1-general/g-7140.md @@ -13,34 +13,34 @@ There is never a better time to review all the steps you took, and to understand ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE my_procedure IS - FUNCTION my_func RETURN NUMBER IS - co_true CONSTANT INTEGER := 1; - BEGIN - RETURN co_true; - END my_func; - BEGIN - NULL; - END my_procedure; -END my_package; +``` sql +create or replace package body my_package is + procedure my_procedure is + function my_func return number is + co_true constant integer := 1; + begin + return co_true; + end my_func; + begin + null; + end my_procedure; +end my_package; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE my_procedure IS - FUNCTION my_func RETURN NUMBER IS - co_true CONSTANT INTEGER := 1; - BEGIN - RETURN co_true; - END my_func; - BEGIN +``` sql +create or replace package body my_package is + procedure my_procedure is + function my_func return number is + co_true constant integer := 1; + begin + return co_true; + end my_func; + begin sys.dbms_output.put_line(my_func()); - END my_procedure; -END my_package; + end my_procedure; +end my_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/1-general/g-7150.md b/docs/4-language-usage/7-stored-objects/1-general/g-7150.md index 4c2425fe..627c62c6 100644 --- a/docs/4-language-usage/7-stored-objects/1-general/g-7150.md +++ b/docs/4-language-usage/7-stored-objects/1-general/g-7150.md @@ -5,55 +5,55 @@ ## Reason -You should go through your programs and remove any partameter that is no longer used. +You should go through your programs and remove any parameter that is no longer used. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY department_api IS - FUNCTION name_by_id (in_department_id IN departments.department_id%TYPE - ,in_manager_id IN departments.manager_id%TYPE) - RETURN departments.department_name%TYPE IS - l_department_name departments.department_name%TYPE; - BEGIN +``` sql +create or replace package body department_api is + function name_by_id (in_department_id in departments.department_id%type + ,in_manager_id in departments.manager_id%type) + return departments.department_name%type is + l_department_name departments.department_name%type; + begin <> - BEGIN - SELECT department_name - INTO l_department_name - FROM departments - WHERE department_id = in_department_id; - EXCEPTION - WHEN NO_DATA_FOUND OR TOO_MANY_ROWS THEN - l_department_name := NULL; - END find_department; + begin + select department_name + into l_department_name + from departments + where department_id = in_department_id; + exception + when no_data_found or too_many_rows then + l_department_name := null; + end find_department; - RETURN l_department_name; - END name_by_id; -END department_api; + return l_department_name; + end name_by_id; +end department_api; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY department_api IS - FUNCTION name_by_id (in_department_id IN departments.department_id%TYPE) - RETURN departments.department_name%TYPE IS - l_department_name departments.department_name%TYPE; - BEGIN +``` sql +create or replace package body department_api is + function name_by_id (in_department_id in departments.department_id%type) + return departments.department_name%type is + l_department_name departments.department_name%type; + begin <> - BEGIN - SELECT department_name - INTO l_department_name - FROM departments - WHERE department_id = in_department_id; - EXCEPTION - WHEN NO_DATA_FOUND OR TOO_MANY_ROWS THEN - l_department_name := NULL; - END find_department; + begin + select department_name + into l_department_name + from departments + where department_id = in_department_id; + exception + when no_data_found or too_many_rows then + l_department_name := null; + end find_department; - RETURN l_department_name; - END name_by_id; -END department_api; + return l_department_name; + end name_by_id; +end department_api; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/1-general/g-7160.md b/docs/4-language-usage/7-stored-objects/1-general/g-7160.md new file mode 100644 index 00000000..4331e0b2 --- /dev/null +++ b/docs/4-language-usage/7-stored-objects/1-general/g-7160.md @@ -0,0 +1,36 @@ +# G-7160: Always explicitly state parameter mode. + +!!! warning "Major" + Maintainability + +## Reason + +By showing the mode of parameters, you help the reader. If you do not specify a parameter mode, the default mode is `in`. Explicitly showing the mode indication of all parameters is a more assertive action than simply taking the default mode. Anyone reviewing the code later will be more confident that you intended the parameter mode to be `in`, `out` or `in out`. + +## Example (bad) + +``` sql +create or replace package employee_api is + procedure upsert (io_id in out employees.id%type + ,in_first_name employees.first_name%type + ,in_last_name employees.last_name%type + ,in_email employees.email%type + ,in_department_id employees.department_id%type + ,out_success out pls_integer); +end employee_up; +/ +``` + +## Example (good) + +``` sql +create or replace package employee_api is + procedure upsert (io_id in out employees.id%type + ,in_first_name in employees.first_name%type + ,in_last_name in employees.last_name%type + ,in_email in employees.email%type + ,in_department_id in employees.department_id%type + ,out_success out pls_integer); +end employee_up; +/ +``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/1-general/g-7170.md b/docs/4-language-usage/7-stored-objects/1-general/g-7170.md new file mode 100644 index 00000000..0453d151 --- /dev/null +++ b/docs/4-language-usage/7-stored-objects/1-general/g-7170.md @@ -0,0 +1,89 @@ +# G-7170: Avoid using an IN OUT parameter as IN or OUT only. + +!!! warning "Major" + Efficiency, Maintainability + +!!! missing "Unsupported in PL/SQL Cop Validators" + Rule G-7170 is not expected to be implemented in the static code analysis validators. + +## Reason + +Avoid using parameter mode `in out` unless you actually use the parameter both as input and output. If the code body only reads from the parameter, use `in`; if the code body only assigns to the parameter, use `out`. If at the beginning of a project you expect a parameter to be both input and output and therefore choose `in out` just in case, but later development shows the parameter actually is only `in` or `out`, you should change the parameter mode accordingly. + +## Example (bad) + +``` sql +create or replace package body employee_up is + procedure rcv_emp (io_first_name in out employees.first_name%type + ,io_last_name in out employees.last_name%type + ,io_email in out employees.email%type + ,io_phone_number in out employees.phone_number%type + ,io_hire_date in out employees.hire_date%type + ,io_job_id in out employees.job_id%type + ,io_salary in out employees.salary%type + ,io_commission_pct in out employees.commission_pct%type + ,io_manager_id in out employees.manager_id%type + ,io_department_id in out employees.department_id%type + ,in_wait in integer) is + l_status pls_integer; + co_dflt_pipe_name constant string(30 char) := 'MyPipe'; + co_ok constant pls_integer := 1; + begin + -- Receive next message and unpack for each column. + l_status := sys.dbms_pipe.receive_message(pipename => co_dflt_pipe_name + ,timeout => in_wait); + if l_status = co_ok then + sys.dbms_pipe.unpack_message (io_first_name); + sys.dbms_pipe.unpack_message (io_last_name); + sys.dbms_pipe.unpack_message (io_email); + sys.dbms_pipe.unpack_message (io_phone_number); + sys.dbms_pipe.unpack_message (io_hire_date); + sys.dbms_pipe.unpack_message (io_job_id); + sys.dbms_pipe.unpack_message (io_salary); + sys.dbms_pipe.unpack_message (io_commission_pct); + sys.dbms_pipe.unpack_message (io_manager_id); + sys.dbms_pipe.unpack_message (io_department_id); + end if; + end rcv_emp; +end employee_up; +/ +``` + +## Example (good) + +``` sql +create or replace package body employee_up is + procedure rcv_emp (out_first_name out employees.first_name%type + ,out_last_name out employees.last_name%type + ,out_email out employees.email%type + ,out_phone_number out employees.phone_number%type + ,out_hire_date out employees.hire_date%type + ,out_job_id out employees.job_id%type + ,out_salary out employees.salary%type + ,out_commission_pct out employees.commission_pct%type + ,out_manager_id out employees.manager_id%type + ,out_department_id out employees.department_id%type + ,in_wait in integer) is + l_status pls_integer; + co_dflt_pipe_name constant string(30 char) := 'MyPipe'; + co_ok constant pls_integer := 1; + begin + -- Receive next message and unpack for each column. + l_status := sys.dbms_pipe.receive_message(pipename => co_dflt_pipe_name + ,timeout => in_wait); + if l_status = co_ok then + sys.dbms_pipe.unpack_message (out_first_name); + sys.dbms_pipe.unpack_message (out_last_name); + sys.dbms_pipe.unpack_message (out_email); + sys.dbms_pipe.unpack_message (out_phone_number); + sys.dbms_pipe.unpack_message (out_hire_date); + sys.dbms_pipe.unpack_message (out_job_id); + sys.dbms_pipe.unpack_message (out_salary); + sys.dbms_pipe.unpack_message (out_commission_pct); + sys.dbms_pipe.unpack_message (out_manager_id); + sys.dbms_pipe.unpack_message (out_department_id); + end if; + end rcv_emp; +end employee_up; +/ +``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/2-packages/g-7220.md b/docs/4-language-usage/7-stored-objects/2-packages/g-7220.md index 167d54f8..1a4b2225 100644 --- a/docs/4-language-usage/7-stored-objects/2-packages/g-7220.md +++ b/docs/4-language-usage/7-stored-objects/2-packages/g-7220.md @@ -9,77 +9,77 @@ Having forward declarations allows you to order the functions and procedures of ## Example (bad) -``` -CREATE OR REPLACE PACKAGE department_api IS - PROCEDURE del (in_department_id IN departments.department_id%TYPE); -END department_api; +``` sql +create or replace package department_api is + procedure del (in_department_id in departments.department_id%type); +end department_api; / -CREATE OR REPLACE PACKAGE BODY department_api IS - FUNCTION does_exist (in_department_id IN departments.department_id%TYPE) - RETURN BOOLEAN IS - l_return PLS_INTEGER; - BEGIN +create or replace package body department_api is + function does_exist (in_department_id in departments.department_id%type) + return boolean is + l_return pls_integer; + begin <> - BEGIN - SELECT 1 - INTO l_return - FROM departments - WHERE department_id = in_department_id; - EXCEPTION - WHEN no_data_found OR too_many_rows THEN + begin + select 1 + into l_return + from departments + where department_id = in_department_id; + exception + when no_data_found or too_many_rows then l_return := 0; - END check_row_exists; + end check_row_exists; - RETURN l_return = 1; - END does_exist; + return l_return = 1; + end does_exist; - PROCEDURE del (in_department_id IN departments.department_id%TYPE) IS - BEGIN - IF does_exist(in_department_id) THEN - NULL; - END IF; - END del; -END department_api; + procedure del (in_department_id in departments.department_id%type) is + begin + if does_exist(in_department_id) then + null; + end if; + end del; +end department_api; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE department_api IS - PROCEDURE del (in_department_id IN departments.department_id%TYPE); -END department_api; +``` sql +create or replace package department_api is + procedure del (in_department_id in departments.department_id%type); +end department_api; / -CREATE OR REPLACE PACKAGE BODY department_api IS - FUNCTION does_exist (in_department_id IN departments.department_id%TYPE) - RETURN BOOLEAN; +create or replace package body department_api is + function does_exist (in_department_id in departments.department_id%type) + return boolean; - PROCEDURE del (in_department_id IN departments.department_id%TYPE) IS - BEGIN - IF does_exist(in_department_id) THEN - NULL; - END IF; - END del; + procedure del (in_department_id in departments.department_id%type) is + begin + if does_exist(in_department_id) then + null; + end if; + end del; - FUNCTION does_exist (in_department_id IN departments.department_id%TYPE) - RETURN BOOLEAN IS - l_return PLS_INTEGER; - BEGIN + function does_exist (in_department_id in departments.department_id%type) + return boolean is + l_return pls_integer; + begin <> - BEGIN - SELECT 1 - INTO l_return - FROM departments - WHERE department_id = in_department_id; - EXCEPTION - WHEN no_data_found OR too_many_rows THEN + begin + select 1 + into l_return + from departments + where department_id = in_department_id; + exception + when no_data_found or too_many_rows then l_return := 0; - END check_row_exists; + end check_row_exists; - RETURN l_return = 1; - END does_exist; -END department_api; + return l_return = 1; + end does_exist; +end department_api; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/2-packages/g-7230.md b/docs/4-language-usage/7-stored-objects/2-packages/g-7230.md index 069558a2..9988a859 100644 --- a/docs/4-language-usage/7-stored-objects/2-packages/g-7230.md +++ b/docs/4-language-usage/7-stored-objects/2-packages/g-7230.md @@ -5,73 +5,81 @@ ## Reason -You should always declare package-level data inside the package body. You can then define "get and set" methods (functions and procedures, respectively) in the package specification to provide controlled access to that data. By doing so you can guarantee data integrity, you can change your data structure implementation, and also track access to those data structures. +You should always declare package-level data (non-constants) inside the package body. You can then define "get and set" methods (functions and procedures, respectively) in the package specification to provide controlled access to that data. By doing so you can guarantee data integrity, you can change your data structure implementation, and also track access to those data structures. -Data structures (scalar variables, collections, cursors) declared in the package specification (not within any specific program) can be referenced directly by any program running in a session with EXECUTE rights to the package. +Data structures (scalar variables, collections, cursors) declared in the package specification (not within any specific program) 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 "get and set" methods - a function to get the value and a procedure to set the value - in the package specification. Developers then can access the data using these methods - and will automatically follow all rules you set upon data modification. +For package-level constants, consider whether the constant should be public and usable from other code, or if only relevant for code within the package. If the latter, declare the constant in the package body. If the former, it is typically good practice to place the constants in a package specification that only holds constants. + ## Example (bad) -``` -CREATE OR REPLACE PACKAGE employee_api AS - co_min_increase CONSTANT types_up.sal_increase_type := 0.01; - co_max_increase CONSTANT types_up.sal_increase_type := 0.5; +``` sql +create or replace package employee_api as + co_min_increase constant types_up.sal_increase_type := 0.01; + co_max_increase constant types_up.sal_increase_type := 0.5; g_salary_increase types_up.sal_increase_type := co_min_increase; - PROCEDURE set_salary_increase (in_increase IN types_up.sal_increase_type); - FUNCTION salary_increase RETURN types_up.sal_increase_type; -END employee_api; + procedure set_salary_increase (in_increase in types_up.sal_increase_type); + function salary_increase return types_up.sal_increase_type; +end employee_api; / -CREATE OR REPLACE PACKAGE BODY employee_api AS - PROCEDURE set_salary_increase (in_increase IN types_up.sal_increase_type) IS - BEGIN - g_salary_increase := GREATEST(LEAST(in_increase,co_max_increase) +create or replace package body employee_api as + procedure set_salary_increase (in_increase in types_up.sal_increase_type) is + begin + g_salary_increase := greatest(least(in_increase,co_max_increase) ,co_min_increase); - END set_salary_increase; + end set_salary_increase; - FUNCTION salary_increase RETURN types_up.sal_increase_type IS - BEGIN - RETURN g_salary_increase; - END salary_increase; -END employee_api; + function salary_increase return types_up.sal_increase_type is + begin + return g_salary_increase; + end salary_increase; +end employee_api; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE employee_api AS - PROCEDURE set_salary_increase (in_increase IN types_up.sal_increase_type); - FUNCTION salary_increase RETURN types_up.sal_increase_type; -END employee_api; +``` sql +create or replace package constants_up as + co_min_increase constant types_up.sal_increase_type := 0.01; + co_max_increase constant types_up.sal_increase_type := 0.5; +end constants_up; +/ + +create or replace package employee_api as + procedure set_salary_increase (in_increase in types_up.sal_increase_type); + function salary_increase return types_up.sal_increase_type; +end employee_api; / -CREATE OR REPLACE PACKAGE BODY employee_api AS +create or replace package body employee_api as g_salary_increase types_up.sal_increase_type(4,2); - PROCEDURE init; + procedure init; - PROCEDURE set_salary_increase (in_increase IN types_up.sal_increase_type) IS - BEGIN - g_salary_increase := GREATEST(LEAST(in_increase - ,constants_up.max_salary_increase()) - ,constants_up.min_salary_increase()); - END set_salary_increase; + procedure set_salary_increase (in_increase in types_up.sal_increase_type) is + begin + g_salary_increase := greatest(least(in_increase + ,constants_up.co_max_increase) + ,constants_up.co_min_increase); + end set_salary_increase; - FUNCTION salary_increase RETURN types_up.sal_increase_type IS - BEGIN - RETURN g_salary_increase; - END salary_increase; + function salary_increase return types_up.sal_increase_type is + begin + return g_salary_increase; + end salary_increase; - PROCEDURE init - IS - BEGIN - g_salary_increase := constants_up.min_salary_increase(); - END init; -BEGIN + procedure init + is + begin + g_salary_increase := constants_up.co_min_increase; + end init; +begin init(); -END employee_api; +end employee_api; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/2-packages/g-7240.md b/docs/4-language-usage/7-stored-objects/2-packages/g-7240.md index f6f5112f..64bcde6c 100644 --- a/docs/4-language-usage/7-stored-objects/2-packages/g-7240.md +++ b/docs/4-language-usage/7-stored-objects/2-packages/g-7240.md @@ -1,87 +1,60 @@ -# G-7240: Avoid using an IN OUT parameter as IN or OUT only. +# G-7240: Never use RETURN in package initialization block. -!!! warning "Major" - Efficiency, Maintainability +!!! tip "Minor" + Maintainability ## Reason -By showing the mode of parameters, you help the reader. If you do not specify a parameter mode, the default mode is `IN`. Explicitly showing the mode indication of all parameters is a more assertive action than simply taking the default mode. Anyone reviewing the code later will be more confident that you intended the parameter mode to be `IN` / `OUT`. +The purpose of the initialization block of a package body is to set initial values of the global variables of the package (initialize the package state). Although `return` is syntactically allowed in this block, it makes no sense. If it is the last keyword of the block, it is superfluous. If it is not the last keyword, then all code after the `return` is unreachable and thus dead code. ## Example (bad) -``` --- Bad -CREATE OR REPLACE PACKAGE BODY employee_up IS - PROCEDURE rcv_emp (io_first_name IN OUT employees.first_name%TYPE - ,io_last_name IN OUT employees.last_name%TYPE - ,io_email IN OUT employees.email%TYPE - ,io_phone_number IN OUT employees.phone_number%TYPE - ,io_hire_date IN OUT employees.hire_date%TYPE - ,io_job_id IN OUT employees.job_id%TYPE - ,io_salary IN OUT employees.salary%TYPE - ,io_commission_pct IN OUT employees.commission_pct%TYPE - ,io_manager_id IN OUT employees.manager_id%TYPE - ,io_department_id IN OUT employees.department_id%TYPE - ,in_wait INTEGER) IS - l_status PLS_INTEGER; - co_dflt_pipe_name CONSTANT STRING(30 CHAR) := 'MyPipe'; - co_ok CONSTANT PLS_INTEGER := 1; - BEGIN - -- Receive next message and unpack for each column. - l_status := SYS.dbms_pipe.receive_message(pipename => co_dflt_pipe_name - ,timeout => in_wait); - IF l_status = co_ok THEN - SYS.dbms_pipe.unpack_message (io_first_name); - SYS.dbms_pipe.unpack_message (io_last_name); - SYS.dbms_pipe.unpack_message (io_email); - SYS.dbms_pipe.unpack_message (io_phone_number); - SYS.dbms_pipe.unpack_message (io_hire_date); - SYS.dbms_pipe.unpack_message (io_job_id); - SYS.dbms_pipe.unpack_message (io_salary); - SYS.dbms_pipe.unpack_message (io_commission_pct); - SYS.dbms_pipe.unpack_message (io_manager_id); - SYS.dbms_pipe.unpack_message (io_department_id); - END IF; - END rcv_emp; -END employee_up; +``` sql +create or replace package body employee_api as + g_salary_increase types_up.sal_increase_type(4,2); + + procedure set_salary_increase (in_increase in types_up.sal_increase_type) is + begin + g_salary_increase := greatest(least(in_increase + ,constants_up.max_salary_increase()) + ,constants_up.min_salary_increase()); + end set_salary_increase; + + function salary_increase return types_up.sal_increase_type is + begin + return g_salary_increase; + end salary_increase; + +begin + g_salary_increase := constants_up.min_salary_increase(); + + return; + + set_salary_increase(constants_up.min_salary_increase()); -- dead code +end employee_api; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY employee_up IS - PROCEDURE rcv_emp (OUT_first_name OUT employees.first_name%TYPE - ,OUT_last_name OUT employees.last_name%TYPE - ,OUT_email OUT employees.email%TYPE - ,OUT_phone_number OUT employees.phone_number%TYPE - ,OUT_hire_date OUT employees.hire_date%TYPE - ,OUT_job_id OUT employees.job_id%TYPE - ,OUT_salary OUT employees.salary%TYPE - ,OUT_commission_pct OUT employees.commission_pct%TYPE - ,OUT_manager_id OUT employees.manager_id%TYPE - ,OUT_department_id OUT employees.department_id%TYPE - ,in_wait IN INTEGER) IS - l_status PLS_INTEGER; - co_dflt_pipe_name CONSTANT STRING(30 CHAR) := 'MyPipe'; - co_ok CONSTANT PLS_INTEGER := 1; - BEGIN - -- Receive next message and unpack for each column. - l_status := SYS.dbms_pipe.receive_message(pipename => co_dflt_pipe_name - ,timeout => in_wait); - IF l_status = co_ok THEN - SYS.dbms_pipe.unpack_message (out_first_name); - SYS.dbms_pipe.unpack_message (out_last_name); - SYS.dbms_pipe.unpack_message (out_email); - SYS.dbms_pipe.unpack_message (out_phone_number); - SYS.dbms_pipe.unpack_message (out_hire_date); - SYS.dbms_pipe.unpack_message (out_job_id); - SYS.dbms_pipe.unpack_message (out_salary); - SYS.dbms_pipe.unpack_message (out_commission_pct); - SYS.dbms_pipe.unpack_message (out_manager_id); - SYS.dbms_pipe.unpack_message (out_department_id); - END IF; - END rcv_emp; -END employee_up; +``` sql +create or replace package body employee_api as + g_salary_increase types_up.sal_increase_type(4,2); + + procedure set_salary_increase (in_increase in types_up.sal_increase_type) is + begin + g_salary_increase := greatest(least(in_increase + ,constants_up.max_salary_increase()) + ,constants_up.min_salary_increase()); + end set_salary_increase; + + function salary_increase return types_up.sal_increase_type is + begin + return g_salary_increase; + end salary_increase; + +begin + g_salary_increase := constants_up.min_salary_increase(); +end employee_api; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/3-procedures/g-7310.md b/docs/4-language-usage/7-stored-objects/3-procedures/g-7310.md index e7d51e86..3d08aad3 100644 --- a/docs/4-language-usage/7-stored-objects/3-procedures/g-7310.md +++ b/docs/4-language-usage/7-stored-objects/3-procedures/g-7310.md @@ -11,27 +11,27 @@ Package bodies may be changed and compiled without invalidating other packages. ## Example (bad) -``` -CREATE OR REPLACE PROCEDURE my_procedure IS -BEGIN - NULL; -END my_procedure; +``` sql +create or replace procedure my_procedure is +begin + null; +end my_procedure; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE my_package IS - PROCEDURE my_procedure; -END my_package; +``` sql +create or replace package my_package is + procedure my_procedure; +end my_package; / -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE my_procedure IS - BEGIN - NULL; - END my_procedure; -END my_package; +create or replace package body my_package is + procedure my_procedure is + begin + null; + end my_procedure; +end my_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/3-procedures/g-7320.md b/docs/4-language-usage/7-stored-objects/3-procedures/g-7320.md index 8ea413d4..ecbbe415 100644 --- a/docs/4-language-usage/7-stored-objects/3-procedures/g-7320.md +++ b/docs/4-language-usage/7-stored-objects/3-procedures/g-7320.md @@ -5,46 +5,46 @@ ## Reason -Use of the `RETURN` statement is legal within a procedure in PL/SQL, but it is very similar to a `GOTO`, which means you end up with poorly structured code that is hard to debug and maintain. +Use of the `return` statement is legal within a procedure in PL/SQL, but it is very similar to a `goto`, which means you end up with poorly structured code that is hard to debug and maintain. A good general rule to follow as you write your PL/SQL programs is "one way in and one way out". In other words, there should be just one way to enter or call a program, and there should be one way out, one exit path from a program (or loop) on successful termination. By following this rule, you end up with code that is much easier to trace, debug, and maintain. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE my_procedure IS - l_idx SIMPLE_INTEGER := 1; - co_modulo CONSTANT SIMPLE_INTEGER := 7; - BEGIN +``` sql +create or replace package body my_package is + procedure my_procedure is + l_idx simple_integer := 1; + co_modulo constant simple_integer := 7; + begin <> - LOOP - IF MOD(l_idx,co_modulo) = 0 THEN - RETURN; - END IF; + loop + if mod(l_idx,co_modulo) = 0 then + return; + end if; l_idx := l_idx + 1; - END LOOP mod7_loop; - END my_procedure; -END my_package; + end loop mod7_loop; + end my_procedure; +end my_package; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - PROCEDURE my_procedure IS - l_idx SIMPLE_INTEGER := 1; - co_modulo CONSTANT SIMPLE_INTEGER := 7; - BEGIN +``` sql +create or replace package body my_package is + procedure my_procedure is + l_idx simple_integer := 1; + co_modulo constant simple_integer := 7; + begin <> - LOOP - EXIT mod7_loop WHEN MOD(l_idx,co_modulo) = 0; + loop + exit mod7_loop when mod(l_idx,co_modulo) = 0; l_idx := l_idx + 1; - END LOOP mod7_loop; - END my_procedure; -END my_package; + end loop mod7_loop; + end my_procedure; +end my_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/3-procedures/g-7330.md b/docs/4-language-usage/7-stored-objects/3-procedures/g-7330.md new file mode 100644 index 00000000..e3fb6b32 --- /dev/null +++ b/docs/4-language-usage/7-stored-objects/3-procedures/g-7330.md @@ -0,0 +1,39 @@ +# G-7330: Always assign values to OUT parameters. + +!!! tip "Major" + Maintainability, Testability + +## Reason + +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. + +## Example (bad) + +``` sql +create or replace package body my_package is + procedure greet( + in_name in varchar2 + , out_greeting out varchar2 + ) is + l_message varchar2(100 char); + begin + l_message := 'Hello, ' || in_name; + end my_procedure; +end my_package; +/ +``` + +## Example (good) + +``` sql +create or replace package body my_package is + procedure greet( + in_name in varchar2 + , out_greeting out varchar2 + ) is + begin + out_greeting := 'Hello, ' || in_name; + end my_procedure; +end my_package; +/ +``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/4-functions/g-7410.md b/docs/4-language-usage/7-stored-objects/4-functions/g-7410.md index 2e819370..f2d74862 100644 --- a/docs/4-language-usage/7-stored-objects/4-functions/g-7410.md +++ b/docs/4-language-usage/7-stored-objects/4-functions/g-7410.md @@ -11,22 +11,22 @@ Package bodies may be changed and compiled without invalidating other packages. ## Example (bad) -``` -CREATE OR REPLACE FUNCTION my_function RETURN VARCHAR2 IS -BEGIN - RETURN NULL; -END my_function; +``` sql +create or replace function my_function return varchar2 is +begin + return null; +end my_function; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - FUNCTION my_function RETURN VARCHAR2 IS - BEGIN - RETURN NULL; - END my_function; -END my_package; +``` sql +create or replace package body my_package is + function my_function return varchar2 is + begin + return null; + end my_function; +end my_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/4-functions/g-7420.md b/docs/4-language-usage/7-stored-objects/4-functions/g-7420.md index 59ec1531..7dadcfa8 100644 --- a/docs/4-language-usage/7-stored-objects/4-functions/g-7420.md +++ b/docs/4-language-usage/7-stored-objects/4-functions/g-7420.md @@ -5,46 +5,46 @@ ## Reason -The reader expects the RETURN statement to be the last statement of a function. +The reader expects the `return` statement to be the last statement of a function. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - FUNCTION my_function (in_from IN PLS_INTEGER - , in_to IN PLS_INTEGER) RETURN PLS_INTEGER IS - l_ret PLS_INTEGER; - BEGIN +``` sql +create or replace package body my_package is + function my_function (in_from in pls_integer + , in_to in pls_integer) return pls_integer is + l_ret pls_integer; + begin l_ret := in_from; <> - FOR i IN in_from .. in_to - LOOP + for i in in_from .. in_to + loop l_ret := l_ret + i; - IF i = in_to THEN - RETURN l_ret; - END IF; - END LOOP for_loop; - END my_function; -END my_package; + if i = in_to then + return l_ret; + end if; + end loop for_loop; + end my_function; +end my_package; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - FUNCTION my_function (in_from IN PLS_INTEGER - , in_to IN PLS_INTEGER) RETURN PLS_INTEGER IS - l_ret PLS_INTEGER; - BEGIN +``` sql +create or replace package body my_package is + function my_function (in_from in pls_integer + , in_to in pls_integer) return pls_integer is + l_ret pls_integer; + begin l_ret := in_from; <> - FOR i IN in_from .. in_to - LOOP + for i in in_from .. in_to + loop l_ret := l_ret + i; - END LOOP for_loop; - RETURN l_ret; - END my_function; -END my_package; + end loop for_loop; + return l_ret; + end my_function; +end my_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/4-functions/g-7430.md b/docs/4-language-usage/7-stored-objects/4-functions/g-7430.md index 35bf12cc..96336712 100644 --- a/docs/4-language-usage/7-stored-objects/4-functions/g-7430.md +++ b/docs/4-language-usage/7-stored-objects/4-functions/g-7430.md @@ -10,48 +10,50 @@ A function should have a single point of entry as well as a single exit-point. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - FUNCTION my_function (in_value IN PLS_INTEGER) RETURN BOOLEAN IS - co_yes CONSTANT PLS_INTEGER := 1; - BEGIN - IF in_value = co_yes THEN - RETURN TRUE; - ELSE - RETURN FALSE; - END IF; - END my_function; -END my_package; +``` sql +create or replace package body my_package is + function my_function (in_value in pls_integer) return boolean is + co_yes constant pls_integer := 1; + begin + if in_value = co_yes then + return true; + else + return false; + end if; + end my_function; +end my_package; / ``` ## Example (better) -CREATE OR REPLACE PACKAGE BODY my_package IS - FUNCTION my_function (in_value IN PLS_INTEGER) RETURN BOOLEAN IS - co_yes CONSTANT PLS_INTEGER := 1; - l_ret BOOLEAN; - BEGIN - IF in_value = co_yes THEN - l_ret := TRUE; - ELSE - l_ret := FALSE; - END IF; +``` sql +create or replace package body my_package is + function my_function (in_value in pls_integer) return boolean is + co_yes constant pls_integer := 1; + l_ret boolean; + begin + if in_value = co_yes then + l_ret := true; + else + l_ret := false; + end if; - RETURN l_ret; - END my_function; -END my_package; + return l_ret; + end my_function; +end my_package; / +``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - FUNCTION my_function (in_value IN PLS_INTEGER) RETURN BOOLEAN IS - co_yes CONSTANT PLS_INTEGER := 1; - BEGIN - RETURN in_value = co_yes; - END my_function; -END my_package; +``` sql +create or replace package body my_package is + function my_function (in_value in pls_integer) return boolean is + co_yes constant pls_integer := 1; + begin + return in_value = co_yes; + end my_function; +end my_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/4-functions/g-7440.md b/docs/4-language-usage/7-stored-objects/4-functions/g-7440.md index 7b7ef461..d0f9a49a 100644 --- a/docs/4-language-usage/7-stored-objects/4-functions/g-7440.md +++ b/docs/4-language-usage/7-stored-objects/4-functions/g-7440.md @@ -5,29 +5,29 @@ ## Reason -A function should return all its data through the RETURN clause. Having an OUT parameter prohibits usage of a function within SQL statements. +A function should return all its data through the `return` clause. Having an `out` parameter prohibits usage of a function within SQL statements. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - FUNCTION my_function (out_date OUT DATE) RETURN BOOLEAN IS - BEGIN - out_date := SYSDATE; - RETURN TRUE; - END my_function; -END my_package; +``` sql +create or replace package body my_package is + function my_function (out_date out date) return boolean is + begin + out_date := sysdate; + return true; + end my_function; +end my_package; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - FUNCTION my_function RETURN DATE IS - BEGIN - RETURN SYSDATE; - END my_function; -END my_package; +``` sql +create or replace package body my_package is + function my_function return date is + begin + return sysdate; + end my_function; +end my_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/4-functions/g-7450.md b/docs/4-language-usage/7-stored-objects/4-functions/g-7450.md index 3a0cc2f4..8a9a61e5 100644 --- a/docs/4-language-usage/7-stored-objects/4-functions/g-7450.md +++ b/docs/4-language-usage/7-stored-objects/4-functions/g-7450.md @@ -5,28 +5,28 @@ ## Reason -If a boolean function returns null, the caller has do deal with it. This makes the usage cumbersome and more error-prone. +If a boolean function returns `null`, the caller has do deal with it. This makes the usage cumbersome and more error-prone. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - FUNCTION my_function RETURN BOOLEAN IS - BEGIN - RETURN NULL; - END my_function; -END my_package; +``` sql +create or replace package body my_package is + function my_function return boolean is + begin + return null; + end my_function; +end my_package; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY my_package IS - FUNCTION my_function RETURN BOOLEAN IS - BEGIN - RETURN TRUE; - END my_function; -END my_package; +``` sql +create or replace package body my_package is + function my_function return boolean is + begin + return true; + end my_function; +end my_package; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/4-functions/g-7460.md b/docs/4-language-usage/7-stored-objects/4-functions/g-7460.md index 9b4e00cf..833c2b13 100644 --- a/docs/4-language-usage/7-stored-objects/4-functions/g-7460.md +++ b/docs/4-language-usage/7-stored-objects/4-functions/g-7460.md @@ -9,20 +9,20 @@ A deterministic function (always return same result for identical parameters) wh ## Example (bad) -``` -CREATE OR REPLACE PACKAGE department_api IS - FUNCTION name_by_id (in_department_id IN departments.department_id%TYPE) - RETURN departments.department_name%TYPE; -END department_api; +``` sql +create or replace package department_api is + function name_by_id (in_department_id in departments.department_id%type) + return departments.department_name%type; +end department_api; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE department_api IS - FUNCTION name_by_id (in_department_id IN departments.department_id%TYPE) - RETURN departments.department_name%TYPE DETERMINISTIC; -END department_api; +``` sql +create or replace package department_api is + function name_by_id (in_department_id in departments.department_id%type) + return departments.department_name%type deterministic; +end department_api; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/5-oracle-supplied-packages/g-7510.md b/docs/4-language-usage/7-stored-objects/5-oracle-supplied-packages/g-7510.md index 3ae56e1f..50d6c25b 100644 --- a/docs/4-language-usage/7-stored-objects/5-oracle-supplied-packages/g-7510.md +++ b/docs/4-language-usage/7-stored-objects/5-oracle-supplied-packages/g-7510.md @@ -9,22 +9,22 @@ The signature of oracle-supplied packages is well known and therefore it is quit ## Example (bad) -``` -DECLARE - co_hello_world CONSTANT STRING(30 CHAR) := 'Hello World'; -BEGIN +``` sql +declare + co_hello_world constant string(30 char) := 'Hello World'; +begin dbms_output.put_line(co_hello_world); -END; +end; / ``` ## Example (good) -``` -DECLARE - co_hello_world CONSTANT STRING(30 CHAR) := 'Hello World'; -BEGIN +``` sql +declare + co_hello_world constant string(30 char) := 'Hello World'; +begin sys.dbms_output.put_line(co_hello_world); -END; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/7-triggers/g-7710.md b/docs/4-language-usage/7-stored-objects/7-triggers/g-7710.md index 64d02a0a..84bbc6b7 100644 --- a/docs/4-language-usage/7-stored-objects/7-triggers/g-7710.md +++ b/docs/4-language-usage/7-stored-objects/7-triggers/g-7710.md @@ -9,59 +9,59 @@ Having triggers that act on other tables in a way that causes triggers on that t ## Example (bad) -``` -CREATE OR REPLACE TRIGGER dept_br_u -BEFORE UPDATE ON departments FOR EACH ROW -BEGIN - INSERT INTO departments_hist (department_id +``` sql +create or replace trigger dept_br_u +before update on departments for each row +begin + insert into departments_hist (department_id ,department_name ,manager_id ,location_id ,modification_date) - VALUES (:OLD.department_id - ,:OLD.department_name - ,:OLD.manager_id - ,:OLD.location_id - ,SYSDATE); -END; + values (:old.department_id + ,:old.department_name + ,:old.manager_id + ,:old.location_id + ,sysdate); +end; / -CREATE OR REPLACE TRIGGER dept_hist_br_i -BEFORE INSERT ON departments_hist FOR EACH ROW -BEGIN - INSERT INTO departments_log (department_id +create or replace trigger dept_hist_br_i +before insert on departments_hist for each row +begin + insert into departments_log (department_id ,department_name ,modification_date) - VALUES (:NEW.department_id - ,:NEW.department_name - ,SYSDATE); -END; + values (:new.department_id + ,:new.department_name + ,sysdate); +end; / ``` ## Example (good) -``` -CREATE OR REPLACE TRIGGER dept_br_u -BEFORE UPDATE ON departments FOR EACH ROW -BEGIN - INSERT INTO departments_hist (department_id +``` sql +create or replace trigger dept_br_u +before update on departments for each row +begin + insert into departments_hist (department_id ,department_name ,manager_id ,location_id ,modification_date) - VALUES (:OLD.department_id - ,:OLD.department_name - ,:OLD.manager_id - ,:OLD.location_id - ,SYSDATE); + values (:old.department_id + ,:old.department_name + ,:old.manager_id + ,:old.location_id + ,sysdate); - INSERT INTO departments_log (department_id + insert into departments_log (department_id ,department_name ,modification_date) - VALUES (:OLD.department_id - ,:OLD.department_name - ,SYSDATE); + values (:old.department_id + ,:old.department_name + ,sysdate); -END; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/7-triggers/g-7720.md b/docs/4-language-usage/7-stored-objects/7-triggers/g-7720.md new file mode 100644 index 00000000..c34c487a --- /dev/null +++ b/docs/4-language-usage/7-stored-objects/7-triggers/g-7720.md @@ -0,0 +1,43 @@ +# G-7720: Never use multiple UPDATE OF in trigger event clause. + +!!! bug "Blocker" + Maintainability, Reliability, Testability + +## Reason + +A DML trigger can have multiple triggering events separated by `or` like `before insert or delete or update of some_column`. If you have multiple `update of` separated by `or`, only one of them (the last one) is actually used and you get no error message, so you have a bug waiting to happen. Instead you always should use a single `update of` with all columns comma-separated, or an `update` without `of` if you wish all columns. + +## Example (bad) + +``` sql +create or replace trigger dept_br_u +before update of department_id or update of department_name +on departments for each row +begin + -- will only fire on updates of department_name + insert into departments_log (department_id + ,department_name + ,modification_date) + values (:old.department_id + ,:old.department_name + ,sysdate); +end; +/ +``` + +## Example (good) + +``` sql +create or replace trigger dept_br_u +before update of department_id, department_name +on departments for each row +begin + insert into departments_log (department_id + ,department_name + ,modification_date) + values (:old.department_id + ,:old.department_name + ,sysdate); +end; +/ +``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/7-triggers/g-7730.md b/docs/4-language-usage/7-stored-objects/7-triggers/g-7730.md new file mode 100644 index 00000000..ac576a09 --- /dev/null +++ b/docs/4-language-usage/7-stored-objects/7-triggers/g-7730.md @@ -0,0 +1,62 @@ +# G-7730: Avoid multiple DML events per trigger if primary key is assigned in trigger. + +!!! warning "Major" + Efficiency, Reliability + +## Reason + +If a trigger makes assignment to the primary key anywhere in the trigger code, that causes the session firing the trigger to take a lock on any child tables with a foreign key to this primary key. Even if the assignment is in for example an `if inserting` block and the trigger is fired by an `update` statement, such locks still happen unnecessarily. The issue is avoided by having one trigger for the insert containing the primary key assignment, and another trigger for the update. Or even better by handling the insert assignment as ´default on null´ clauses, so that only an `on update` trigger is needed. + +## Example (bad) + +``` sql +create or replace trigger dept_br_iu +before insert or update +on departments for each row +begin + if inserting then + :new.department_id := department_seq.nextval; + :new.created_date := sysdate; + end if; + if updating then + :new.changed_date := sysdate; + end if; +end; +/ +``` + +## Example (better) + +``` sql +create or replace trigger dept_br_i +before insert +on departments for each row +begin + :new.department_id := department_seq.nextval; + :new.created_date := sysdate; +end; +/ + +create or replace trigger dept_br_u +before update +on departments for each row +begin + :new.changed_date := sysdate; +end; +/ +``` + +## Example (good) + +``` sql +alter table department modify department_id default on null department_seq.nextval; +alter table department modify created_date default on null sysdate; + +create or replace trigger dept_br_u +before update +on departments for each row +begin + :new.changed_date := sysdate; +end; +/ +``` \ No newline at end of file diff --git a/docs/4-language-usage/7-stored-objects/8-sequences/g-7810.md b/docs/4-language-usage/7-stored-objects/8-sequences/g-7810.md index b7edd72e..a028264b 100644 --- a/docs/4-language-usage/7-stored-objects/8-sequences/g-7810.md +++ b/docs/4-language-usage/7-stored-objects/8-sequences/g-7810.md @@ -5,28 +5,28 @@ ## Reason -Since ORACLE 11g it is no longer needed to use a SELECT statement to read a sequence (which would imply a context switch). +Since ORACLE 11g it is no longer needed to use a `select` statement to read a sequence (which would imply a context switch). ## Example (bad) -``` -DECLARE +``` sql +declare l_sequence_number employees.emloyee_id%type; -BEGIN - SELECT employees_seq.NEXTVAL - INTO l_sequence_number - FROM DUAL; -END; +begin + select employees_seq.nextval + into l_sequence_number + from dual; +end; / ``` ## Example (good) -``` -DECLARE +``` sql +declare l_sequence_number employees.emloyee_id%type; -BEGIN - l_sequence_number := employees_seq.NEXTVAL; -END; +begin + l_sequence_number := employees_seq.nextval; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/8-patterns/1-checking-the-number-of-rows/g-8110.md b/docs/4-language-usage/8-patterns/1-checking-the-number-of-rows/g-8110.md index 287c4cdc..ae04033d 100644 --- a/docs/4-language-usage/8-patterns/1-checking-the-number-of-rows/g-8110.md +++ b/docs/4-language-usage/8-patterns/1-checking-the-number-of-rows/g-8110.md @@ -5,49 +5,49 @@ ## Reason -If you do a SELECT count(*) all rows will be read according to the WHERE clause, even if only the availability of data is of interest. For this we have a big performance overhead. If we do a SELECT count(*) ... WHERE ROWNUM = 1 there is also a overhead as there will be two communications between the PL/SQL and the SQL engine. See the following example for a better solution. +If you do a `select count(*)` all rows will be read according to the `where` clause, even if only the availability of data is of interest. For this we have a big performance overhead. If we do a `select count(*) ... where rownum = 1` there is also a overhead as there will be two communications between the PL/SQL and the SQL engine. See the following example for a better solution. ## Example (bad) -``` -DECLARE - l_count PLS_INTEGER; - co_zero CONSTANT SIMPLE_INTEGER := 0; - co_salary CONSTANT employees.salary%TYPE := 5000; -BEGIN - SELECT count(*) - INTO l_count - FROM employees - WHERE salary < co_salary; - IF l_count > co_zero THEN +``` sql +declare + l_count pls_integer; + co_zero constant simple_integer := 0; + co_salary constant employees.salary%type := 5000; +begin + select count(*) + into l_count + from employees + where salary < co_salary; + if l_count > co_zero then <> - FOR r_emp IN (SELECT employee_id - FROM employees) - LOOP - IF r_emp.salary < co_salary THEN + for r_emp in (select employee_id + from employees) + loop + if r_emp.salary < co_salary then my_package.my_proc(in_employee_id => r_emp.employee_id); - END IF; - END LOOP emp_loop; - END IF; -END; + end if; + end loop emp_loop; + end if; +end; / ``` ## Example (good) -``` -DECLARE - co_salary CONSTANT employees.salary%TYPE := 5000; -BEGIN +``` sql +declare + co_salary constant employees.salary%type := 5000; +begin <> - FOR r_emp IN (SELECT e1.employee_id - FROM employees e1 - WHERE EXISTS(SELECT e2.salary - FROM employees e2 - WHERE e2.salary < co_salary)) - LOOP + for r_emp in (select e1.employee_id + from employees e1 + where exists(select e2.salary + from employees e2 + where e2.salary < co_salary)) + loop my_package.my_proc(in_employee_id => r_emp.employee_id); - END LOOP emp_loop; -END; + end loop emp_loop; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/8-patterns/1-checking-the-number-of-rows/g-8120.md b/docs/4-language-usage/8-patterns/1-checking-the-number-of-rows/g-8120.md index 394bff7b..15350bab 100644 --- a/docs/4-language-usage/8-patterns/1-checking-the-number-of-rows/g-8120.md +++ b/docs/4-language-usage/8-patterns/1-checking-the-number-of-rows/g-8120.md @@ -5,40 +5,40 @@ ## Reason -The result of an existence check is a snapshot of the current situation. You never know whether in the time between the check and the (insert) action someone else has decided to create a row with the values you checked. Therefore, you should only rely on constraints when it comes to preventioin of duplicate records. +The result of an existence check is a snapshot of the current situation. You never know whether in the time between the check and the (insert) action someone else has decided to create a row with the values you checked. Therefore, you should only rely on constraints when it comes to prevention of duplicate records. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY department_api IS - PROCEDURE ins (in_r_department IN departments%ROWTYPE) IS - l_count PLS_INTEGER; - BEGIN - SELECT count(*) - INTO l_count - FROM departments - WHERE department_id = in_r_department.department_id; +``` sql +create or replace package body department_api is + procedure ins (in_r_department in departments%rowtype) is + l_count pls_integer; + begin + select count(*) + into l_count + from departments + where department_id = in_r_department.department_id; - IF l_count = 0 THEN - INSERT INTO departments - VALUES in_r_department; - END IF; - END ins; -END department_api; + if l_count = 0 then + insert into departments + values in_r_department; + end if; + end ins; +end department_api; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY department_api IS - PROCEDURE ins (in_r_department IN departments%ROWTYPE) IS - BEGIN - INSERT INTO departments - VALUES in_r_department; - EXCEPTION - WHEN dup_val_on_index THEN NULL; -- handle exception - END ins; -END department_api; +``` sql +create or replace package body department_api is + procedure ins (in_r_department in departments%rowtype) is + begin + insert into departments + values in_r_department; + exception + when dup_val_on_index then null; -- handle exception + end ins; +end department_api; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/8-patterns/2-access-objects-of-foreign-application-schemas/g-8210.md b/docs/4-language-usage/8-patterns/2-access-objects-of-foreign-application-schemas/g-8210.md index 6044ddaf..91dbf188 100644 --- a/docs/4-language-usage/8-patterns/2-access-objects-of-foreign-application-schemas/g-8210.md +++ b/docs/4-language-usage/8-patterns/2-access-objects-of-foreign-application-schemas/g-8210.md @@ -9,42 +9,42 @@ If a connection is needed to a table that is placed in a foreign schema, using s ## Example (bad) -``` -DECLARE - l_product_name oe.products.product_name%TYPE; - co_price CONSTANT oe.products@list_price%TYPE := 1000; -BEGIN - SELECT p.product_name - INTO l_product_name - FROM oe.products p - WHERE list_price > co_price; -EXCEPTION - WHEN NO_DATA_FOUND THEN - NULL; -- handle_no_data_found; - WHEN TOO_MANY_ROWS THEN - NULL; -- handle_too_many_rows; -END; +``` sql +declare + l_product_name oe.products.product_name%type; + co_price constant oe.products@list_price%type := 1000; +begin + select p.product_name + into l_product_name + from oe.products p + where list_price > co_price; +exception + when no_data_found then + null; -- handle_no_data_found; + when too_many_rows then + null; -- handle_too_many_rows; +end; / ``` ## Example (good) -``` -CREATE SYNONYM oe_products FOR oe.products; - -DECLARE - l_product_name oe_products.product_name%TYPE; - co_price CONSTANT oe_products.list_price%TYPE := 1000; -BEGIN - SELECT p.product_name - INTO l_product_name - FROM oe_products p - WHERE list_price > co_price; -EXCEPTION - WHEN NO_DATA_FOUND THEN - NULL; -- handle_no_data_found; - WHEN TOO_MANY_ROWS THEN - NULL; -- handle_too_many_rows; -END; +``` sql +create synonym oe_products for oe.products; + +declare + l_product_name oe_products.product_name%type; + co_price constant oe_products.list_price%type := 1000; +begin + select p.product_name + into l_product_name + from oe_products p + where list_price > co_price; +exception + when no_data_found then + null; -- handle_no_data_found; + when too_many_rows then + null; -- handle_too_many_rows; +end; / ``` \ No newline at end of file diff --git a/docs/4-language-usage/8-patterns/3-validating-input-parameter-size/g-8310.md b/docs/4-language-usage/8-patterns/3-validating-input-parameter-size/g-8310.md index 69a9e7ea..f830e3fd 100644 --- a/docs/4-language-usage/8-patterns/3-validating-input-parameter-size/g-8310.md +++ b/docs/4-language-usage/8-patterns/3-validating-input-parameter-size/g-8310.md @@ -5,58 +5,58 @@ ## Reason -This technique raises an error (value_error) which may not be handled in the called program unit. This is the right way to do it, as the error is not within this unit but when calling it, so the caller should handle the error. +This technique raises an error (`value_error`) which may not be handled in the called program unit. This is the right way to do it, as the error is not within this unit but when calling it, so the caller should handle the error. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY department_api IS - FUNCTION dept_by_name (in_dept_name IN departments.department_name%TYPE) - RETURN departments%ROWTYPE IS +``` sql +create or replace package body department_api is + function dept_by_name (in_dept_name in departments.department_name%type) + return departments%rowtype is l_return departments%rowtype; - BEGIN - IF in_dept_name IS NULL - OR LENGTH(in_dept_name) > 20 - THEN - RAISE err.e_param_to_large; - END IF; + begin + if in_dept_name is null + or length(in_dept_name) > 20 + then + raise err.e_param_to_large; + end if; -- get the department by name - SELECT * - FROM departments - WHERE department_name = in_dept_name; + select * + from departments + where department_name = in_dept_name; - RETURN l_return; - END dept_by_name; -END department_api; + return l_return; + end dept_by_name; +end department_api; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY department_api IS - FUNCTION dept_by_name (in_dept_name IN departments.department_name%TYPE) - RETURN departments%ROWTYPE IS - l_dept_name departments.department_name%TYPE NOT NULL := in_dept_name; +``` sql +create or replace package body department_api is + function dept_by_name (in_dept_name in departments.department_name%type) + return departments%rowtype is + l_dept_name departments.department_name%type not null := in_dept_name; l_return departments%rowtype; - BEGIN + begin -- get the department by name - SELECT * - FROM departments - WHERE department_name = l_dept_name; + select * + from departments + where department_name = l_dept_name; - RETURN l_return; - END dept_by_name; -END department_api; + return l_return; + end dept_by_name; +end department_api; / ``` ## Function call -``` +``` sql ... - r_deparment := department_api.dept_by_name('Far to long name of a department'); + r_department := department_api.dept_by_name('Far to long name of a department'); ... -EXCEPTION - WHEN VALUE_ERROR THEN ... +exception + when value_error then ... ``` \ No newline at end of file diff --git a/docs/4-language-usage/8-patterns/4-ensure-single-execution-at-a-time-of-a-program-unit/g-8410.md b/docs/4-language-usage/8-patterns/4-ensure-single-execution-at-a-time-of-a-program-unit/g-8410.md index 4f2cf5ec..5f869786 100644 --- a/docs/4-language-usage/8-patterns/4-ensure-single-execution-at-a-time-of-a-program-unit/g-8410.md +++ b/docs/4-language-usage/8-patterns/4-ensure-single-execution-at-a-time-of-a-program-unit/g-8410.md @@ -11,94 +11,94 @@ The alternative using a table where a “Lock-Row” is stored has the disadvant ## Example (bad) -``` +``` sql -- Bad /* Example */ -CREATE OR REPLACE PACKAGE BODY lock_up IS +create or replace package body lock_up is -- manage locks in a dedicated table created as follows: -- CREATE TABLE app_locks ( -- lock_name VARCHAR2(128 CHAR) NOT NULL primary key -- ); - PROCEDURE request_lock (in_lock_name IN VARCHAR2) IS - BEGIN + procedure request_lock (in_lock_name in varchar2) is + begin -- raises dup_val_on_index - INSERT INTO app_locks (lock_name) VALUES (in_lock_name); - END request_lock; + insert into app_locks (lock_name) values (in_lock_name); + end request_lock; - PROCEDURE release_lock(in_lock_name IN VARCHAR2) IS - BEGIN - DELETE FROM app_locks WHERE lock_name = in_lock_name; - END release_lock; -END lock_up; + procedure release_lock(in_lock_name in varchar2) is + begin + delete from app_locks where lock_name = in_lock_name; + end release_lock; +end lock_up; / /* Call bad example */ -DECLARE - co_lock_name CONSTANT VARCHAR2(30 CHAR) := 'APPLICATION_LOCK'; -BEGIN +declare + co_lock_name constant varchar2(30 char) := 'APPLICATION_LOCK'; +begin lock_up.request_lock(in_lock_name => co_lock_name); -- processing lock_up.release_lock(in_lock_name => co_lock_name); -EXCEPTION - WHEN OTHERS THEN +exception + when others then -- log error lock_up.release_lock(in_lock_name => co_lock_name); - RAISE; -END; + raise; +end; / ``` ## Example (good) -``` +``` sql /* Example */ -CREATE OR REPLACE PACKAGE BODY lock_up IS - FUNCTION request_lock( - in_lock_name IN VARCHAR2, - in_release_on_commit IN BOOLEAN := FALSE) - RETURN VARCHAR2 IS - l_lock_handle VARCHAR2(128 CHAR); - BEGIN +create or replace package body lock_up is + function request_lock( + in_lock_name in varchar2, + in_release_on_commit in boolean := false) + return varchar2 is + l_lock_handle varchar2(128 char); + begin sys.dbms_lock.allocate_unique( lockname => in_lock_name, lockhandle => l_lock_handle, expiration_secs => constants_up.co_one_week ); - IF sys.dbms_lock.request( + if sys.dbms_lock.request( lockhandle => l_lock_handle, lockmode => sys.dbms_lock.x_mode, timeout => sys.dbms_lock.maxwait, - release_on_commit => COALESCE(in_release_on_commit, FALSE) + release_on_commit => coalesce(in_release_on_commit, false) ) > 0 - THEN - RAISE err.e_lock_request_failed; - END IF; - RETURN l_lock_handle; - END request_lock; + then + raise err.e_lock_request_failed; + end if; + return l_lock_handle; + end request_lock; - PROCEDURE release_lock(in_lock_handle IN VARCHAR2) IS - BEGIN - IF sys.dbms_lock.release(lockhandle => in_lock_handle) > 0 THEN - RAISE err.e_lock_request_failed; - END IF; - END release_lock; -END lock_up; + procedure release_lock(in_lock_handle in varchar2) is + begin + if sys.dbms_lock.release(lockhandle => in_lock_handle) > 0 then + raise err.e_lock_request_failed; + end if; + end release_lock; +end lock_up; / /* Call good example */ -DECLARE - l_handle VARCHAR2(128 CHAR); - co_lock_name CONSTANT VARCHAR2(30 CHAR) := 'APPLICATION_LOCK'; -BEGIN +declare + l_handle varchar2(128 char); + co_lock_name constant varchar2(30 char) := 'APPLICATION_LOCK'; +begin l_handle := lock_up.request_lock(in_lock_name => co_lock_name); -- processing lock_up.release_lock(in_lock_handle => l_handle); -EXCEPTION - WHEN OTHERS THEN +exception + when others then -- log error lock_up.release_lock(in_lock_handle => l_handle); - RAISE; -END; + raise; +end; / ``` diff --git a/docs/4-language-usage/8-patterns/5-use-dbms-application-info-package-to-follow-progress-of-a-process/g-8510.md b/docs/4-language-usage/8-patterns/5-use-dbms-application-info-package-to-follow-progress-of-a-process/g-8510.md index d1b0a4a2..04f3b526 100644 --- a/docs/4-language-usage/8-patterns/5-use-dbms-application-info-package-to-follow-progress-of-a-process/g-8510.md +++ b/docs/4-language-usage/8-patterns/5-use-dbms-application-info-package-to-follow-progress-of-a-process/g-8510.md @@ -5,42 +5,42 @@ ## Reason -This technique allows us to view progress of a process without having to persistently write log data in either a table or a file. The information is accessible through the `V$SESSION` view. +This technique allows us to view progress of a process without having to persistently write log data in either a table or a file. The information is accessible through the `v$session` view. ## Example (bad) -``` -CREATE OR REPLACE PACKAGE BODY employee_api IS - PROCEDURE process_emps IS - BEGIN +``` sql +create or replace package body employee_api is + procedure process_emps is + begin <> - FOR emp_rec IN (SELECT employee_id - FROM employees - ORDER BY employee_id) - LOOP - NULL; -- some processing - END LOOP employees; - END process_emps; -END employee_api; + for emp_rec in (select employee_id + from employees + order by employee_id) + loop + null; -- some processing + end loop employees; + end process_emps; +end employee_api; / ``` ## Example (good) -``` -CREATE OR REPLACE PACKAGE BODY employee_api IS - PROCEDURE process_emps IS - BEGIN - SYS.DBMS_APPLICATION_INFO.SET_MODULE(module_name => $$PLSQL_UNIT +``` sql +create or replace package body employee_api is + procedure process_emps is + begin + sys.dbms_application_info.set_module(module_name => $$plsql_unit ,action_name => 'Init'); <> - FOR emp_rec IN (SELECT employee_id - FROM employees - ORDER BY employee_id) - LOOP - SYS.DBMS_APPLICATION_INFO.SET_ACTION('Processing ' || emp_rec.employee_id); - END LOOP employees; + for emp_rec in (select employee_id + from employees + order by employee_id) + loop + sys.dbms_application_info.set_action('Processing ' || emp_rec.employee_id); + end loop employees; end process_emps; -END employee_api; +end employee_api; / ``` \ No newline at end of file diff --git a/docs/5-complexity-analysis/complexity-analysis.md b/docs/5-complexity-analysis/complexity-analysis.md index ef94d892..d2e62ccb 100644 --- a/docs/5-complexity-analysis/complexity-analysis.md +++ b/docs/5-complexity-analysis/complexity-analysis.md @@ -33,7 +33,7 @@ Cyclomatic complexity (or conditional complexity) is a software metric used to m Cyclomatic complexity is computed using the control flow graph of the program: the nodes of the graph correspond to indivisible groups of commands of a program, and a directed edge connects two nodes if the second command might be executed immediately after the first command. Cyclomatic complexity may also be applied to individual functions, modules, methods or classes within a program. -The cyclomatic complexity of a section of source code is the count of the number of linearly independent paths through the source code. For instance, if the source code contains no decision points, such as IF statements or FOR loops, the complexity would be 1, since there is only a single path through the code. If the code has a single IF statement containing a single condition there would be two paths through the code, one path where the IF statement is evaluated as TRUE and one path where the IF statement is evaluated as FALSE. +The cyclomatic complexity of a section of source code is the count of the number of linearly independent paths through the source code. For instance, if the source code contains no decision points, such as `if` statements or `for` loops, the complexity would be 1, since there is only a single path through the code. If the code has a single `if` statement containing a single condition there would be two paths through the code, one path where the `if` statement is evaluated as `true` and one path where the `if` statement is evaluated as `false`. ### Calculation @@ -54,26 +54,26 @@ where Take, for example, a control flow graph of a simple program. The program begins executing at the red node, then enters a loop (group of three nodes immediately below the red node). On exiting the loop, there is a conditional statement (group below the loop), and finally the program exits at the blue node. For this graph, $E = 9$, $N = 8$ and $P = 1$, so the cyclomatic complexity of the program is $3$.

-``` -BEGIN - FOR i IN 1..3 - LOOP +``` sql +begin + for i in 1..3 + loop dbms_output.put_line('in loop'); - END LOOP; + end loop; -- - IF 1 = 1 - THEN + if 1 = 1 + then dbms_output.put_line('yes'); - END IF; + end if; -- dbms_output.put_line('end'); -END; +end; / ``` For a single program (or subroutine or method), P is always equal to 1. Cyclomatic complexity may, however, be applied to several such programs or subprograms at the same time (e.g., to all of the methods in a class), and in these cases P will be equal to the number of programs in question, as each subprogram will appear as a disconnected subset of the graph. -It can be shown that the cyclomatic complexity of any structured program with only one entrance point and one exit point is equal to the number of decision points (i.e., 'if' statements or conditional loops) contained in that program plus one. +It can be shown that the cyclomatic complexity of any structured program with only one entrance point and one exit point is equal to the number of decision points (i.e., `if` statements or conditional loops) contained in that program plus one. Cyclomatic complexity may be extended to a program with multiple exit points; in this case it is equal to: diff --git a/docs/cover-template.html b/docs/cover-template.html index 769583cc..39a566ae 100644 --- a/docs/cover-template.html +++ b/docs/cover-template.html @@ -11,26 +11,26 @@ .page { margin-top: 0mm; } - .page .cover-image .title { - position: absolute; - top: 20px; - left: 25px; - color:white; + .page .title { + color: #6D65AB; font-size: 77pt; font-weight: bold; line-height: 90%; } - .page .cover-image .subtitle { + .page .cover-image { + position: relative; + top: 50px; + } + .page .subtitle { position: relative; - bottom: 58px; - left: 25px; - color: white; + top: 100px; + color: #FC7C77; font-size: 28pt; font-weight: bold; } .page .version { position: relative; - top: -20px; + top: 150px; font-size: 14pt; } .page .copyright { @@ -44,14 +44,14 @@
+
+ PL/SQL & SQL
Coding Guidelines +
-
- PL/SQL & SQL
Coding Guidelines -
-
- Tips for Development & Operation -
+
+
+ Tips for Development & Operation
Document Version #VERSION# @@ -60,7 +60,7 @@
diff --git a/docs/images/cover.png b/docs/images/cover.png index 4cd4f21d..1ce705e9 100644 Binary files a/docs/images/cover.png and b/docs/images/cover.png differ diff --git a/docs/images/favicon.ico b/docs/images/favicon.ico index 7e3cabfd..8016c3f8 100644 Binary files a/docs/images/favicon.ico and b/docs/images/favicon.ico differ diff --git a/docs/images/pdf-thumbnail.png b/docs/images/pdf-thumbnail.png index fd0edfa4..0dcf4f51 100644 Binary files a/docs/images/pdf-thumbnail.png and b/docs/images/pdf-thumbnail.png differ diff --git a/docs/index.md b/docs/index.md index 7417730b..cba41e9a 100644 --- a/docs/index.md +++ b/docs/index.md @@ -48,7 +48,7 @@ The authors and publisher shall have neither liability nor responsibility to any ## Revision History -The first version of these guidelines was compiled by Roger Troller on March 17, 2009. Jörn Kulessa, Daniela Reiner, Richard Bushnell, Andreas Flubacher and Thomas Mauch helped Roger complete version 1.2 until August 21, 2009. This was the first GA version. The handy printed version in A5 format was distributed free of charge at the DOAG Annual Conference and on other occasions. Since then Roger updated the guidelines regularily. Philipp Salvisberg was involved in the review process for version 3.0 which was a major update. Philipp took the lead, after Roger left Trivadis in 2016. +The first version of these guidelines was compiled by Roger Troller on March 17, 2009. Jörn Kulessa, Daniela Reiner, Richard Bushnell, Andreas Flubacher and Thomas Mauch helped Roger complete version 1.2 until August 21, 2009. This was the first GA version. The handy printed version in A5 format was distributed free of charge at the DOAG Annual Conference and on other occasions. Since then Roger updated the guidelines regularily. Philipp Salvisberg was involved in the review process for version 3.0 which was a major update. Philipp took the lead, after Roger left Trivadis in 2016. In 2020 Kim Berg Hansen started handling guidelines maintenance, letting Philipp concentrate on the related [Trivadis PL/SQL Cop](https://www.trivadis.com/en/plsql-cop) tool. Since July, 7 2018 these guidelines are hosted on GitHub. Ready to be enhanced by the community and forked to fit specific needs. diff --git a/docs/stylesheets/extra.css b/docs/stylesheets/extra.css index a7d12a4a..05045992 100644 --- a/docs/stylesheets/extra.css +++ b/docs/stylesheets/extra.css @@ -89,6 +89,7 @@ [id="large-objects"], [id="dml-sql"], [id="bulk-operations"], + [id="transaction-control"], [id="control-structures"], [id="case-if-decode-nvl-nvl2-coalesce"], [id="flow-control"], diff --git a/mkdocs.yml b/mkdocs.yml index 70ffbdac..d3f2d276 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -22,6 +22,8 @@ extra_javascript: markdown_extensions: - admonition + - codehilite: + linenums: true - pymdownx.arithmatex - footnotes @@ -36,4 +38,4 @@ extra: plugins: - search - awesome-pages: - collapse_single_pages: false \ No newline at end of file + collapse_single_pages: false diff --git a/tools/run-in-container/genpdf.sh b/tools/run-in-container/genpdf.sh index 0f14bb8a..756b6fac 100755 --- a/tools/run-in-container/genpdf.sh +++ b/tools/run-in-container/genpdf.sh @@ -44,7 +44,9 @@ function write_guidelines(){ for f in ${DATA_DIR}/docs/${DIR}/*.md do echo "" >> ${TARGET_DIR}/docs/index.md - sed -e "s|# |${HEADER} |g" $f >> ${TARGET_DIR}/docs/index.md + sed -e "s|# |${HEADER} |g" $f | \ + sed -e 's|../../../../4-language-usage/3-dml-and-sql/3-transaction-control/g-3310|#g-3310-never-commit-within-a-cursor-loop|g' | \ + sed -e 's|../../../../4-language-usage/3-dml-and-sql/3-transaction-control/g-3320|#g-3320-try-to-move-transactions-within-a-non-cursor-loop-into-procedures|g' >> ${TARGET_DIR}/docs/index.md done } @@ -107,6 +109,8 @@ write_text "### General" write_guidelines "4-language-usage/3-dml-and-sql/1-general" "####" write_text "### Bulk Operations" write_guidelines "4-language-usage/3-dml-and-sql/2-bulk-operations" "####" +write_text "### Transaction Control" +write_guidelines "4-language-usage/3-dml-and-sql/3-transaction-control" "####" write_text "## Control Structures" write_text "### CURSOR" write_guidelines "4-language-usage/4-control-structures/1-cursor" "####"