Обсуждение: plpgsql FOR loop doesn't guard against strange step values
I just noticed that when the BY option was added to plpgsql FOR loops, no real error checking was done. If you specify a zero step value, you'll have an infinite loop. If you specify a negative value, the loop variable will increment in the "wrong direction" until integer overflow occurs. Neither of these behaviors seem desirable in the least. Another problem is that no check for overflow is done when incrementing the loop variable, which means that an infinite loop is possible if the step value is larger than the distance from the loop upper bound to INT_MAX --- the loop variable could overflow before it is seen to be greater than the upper bound, and after wrapping around to negative it's still less than the upper bound, so the loop continues to run. I suggest throwing an error for zero or negative step value, and terminating the loop if the loop variable overflows. Any objections? regards, tom lane
Tom Lane wrote: > I just noticed that when the BY option was added to plpgsql FOR > loops, no real error checking was done. If you specify a zero step > value, you'll have an infinite loop. If you specify a negative > value, the loop variable will increment in the "wrong direction" > until integer overflow occurs. Neither of these behaviors seem > desirable in the least. That seems to be fairly normal proramming language behavior. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut <peter_e@gmx.net> writes: > Tom Lane wrote: >> I just noticed that when the BY option was added to plpgsql FOR >> loops, no real error checking was done. If you specify a zero step >> value, you'll have an infinite loop. If you specify a negative >> value, the loop variable will increment in the "wrong direction" >> until integer overflow occurs. Neither of these behaviors seem >> desirable in the least. > That seems to be fairly normal proramming language behavior. Well, it's about what I'd expect from C or something at a similar level of (non) abstraction. But I dislike the idea that plpgsql should have behavior as machine-dependent as that the number of iterations will depend on the value of INT_MIN. Also, at the SQL level our usual policy is to throw errors for obvious programmer mistakes, and it's hard to argue that a zero or negative step isn't a programmer mistake. Had we defined the stepping behavior differently (ie, make "BY -1" work like REVERSE) then there would be some sanity in allowing negative steps, but I don't see the sanity in it given the implemented behavior. regards, tom lane
Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > >> Tom Lane wrote: >> >>> I just noticed that when the BY option was added to plpgsql FOR >>> loops, no real error checking was done. If you specify a zero step >>> value, you'll have an infinite loop. If you specify a negative >>> value, the loop variable will increment in the "wrong direction" >>> until integer overflow occurs. Neither of these behaviors seem >>> desirable in the least. >>> > > >> That seems to be fairly normal proramming language behavior. >> > > Well, it's about what I'd expect from C or something at a similar level > of (non) abstraction. But I dislike the idea that plpgsql should have > behavior as machine-dependent as that the number of iterations will > depend on the value of INT_MIN. Also, at the SQL level our usual policy > is to throw errors for obvious programmer mistakes, and it's hard to > argue that a zero or negative step isn't a programmer mistake. Had we > defined the stepping behavior differently (ie, make "BY -1" work like > REVERSE) then there would be some sanity in allowing negative steps, > but I don't see the sanity in it given the implemented behavior. > I suspect we have a significant incompatibility with PLSQL in this area. The docs give this example: FOR i IN REVERSE 10..1 LOOP -- some computations here END LOOP; In PLSQL, as I understand it, (and certainly in its ancestor Ada) this loop will execute 0 times, not 10. To iterate from10 down to 1 one would need to say: FOR i IN REVERSE 1..10 LOOP -- some computations here END LOOP; I'm not sure if this has been noticed before. It's actually quite unfortunate. At least it should be mentioned in the sectionof the docs relating to porting from PLSQL. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes: > I suspect we have a significant incompatibility with PLSQL in this area. Ugh. Google seems to confirm your thought that Oracle expects > FOR i IN REVERSE 1..10 LOOP which is not the way we are doing it. Not sure if it's worth trying to fix this --- the conversion pain would be significant. I agree we gotta document it, however; will go do so. Note that in the Oracle worldview it still wouldn't be sensible to use a negative step. regards, tom lane
Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > >> I suspect we have a significant incompatibility with PLSQL in this area. >> > > Ugh. Google seems to confirm your thought that Oracle expects > > >> FOR i IN REVERSE 1..10 LOOP >> > > which is not the way we are doing it. Not sure if it's worth trying to > fix this --- the conversion pain would be significant. I agree we gotta > document it, however; will go do so. > > Note that in the Oracle worldview it still wouldn't be sensible to use > a negative step. > > > Quite so. I think we should probably require the step to be greater than 0, whether or not we are using REVERSE, and choose to use it as an increment or decrement as appropriate. cheers andrew
On 7/14/07, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I just noticed that when the BY option was added to plpgsql FOR loops, > no real error checking was done. If you specify a zero step value, > you'll have an infinite loop. If you specify a negative value, the > loop variable will increment in the "wrong direction" until integer > overflow occurs. Neither of these behaviors seem desirable in the > least. > while i read this the day you posted it, i didn't have time to answer until now... my answer is: sorry, my bad... have to admit that the idea of preventing zero in the BY doesn't cross my mind. http://archives.postgresql.org/pgsql-hackers/2006-04/msg01100.php i remember my original proposal include that negative values shouldn't be allowed, i don't know where my way was corrupted... maybe because for statement didn't make any effort to prevent things like: FOR i IN 10..1 LOOP > Another problem is that no check for overflow is done when incrementing > the loop variable, which means that an infinite loop is possible if the > step value is larger than the distance from the loop upper bound to > INT_MAX --- the loop variable could overflow before it is seen to be > greater than the upper bound, and after wrapping around to negative > it's still less than the upper bound, so the loop continues to run. > mmm... yeah! > I suggest throwing an error for zero or negative step value, and > terminating the loop if the loop variable overflows. > http://archives.postgresql.org/pgsql-committers/2007-07/msg00142.php at least the part that prevents overflow and probably the one that reject zero in BY are clearly bugs and should be backpatched to 8.2, aren't they? -- regards, Jaime Casanova "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs and the universe trying to produce bigger and better idiots. So far, the universe is winning." Richard Cook
"Jaime Casanova" <systemguards@gmail.com> writes: > http://archives.postgresql.org/pgsql-committers/2007-07/msg00142.php > at least the part that prevents overflow and probably the one that > reject zero in BY are clearly bugs and should be backpatched to 8.2, > aren't they? Well, it's a behavioral change, so given the lack of complaints from the field I'm inclined not to back-patch. regards, tom lane