How to debug your dynamic SQL code

Got this plea for help via our AskTOM PL/SQL Office Hours program:

Dear Experts, I have below written below code:
----------------------------------------------
       Declare
       v_Table all_tables.TABLE_NAME%type;
       v_Mnt varchar2(2):='08';
       Type Cur_type is Ref Cursor;
        C Cur_type;
       
       Begin
               v_Table:='ddi_ticket_10_1018';           
               Open C for 'SELECT * from bill.'||v_Table||v_Mnt||'Where called_nbr=123';
                      
               End;
-------------------------------------------------------------------
When executing this code, I face this Error message.
ORA-00933-SQL Command not properly ended
ORA-06512: At Line 9.
Please check the above code and modify for syntax correction

I could, at a glance, pretty well guess what the problem is.

Can you?

I am not trying to boast. I just encourage you to not read further and instead examine the code. What could be causing his problem?

Dynamic SQL can be tricky - not so much before OPEN-FOR or EXECUTE IMMEDIATE are complicated parts of the PL/SQL language. But because it's just so darned easy to mess up the SQL or PL/SQL you are constructing dynamically. You could:
  • Leave out a ";" (from PL/SQL code).
  • Forget to leave white space between sections of your SQL.
  • Have unmatched parentheses.
  • On and on and on.
In this case, I wrote back to say: "I am pretty sure you will find the problem is that you don't have a space before the "Where" keyword in:

v_Mnt||'Where called_nbr=123';


This exchange then reminded me that I should write a blog post with some simple tips for making it so much easier to debug your dynamic SQL code - and ensure that it works as intended. Here goes.
  1. Define your subprogram with AUTHID CURRENT_USER (invokers rights).
  2. If you're executing dynamic DDL, make the subprogram an autonomous transaction.
  3. Always EXECUTE IMMEDIATE or OPEN FOR a variable.
  4. Always handle the exception that may arise from the dynamic SQL execution.
  5. Log the error information plus the variable that you tried to execute.
  6. Build a test mode into your subprogram.
I will demonstrate the value of these points by starting with a version of a super-duper useful+dangerous program that ignores all of them: the drop_whatever procedure.

PROCEDURE drop_whatever (nm  IN VARCHAR2 DEFAULT '%',
                         typ IN VARCHAR2 DEFAULT '%')
IS
   CURSOR object_cur
   IS
      SELECT object_name, object_type
        FROM user_objects
       WHERE     object_name LIKE UPPER (nm)
             AND object_type LIKE UPPER (typ)
             AND object_name <> 'DROP_WHATEVER';
BEGIN
   FOR rec IN object_cur
   LOOP
      EXECUTE IMMEDIATE
            'DROP '
         || rec.object_type
         || ' '
         || rec.object_name
         || CASE
               WHEN rec.object_type IN ('TABLE', 'OBJECT')
               THEN
                  ' CASCADE CONSTRAINTS'
               ELSE
                  NULL
            END;
   END LOOP;
END;

In this procedure, I use a static cursor to find all matching objects, then for each object found, I execute a dynamic DDL DROP statement.

It's useful because I can drop all database objects in my schema by typing nothing more than

EXEC drop_whatever()

And it's dangerous for precisely the same reason.

Oh, but wait. Given how useful it is, maybe we should let everyone be able to use it. I know, I will run this command:

GRANT EXECUTE ON drop_whatever TO PUBLIC

Hey what could go wrong? :-)

So very, very much. Let's step through my recommendations and highlight the potential problems.

1. Define your subprogram with AUTHID CURRENT_USER (invokers rights).

The procedure does not have an AUTHID clause (I bet most of your stored program units do not). This means that it defaults to "definer rights". This means that it always executes with the privileges of the definer/owner of the procedure.

Which means that if, say, HR owns drop_whatever and then SCOTT executes it (thank you, GRANT to PUBLIC!) as in:

EXEC HR.drop_whatever()

Then SCOTT will have just dropped all of the database objects in HR's schema!

2. If you're executing dynamic DDL, make the subprogram an autonomous transaction.

