22 July 2011

Valid partial step in optimizing code? (4482)

In the Q2 2011 playoff, one of the questions examined the nuances of using BULK COLLECT and FORALL to optimize one's code. We asked players "Which of the statements below describe a complete or partial step towards improving the performance of this program, while keeping intact all current functionality?"

And we scored as incorrect this choice:

"Keep the cursor FOR loop in place (they are automatically optimized by Oracle into BULK COLLECT levels of performance). Simply replace the two DML statements with FORALL statements within that cursor FOR loop."

A player objected, feeling that this should be scored as correct, arguing:

"I checked this answer as correct because, as another player mentioned, the question did ask “complete or partial step towards improving the performance of this program”. It is not a solution, of course, because there needs to be a bulk collect off the cursor to create the collection(s) necessary for a forall statement to work. Of course, that would also be a “step”. But this answer also is a “partial step towards improving the performance”. I think that this answer should be scored as correct. I interpreted the word “step” to mean “one of things that I would need to do to improve the performance”. To make this answer incorrect, I would have added the following clarifying language to the question: “and resulting in a working program that compiles without error”. But the English language is much more ambiguous than the PL/SQL language."

Another player wrote to say about two other of the choices:

"I have to disagree with the answers to this question. The question clearly states "Which of the statements below describe a complete OR PARTIAL step towards improving the performance of this program, while keeping intact all current functionality?" (emphasis mine). Answers that describe a reasonable step that the programmer may take, even if they are not complete, should still be marked correct in my opinion. For example: "Replace the two DML statements (insert and update) with one FORALL statement that executes those same two statements, using data in collections populated by a BULK COLLECT." - you object to this answer because it doesn't completely fulfill the requirements. But a FORALL statement could most definitely be used to improve the performance - sure, it doesn't completely fulfill the requirements, the programmer has to do more than that, but it is still a partial step towards improving the program. Similarlarly, "Add a LOG ERRORS to both DML statements and put them inside FORALL statements." - this is a good partial step. All I have to do is add some code to raise an exception if I find an error in SQL%BULK_EXCEPTIONS. The other answers were steps in the *wrong* direction, and were correctly marked as wrong for that reason."

My responses:

First, I recognize the problematic aspect of including "or partial" in our question text. We actually included it so that a player couldn't argue that a single choice was or wasn't a complete solution. Ah, well...

Still, I am ready to defend with full vigor my scoring. Here goes:

"Keep the cursor FOR loop in place (they are automatically optimized by Oracle into BULK COLLECT levels of performance). Simply replace the two DML statements with FORALL statements within that cursor FOR loop." [7281]

It sounds like this rationale for marking this as correct is:

Sure, Steven, you say you will keep the cursor for loop "in place" but that's just for now. Later you will come back and change it to a bulk collect. So it is a partial step.

I certainly do understand that rationale. But I also find myself objecting to it. Why? Because the language of the choice implies (strongly, to my mind) that this is not simply a transitional step. It is a misunderstanding of the technology. Compare the text of answer 7281 to this:

Change the two DML statements into FORALL statements within the cursor FOR loop.

This actually still makes little sense (what collection would "feed" the FORALL statement?), but one could argue that the bulk collect step is "next." OK.

But in 7281, I explicitly say I will keep the cursor for loop "in place" and even include a parenthetical clause explaining why - this sentence is saying "This developer thinks that because the cursor FOR loop is auto-optimized, there is no need to change it. Then I include "Simply" in the second sentence to imply that I will do nothing more.

I realize that I am now having a discussion about English syntax and semantics, not PL/SQL code per se. But that is certainly the rationale for why I wrote it that way. I find it hard to read that choice and think to myself, "Oh, that is just the first step in a series planned by this person."

On to the others:

Replace the two DML statements (insert and update) with one FORALL statement that executes those same two statements, using data in collections populated by a BULK COLLECT. [7282]

I don't see how this could be interpreted as a partial/transitional step towards an improved program. You absolutely cannot replace these two DML statements with a single FORALL statement. Again, that reveals a misunderstanding of FORALL. You can only have one DML statement in each FORALL. So how can this be a partial step? You'd have to undo everything you do in this step to get it to work!

Add a LOG ERRORS to both DML statements and put them inside FORALL statements. [7286]

Again, I don't see how to recognize this as a valid step towards a properly refactored program. You cannot use LOG ERRORS without changing the behavior of the program. LOG ERRORS suppresses errors at the row level, and we need to do statement-level suppression of exceptions. You certainly can't add code using SQL%BULK_EXCEPTIONS, because when you use LOG ERRORS, no exceptions are raised at all. So that pseudo collection will remain empty.

Your thoughts?

6 comments:

  1. I agree, I think that it is a misunderstanding of the technology 1) to attempt to use multiple SQL statements inside one FORALL statement, which is incorrect syntax, or 2) to add LOG ERRORS, which changes the behavior of the program.

    However, it is possible (and I have done so before) to use bulk collect explicitly inside the cursor for loop, to create collections for the purpose of using a FORALL statement later in the loop.

    I believe that this is a valid "partial step" towards improving the performance of the program without changing the behavior of the program.

    ReplyDelete
  2. Randy, I can definitely see a use case for having a bulk collect inside a cursor FOR loop, to then feed the FORALLs...but I don't see how that use case applies to this specific program.

    ReplyDelete
  3. You demonstrated in an earlier question (Sept 15 2010), you can use a single FORALL for multiple DML statements using EXECUTE IMMEDIATE and an anonymous PL/SQL block.

    You also pointed out at the time, it negates any performance improvement so it isn't a valid choice for this question.

    http://plsql-challenge.blogspot.com/2010/09/typo-in-15-september-quiz-but-no-need.html

    ReplyDelete
  4. Hello All,
    First, many congratulations to Gary for winning this playoff !

    Regarding this quiz, it is hard to say what exactly can be considered a "partial" valid step towards first of all a WORKING solution, and afterwards towards a FASTER WORKING solution.

    Maybe an approach which would have won a wider acceptance could have been to offer several COMPLETE solutions, correct and incorrect, and to ask which one of them will definitely perform faster, though, this would have probably made the overall code a little bit too long.

    Maybe part of the frustration of many players regarding this quiz relies in the fact that none of the 6 choices was correct, which is quite unusual for a playoff.

    The solution presented in the answer is very nice and it goes along the lines that I thought of when I marked as correct the choice suggesting the 2 FORALL loops (again, as a PARTIAL idea, not completely elaborated).

    But I think I already can suggest two other possible solutions ... maybe in the next playoff :) :) :).

    Problems usually have several good solutions,
    otherwise life would have been too simple ...

    I think it probably should be extremely exciting to be in the Reviewers team for a playoff quiz, hope to share that honor one day ...

    I still expect a blog thread regarding the PLSQL_WARNINGS quiz, where the objections are much more "straight-forward and technical".

    Thanks & Best Regards,
    Iudith

    ReplyDelete
  5. Sure, Iudith, I will post a separate blog about the warnings quiz.

    Regarding "Maybe part of the frustration of many players regarding this quiz...", I would just like to point out that 84% of the people taking the playoff chose "incorrect" for this choice.

    ReplyDelete
  6. I have decided to give all playoff participants credit for choice 7281. My argument requires too much "slicing and dicing" of the meanings of words and the context in which they are used. It is not sufficiently clear.

    ReplyDelete