Gavin Sherry <[EMAIL PROTECTED]> writes:
> On Tue, 3 Aug 2004, Jeff Davis wrote:
>> I have a question for you also. I just posted a patch at about the same
>> time you did (I sent it to pgsql-patches, but I haven't seen it appear
>> yet). Mine was a one-liner (appended to end of this email) and all it
>> did was add a check into the aforementioned line for a non-null target
>> pointer. My patch seemed to work, so I'd like to know why you changed
>> the structure around more. I did notice some things were a little
>> cleaner, so was it just clean-up or does my patch fail in some way?

> Just a clean up.

Actually, there's an easier fix than either of these, which is just to
pull the savepointLevel test out of the loop and only do it after we've
found a candidate savepoint.  There's no point in checking the level at
every loop iteration (especially since the feature isn't even being used
yet, as noted).

Attached is the patch I just applied (which also includes my own notions
about how to make the loop prettier...)

                        regards, tom lane


*** src/backend/access/transam/xact.c.orig      Sun Aug  1 16:57:59 2004
--- src/backend/access/transam/xact.c   Tue Aug  3 11:53:41 2004
***************
*** 2520,2541 ****
  
        Assert(PointerIsValid(name));
  
!       target = CurrentTransactionState;
! 
!       while (target != NULL)
        {
                if (PointerIsValid(target->name) && strcmp(target->name, name) == 0)
                        break;
-               target = target->parent;
- 
-               /* we don't cross savepoint level boundaries */
-               if (target->savepointLevel != s->savepointLevel)
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
-                                        errmsg("no such savepoint")));
        }
  
        if (!PointerIsValid(target))
                ereport(ERROR,
                                (errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
                                 errmsg("no such savepoint")));
--- 2520,2538 ----
  
        Assert(PointerIsValid(name));
  
!       for (target = s; PointerIsValid(target); target = target->parent)
        {
                if (PointerIsValid(target->name) && strcmp(target->name, name) == 0)
                        break;
        }
  
        if (!PointerIsValid(target))
+               ereport(ERROR,
+                               (errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
+                                errmsg("no such savepoint")));
+ 
+       /* disallow crossing savepoint level boundaries */
+       if (target->savepointLevel != s->savepointLevel)
                ereport(ERROR,
                                (errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
                                 errmsg("no such savepoint")));

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

               http://www.postgresql.org/docs/faqs/FAQ.html

Reply via email to