Tuesday, November 25, 2014

Feedback on my Planning for Trouble Oracle Magazine article

Received feedback today from James S on my latest Oracle Magazine article, Planning for Trouble. I will respond here (there is no mechanism for comments/discussion at the Oracle Magazine page). If you haven't yet read it, I suggest you do so before proceeding with this post.

Comment 1. This is regarding exception handling.  Let’s say the client calls procedure A, and A calls B, B calls C. Are you suggesting we add a “When others” + error logging logic in each level? Does it mean one error will be logged three times? (because it is captured and re-raised each time).

Good point, James. I make the argument in the article that you should trap the exception as close as possible to where it was raised, so that you can log the values of local variables that may help you diagnose the problem. 

[Here's reinforcement of that point (and other great points about doing effective execution tracing) - they call it contextual logging: http://www.stackify.com/smarter-errors-logs-putting-data-work

But if you apply this rule to A calls B calls C, and C raises an exception, then you could well have three rows inserted into your error log. 

So, first, ask yourself: is this a problem? Generally more info is better than less, but of course you can overwhelmed by too much data.

If you decide that you really don't want that, then you could get "fancy" and in your logging routine set a flag in a package that indicates "already logged". Then in outer blocks, your logger could check the flag and only insert into the log table if it's not already logged.

Does anyone do this now?

Comment 2. I don’t think there’s an issue with the Miguel way (only capture at level A). If you don’t know what will happen, just let it happen. The outer layer or the client can log it and hide it and display a friendly information on the UI.

Well, I guess I was not able to convince James of the value of logging local variable values. What do you think? Is this valuable to you? Am I missing something?

Comment 3: And I don’t think “ORA-00942: table or view does not exist” is a good example. If you try to access a non-existing table your code won’t compile at all (unless you use dynamic sql). And I think Oracle should tell us what is missing. Prior to 10g when you insert a string longer than column width it doesn’t  tell you which column goes wrong! Now the information makes more sense, it not only tells you the column name but also the column width and actual length of value.

Well, your "unless" certainly indicates it could be a runtime issue for logging, and we certainly do write lots of dynamic SQL, these days. As for "I think Oracle should tell us what is missing", I can only tell you that Oracle is following well-accepted guidelines for keeping databases secure. That's specifically for this error. Obviously, as you point out, in other cases, Oracle does give us very excellent context-specific information in the error message. 

Comment 4: Regarding the “assignment in the declaration section”: do we have a choice for constants? It has to be in the declaration right?

You are correct. If you are declaring a constant, it must be done in the declaration section. So you need to assess the risk: am I assigning a value that could have wildly different sizes or format? If so and if I need to trap the exception inside that block and log the problem, then I need to remove the CONSTANT keyword. if you are assigning a literal, then you should consider it part of your job as a disciplined professional to READ YOUR CODE and make sure you are not making a blooper as in:

c_number CONSTANT NUMBER(3) := 1234;

Wednesday, November 19, 2014

Planning for trouble: comments on my latest Oracle Magazine article.

In my November/December 2014 article for Oracle Magazine, Planning for Trouble, I urge developers to realize that regardless of best intentions, not everything related to our apps is under our control, and we need to assume that trouble might be coming our way.

I received today, the following comments from Gary Malandro, which I thought you might enjoy reading:

Enjoyed your article in Oracle Magazine, and I have a few comments.

1.       You mentioned “Documents that spell out naming conventions…fit very nicely inside desk drawers”.  On our team, we have a number of policies that include the requirements for a technical design document, a software change request, source control, and some regarding style.  First thought upon hearing that is probably what-a-load-of-bureaucratic-nonsense.  Well, there are reasons for creating and enforcing these standards.  Compliance for one.  Another is we had code going into production systems that performed poorly, contained logic errors, was difficult to understand, already existed, no exception handling, etc.  With standards, we are able to peer review designs and code to make sure it is understood.  These are necessary when doing reviews otherwise it is a matter of style between developers – what 1 developer thinks is cool another finds difficult to understand.  Anyway, since we’ve implemented these policies our defect rate to test and error rate to production has improved considerably.  Besides PLSQL, we also develop in Java and .Net.  I’ve heard all the excuses as to the evil of standards and documentation (and probably said them many times myself), but the people who say this typically have to rework their code multiple times.  So pay now or pay later. 

2.       In my previous role I was supporting/developing a large volume of PL/SQL code.  I created a separate package (XXSA_DEBUG) which contained procedures to output data in various ways (a very, very, very poor man’s log4j).  In procedures I would include a way to get a debug variable and could either decide to call the output package, or just directly call it and let it decide where/how to output.   Production access is limited, so being able to change the debug flag using an application, or even running with a stub program, greatly aided when troubleshooting was required.

3.       Another thing we enforce is exception handling with clear messages.  If it’s something going to a log file or table (aimed for the developer) it should contain the procedure name and where the error occurred.  This allows any developer to trace where the error occurred.  Too many times we had log files loaded with “Failed”, and no explanation.  And thousands of lines of code that contained “Failed” for the output message. 

Thanks, Gary, for taking the time to share your experience. I would be very happy to hear from others!

Thursday, November 13, 2014

FORALLs and COMMITs

Received this note today from Debbie B:

I attended your Turbo Charge PL/SQL seminar today. I still have a question about where to put COMMIT. I’m using 10g. See (pseudo) code below.

Say I have 50,000 records and the LIMIT is 100. If an exception is thrown:
  • Do I need a COMMIT in the WHEN clauses so successful DML gets committed, then loop processing will continue?
  • I was thinking each DML needed it’s own BEGIN EXCEPTION END block so I would know if the error happened due to the insert or the update and could log the appropriate error. Is this wrong?

  OPEN v_cursor;
  LOOP
    FETCH v_cursor BULK COLLECT INTO data_array LIMIT i_limit;
    EXIT WHEN data_array.COUNT = 0;

    BEGIN
      FORALL i IN 1.. data_array.COUNT SAVE EXCEPTIONS
        INSERT INTO some_table VALUES data_array (i);
        COMMIT;
    EXCEPTION
        WHEN dml_errors THEN
             log_error;
             COMMIT;
             /* Don't RAISE, so execution will continue */
        WHEN OTHERS
             log_error;
             COMMIT;
             /* Don't RAISE, so execution will continue */
    END;

    BEGIN
      FORALL i IN 1.. data_array.COUNT SAVE EXCEPTIONS
        UPDATE some_table SET…
        COMMIT;
    EXCEPTION
        WHEN dml_errors THEN
             log_error;
             COMMIT;
             /* Don't RAISE, so execution will continue */
        WHEN OTHERS
             log_error;
             COMMIT;
             /* Don't RAISE, so execution will continue */
    END
  END LOOP;
  COMMIT;
  CLOSE v_cursor;

So many commits, so little time. 

Before addressing the specifics of this code, let's consider a more general question:

When and where should I put commits in my code?

Answer: How the heck should I know? Or, perhaps a little more politely: What is your transaction?

When you commit, you are really saying: I have finished making all changes needed for this transaction. Now it's time to save them, all together, all at once.

When we write back-end code that acts as an API to front end (user-driven) code, we usually don't have any commits. That's because we leave it up to the user to decide when they are done and ready to save their changes.

When we write code implementing backend processes not controlled by the user, then we do usually need to explicitly commit (and rollback). But again the primary question is very application-specific: what constitutes a transaction? 

Of course, the real world is messy - even when we clean it up, shear off rough edges, and make that real world fit into our cyber world.

For example, sometimes we have to do "incremental commits" - save changes to N rows at a time, to avoid rollback segment errors.

That may be the reason that Debbie has put COMMIT statements after each FORALL and after each logging of an error. 

But do you need a COMMIT in that exception section to ensure that successfully executed statements are not rolled back? NO! The failure of a SQL statement in your block will not force the rollback of previously completed statements.

As for putting each FORALL inside its own block, sure, that makes a lot of sense - if you want to make sure that both FORALLs always execute for each iteration of the loop. I would, however, put each inside its own nested subprogram (see my rewritten version of your code below).

I probably would not, however, use the same exceptional handling strategy for ORA-24381 (which your code implies was associated with the dml_errors exception) and for WHEN OTHERS. If something else went wrong in FORALL execution that was not caught by SAVE EXCEPTIONS, you should consider it "catastrophic" and stop the processing.

And, by the way, so far as I can tell, that final COMMIT before the CLOSE statement? Totally unnecessary - if you keep all those other COMMITs. You've already covered every path out of the loop!

OK, here's my offering of an alternative implementation. It's still kinda pseudo-codish because I lack the full details of your requirements, but:
  • Just one COMMIT at the end
  • Nested subprograms for each FORALL
  • No handling of unanticipated errors.

IS
   failure_in_forall   EXCEPTION;
   PRAGMA EXCEPTION_INIT (failure_in_forall, -24381);

   PROCEDURE bulk_inserts (data_array_in your_type)
   IS
   BEGIN
      FORALL i IN 1 .. data_array_in.COUNT SAVE EXCEPTIONS
         INSERT INTO some_table
              VALUES data_array_in (i);
   EXCEPTION
      WHEN failure_in_forall
      THEN
         log_error;
   END;

   PROCEDURE bulk_updates (data_array_in your_type)
   IS
   BEGIN
      FORALL i IN 1 .. data_array.COUNT SAVE EXCEPTIONS
         UPDATE some_table
            SET x = data_array (i);
   EXCEPTION
      WHEN failure_in_forall
      THEN
         log_error;
   END;
BEGIN
   OPEN v_cursor;

   LOOP
      FETCH v_cursor BULK COLLECT INTO data_array LIMIT i_limit;

      EXIT WHEN data_array.COUNT = 0;

      bulk_inserts (data_array);
      bulk_updates (data_array);
   END LOOP;

   CLOSE v_cursor;

   COMMIT;
END;