Code Analysis Incorrectly Reporting Violations

I’m just now playing around with the Code Analysis feature and found some rules that are incorrectly being reported as being violated. . .

When I run Code Analysis on a trigger, I seem to always get a violation of Rule 2118: Avoid DDLs other than Truncate, Exhange/Split partition. Here is an example of a trigger that “breaks” that rule:

CREATE OR REPLACE TRIGGER ANT.TAB_INSERT_STEP_TYPES_TRG

BEFORE INSERT

ON ant.tab_insert_step_types

REFERENCING NEW AS new OLD AS old

FOR EACH ROW

WHEN(new.ist_no IS NULL)

BEGIN

:new.ist_no := ist_seq.NEXTVAL;

END tab_insert_step_types_trg;

/

These next two rules are both raised on the same trigger. I’m including just the pertinent part of the trigger code

Rule 5914: Use untransformed column values in the WHERE clause:

In this case, the untransformed “column” it is referring to is a reference to the value in :new.columname where I am doing an UPPER

Rule 5903: All queries (dynamic and fixed) must leverage bind variables

My query is using :new.field name references and an IN parameter to the cursor. So it is using bind variables

CREATE OR REPLACE TRIGGER ANT.TAB_ZERO_ROW_WID_COLUMNS_T

BEFORE INSERT OR UPDATE

ON ANT.TAB_ZERO_ROW_WID_COLUMNS

REFERENCING NEW AS NEW OLD AS OLD

FOR EACH ROW

DISABLE

DECLARE

CURSOR tab(in_column_name IN dba_tab_columns.column_name%TYPE)

IS

SELECT *

FROM dba_tab_columns

WHERE 1 = 1

AND table_name = UPPER(:new.zrw_table_name)

AND owner = UPPER(:new.zrw_owner)

AND column_name = UPPER(in_column_name);

i PLS_INTEGER := 0;

col_rec dba_tab_columns%ROWTYPE;

v_exception_message types.t_sql;

v_datatypes_selected types.t_sql;

That’s all I’ll report for now, but may have others to mention later. :slight_smile:

Blessings,

Phyllis


Phyllis Helton

Data Magician

Security Gestapo
Digital Products & Strategies, Cru | Data Sciences & Analytics
Office :phone: 407-515-4452

phyllis.helton@cru.org

Hello Phyllis,

Rule 2118: Avoid DDLs other than Truncate, Exhange/Split partition

This rule was suggested by a customer back in the year 2011. Well, looking at the original spec it seems that it should hunt for DDL inside an EXECUTE IMMEDIATE statement, not for any DDL! Ok, to be fixed.

Rule 5914: Use untransformed column values in the WHERE clause:

Good catch! Detecting column transformations such as by UPPER short circuits index usage in some cases. In our case, the :new.name symbols do not refer to columns retrieved by the query so they should not be considered. We’ll fix that!

Rule 5903: All queries (dynamic and fixed) must leverage bind variables

The rule’s rationale is: "SQL statements should use bind variables instead of hard coded text literals, dates and/or numeric values to improve performance, security and maintainability. "

It complains about the 1 = 1 predicate. Why is it there, is that generated code?

The WHERE 1 = 1 seems to be a widespread practice, we could easily disregard it in the rule.

Thank you very much for this valuable feedback!

Andre

(refs. QP-2398, 2399, 2400)

Thanks Andre!

I totally missed the 1=1. That makes sense it would fail the rule. I have been looking at code like that for so long, I don't think I actually even see it any longer. We do that for a few reasons, mostly laziness. :slight_smile:

  1. It makes it very easy when testing and commenting out various conditions. Each line of the where clause that I might comment out will always start with an AND so it helps with commenting out various lines to test things.

  2. When building dynamic SQL, it allows me to always have a WHERE clause so I don't have to test for it's existence before adding conditions dynamically

There are probably a few other reasons I've used it over the years, but I can't think of what the others are right now.

On Sat, May 6, 2017 at 6:28 AM, Andre Vergison bounce-avergison@toadworld.com wrote:

RE: Code Analysis Incorrectly Reporting Violations

Reply by Andre Vergison
Hello Phyllis,

Rule 2118: Avoid DDLs other than Truncate, Exhange/Split partition

This rule was suggested by a customer back in the year 2011. Well, looking at the original spec it seems that it should hunt for DDL inside an EXECUTE IMMEDIATE statement, not for any DDL! Ok, to be fixed.

Rule 5914: Use untransformed column values in the WHERE clause:

Good catch! Detecting column transformations such as by UPPER short circuits index usage in some cases. In our case, the :new.name symbols do not refer to columns retrieved by the query so they should not be considered. We'll fix that!

Rule 5903: All queries (dynamic and fixed) must leverage bind variables

The rule's rationale is: "SQL statements should use bind variables instead of hard coded text literals, dates and/or numeric values to improve performance, security and maintainability. "

It complains about the 1 = 1 predicate. Why is it there, is that generated code?

The WHERE 1 = 1 seems to be a widespread practice, we could easily disregard it in the rule.

Thank you very much for this valuable feedback!

Andre

To reply, please reply-all to this email.

Stop receiving emails on this subject.

Or Unsubscribe from Toad for Oracle - Beta Forum notifications altogether.

Toad for Oracle - Beta Discussion Forum

Flag this post as spam/abuse.

--
Phyllis Helton

Data Magician

Security Gestapo
Digital Products & Strategies, Cru | Data Sciences & Analytics
Office :phone: 407-515-4452

phyllis.helton@cru.org

Phyllis those are very “logical” reasons. I looked up the internet and everybody has the same reasons. Ok, as said before, we’ll just enhance the rule to disregard things of the form ‘my_literal = my_literal’ and we’ll be done with it.

Thanks,
Andre

Hello Phyllis,

Some feeback…

The rule 2118 (DDLs) fix should already have made it into the beta. It will now process DDL quoted inside an EXECUTE IMMEDIATE statement only, excluding all other normal DDL statements.

Rule 5914 (untransformad values) should normally be in next beta. Feel free to comment if it doesn’t work well enough … :slight_smile:

Rule 5903 (bind vars) is due within a week or three.

Hope this helps,
Andre

Thanks Andre!

On Tue, Sep 5, 2017 at 11:32 AM, Andre Vergison bounce-avergison@toadworld.com wrote:

RE: Code Analysis Incorrectly Reporting Violations

Reply by Andre Vergison
Hello Phyllis,

Some feeback...

The rule 2118 (DDLs) fix should already have made it into the beta. It will now process DDL quoted inside an EXECUTE IMMEDIATE statement only, excluding all other normal DDL statements.

Rule 5914 (untransformad values) should normally be in next beta. Feel free to comment if it doesn't work well enough ... :slight_smile:

Rule 5903 (bind vars) is due within a week or three.

Hope this helps,
Andre

To reply, please reply-all to this email.

Stop receiving emails on this subject.

Or Unsubscribe from Toad for Oracle - Beta Forum notifications altogether.

Toad for Oracle - Beta Discussion Forum

Flag this post as spam/abuse.

--
Phyllis Helton

Data Magician

Security Gestapo
Digital Products & Strategies, Cru | Data Sciences & Analytics
Office :phone: 407-515-4452

phyllis.helton@cru.org