The thing about DDL statements is that Oracle performs an implicit commit both before and after the DDL statement is executed. So if you have a stored procedure that executes dynamic DDL, either you have to warn everyone who might use it that any outstanding changes in their session (that's just rude) or you add this statement to your procedure:

PRAGMA AUTONOMOUS_TRANSACTION;

Now, any commits (or rollbacks) executed in the procedure will affect only those changes made within the procedure.

Also, please note the comment from Christian below, which prompted me to add this caution: you should almost never execute DDL in your code, certainly not in production code under the control of users. When you modify DDL, you can cause a ripple of invalidations and disruptions through your application (unless you are using edition-based redefinition!).

3. Always EXECUTE IMMEDIATE or OPEN FOR a variable.

It's such a simple thing, but it could save you lots of time when trying to figure out what's wrong with your program.

Here's the thing: it's not hard to figure how to use EXECUTE IMMEDIATE. But it can be very tricky to properly construct your string at run-time. So many small mistakes can cause errors. And if you construct your string directly within the EXECUTE IMMEDIATE statement, how can you see what was executed and where you might have gone wrong?

Suppose, for example, that in the drop_whatever procedure, I constructed my DROP statement as follows:

EXECUTE IMMEDIATE
   'DROP '
 || rec.object_type
 || rec.object_name ...

When I try to drop my table, I see:

ORA-00950: invalid DROP option

And what does that tell me? Not much. What option does it think I gave it that is invalid? What did I just try to do?

If, on the other hand, I assign the expression I wish to execute to a variable, and then call EXECUTE IMMEDIATE, I can trap the error, and log / display that variable (see second implementation of drop_whatever below). And then I might see something like:

DROP SYNONYMV$SQL - FAILURE

Oh! I see now. I did not include a space between the object type and the object name. Silly me. So always declare a variable, assign the dynamically-constructed SQL statement to that variable, and EXECUTE IMMEDIATE it.

4. Always handle the exception that may arise from the dynamic SQL execution.
5. Log the error information plus the variable that you tried to execute.

If you don't trap the exception, you can't log or display that variable. If you don't persist that variable value, it's awfully hard to make a useful report of the problem to your support team.

You can't do much except whimper at the crappy design of your code.

6. Build a test mode into your subprogram.

I have been writing code for long and screwing up that code for so long, I have learned that it is very helpful - especially when that code makes changes to data in tables - to implement a test mode that doesn't "do" anything. Just shows me what it would have done if I'd let it.

You can see it in the code below, when I pass TRUE (the default) for the just_checking parameter.

A Much Better (?) Drop_Whatever

The "?" in that title is just to remind us that this procedure is inherently dangerous.

Here's the version of drop_whatever following my recommendations. Note that for real, production code, you should never "report" or "log" an error by calling DBMS_OUTPUT.PUT_LINE. Who's going to see that? Instead, call your standard error logging procedure and if you don't have one then get and use Logger.

PROCEDURE drop_whatever (
   nm              IN   VARCHAR2 DEFAULT '%'
 , typ             IN   VARCHAR2 DEFAULT '%'
 , just_checking   IN   BOOLEAN DEFAULT TRUE
)
AUTHID CURRENT_USER
IS
   PRAGMA AUTONOMOUS_TRANSACTION;                             
   dropstr   VARCHAR2 (32767);

   CURSOR object_cur
   IS
      SELECT object_name, object_type
        FROM user_objects
       WHERE object_name LIKE UPPER (nm)
         AND object_type LIKE UPPER (typ)
         AND object_name <> 'DROP_WHATEVER';         
BEGIN
   FOR rec IN object_cur
   LOOP
      dropstr :=
            'DROP '
         || rec.object_type
         || ' '
         || rec.object_name
         || CASE
               WHEN rec.object_type IN ('TABLE', 'OBJECT')
                  THEN ' CASCADE CONSTRAINTS'
               ELSE NULL
            END;                                              

      BEGIN
         IF just_checking
         THEN
            DBMS_OUTPUT.put_line (dropstr || ' - just checking!');
         ELSE
            EXECUTE IMMEDIATE dropstr;
            DBMS_OUTPUT.put_line (dropstr || ' - SUCCESSFUL!');
         END IF;

      EXCEPTION
         WHEN OTHERS
         THEN
            DBMS_OUTPUT.put_line (dropstr || ' - FAILURE!');
            DBMS_OUTPUT.put_line (DBMS_UTILITY.format_error_stack);
      END;
   END LOOP;
END;

As you will see in the comments, Kevan Gelling pointed out the benefit of using a template for your dynamic SQL string, with calls to REPLACE to substitute actual values for placeholders. I agree and offer yet a third implementation of drop_whatever below utilizing the approach (I won't repeat the BEGIN-END encapsulating the EXECUTE IMMEDIATE. That doesn't change.

PROCEDURE drop_whatever (
   nm              IN   VARCHAR2 DEFAULT '%'
 , typ             IN   VARCHAR2 DEFAULT '%'
 , just_checking   IN   BOOLEAN DEFAULT TRUE
)
AUTHID CURRENT_USER
IS
   PRAGMA AUTONOMOUS_TRANSACTION;                             
   dropstr VARCHAR2 (32767) := 'DROP [object_type] [object_name] [cascade]';

   CURSOR object_cur ... same as above ;         
BEGIN
   FOR rec IN object_cur
   LOOP
      dropstr := REPLACE (dropstr, '[object_type]', rec.object_type);
      dropstr := REPLACE (dropstr, '[object_name]', rec.object_name);
      dropstr :=
         REPLACE (
            dropstr,
            '[cascase]',
            CASE
               WHEN rec.object_type IN ('TABLE', 'OBJECT')
               THEN
                  'CASCADE CONSTRAINTS'
            END);                                             

      BEGIN ... EXECUTE IMMEDIATE ... END;
   END LOOP;
END;

You could also do all three of those replaces in a single assignment, but you sacrifice some readability. Thanks, Kevan, for the reminder and the code!

Let's Recap

When you write a stored program unit that contains dynamic SQL:
  1. Define your subprogram with AUTHID CURRENT_USER (invokers rights).
  2. If you're executing dynamic DDL, make the subprogram an autonomous transaction.
  3. Always EXECUTE IMMEDIATE or OPEN FOR a variable.
  4. Always handle the exception that may arise from the dynamic SQL execution.
  5. Log the error information plus the variable that you tried to execute.
  6. Build a test mode into your subprogram.

Comments

  1. My personal preference is to create a full formed SQL string template first (it's easier to read than a screen of concatenated fields, imho) and then replace the placeholders as required.

    For example:

    dropstr := 'DROP [object_type] [object_name] [cascade]';

    dropstr := REPLACE( dropstr, '[object_type]', rec.object_type );
    dropstr := REPLACE( dropstr, '[object_name]', rec.object_name );
    dropstr := REPLACE( dropstr, '[cascase]', CASE WHEN rec.object_type IN ('TABLE', 'OBJECT')
    THEN 'CASCADE CONSTRAINTS' END );

    ReplyDelete
    Replies
    1. Sheesh. Don't you hate it when right after you publish a new post with recommendations, some smarty pants comes up with an even BETTER recommendation? :-)

      Yes, that is a very good idea (and one that @BrynLite promotes heavily as well).

      One nice advantage of this approach is that if, for example, the object_name appears more than once in the string, the REPLACE will take care of them all.

      Thanks, Kevan.

      Delete
    2. Very good tips, Steven! Thanks!

      Although I've never needed to execute a DDL with EXECUTE IMMEDIATE before, I didn't know about the implicit commit before its execution! This is the kind of dangerous thing which is very hard to realize when errors happen!

      Just one suggestion similar to Kevan's one: instead of concatenating the string you could use the utl_lms.format_message() function. It also gives you more readability of the template text.

      What do you think?

      Delete
    3. I've never used utl_lms.format_message, but it looks like an interesting way to go.

      https://docs.oracle.com/en/database/oracle/oracle-database/18/arpls/UTL_LMS.html#GUID-9EBA2B74-FAC1-480D-B416-29F19A47F224

      Thanks, Rafael!

      Delete
  2. Whenever I see Dynamic SQL one very big point is forgotten: check for SQL Injection. In most if not all Dynamic SQL Programs I see SQL Injection is possible in some way. Verify your inputs wherever needed and possible:

    https://xkcd.com/327/

    dbms_assert really makes your life easier. Also v$reserved_words also helps a great deal to check if someone is trying something out.

    And last but not least (which kind of contradicts parts of the article, but has to be said): avoid DDL statements from within PL/SQL at any cost. I would even go so far to say: never ever do it.

    I have written my fair share of PL/SQL programs that do DDL statements to be able to say this safe and sound (our database upgrade program was previously written in Forms PL/SQL and in some cases in Database PL/SQL for reasons that are to much to explain here - but let me tell you that it is all gone now and the whole process gained about 100% stability for not doing this in PL/SQL).

    Even if there is a perfect valid reason for doing DDL from within PL/SQL and you are implementing everything correctly: Do. Not. Do it.
    Maybe someone builds a dependency on the Object you are trying to manipulate in your Package. ORA-4061. ORA-4062. ORA-4063. ORA-4064. ORA-4068. (in some cases even ORA-00600 or ORA-07445 if you really want to solve a good one). Thats when you start having to struggle with suddenly failing processes which inevitably will result in you tearing your hair from your head.

    IF and really IF objects have to be created / modified dynamically in your process involving PL/SQL programs: change the sequence when that has to be done.
    - create / modify the objects in the caller of your PL/SQL API (Java, C#,....).
    - Validate all Objects in the caller program (dbms_utility.compile_schema)
    - Clear the Package state if need be in the caller program (dbms_session.reset_package)
    - Execute your Stored Procedure (which does not do *any* DDL on *any* object involved).

    cheers

    ReplyDelete
    Replies
    1. Great points, Christian. Thanks for writing it up. I will add a warning about DDL in the post.

      As for "never ever do it" - yeah, well, maybe. Certainly in non-production scripts it can have a place and be very useful. But in production code executed under the control of end users? Yep, that would be a very dangerous thing to do.

      Delete

Post a Comment

Popular posts from this blog

Table Functions, Part 1: Introduction and Exploration

Quick Guide to User-Defined Types in Oracle PL/SQL

Recommendations for unit testing PL/SQL programs