Thread: Better testing coverage and unified coding for plpgsql loops

Better testing coverage and unified coding for plpgsql loops

From
Tom Lane
Date:
While I've been fooling around with plpgsql, I've been paying close
attention to code coverage reports to make sure that the regression tests
exercise all that new code.  It started to bug me that there were some
serious gaps in the test coverage for existing code in pl_exec.c.
One thing I noticed in particular was that coverage for the
PLPGSQL_RC_EXIT/PLPGSQL_RC_RETURN/PLPGSQL_RC_CONTINUE management code
in the various looping statements was nearly nonexistent, and coverage
for integer FOR loops was pretty awful too (eg, not a single BY clause
in the whole test corpus :-().  So I said to myself "let's fix that",
and wrote up a new regression test file plpgsql_control.sql with a
charter to test plpgsql's control structures.  I moved a few existing
tests that seemed to fall into that charter into the new file, and
added tests until I was happier with the state of the coverage report.
The result is attached as plpgsql-better-code-coverage.patch.

However, while I was doing that, it seemed like the tests I was adding
were mighty repetitive, as many of them were just exactly the same thing
adjusted for a different kind of loop statement.  And so I began to wonder
why it was that we had five copies of the RC_FOO management logic, no two
quite alike.  If we only had *one* copy then it would not seem necessary
to have such duplicative test cases for it.  A bit of hacking later, and
I had the management logic expressed as a macro, with only one copy for
all five kinds of loop.  I verified it still passes the previous set of
tests and then removed the ones that seemed redundant, yielding
plpgsql-unify-loop-rc-code.patch below.  So what I propose actually
committing is the combination of these two patches.

Anyone feel like reviewing this, or should I just push it?  It seems
pretty noncontroversial to me, but maybe I'm wrong about that.

            regards, tom lane

diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile
index 76ac247..14a4d83 100644
*** a/src/pl/plpgsql/src/Makefile
--- b/src/pl/plpgsql/src/Makefile
*************** DATA = plpgsql.control plpgsql--1.0.sql
*** 26,32 ****

  REGRESS_OPTS = --dbname=$(PL_TESTDB)

! REGRESS = plpgsql_call

  all: all-lib

--- 26,32 ----

  REGRESS_OPTS = --dbname=$(PL_TESTDB)

! REGRESS = plpgsql_call plpgsql_control

  all: all-lib

diff --git a/src/pl/plpgsql/src/expected/plpgsql_control.out b/src/pl/plpgsql/src/expected/plpgsql_control.out
index ...b7089e3 .
*** a/src/pl/plpgsql/src/expected/plpgsql_control.out
--- b/src/pl/plpgsql/src/expected/plpgsql_control.out
***************
*** 0 ****
--- 1,833 ----
+ --
+ -- Tests for PL/pgSQL control structures
+ --
+ -- integer FOR loop
+ do $$
+ begin
+   -- basic case
+   for i in 1..3 loop
+     raise notice '1..3: i = %', i;
+   end loop;
+   -- with BY, end matches exactly
+   for i in 1..10 by 3 loop
+     raise notice '1..10 by 3: i = %', i;
+   end loop;
+   -- with BY, end does not match
+   for i in 1..11 by 3 loop
+     raise notice '1..11 by 3: i = %', i;
+   end loop;
+   -- zero iterations
+   for i in 1..0 by 3 loop
+     raise notice '1..0 by 3: i = %', i;
+   end loop;
+   -- REVERSE
+   for i in reverse 10..0 by 3 loop
+     raise notice 'reverse 10..0 by 3: i = %', i;
+   end loop;
+   -- potential overflow
+   for i in 2147483620..2147483647 by 10 loop
+     raise notice '2147483620..2147483647 by 10: i = %', i;
+   end loop;
+   -- potential overflow, reverse direction
+   for i in reverse -2147483620..-2147483647 by 10 loop
+     raise notice 'reverse -2147483620..-2147483647 by 10: i = %', i;
+   end loop;
+ end$$;
+ NOTICE:  1..3: i = 1
+ NOTICE:  1..3: i = 2
+ NOTICE:  1..3: i = 3
+ NOTICE:  1..10 by 3: i = 1
+ NOTICE:  1..10 by 3: i = 4
+ NOTICE:  1..10 by 3: i = 7
+ NOTICE:  1..10 by 3: i = 10
+ NOTICE:  1..11 by 3: i = 1
+ NOTICE:  1..11 by 3: i = 4
+ NOTICE:  1..11 by 3: i = 7
+ NOTICE:  1..11 by 3: i = 10
+ NOTICE:  reverse 10..0 by 3: i = 10
+ NOTICE:  reverse 10..0 by 3: i = 7
+ NOTICE:  reverse 10..0 by 3: i = 4
+ NOTICE:  reverse 10..0 by 3: i = 1
+ NOTICE:  2147483620..2147483647 by 10: i = 2147483620
+ NOTICE:  2147483620..2147483647 by 10: i = 2147483630
+ NOTICE:  2147483620..2147483647 by 10: i = 2147483640
+ NOTICE:  reverse -2147483620..-2147483647 by 10: i = -2147483620
+ NOTICE:  reverse -2147483620..-2147483647 by 10: i = -2147483630
+ NOTICE:  reverse -2147483620..-2147483647 by 10: i = -2147483640
+ -- BY can't be zero or negative
+ do $$
+ begin
+   for i in 1..3 by 0 loop
+     raise notice '1..3 by 0: i = %', i;
+   end loop;
+ end$$;
+ ERROR:  BY value of FOR loop must be greater than zero
+ CONTEXT:  PL/pgSQL function inline_code_block line 3 at FOR with integer loop variable
+ do $$
+ begin
+   for i in 1..3 by -1 loop
+     raise notice '1..3 by -1: i = %', i;
+   end loop;
+ end$$;
+ ERROR:  BY value of FOR loop must be greater than zero
+ CONTEXT:  PL/pgSQL function inline_code_block line 3 at FOR with integer loop variable
+ do $$
+ begin
+   for i in reverse 1..3 by -1 loop
+     raise notice 'reverse 1..3 by -1: i = %', i;
+   end loop;
+ end$$;
+ ERROR:  BY value of FOR loop must be greater than zero
+ CONTEXT:  PL/pgSQL function inline_code_block line 3 at FOR with integer loop variable
+ -- CONTINUE statement
+ create table conttesttbl(idx serial, v integer);
+ insert into conttesttbl(v) values(10);
+ insert into conttesttbl(v) values(20);
+ insert into conttesttbl(v) values(30);
+ insert into conttesttbl(v) values(40);
+ create function continue_test1() returns void as $$
+ declare _i integer = 0; _r record;
+ begin
+   raise notice '---1---';
+   loop
+     _i := _i + 1;
+     raise notice '%', _i;
+     continue when _i < 10;
+     exit;
+   end loop;
+
+   raise notice '---2---';
+   <<lbl>>
+   loop
+     _i := _i - 1;
+     loop
+       raise notice '%', _i;
+       continue lbl when _i > 0;
+       exit lbl;
+     end loop;
+   end loop;
+
+   raise notice '---3---';
+   <<the_loop>>
+   while _i < 10 loop
+     _i := _i + 1;
+     continue the_loop when _i % 2 = 0;
+     raise notice '%', _i;
+   end loop;
+
+   raise notice '---4---';
+   for _i in 1..10 loop
+     begin
+       -- applies to outer loop, not the nested begin block
+       continue when _i < 5;
+       raise notice '%', _i;
+     end;
+   end loop;
+
+   raise notice '---5---';
+   for _r in select * from conttesttbl loop
+     continue when _r.v <= 20;
+     raise notice '%', _r.v;
+   end loop;
+
+   raise notice '---6---';
+   for _r in execute 'select * from conttesttbl' loop
+     continue when _r.v <= 20;
+     raise notice '%', _r.v;
+   end loop;
+
+   raise notice '---7---';
+   <<looplabel>>
+   for _i in 1..3 loop
+     continue looplabel when _i = 2;
+     raise notice '%', _i;
+   end loop;
+
+   raise notice '---8---';
+   _i := 1;
+   while _i <= 3 loop
+     raise notice '%', _i;
+     _i := _i + 1;
+     continue when _i = 3;
+   end loop;
+
+   raise notice '---9---';
+   for _r in select * from conttesttbl order by v limit 1 loop
+     raise notice '%', _r.v;
+     continue;
+   end loop;
+
+   raise notice '---10---';
+   for _r in execute 'select * from conttesttbl order by v limit 1' loop
+     raise notice '%', _r.v;
+     continue;
+   end loop;
+
+   raise notice '---11---';
+   <<outerlooplabel>>
+   for _i in 1..2 loop
+     raise notice 'outer %', _i;
+     <<innerlooplabel>>
+     for _j in 1..3 loop
+       continue outerlooplabel when _j = 2;
+       raise notice 'inner %', _j;
+     end loop;
+   end loop;
+ end; $$ language plpgsql;
+ select continue_test1();
+ NOTICE:  ---1---
+ NOTICE:  1
+ NOTICE:  2
+ NOTICE:  3
+ NOTICE:  4
+ NOTICE:  5
+ NOTICE:  6
+ NOTICE:  7
+ NOTICE:  8
+ NOTICE:  9
+ NOTICE:  10
+ NOTICE:  ---2---
+ NOTICE:  9
+ NOTICE:  8
+ NOTICE:  7
+ NOTICE:  6
+ NOTICE:  5
+ NOTICE:  4
+ NOTICE:  3
+ NOTICE:  2
+ NOTICE:  1
+ NOTICE:  0
+ NOTICE:  ---3---
+ NOTICE:  1
+ NOTICE:  3
+ NOTICE:  5
+ NOTICE:  7
+ NOTICE:  9
+ NOTICE:  ---4---
+ NOTICE:  5
+ NOTICE:  6
+ NOTICE:  7
+ NOTICE:  8
+ NOTICE:  9
+ NOTICE:  10
+ NOTICE:  ---5---
+ NOTICE:  30
+ NOTICE:  40
+ NOTICE:  ---6---
+ NOTICE:  30
+ NOTICE:  40
+ NOTICE:  ---7---
+ NOTICE:  1
+ NOTICE:  3
+ NOTICE:  ---8---
+ NOTICE:  1
+ NOTICE:  2
+ NOTICE:  3
+ NOTICE:  ---9---
+ NOTICE:  10
+ NOTICE:  ---10---
+ NOTICE:  10
+ NOTICE:  ---11---
+ NOTICE:  outer 1
+ NOTICE:  inner 1
+ NOTICE:  outer 2
+ NOTICE:  inner 1
+  continue_test1
+ ----------------
+
+ (1 row)
+
+ -- should fail: CONTINUE is only legal inside a loop
+ create function continue_error1() returns void as $$
+ begin
+     begin
+         continue;
+     end;
+ end;
+ $$ language plpgsql;
+ ERROR:  CONTINUE cannot be used outside a loop
+ LINE 4:         continue;
+                 ^
+ -- should fail: unlabeled EXIT is only legal inside a loop
+ create function exit_error1() returns void as $$
+ begin
+     begin
+         exit;
+     end;
+ end;
+ $$ language plpgsql;
+ ERROR:  EXIT cannot be used outside a loop, unless it has a label
+ LINE 4:         exit;
+                 ^
+ -- should fail: no such label
+ create function continue_error2() returns void as $$
+ begin
+     begin
+         loop
+             continue no_such_label;
+         end loop;
+     end;
+ end;
+ $$ language plpgsql;
+ ERROR:  there is no label "no_such_label" attached to any block or loop enclosing this statement
+ LINE 5:             continue no_such_label;
+                              ^
+ -- should fail: no such label
+ create function exit_error2() returns void as $$
+ begin
+     begin
+         loop
+             exit no_such_label;
+         end loop;
+     end;
+ end;
+ $$ language plpgsql;
+ ERROR:  there is no label "no_such_label" attached to any block or loop enclosing this statement
+ LINE 5:             exit no_such_label;
+                          ^
+ -- should fail: CONTINUE can't reference the label of a named block
+ create function continue_error3() returns void as $$
+ begin
+     <<begin_block1>>
+     begin
+         loop
+             continue begin_block1;
+         end loop;
+     end;
+ end;
+ $$ language plpgsql;
+ ERROR:  block label "begin_block1" cannot be used in CONTINUE
+ LINE 6:             continue begin_block1;
+                              ^
+ -- On the other hand, EXIT *can* reference the label of a named block
+ create function exit_block1() returns void as $$
+ begin
+     <<begin_block1>>
+     begin
+         loop
+             exit begin_block1;
+             raise exception 'should not get here';
+         end loop;
+     end;
+ end;
+ $$ language plpgsql;
+ select exit_block1();
+  exit_block1
+ -------------
+
+ (1 row)
+
+ -- verbose end block and end loop
+ create function end_label1() returns void as $$
+ <<blbl>>
+ begin
+   <<flbl1>>
+   for i in 1 .. 10 loop
+     raise notice 'i = %', i;
+     exit flbl1;
+   end loop flbl1;
+   <<flbl2>>
+   for j in 1 .. 10 loop
+     raise notice 'j = %', j;
+     exit flbl2;
+   end loop;
+ end blbl;
+ $$ language plpgsql;
+ select end_label1();
+ NOTICE:  i = 1
+ NOTICE:  j = 1
+  end_label1
+ ------------
+
+ (1 row)
+
+ -- should fail: undefined end label
+ create function end_label2() returns void as $$
+ begin
+   for _i in 1 .. 10 loop
+     exit;
+   end loop flbl1;
+ end;
+ $$ language plpgsql;
+ ERROR:  end label "flbl1" specified for unlabelled block
+ LINE 5:   end loop flbl1;
+                    ^
+ -- should fail: end label does not match start label
+ create function end_label3() returns void as $$
+ <<outer_label>>
+ begin
+   <<inner_label>>
+   for _i in 1 .. 10 loop
+     exit;
+   end loop outer_label;
+ end;
+ $$ language plpgsql;
+ ERROR:  end label "outer_label" differs from block's label "inner_label"
+ LINE 7:   end loop outer_label;
+                    ^
+ -- should fail: end label on a block without a start label
+ create function end_label4() returns void as $$
+ <<outer_label>>
+ begin
+   for _i in 1 .. 10 loop
+     exit;
+   end loop outer_label;
+ end;
+ $$ language plpgsql;
+ ERROR:  end label "outer_label" specified for unlabelled block
+ LINE 6:   end loop outer_label;
+                    ^
+ -- unlabeled exit matches no blocks
+ do $$
+ begin
+ for i in 1..10 loop
+   <<innerblock>>
+   begin
+     begin  -- unlabeled block
+       exit;
+       raise notice 'should not get here';
+     end;
+     raise notice 'should not get here, either';
+   end;
+   raise notice 'nor here';
+ end loop;
+ raise notice 'should get here';
+ end$$;
+ NOTICE:  should get here
+ -- check exit out of an unlabeled block to a labeled one
+ do $$
+ <<outerblock>>
+ begin
+   <<innerblock>>
+   begin
+     <<moreinnerblock>>
+     begin
+       begin  -- unlabeled block
+         exit innerblock;
+         raise notice 'should not get here';
+       end;
+       raise notice 'should not get here, either';
+     end;
+     raise notice 'nor here';
+   end;
+   raise notice 'should get here';
+ end$$;
+ NOTICE:  should get here
+ -- check exit to a matching outer loop (other cases are covered elsewhere)
+ do $$
+ begin
+ <<outerloop>>
+ for i in 1..10 loop
+   <<innerloop>>
+   for j in 1..10 loop
+     exit outerloop;
+     raise notice 'should not get here';
+   end loop;
+   raise notice 'should not get here, either';
+ end loop;
+ raise notice 'should get here';
+ end$$;
+ NOTICE:  should get here
+ -- return out of a loop
+ create function return_from_loop() returns int language plpgsql as $$
+ begin
+   for i in 1..10 loop
+     if i > 3 then
+       return i;
+     end if;
+   end loop;
+   return null;
+ end$$;
+ select return_from_loop();
+  return_from_loop
+ ------------------
+                 4
+ (1 row)
+
+ -- unlabeled exit does match a while loop
+ do $$
+ begin
+   <<outermostwhile>>
+   while 1 > 0 loop
+     <<outerwhile>>
+     while 1 > 0 loop
+       <<innerwhile>>
+       while 1 > 0 loop
+         exit;
+         raise notice 'should not get here';
+       end loop;
+       raise notice 'should get here';
+       exit outermostwhile;
+       raise notice 'should not get here, either';
+     end loop;
+     raise notice 'nor here';
+   end loop;
+   raise notice 'should get here, too';
+ end$$;
+ NOTICE:  should get here
+ NOTICE:  should get here, too
+ -- check exit out of an unlabeled while to a labeled one
+ do $$
+ begin
+   <<outerwhile>>
+   while 1 > 0 loop
+     while 1 > 0 loop
+       exit outerwhile;
+       raise notice 'should not get here';
+     end loop;
+     raise notice 'should not get here, either';
+   end loop;
+   raise notice 'should get here';
+ end$$;
+ NOTICE:  should get here
+ -- continue to an outer while
+ do $$
+ declare i int := 0;
+ begin
+   <<outermostwhile>>
+   while i < 2 loop
+     raise notice 'outermostwhile, i = %', i;
+     i := i + 1;
+     <<outerwhile>>
+     while 1 > 0 loop
+       <<innerwhile>>
+       while 1 > 0 loop
+         continue outermostwhile;
+         raise notice 'should not get here';
+       end loop;
+       raise notice 'should not get here, either';
+     end loop;
+     raise notice 'nor here';
+   end loop;
+   raise notice 'out of outermostwhile, i = %', i;
+ end$$;
+ NOTICE:  outermostwhile, i = 0
+ NOTICE:  outermostwhile, i = 1
+ NOTICE:  out of outermostwhile, i = 2
+ -- return out of a while
+ create function return_from_while() returns int language plpgsql as $$
+ declare i int := 0;
+ begin
+   while i < 10 loop
+     if i > 2 then
+       return i;
+     end if;
+     i := i + 1;
+   end loop;
+   return null;
+ end$$;
+ select return_from_while();
+  return_from_while
+ -------------------
+                  3
+ (1 row)
+
+ -- exit out of a foreach
+ do $$
+ declare i int; j int; k int;
+ begin
+   <<outermostforeach>>
+   foreach i in array array[1,2,3] loop
+     <<outerforeach>>
+     foreach j in array array[1,2,3] loop
+       <<innerforeach>>
+       foreach k in array array[1,2,3] loop
+         exit;
+         raise notice 'should not get here';
+       end loop;
+       raise notice 'should get here';
+       exit outermostforeach;
+       raise notice 'should not get here, either';
+     end loop;
+     raise notice 'nor here';
+   end loop;
+   raise notice 'should get here, too';
+ end$$;
+ NOTICE:  should get here
+ NOTICE:  should get here, too
+ -- continue out of a foreach
+ do $$
+ declare i int; j int; k int;
+ begin
+   <<outermostforeach>>
+   foreach i in array array[1,2,3] loop
+     <<outerforeach>>
+     foreach j in array array[1,2,3] loop
+       <<innerforeach>>
+       foreach k in array array[1,2,3] loop
+         continue;
+         raise notice 'should not get here';
+       end loop;
+       raise notice 'should get here, k = %', k;
+       continue outermostforeach;
+       raise notice 'should not get here, either';
+     end loop;
+     raise notice 'nor here';
+   end loop;
+   raise notice 'should get here, i = %, j = %, k = %', i, j, k;
+ end$$;
+ NOTICE:  should get here, k = 3
+ NOTICE:  should get here, k = 3
+ NOTICE:  should get here, k = 3
+ NOTICE:  should get here, i = 3, j = 1, k = 3
+ -- return out of a foreach
+ create function return_from_foreach() returns int language plpgsql as $$
+ declare i int := 0;
+ begin
+   foreach i in array array[1,2,3] loop
+     if i > 1 then
+       return i;
+     end if;
+   end loop;
+   return null;
+ end$$;
+ select return_from_foreach();
+  return_from_foreach
+ ---------------------
+                    2
+ (1 row)
+
+ -- exit out of a for-over-query
+ do $$
+ declare i int; j int; k int;
+ begin
+   <<outermostfor>>
+   for i in select generate_series(1,3) loop
+     <<outerfor>>
+     for j in select generate_series(1,3) loop
+       <<innerfor>>
+       for k in select generate_series(1,3) loop
+         exit;
+         raise notice 'should not get here';
+       end loop;
+       raise notice 'should get here';
+       exit outermostfor;
+       raise notice 'should not get here, either';
+     end loop;
+     raise notice 'nor here';
+   end loop;
+   raise notice 'should get here, too';
+ end$$;
+ NOTICE:  should get here
+ NOTICE:  should get here, too
+ -- continue out of a for-over-query
+ do $$
+ declare i int; j int; k int;
+ begin
+   <<outermostfor>>
+   for i in select generate_series(1,3) loop
+     <<outerfor>>
+     for j in select generate_series(1,3) loop
+       <<innerfor>>
+       for k in select generate_series(1,3) loop
+         continue;
+         raise notice 'should not get here';
+       end loop;
+       raise notice 'should get here, k = %', k;
+       continue outermostfor;
+       raise notice 'should not get here, either';
+     end loop;
+     raise notice 'nor here';
+   end loop;
+   raise notice 'should get here, i = %, j = %, k = %', i, j, k;
+ end$$;
+ NOTICE:  should get here, k = 3
+ NOTICE:  should get here, k = 3
+ NOTICE:  should get here, k = 3
+ NOTICE:  should get here, i = 3, j = 1, k = 3
+ -- return out of a for-over-query
+ create function return_from_for_query() returns int language plpgsql as $$
+ declare i int := 0;
+ begin
+   for i in select generate_series(1,3) loop
+     if i > 1 then
+       return i;
+     end if;
+   end loop;
+   return null;
+ end$$;
+ select return_from_for_query();
+  return_from_for_query
+ -----------------------
+                      2
+ (1 row)
+
+ -- using list of scalars in fori and fore stmts
+ create function for_vect() returns void as $proc$
+ <<lbl>>declare a integer; b varchar; c varchar; r record;
+ begin
+   -- fori
+   for i in 1 .. 3 loop
+     raise notice '%', i;
+   end loop;
+   -- fore with record var
+   for r in select gs as aa, 'BB' as bb, 'CC' as cc from generate_series(1,4) gs loop
+     raise notice '% % %', r.aa, r.bb, r.cc;
+   end loop;
+   -- fore with single scalar
+   for a in select gs from generate_series(1,4) gs loop
+     raise notice '%', a;
+   end loop;
+   -- fore with multiple scalars
+   for a,b,c in select gs, 'BB','CC' from generate_series(1,4) gs loop
+     raise notice '% % %', a, b, c;
+   end loop;
+   -- using qualified names in fors, fore is enabled, disabled only for fori
+   for lbl.a, lbl.b, lbl.c in execute $$select gs, 'bb','cc' from generate_series(1,4) gs$$ loop
+     raise notice '% % %', a, b, c;
+   end loop;
+ end;
+ $proc$ language plpgsql;
+ select for_vect();
+ NOTICE:  1
+ NOTICE:  2
+ NOTICE:  3
+ NOTICE:  1 BB CC
+ NOTICE:  2 BB CC
+ NOTICE:  3 BB CC
+ NOTICE:  4 BB CC
+ NOTICE:  1
+ NOTICE:  2
+ NOTICE:  3
+ NOTICE:  4
+ NOTICE:  1 BB CC
+ NOTICE:  2 BB CC
+ NOTICE:  3 BB CC
+ NOTICE:  4 BB CC
+ NOTICE:  1 bb cc
+ NOTICE:  2 bb cc
+ NOTICE:  3 bb cc
+ NOTICE:  4 bb cc
+  for_vect
+ ----------
+
+ (1 row)
+
+ -- CASE statement
+ create or replace function case_test(bigint) returns text as $$
+ declare a int = 10;
+         b int = 1;
+ begin
+   case $1
+     when 1 then
+       return 'one';
+     when 2 then
+       return 'two';
+     when 3,4,3+5 then
+       return 'three, four or eight';
+     when a then
+       return 'ten';
+     when a+b, a+b+1 then
+       return 'eleven, twelve';
+   end case;
+ end;
+ $$ language plpgsql immutable;
+ select case_test(1);
+  case_test
+ -----------
+  one
+ (1 row)
+
+ select case_test(2);
+  case_test
+ -----------
+  two
+ (1 row)
+
+ select case_test(3);
+       case_test
+ ----------------------
+  three, four or eight
+ (1 row)
+
+ select case_test(4);
+       case_test
+ ----------------------
+  three, four or eight
+ (1 row)
+
+ select case_test(5); -- fails
+ ERROR:  case not found
+ HINT:  CASE statement is missing ELSE part.
+ CONTEXT:  PL/pgSQL function case_test(bigint) line 5 at CASE
+ select case_test(8);
+       case_test
+ ----------------------
+  three, four or eight
+ (1 row)
+
+ select case_test(10);
+  case_test
+ -----------
+  ten
+ (1 row)
+
+ select case_test(11);
+    case_test
+ ----------------
+  eleven, twelve
+ (1 row)
+
+ select case_test(12);
+    case_test
+ ----------------
+  eleven, twelve
+ (1 row)
+
+ select case_test(13); -- fails
+ ERROR:  case not found
+ HINT:  CASE statement is missing ELSE part.
+ CONTEXT:  PL/pgSQL function case_test(bigint) line 5 at CASE
+ create or replace function catch() returns void as $$
+ begin
+   raise notice '%', case_test(6);
+ exception
+   when case_not_found then
+     raise notice 'caught case_not_found % %', SQLSTATE, SQLERRM;
+ end
+ $$ language plpgsql;
+ select catch();
+ NOTICE:  caught case_not_found 20000 case not found
+  catch
+ -------
+
+ (1 row)
+
+ -- test the searched variant too, as well as ELSE
+ create or replace function case_test(bigint) returns text as $$
+ declare a int = 10;
+ begin
+   case
+     when $1 = 1 then
+       return 'one';
+     when $1 = a + 2 then
+       return 'twelve';
+     else
+       return 'other';
+   end case;
+ end;
+ $$ language plpgsql immutable;
+ select case_test(1);
+  case_test
+ -----------
+  one
+ (1 row)
+
+ select case_test(2);
+  case_test
+ -----------
+  other
+ (1 row)
+
+ select case_test(12);
+  case_test
+ -----------
+  twelve
+ (1 row)
+
+ select case_test(13);
+  case_test
+ -----------
+  other
+ (1 row)
+
diff --git a/src/pl/plpgsql/src/sql/plpgsql_control.sql b/src/pl/plpgsql/src/sql/plpgsql_control.sql
index ...2a9dfa9 .
*** a/src/pl/plpgsql/src/sql/plpgsql_control.sql
--- b/src/pl/plpgsql/src/sql/plpgsql_control.sql
***************
*** 0 ****
--- 1,620 ----
+ --
+ -- Tests for PL/pgSQL control structures
+ --
+
+ -- integer FOR loop
+
+ do $$
+ begin
+   -- basic case
+   for i in 1..3 loop
+     raise notice '1..3: i = %', i;
+   end loop;
+   -- with BY, end matches exactly
+   for i in 1..10 by 3 loop
+     raise notice '1..10 by 3: i = %', i;
+   end loop;
+   -- with BY, end does not match
+   for i in 1..11 by 3 loop
+     raise notice '1..11 by 3: i = %', i;
+   end loop;
+   -- zero iterations
+   for i in 1..0 by 3 loop
+     raise notice '1..0 by 3: i = %', i;
+   end loop;
+   -- REVERSE
+   for i in reverse 10..0 by 3 loop
+     raise notice 'reverse 10..0 by 3: i = %', i;
+   end loop;
+   -- potential overflow
+   for i in 2147483620..2147483647 by 10 loop
+     raise notice '2147483620..2147483647 by 10: i = %', i;
+   end loop;
+   -- potential overflow, reverse direction
+   for i in reverse -2147483620..-2147483647 by 10 loop
+     raise notice 'reverse -2147483620..-2147483647 by 10: i = %', i;
+   end loop;
+ end$$;
+
+ -- BY can't be zero or negative
+ do $$
+ begin
+   for i in 1..3 by 0 loop
+     raise notice '1..3 by 0: i = %', i;
+   end loop;
+ end$$;
+
+ do $$
+ begin
+   for i in 1..3 by -1 loop
+     raise notice '1..3 by -1: i = %', i;
+   end loop;
+ end$$;
+
+ do $$
+ begin
+   for i in reverse 1..3 by -1 loop
+     raise notice 'reverse 1..3 by -1: i = %', i;
+   end loop;
+ end$$;
+
+
+ -- CONTINUE statement
+
+ create table conttesttbl(idx serial, v integer);
+ insert into conttesttbl(v) values(10);
+ insert into conttesttbl(v) values(20);
+ insert into conttesttbl(v) values(30);
+ insert into conttesttbl(v) values(40);
+
+ create function continue_test1() returns void as $$
+ declare _i integer = 0; _r record;
+ begin
+   raise notice '---1---';
+   loop
+     _i := _i + 1;
+     raise notice '%', _i;
+     continue when _i < 10;
+     exit;
+   end loop;
+
+   raise notice '---2---';
+   <<lbl>>
+   loop
+     _i := _i - 1;
+     loop
+       raise notice '%', _i;
+       continue lbl when _i > 0;
+       exit lbl;
+     end loop;
+   end loop;
+
+   raise notice '---3---';
+   <<the_loop>>
+   while _i < 10 loop
+     _i := _i + 1;
+     continue the_loop when _i % 2 = 0;
+     raise notice '%', _i;
+   end loop;
+
+   raise notice '---4---';
+   for _i in 1..10 loop
+     begin
+       -- applies to outer loop, not the nested begin block
+       continue when _i < 5;
+       raise notice '%', _i;
+     end;
+   end loop;
+
+   raise notice '---5---';
+   for _r in select * from conttesttbl loop
+     continue when _r.v <= 20;
+     raise notice '%', _r.v;
+   end loop;
+
+   raise notice '---6---';
+   for _r in execute 'select * from conttesttbl' loop
+     continue when _r.v <= 20;
+     raise notice '%', _r.v;
+   end loop;
+
+   raise notice '---7---';
+   <<looplabel>>
+   for _i in 1..3 loop
+     continue looplabel when _i = 2;
+     raise notice '%', _i;
+   end loop;
+
+   raise notice '---8---';
+   _i := 1;
+   while _i <= 3 loop
+     raise notice '%', _i;
+     _i := _i + 1;
+     continue when _i = 3;
+   end loop;
+
+   raise notice '---9---';
+   for _r in select * from conttesttbl order by v limit 1 loop
+     raise notice '%', _r.v;
+     continue;
+   end loop;
+
+   raise notice '---10---';
+   for _r in execute 'select * from conttesttbl order by v limit 1' loop
+     raise notice '%', _r.v;
+     continue;
+   end loop;
+
+   raise notice '---11---';
+   <<outerlooplabel>>
+   for _i in 1..2 loop
+     raise notice 'outer %', _i;
+     <<innerlooplabel>>
+     for _j in 1..3 loop
+       continue outerlooplabel when _j = 2;
+       raise notice 'inner %', _j;
+     end loop;
+   end loop;
+ end; $$ language plpgsql;
+
+ select continue_test1();
+
+ -- should fail: CONTINUE is only legal inside a loop
+ create function continue_error1() returns void as $$
+ begin
+     begin
+         continue;
+     end;
+ end;
+ $$ language plpgsql;
+
+ -- should fail: unlabeled EXIT is only legal inside a loop
+ create function exit_error1() returns void as $$
+ begin
+     begin
+         exit;
+     end;
+ end;
+ $$ language plpgsql;
+
+ -- should fail: no such label
+ create function continue_error2() returns void as $$
+ begin
+     begin
+         loop
+             continue no_such_label;
+         end loop;
+     end;
+ end;
+ $$ language plpgsql;
+
+ -- should fail: no such label
+ create function exit_error2() returns void as $$
+ begin
+     begin
+         loop
+             exit no_such_label;
+         end loop;
+     end;
+ end;
+ $$ language plpgsql;
+
+ -- should fail: CONTINUE can't reference the label of a named block
+ create function continue_error3() returns void as $$
+ begin
+     <<begin_block1>>
+     begin
+         loop
+             continue begin_block1;
+         end loop;
+     end;
+ end;
+ $$ language plpgsql;
+
+ -- On the other hand, EXIT *can* reference the label of a named block
+ create function exit_block1() returns void as $$
+ begin
+     <<begin_block1>>
+     begin
+         loop
+             exit begin_block1;
+             raise exception 'should not get here';
+         end loop;
+     end;
+ end;
+ $$ language plpgsql;
+
+ select exit_block1();
+
+ -- verbose end block and end loop
+ create function end_label1() returns void as $$
+ <<blbl>>
+ begin
+   <<flbl1>>
+   for i in 1 .. 10 loop
+     raise notice 'i = %', i;
+     exit flbl1;
+   end loop flbl1;
+   <<flbl2>>
+   for j in 1 .. 10 loop
+     raise notice 'j = %', j;
+     exit flbl2;
+   end loop;
+ end blbl;
+ $$ language plpgsql;
+
+ select end_label1();
+
+ -- should fail: undefined end label
+ create function end_label2() returns void as $$
+ begin
+   for _i in 1 .. 10 loop
+     exit;
+   end loop flbl1;
+ end;
+ $$ language plpgsql;
+
+ -- should fail: end label does not match start label
+ create function end_label3() returns void as $$
+ <<outer_label>>
+ begin
+   <<inner_label>>
+   for _i in 1 .. 10 loop
+     exit;
+   end loop outer_label;
+ end;
+ $$ language plpgsql;
+
+ -- should fail: end label on a block without a start label
+ create function end_label4() returns void as $$
+ <<outer_label>>
+ begin
+   for _i in 1 .. 10 loop
+     exit;
+   end loop outer_label;
+ end;
+ $$ language plpgsql;
+
+ -- unlabeled exit matches no blocks
+ do $$
+ begin
+ for i in 1..10 loop
+   <<innerblock>>
+   begin
+     begin  -- unlabeled block
+       exit;
+       raise notice 'should not get here';
+     end;
+     raise notice 'should not get here, either';
+   end;
+   raise notice 'nor here';
+ end loop;
+ raise notice 'should get here';
+ end$$;
+
+ -- check exit out of an unlabeled block to a labeled one
+ do $$
+ <<outerblock>>
+ begin
+   <<innerblock>>
+   begin
+     <<moreinnerblock>>
+     begin
+       begin  -- unlabeled block
+         exit innerblock;
+         raise notice 'should not get here';
+       end;
+       raise notice 'should not get here, either';
+     end;
+     raise notice 'nor here';
+   end;
+   raise notice 'should get here';
+ end$$;
+
+ -- check exit to a matching outer loop (other cases are covered elsewhere)
+ do $$
+ begin
+ <<outerloop>>
+ for i in 1..10 loop
+   <<innerloop>>
+   for j in 1..10 loop
+     exit outerloop;
+     raise notice 'should not get here';
+   end loop;
+   raise notice 'should not get here, either';
+ end loop;
+ raise notice 'should get here';
+ end$$;
+
+ -- return out of a loop
+ create function return_from_loop() returns int language plpgsql as $$
+ begin
+   for i in 1..10 loop
+     if i > 3 then
+       return i;
+     end if;
+   end loop;
+   return null;
+ end$$;
+
+ select return_from_loop();
+
+ -- unlabeled exit does match a while loop
+ do $$
+ begin
+   <<outermostwhile>>
+   while 1 > 0 loop
+     <<outerwhile>>
+     while 1 > 0 loop
+       <<innerwhile>>
+       while 1 > 0 loop
+         exit;
+         raise notice 'should not get here';
+       end loop;
+       raise notice 'should get here';
+       exit outermostwhile;
+       raise notice 'should not get here, either';
+     end loop;
+     raise notice 'nor here';
+   end loop;
+   raise notice 'should get here, too';
+ end$$;
+
+ -- check exit out of an unlabeled while to a labeled one
+ do $$
+ begin
+   <<outerwhile>>
+   while 1 > 0 loop
+     while 1 > 0 loop
+       exit outerwhile;
+       raise notice 'should not get here';
+     end loop;
+     raise notice 'should not get here, either';
+   end loop;
+   raise notice 'should get here';
+ end$$;
+
+ -- continue to an outer while
+ do $$
+ declare i int := 0;
+ begin
+   <<outermostwhile>>
+   while i < 2 loop
+     raise notice 'outermostwhile, i = %', i;
+     i := i + 1;
+     <<outerwhile>>
+     while 1 > 0 loop
+       <<innerwhile>>
+       while 1 > 0 loop
+         continue outermostwhile;
+         raise notice 'should not get here';
+       end loop;
+       raise notice 'should not get here, either';
+     end loop;
+     raise notice 'nor here';
+   end loop;
+   raise notice 'out of outermostwhile, i = %', i;
+ end$$;
+
+ -- return out of a while
+ create function return_from_while() returns int language plpgsql as $$
+ declare i int := 0;
+ begin
+   while i < 10 loop
+     if i > 2 then
+       return i;
+     end if;
+     i := i + 1;
+   end loop;
+   return null;
+ end$$;
+
+ select return_from_while();
+
+ -- exit out of a foreach
+ do $$
+ declare i int; j int; k int;
+ begin
+   <<outermostforeach>>
+   foreach i in array array[1,2,3] loop
+     <<outerforeach>>
+     foreach j in array array[1,2,3] loop
+       <<innerforeach>>
+       foreach k in array array[1,2,3] loop
+         exit;
+         raise notice 'should not get here';
+       end loop;
+       raise notice 'should get here';
+       exit outermostforeach;
+       raise notice 'should not get here, either';
+     end loop;
+     raise notice 'nor here';
+   end loop;
+   raise notice 'should get here, too';
+ end$$;
+
+ -- continue out of a foreach
+ do $$
+ declare i int; j int; k int;
+ begin
+   <<outermostforeach>>
+   foreach i in array array[1,2,3] loop
+     <<outerforeach>>
+     foreach j in array array[1,2,3] loop
+       <<innerforeach>>
+       foreach k in array array[1,2,3] loop
+         continue;
+         raise notice 'should not get here';
+       end loop;
+       raise notice 'should get here, k = %', k;
+       continue outermostforeach;
+       raise notice 'should not get here, either';
+     end loop;
+     raise notice 'nor here';
+   end loop;
+   raise notice 'should get here, i = %, j = %, k = %', i, j, k;
+ end$$;
+
+ -- return out of a foreach
+ create function return_from_foreach() returns int language plpgsql as $$
+ declare i int := 0;
+ begin
+   foreach i in array array[1,2,3] loop
+     if i > 1 then
+       return i;
+     end if;
+   end loop;
+   return null;
+ end$$;
+
+ select return_from_foreach();
+
+ -- exit out of a for-over-query
+ do $$
+ declare i int; j int; k int;
+ begin
+   <<outermostfor>>
+   for i in select generate_series(1,3) loop
+     <<outerfor>>
+     for j in select generate_series(1,3) loop
+       <<innerfor>>
+       for k in select generate_series(1,3) loop
+         exit;
+         raise notice 'should not get here';
+       end loop;
+       raise notice 'should get here';
+       exit outermostfor;
+       raise notice 'should not get here, either';
+     end loop;
+     raise notice 'nor here';
+   end loop;
+   raise notice 'should get here, too';
+ end$$;
+
+ -- continue out of a for-over-query
+ do $$
+ declare i int; j int; k int;
+ begin
+   <<outermostfor>>
+   for i in select generate_series(1,3) loop
+     <<outerfor>>
+     for j in select generate_series(1,3) loop
+       <<innerfor>>
+       for k in select generate_series(1,3) loop
+         continue;
+         raise notice 'should not get here';
+       end loop;
+       raise notice 'should get here, k = %', k;
+       continue outermostfor;
+       raise notice 'should not get here, either';
+     end loop;
+     raise notice 'nor here';
+   end loop;
+   raise notice 'should get here, i = %, j = %, k = %', i, j, k;
+ end$$;
+
+ -- return out of a for-over-query
+ create function return_from_for_query() returns int language plpgsql as $$
+ declare i int := 0;
+ begin
+   for i in select generate_series(1,3) loop
+     if i > 1 then
+       return i;
+     end if;
+   end loop;
+   return null;
+ end$$;
+
+ select return_from_for_query();
+
+ -- using list of scalars in fori and fore stmts
+ create function for_vect() returns void as $proc$
+ <<lbl>>declare a integer; b varchar; c varchar; r record;
+ begin
+   -- fori
+   for i in 1 .. 3 loop
+     raise notice '%', i;
+   end loop;
+   -- fore with record var
+   for r in select gs as aa, 'BB' as bb, 'CC' as cc from generate_series(1,4) gs loop
+     raise notice '% % %', r.aa, r.bb, r.cc;
+   end loop;
+   -- fore with single scalar
+   for a in select gs from generate_series(1,4) gs loop
+     raise notice '%', a;
+   end loop;
+   -- fore with multiple scalars
+   for a,b,c in select gs, 'BB','CC' from generate_series(1,4) gs loop
+     raise notice '% % %', a, b, c;
+   end loop;
+   -- using qualified names in fors, fore is enabled, disabled only for fori
+   for lbl.a, lbl.b, lbl.c in execute $$select gs, 'bb','cc' from generate_series(1,4) gs$$ loop
+     raise notice '% % %', a, b, c;
+   end loop;
+ end;
+ $proc$ language plpgsql;
+
+ select for_vect();
+
+ -- CASE statement
+
+ create or replace function case_test(bigint) returns text as $$
+ declare a int = 10;
+         b int = 1;
+ begin
+   case $1
+     when 1 then
+       return 'one';
+     when 2 then
+       return 'two';
+     when 3,4,3+5 then
+       return 'three, four or eight';
+     when a then
+       return 'ten';
+     when a+b, a+b+1 then
+       return 'eleven, twelve';
+   end case;
+ end;
+ $$ language plpgsql immutable;
+
+ select case_test(1);
+ select case_test(2);
+ select case_test(3);
+ select case_test(4);
+ select case_test(5); -- fails
+ select case_test(8);
+ select case_test(10);
+ select case_test(11);
+ select case_test(12);
+ select case_test(13); -- fails
+
+ create or replace function catch() returns void as $$
+ begin
+   raise notice '%', case_test(6);
+ exception
+   when case_not_found then
+     raise notice 'caught case_not_found % %', SQLSTATE, SQLERRM;
+ end
+ $$ language plpgsql;
+
+ select catch();
+
+ -- test the searched variant too, as well as ELSE
+ create or replace function case_test(bigint) returns text as $$
+ declare a int = 10;
+ begin
+   case
+     when $1 = 1 then
+       return 'one';
+     when $1 = a + 2 then
+       return 'twelve';
+     else
+       return 'other';
+   end case;
+ end;
+ $$ language plpgsql immutable;
+
+ select case_test(1);
+ select case_test(2);
+ select case_test(12);
+ select case_test(13);
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index a2df411..4783807 100644
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
*************** NOTICE:  {10,20,30}; 20; xyz; xyzabc; (1
*** 2706,3044 ****
  (1 row)

  drop function raise_exprs();
- -- continue statement
- create table conttesttbl(idx serial, v integer);
- insert into conttesttbl(v) values(10);
- insert into conttesttbl(v) values(20);
- insert into conttesttbl(v) values(30);
- insert into conttesttbl(v) values(40);
- create function continue_test1() returns void as $$
- declare _i integer = 0; _r record;
- begin
-   raise notice '---1---';
-   loop
-     _i := _i + 1;
-     raise notice '%', _i;
-     continue when _i < 10;
-     exit;
-   end loop;
-
-   raise notice '---2---';
-   <<lbl>>
-   loop
-     _i := _i - 1;
-     loop
-       raise notice '%', _i;
-       continue lbl when _i > 0;
-       exit lbl;
-     end loop;
-   end loop;
-
-   raise notice '---3---';
-   <<the_loop>>
-   while _i < 10 loop
-     _i := _i + 1;
-     continue the_loop when _i % 2 = 0;
-     raise notice '%', _i;
-   end loop;
-
-   raise notice '---4---';
-   for _i in 1..10 loop
-     begin
-       -- applies to outer loop, not the nested begin block
-       continue when _i < 5;
-       raise notice '%', _i;
-     end;
-   end loop;
-
-   raise notice '---5---';
-   for _r in select * from conttesttbl loop
-     continue when _r.v <= 20;
-     raise notice '%', _r.v;
-   end loop;
-
-   raise notice '---6---';
-   for _r in execute 'select * from conttesttbl' loop
-     continue when _r.v <= 20;
-     raise notice '%', _r.v;
-   end loop;
-
-   raise notice '---7---';
-   for _i in 1..3 loop
-     raise notice '%', _i;
-     continue when _i = 3;
-   end loop;
-
-   raise notice '---8---';
-   _i := 1;
-   while _i <= 3 loop
-     raise notice '%', _i;
-     _i := _i + 1;
-     continue when _i = 3;
-   end loop;
-
-   raise notice '---9---';
-   for _r in select * from conttesttbl order by v limit 1 loop
-     raise notice '%', _r.v;
-     continue;
-   end loop;
-
-   raise notice '---10---';
-   for _r in execute 'select * from conttesttbl order by v limit 1' loop
-     raise notice '%', _r.v;
-     continue;
-   end loop;
- end; $$ language plpgsql;
- select continue_test1();
- NOTICE:  ---1---
- NOTICE:  1
- NOTICE:  2
- NOTICE:  3
- NOTICE:  4
- NOTICE:  5
- NOTICE:  6
- NOTICE:  7
- NOTICE:  8
- NOTICE:  9
- NOTICE:  10
- NOTICE:  ---2---
- NOTICE:  9
- NOTICE:  8
- NOTICE:  7
- NOTICE:  6
- NOTICE:  5
- NOTICE:  4
- NOTICE:  3
- NOTICE:  2
- NOTICE:  1
- NOTICE:  0
- NOTICE:  ---3---
- NOTICE:  1
- NOTICE:  3
- NOTICE:  5
- NOTICE:  7
- NOTICE:  9
- NOTICE:  ---4---
- NOTICE:  5
- NOTICE:  6
- NOTICE:  7
- NOTICE:  8
- NOTICE:  9
- NOTICE:  10
- NOTICE:  ---5---
- NOTICE:  30
- NOTICE:  40
- NOTICE:  ---6---
- NOTICE:  30
- NOTICE:  40
- NOTICE:  ---7---
- NOTICE:  1
- NOTICE:  2
- NOTICE:  3
- NOTICE:  ---8---
- NOTICE:  1
- NOTICE:  2
- NOTICE:  3
- NOTICE:  ---9---
- NOTICE:  10
- NOTICE:  ---10---
- NOTICE:  10
-  continue_test1
- ----------------
-
- (1 row)
-
- drop function continue_test1();
- drop table conttesttbl;
- -- should fail: CONTINUE is only legal inside a loop
- create function continue_error1() returns void as $$
- begin
-     begin
-         continue;
-     end;
- end;
- $$ language plpgsql;
- ERROR:  CONTINUE cannot be used outside a loop
- LINE 4:         continue;
-                 ^
- -- should fail: unlabeled EXIT is only legal inside a loop
- create function exit_error1() returns void as $$
- begin
-     begin
-         exit;
-     end;
- end;
- $$ language plpgsql;
- ERROR:  EXIT cannot be used outside a loop, unless it has a label
- LINE 4:         exit;
-                 ^
- -- should fail: no such label
- create function continue_error2() returns void as $$
- begin
-     begin
-         loop
-             continue no_such_label;
-         end loop;
-     end;
- end;
- $$ language plpgsql;
- ERROR:  there is no label "no_such_label" attached to any block or loop enclosing this statement
- LINE 5:             continue no_such_label;
-                              ^
- -- should fail: no such label
- create function exit_error2() returns void as $$
- begin
-     begin
-         loop
-             exit no_such_label;
-         end loop;
-     end;
- end;
- $$ language plpgsql;
- ERROR:  there is no label "no_such_label" attached to any block or loop enclosing this statement
- LINE 5:             exit no_such_label;
-                          ^
- -- should fail: CONTINUE can't reference the label of a named block
- create function continue_error3() returns void as $$
- begin
-     <<begin_block1>>
-     begin
-         loop
-             continue begin_block1;
-         end loop;
-     end;
- end;
- $$ language plpgsql;
- ERROR:  block label "begin_block1" cannot be used in CONTINUE
- LINE 6:             continue begin_block1;
-                              ^
- -- On the other hand, EXIT *can* reference the label of a named block
- create function exit_block1() returns void as $$
- begin
-     <<begin_block1>>
-     begin
-         loop
-             exit begin_block1;
-             raise exception 'should not get here';
-         end loop;
-     end;
- end;
- $$ language plpgsql;
- select exit_block1();
-  exit_block1
- -------------
-
- (1 row)
-
- drop function exit_block1();
- -- verbose end block and end loop
- create function end_label1() returns void as $$
- <<blbl>>
- begin
-   <<flbl1>>
-   for _i in 1 .. 10 loop
-     exit flbl1;
-   end loop flbl1;
-   <<flbl2>>
-   for _i in 1 .. 10 loop
-     exit flbl2;
-   end loop;
- end blbl;
- $$ language plpgsql;
- select end_label1();
-  end_label1
- ------------
-
- (1 row)
-
- drop function end_label1();
- -- should fail: undefined end label
- create function end_label2() returns void as $$
- begin
-   for _i in 1 .. 10 loop
-     exit;
-   end loop flbl1;
- end;
- $$ language plpgsql;
- ERROR:  end label "flbl1" specified for unlabelled block
- LINE 5:   end loop flbl1;
-                    ^
- -- should fail: end label does not match start label
- create function end_label3() returns void as $$
- <<outer_label>>
- begin
-   <<inner_label>>
-   for _i in 1 .. 10 loop
-     exit;
-   end loop outer_label;
- end;
- $$ language plpgsql;
- ERROR:  end label "outer_label" differs from block's label "inner_label"
- LINE 7:   end loop outer_label;
-                    ^
- -- should fail: end label on a block without a start label
- create function end_label4() returns void as $$
- <<outer_label>>
- begin
-   for _i in 1 .. 10 loop
-     exit;
-   end loop outer_label;
- end;
- $$ language plpgsql;
- ERROR:  end label "outer_label" specified for unlabelled block
- LINE 6:   end loop outer_label;
-                    ^
- -- using list of scalars in fori and fore stmts
- create function for_vect() returns void as $proc$
- <<lbl>>declare a integer; b varchar; c varchar; r record;
- begin
-   -- fori
-   for i in 1 .. 3 loop
-     raise notice '%', i;
-   end loop;
-   -- fore with record var
-   for r in select gs as aa, 'BB' as bb, 'CC' as cc from generate_series(1,4) gs loop
-     raise notice '% % %', r.aa, r.bb, r.cc;
-   end loop;
-   -- fore with single scalar
-   for a in select gs from generate_series(1,4) gs loop
-     raise notice '%', a;
-   end loop;
-   -- fore with multiple scalars
-   for a,b,c in select gs, 'BB','CC' from generate_series(1,4) gs loop
-     raise notice '% % %', a, b, c;
-   end loop;
-   -- using qualified names in fors, fore is enabled, disabled only for fori
-   for lbl.a, lbl.b, lbl.c in execute $$select gs, 'bb','cc' from generate_series(1,4) gs$$ loop
-     raise notice '% % %', a, b, c;
-   end loop;
- end;
- $proc$ language plpgsql;
- select for_vect();
- NOTICE:  1
- NOTICE:  2
- NOTICE:  3
- NOTICE:  1 BB CC
- NOTICE:  2 BB CC
- NOTICE:  3 BB CC
- NOTICE:  4 BB CC
- NOTICE:  1
- NOTICE:  2
- NOTICE:  3
- NOTICE:  4
- NOTICE:  1 BB CC
- NOTICE:  2 BB CC
- NOTICE:  3 BB CC
- NOTICE:  4 BB CC
- NOTICE:  1 bb cc
- NOTICE:  2 bb cc
- NOTICE:  3 bb cc
- NOTICE:  4 bb cc
-  for_vect
- ----------
-
- (1 row)
-
  -- regression test: verify that multiple uses of same plpgsql datum within
  -- a SQL command all get mapped to the same $n parameter.  The return value
  -- of the SELECT is not important, we only care that it doesn't fail with
--- 2706,2711 ----
*************** NOTICE:  column >>some column name<<, co
*** 4368,4503 ****
  (1 row)

  drop function stacked_diagnostics_test();
- -- test CASE statement
- create or replace function case_test(bigint) returns text as $$
- declare a int = 10;
-         b int = 1;
- begin
-   case $1
-     when 1 then
-       return 'one';
-     when 2 then
-       return 'two';
-     when 3,4,3+5 then
-       return 'three, four or eight';
-     when a then
-       return 'ten';
-     when a+b, a+b+1 then
-       return 'eleven, twelve';
-   end case;
- end;
- $$ language plpgsql immutable;
- select case_test(1);
-  case_test
- -----------
-  one
- (1 row)
-
- select case_test(2);
-  case_test
- -----------
-  two
- (1 row)
-
- select case_test(3);
-       case_test
- ----------------------
-  three, four or eight
- (1 row)
-
- select case_test(4);
-       case_test
- ----------------------
-  three, four or eight
- (1 row)
-
- select case_test(5); -- fails
- ERROR:  case not found
- HINT:  CASE statement is missing ELSE part.
- CONTEXT:  PL/pgSQL function case_test(bigint) line 5 at CASE
- select case_test(8);
-       case_test
- ----------------------
-  three, four or eight
- (1 row)
-
- select case_test(10);
-  case_test
- -----------
-  ten
- (1 row)
-
- select case_test(11);
-    case_test
- ----------------
-  eleven, twelve
- (1 row)
-
- select case_test(12);
-    case_test
- ----------------
-  eleven, twelve
- (1 row)
-
- select case_test(13); -- fails
- ERROR:  case not found
- HINT:  CASE statement is missing ELSE part.
- CONTEXT:  PL/pgSQL function case_test(bigint) line 5 at CASE
- create or replace function catch() returns void as $$
- begin
-   raise notice '%', case_test(6);
- exception
-   when case_not_found then
-     raise notice 'caught case_not_found % %', SQLSTATE, SQLERRM;
- end
- $$ language plpgsql;
- select catch();
- NOTICE:  caught case_not_found 20000 case not found
-  catch
- -------
-
- (1 row)
-
- -- test the searched variant too, as well as ELSE
- create or replace function case_test(bigint) returns text as $$
- declare a int = 10;
- begin
-   case
-     when $1 = 1 then
-       return 'one';
-     when $1 = a + 2 then
-       return 'twelve';
-     else
-       return 'other';
-   end case;
- end;
- $$ language plpgsql immutable;
- select case_test(1);
-  case_test
- -----------
-  one
- (1 row)
-
- select case_test(2);
-  case_test
- -----------
-  other
- (1 row)
-
- select case_test(12);
-  case_test
- -----------
-  twelve
- (1 row)
-
- select case_test(13);
-  case_test
- -----------
-  other
- (1 row)
-
- drop function catch();
- drop function case_test(bigint);
  -- test variadic functions
  create or replace function vari(variadic int[])
  returns void as $$
--- 4035,4040 ----
*************** create function consumes_rw_array(int[])
*** 5409,5414 ****
--- 4946,4957 ----
  language plpgsql as $$
    begin return $1[1]; end;
  $$ stable;
+ select consumes_rw_array(returns_rw_array(42));
+  consumes_rw_array
+ -------------------
+                 42
+ (1 row)
+
  -- bug #14174
  explain (verbose, costs off)
  select i, a from
*************** select consumes_rw_array(a), a from
*** 5465,5470 ****
--- 5008,5020 ----
                   2 | {2,2}
  (2 rows)

+ do $$
+ declare a int[] := array[1,2];
+ begin
+   a := a || 3;
+   raise notice 'a = %', a;
+ end$$;
+ NOTICE:  a = {1,2,3}
  --
  -- Test access to call stack
  --
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 02c8913..768270d 100644
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
*************** end;$$ language plpgsql;
*** 2285,2525 ****
  select raise_exprs();
  drop function raise_exprs();

- -- continue statement
- create table conttesttbl(idx serial, v integer);
- insert into conttesttbl(v) values(10);
- insert into conttesttbl(v) values(20);
- insert into conttesttbl(v) values(30);
- insert into conttesttbl(v) values(40);
-
- create function continue_test1() returns void as $$
- declare _i integer = 0; _r record;
- begin
-   raise notice '---1---';
-   loop
-     _i := _i + 1;
-     raise notice '%', _i;
-     continue when _i < 10;
-     exit;
-   end loop;
-
-   raise notice '---2---';
-   <<lbl>>
-   loop
-     _i := _i - 1;
-     loop
-       raise notice '%', _i;
-       continue lbl when _i > 0;
-       exit lbl;
-     end loop;
-   end loop;
-
-   raise notice '---3---';
-   <<the_loop>>
-   while _i < 10 loop
-     _i := _i + 1;
-     continue the_loop when _i % 2 = 0;
-     raise notice '%', _i;
-   end loop;
-
-   raise notice '---4---';
-   for _i in 1..10 loop
-     begin
-       -- applies to outer loop, not the nested begin block
-       continue when _i < 5;
-       raise notice '%', _i;
-     end;
-   end loop;
-
-   raise notice '---5---';
-   for _r in select * from conttesttbl loop
-     continue when _r.v <= 20;
-     raise notice '%', _r.v;
-   end loop;
-
-   raise notice '---6---';
-   for _r in execute 'select * from conttesttbl' loop
-     continue when _r.v <= 20;
-     raise notice '%', _r.v;
-   end loop;
-
-   raise notice '---7---';
-   for _i in 1..3 loop
-     raise notice '%', _i;
-     continue when _i = 3;
-   end loop;
-
-   raise notice '---8---';
-   _i := 1;
-   while _i <= 3 loop
-     raise notice '%', _i;
-     _i := _i + 1;
-     continue when _i = 3;
-   end loop;
-
-   raise notice '---9---';
-   for _r in select * from conttesttbl order by v limit 1 loop
-     raise notice '%', _r.v;
-     continue;
-   end loop;
-
-   raise notice '---10---';
-   for _r in execute 'select * from conttesttbl order by v limit 1' loop
-     raise notice '%', _r.v;
-     continue;
-   end loop;
- end; $$ language plpgsql;
-
- select continue_test1();
-
- drop function continue_test1();
- drop table conttesttbl;
-
- -- should fail: CONTINUE is only legal inside a loop
- create function continue_error1() returns void as $$
- begin
-     begin
-         continue;
-     end;
- end;
- $$ language plpgsql;
-
- -- should fail: unlabeled EXIT is only legal inside a loop
- create function exit_error1() returns void as $$
- begin
-     begin
-         exit;
-     end;
- end;
- $$ language plpgsql;
-
- -- should fail: no such label
- create function continue_error2() returns void as $$
- begin
-     begin
-         loop
-             continue no_such_label;
-         end loop;
-     end;
- end;
- $$ language plpgsql;
-
- -- should fail: no such label
- create function exit_error2() returns void as $$
- begin
-     begin
-         loop
-             exit no_such_label;
-         end loop;
-     end;
- end;
- $$ language plpgsql;
-
- -- should fail: CONTINUE can't reference the label of a named block
- create function continue_error3() returns void as $$
- begin
-     <<begin_block1>>
-     begin
-         loop
-             continue begin_block1;
-         end loop;
-     end;
- end;
- $$ language plpgsql;
-
- -- On the other hand, EXIT *can* reference the label of a named block
- create function exit_block1() returns void as $$
- begin
-     <<begin_block1>>
-     begin
-         loop
-             exit begin_block1;
-             raise exception 'should not get here';
-         end loop;
-     end;
- end;
- $$ language plpgsql;
-
- select exit_block1();
- drop function exit_block1();
-
- -- verbose end block and end loop
- create function end_label1() returns void as $$
- <<blbl>>
- begin
-   <<flbl1>>
-   for _i in 1 .. 10 loop
-     exit flbl1;
-   end loop flbl1;
-   <<flbl2>>
-   for _i in 1 .. 10 loop
-     exit flbl2;
-   end loop;
- end blbl;
- $$ language plpgsql;
-
- select end_label1();
- drop function end_label1();
-
- -- should fail: undefined end label
- create function end_label2() returns void as $$
- begin
-   for _i in 1 .. 10 loop
-     exit;
-   end loop flbl1;
- end;
- $$ language plpgsql;
-
- -- should fail: end label does not match start label
- create function end_label3() returns void as $$
- <<outer_label>>
- begin
-   <<inner_label>>
-   for _i in 1 .. 10 loop
-     exit;
-   end loop outer_label;
- end;
- $$ language plpgsql;
-
- -- should fail: end label on a block without a start label
- create function end_label4() returns void as $$
- <<outer_label>>
- begin
-   for _i in 1 .. 10 loop
-     exit;
-   end loop outer_label;
- end;
- $$ language plpgsql;
-
- -- using list of scalars in fori and fore stmts
- create function for_vect() returns void as $proc$
- <<lbl>>declare a integer; b varchar; c varchar; r record;
- begin
-   -- fori
-   for i in 1 .. 3 loop
-     raise notice '%', i;
-   end loop;
-   -- fore with record var
-   for r in select gs as aa, 'BB' as bb, 'CC' as cc from generate_series(1,4) gs loop
-     raise notice '% % %', r.aa, r.bb, r.cc;
-   end loop;
-   -- fore with single scalar
-   for a in select gs from generate_series(1,4) gs loop
-     raise notice '%', a;
-   end loop;
-   -- fore with multiple scalars
-   for a,b,c in select gs, 'BB','CC' from generate_series(1,4) gs loop
-     raise notice '% % %', a, b, c;
-   end loop;
-   -- using qualified names in fors, fore is enabled, disabled only for fori
-   for lbl.a, lbl.b, lbl.c in execute $$select gs, 'bb','cc' from generate_series(1,4) gs$$ loop
-     raise notice '% % %', a, b, c;
-   end loop;
- end;
- $proc$ language plpgsql;
-
- select for_vect();
-
  -- regression test: verify that multiple uses of same plpgsql datum within
  -- a SQL command all get mapped to the same $n parameter.  The return value
  -- of the SELECT is not important, we only care that it doesn't fail with
--- 2285,2290 ----
*************** select stacked_diagnostics_test();
*** 3580,3651 ****

  drop function stacked_diagnostics_test();

- -- test CASE statement
-
- create or replace function case_test(bigint) returns text as $$
- declare a int = 10;
-         b int = 1;
- begin
-   case $1
-     when 1 then
-       return 'one';
-     when 2 then
-       return 'two';
-     when 3,4,3+5 then
-       return 'three, four or eight';
-     when a then
-       return 'ten';
-     when a+b, a+b+1 then
-       return 'eleven, twelve';
-   end case;
- end;
- $$ language plpgsql immutable;
-
- select case_test(1);
- select case_test(2);
- select case_test(3);
- select case_test(4);
- select case_test(5); -- fails
- select case_test(8);
- select case_test(10);
- select case_test(11);
- select case_test(12);
- select case_test(13); -- fails
-
- create or replace function catch() returns void as $$
- begin
-   raise notice '%', case_test(6);
- exception
-   when case_not_found then
-     raise notice 'caught case_not_found % %', SQLSTATE, SQLERRM;
- end
- $$ language plpgsql;
-
- select catch();
-
- -- test the searched variant too, as well as ELSE
- create or replace function case_test(bigint) returns text as $$
- declare a int = 10;
- begin
-   case
-     when $1 = 1 then
-       return 'one';
-     when $1 = a + 2 then
-       return 'twelve';
-     else
-       return 'other';
-   end case;
- end;
- $$ language plpgsql immutable;
-
- select case_test(1);
- select case_test(2);
- select case_test(12);
- select case_test(13);
-
- drop function catch();
- drop function case_test(bigint);
-
  -- test variadic functions

  create or replace function vari(variadic int[])
--- 3345,3350 ----
*************** language plpgsql as $$
*** 4278,4283 ****
--- 3977,3984 ----
    begin return $1[1]; end;
  $$ stable;

+ select consumes_rw_array(returns_rw_array(42));
+
  -- bug #14174
  explain (verbose, costs off)
  select i, a from
*************** select consumes_rw_array(a), a from
*** 4300,4305 ****
--- 4001,4013 ----
  select consumes_rw_array(a), a from
    (values (returns_rw_array(1)), (returns_rw_array(2))) v(a);

+ do $$
+ declare a int[] := array[1,2];
+ begin
+   a := a || 3;
+   raise notice 'a = %', a;
+ end$$;
+

  --
  -- Test access to call stack
diff --git a/src/pl/plpgsql/src/expected/plpgsql_control.out b/src/pl/plpgsql/src/expected/plpgsql_control.out
index b7089e3..73b23a3 100644
*** a/src/pl/plpgsql/src/expected/plpgsql_control.out
--- b/src/pl/plpgsql/src/expected/plpgsql_control.out
*************** begin
*** 413,449 ****
    raise notice 'should get here';
  end$$;
  NOTICE:  should get here
- -- check exit to a matching outer loop (other cases are covered elsewhere)
- do $$
- begin
- <<outerloop>>
- for i in 1..10 loop
-   <<innerloop>>
-   for j in 1..10 loop
-     exit outerloop;
-     raise notice 'should not get here';
-   end loop;
-   raise notice 'should not get here, either';
- end loop;
- raise notice 'should get here';
- end$$;
- NOTICE:  should get here
- -- return out of a loop
- create function return_from_loop() returns int language plpgsql as $$
- begin
-   for i in 1..10 loop
-     if i > 3 then
-       return i;
-     end if;
-   end loop;
-   return null;
- end$$;
- select return_from_loop();
-  return_from_loop
- ------------------
-                 4
- (1 row)
-
  -- unlabeled exit does match a while loop
  do $$
  begin
--- 413,418 ----
*************** select return_from_while();
*** 522,657 ****
                   3
  (1 row)

- -- exit out of a foreach
- do $$
- declare i int; j int; k int;
- begin
-   <<outermostforeach>>
-   foreach i in array array[1,2,3] loop
-     <<outerforeach>>
-     foreach j in array array[1,2,3] loop
-       <<innerforeach>>
-       foreach k in array array[1,2,3] loop
-         exit;
-         raise notice 'should not get here';
-       end loop;
-       raise notice 'should get here';
-       exit outermostforeach;
-       raise notice 'should not get here, either';
-     end loop;
-     raise notice 'nor here';
-   end loop;
-   raise notice 'should get here, too';
- end$$;
- NOTICE:  should get here
- NOTICE:  should get here, too
- -- continue out of a foreach
- do $$
- declare i int; j int; k int;
- begin
-   <<outermostforeach>>
-   foreach i in array array[1,2,3] loop
-     <<outerforeach>>
-     foreach j in array array[1,2,3] loop
-       <<innerforeach>>
-       foreach k in array array[1,2,3] loop
-         continue;
-         raise notice 'should not get here';
-       end loop;
-       raise notice 'should get here, k = %', k;
-       continue outermostforeach;
-       raise notice 'should not get here, either';
-     end loop;
-     raise notice 'nor here';
-   end loop;
-   raise notice 'should get here, i = %, j = %, k = %', i, j, k;
- end$$;
- NOTICE:  should get here, k = 3
- NOTICE:  should get here, k = 3
- NOTICE:  should get here, k = 3
- NOTICE:  should get here, i = 3, j = 1, k = 3
- -- return out of a foreach
- create function return_from_foreach() returns int language plpgsql as $$
- declare i int := 0;
- begin
-   foreach i in array array[1,2,3] loop
-     if i > 1 then
-       return i;
-     end if;
-   end loop;
-   return null;
- end$$;
- select return_from_foreach();
-  return_from_foreach
- ---------------------
-                    2
- (1 row)
-
- -- exit out of a for-over-query
- do $$
- declare i int; j int; k int;
- begin
-   <<outermostfor>>
-   for i in select generate_series(1,3) loop
-     <<outerfor>>
-     for j in select generate_series(1,3) loop
-       <<innerfor>>
-       for k in select generate_series(1,3) loop
-         exit;
-         raise notice 'should not get here';
-       end loop;
-       raise notice 'should get here';
-       exit outermostfor;
-       raise notice 'should not get here, either';
-     end loop;
-     raise notice 'nor here';
-   end loop;
-   raise notice 'should get here, too';
- end$$;
- NOTICE:  should get here
- NOTICE:  should get here, too
- -- continue out of a for-over-query
- do $$
- declare i int; j int; k int;
- begin
-   <<outermostfor>>
-   for i in select generate_series(1,3) loop
-     <<outerfor>>
-     for j in select generate_series(1,3) loop
-       <<innerfor>>
-       for k in select generate_series(1,3) loop
-         continue;
-         raise notice 'should not get here';
-       end loop;
-       raise notice 'should get here, k = %', k;
-       continue outermostfor;
-       raise notice 'should not get here, either';
-     end loop;
-     raise notice 'nor here';
-   end loop;
-   raise notice 'should get here, i = %, j = %, k = %', i, j, k;
- end$$;
- NOTICE:  should get here, k = 3
- NOTICE:  should get here, k = 3
- NOTICE:  should get here, k = 3
- NOTICE:  should get here, i = 3, j = 1, k = 3
- -- return out of a for-over-query
- create function return_from_for_query() returns int language plpgsql as $$
- declare i int := 0;
- begin
-   for i in select generate_series(1,3) loop
-     if i > 1 then
-       return i;
-     end if;
-   end loop;
-   return null;
- end$$;
- select return_from_for_query();
-  return_from_for_query
- -----------------------
-                      2
- (1 row)
-
  -- using list of scalars in fori and fore stmts
  create function for_vect() returns void as $proc$
  <<lbl>>declare a integer; b varchar; c varchar; r record;
--- 491,496 ----
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index dd575e7..a05a52a 100644
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
*************** typedef struct                    /* cast_hash table en
*** 155,160 ****
--- 155,234 ----
  static MemoryContext shared_cast_context = NULL;
  static HTAB *shared_cast_hash = NULL;

+ /*
+  * LOOP_RC_PROCESSING encapsulates common logic for looping statements to
+  * handle return/exit/continue result codes from the loop body statement(s).
+  * It's meant to be used like this:
+  *
+  *        int rc = PLPGSQL_RC_OK;
+  *        for (...)
+  *        {
+  *            ...
+  *            rc = exec_stmts(estate, stmt->body);
+  *            LOOP_RC_PROCESSING(stmt->label);
+  *            ...
+  *        }
+  *        return rc;
+  *
+  * If execution of the loop should terminate, LOOP_RC_PROCESSING will execute
+  * a "break", after updating "rc" if necessary to the value the current
+  * statement should return.  If execution should continue, LOOP_RC_PROCESSING
+  * will do nothing except reset "rc" to PLPGSQL_RC_OK.
+  *
+  * estate and rc are implicit arguments to the macro.
+  * estate->exitlabel is examined and possibly updated.
+  */
+ #define LOOP_RC_PROCESSING(looplabel) \
+     if (rc == PLPGSQL_RC_RETURN) \
+     { \
+         /* RETURN, so propagate RC_RETURN out */ \
+         break; \
+     } \
+     else if (rc == PLPGSQL_RC_EXIT) \
+     { \
+         if (estate->exitlabel == NULL) \
+         { \
+             /* unlabelled EXIT terminates this loop */ \
+             rc = PLPGSQL_RC_OK; \
+             break; \
+         } \
+         else if ((looplabel) != NULL && \
+                  strcmp(looplabel, estate->exitlabel) == 0) \
+         { \
+             /* labelled EXIT matching this loop, so terminate loop */ \
+             estate->exitlabel = NULL; \
+             rc = PLPGSQL_RC_OK; \
+             break; \
+         } \
+         else \
+         { \
+             /* non-matching labelled EXIT, propagate RC_EXIT out */ \
+             break; \
+         } \
+     } \
+     else if (rc == PLPGSQL_RC_CONTINUE) \
+     { \
+         if (estate->exitlabel == NULL) \
+         { \
+             /* unlabelled CONTINUE matches this loop, so continue in loop */ \
+             rc = PLPGSQL_RC_OK; \
+         } \
+         else if ((looplabel) != NULL && \
+                  strcmp(looplabel, estate->exitlabel) == 0) \
+         { \
+             /* labelled CONTINUE matching this loop, so continue in loop */ \
+             estate->exitlabel = NULL; \
+             rc = PLPGSQL_RC_OK; \
+         } \
+         else \
+         { \
+             /* non-matching labelled CONTINUE, propagate RC_CONTINUE out */ \
+             break; \
+         } \
+     } \
+     else \
+         Assert(rc == PLPGSQL_RC_OK)
+
  /************************************************************
   * Local function forward declarations
   ************************************************************/
*************** exec_stmt_block(PLpgSQL_execstate *estat
*** 1476,1482 ****
      estate->err_text = NULL;

      /*
!      * Handle the return code.
       */
      switch (rc)
      {
--- 1550,1558 ----
      estate->err_text = NULL;

      /*
!      * Handle the return code.  This is intentionally different from
!      * LOOP_RC_PROCESSING(): CONTINUE never matches a block, and EXIT matches
!      * a block only if there is a label match.
       */
      switch (rc)
      {
*************** exec_stmt_block(PLpgSQL_execstate *estat
*** 1486,1496 ****
              return rc;

          case PLPGSQL_RC_EXIT:
-
-             /*
-              * This is intentionally different from the handling of RC_EXIT
-              * for loops: to match a block, we require a match by label.
-              */
              if (estate->exitlabel == NULL)
                  return PLPGSQL_RC_EXIT;
              if (block->label == NULL)
--- 1562,1567 ----
*************** exec_stmt_case(PLpgSQL_execstate *estate
*** 1948,1992 ****
  static int
  exec_stmt_loop(PLpgSQL_execstate *estate, PLpgSQL_stmt_loop *stmt)
  {
      for (;;)
      {
!         int            rc = exec_stmts(estate, stmt->body);
!
!         switch (rc)
!         {
!             case PLPGSQL_RC_OK:
!                 break;
!
!             case PLPGSQL_RC_EXIT:
!                 if (estate->exitlabel == NULL)
!                     return PLPGSQL_RC_OK;
!                 if (stmt->label == NULL)
!                     return PLPGSQL_RC_EXIT;
!                 if (strcmp(stmt->label, estate->exitlabel) != 0)
!                     return PLPGSQL_RC_EXIT;
!                 estate->exitlabel = NULL;
!                 return PLPGSQL_RC_OK;
!
!             case PLPGSQL_RC_CONTINUE:
!                 if (estate->exitlabel == NULL)
!                     /* anonymous continue, so re-run the loop */
!                     break;
!                 else if (stmt->label != NULL &&
!                          strcmp(stmt->label, estate->exitlabel) == 0)
!                     /* label matches named continue, so re-run loop */
!                     estate->exitlabel = NULL;
!                 else
!                     /* label doesn't match named continue, so propagate upward */
!                     return PLPGSQL_RC_CONTINUE;
!                 break;
!
!             case PLPGSQL_RC_RETURN:
!                 return rc;

!             default:
!                 elog(ERROR, "unrecognized rc: %d", rc);
!         }
      }
  }


--- 2019,2034 ----
  static int
  exec_stmt_loop(PLpgSQL_execstate *estate, PLpgSQL_stmt_loop *stmt)
  {
+     int            rc = PLPGSQL_RC_OK;
+
      for (;;)
      {
!         rc = exec_stmts(estate, stmt->body);

!         LOOP_RC_PROCESSING(stmt->label);
      }
+
+     return rc;
  }


*************** exec_stmt_loop(PLpgSQL_execstate *estate
*** 1999,2007 ****
  static int
  exec_stmt_while(PLpgSQL_execstate *estate, PLpgSQL_stmt_while *stmt)
  {
      for (;;)
      {
-         int            rc;
          bool        value;
          bool        isnull;

--- 2041,2050 ----
  static int
  exec_stmt_while(PLpgSQL_execstate *estate, PLpgSQL_stmt_while *stmt)
  {
+     int            rc = PLPGSQL_RC_OK;
+
      for (;;)
      {
          bool        value;
          bool        isnull;

*************** exec_stmt_while(PLpgSQL_execstate *estat
*** 2013,2055 ****

          rc = exec_stmts(estate, stmt->body);

!         switch (rc)
!         {
!             case PLPGSQL_RC_OK:
!                 break;
!
!             case PLPGSQL_RC_EXIT:
!                 if (estate->exitlabel == NULL)
!                     return PLPGSQL_RC_OK;
!                 if (stmt->label == NULL)
!                     return PLPGSQL_RC_EXIT;
!                 if (strcmp(stmt->label, estate->exitlabel) != 0)
!                     return PLPGSQL_RC_EXIT;
!                 estate->exitlabel = NULL;
!                 return PLPGSQL_RC_OK;
!
!             case PLPGSQL_RC_CONTINUE:
!                 if (estate->exitlabel == NULL)
!                     /* anonymous continue, so re-run loop */
!                     break;
!                 else if (stmt->label != NULL &&
!                          strcmp(stmt->label, estate->exitlabel) == 0)
!                     /* label matches named continue, so re-run loop */
!                     estate->exitlabel = NULL;
!                 else
!                     /* label doesn't match named continue, propagate upward */
!                     return PLPGSQL_RC_CONTINUE;
!                 break;
!
!             case PLPGSQL_RC_RETURN:
!                 return rc;
!
!             default:
!                 elog(ERROR, "unrecognized rc: %d", rc);
!         }
      }

!     return PLPGSQL_RC_OK;
  }


--- 2056,2065 ----

          rc = exec_stmts(estate, stmt->body);

!         LOOP_RC_PROCESSING(stmt->label);
      }

!     return rc;
  }


*************** exec_stmt_fori(PLpgSQL_execstate *estate
*** 2163,2212 ****
           */
          rc = exec_stmts(estate, stmt->body);

!         if (rc == PLPGSQL_RC_RETURN)
!             break;                /* break out of the loop */
!         else if (rc == PLPGSQL_RC_EXIT)
!         {
!             if (estate->exitlabel == NULL)
!                 /* unlabelled exit, finish the current loop */
!                 rc = PLPGSQL_RC_OK;
!             else if (stmt->label != NULL &&
!                      strcmp(stmt->label, estate->exitlabel) == 0)
!             {
!                 /* labelled exit, matches the current stmt's label */
!                 estate->exitlabel = NULL;
!                 rc = PLPGSQL_RC_OK;
!             }
!
!             /*
!              * otherwise, this is a labelled exit that does not match the
!              * current statement's label, if any: return RC_EXIT so that the
!              * EXIT continues to propagate up the stack.
!              */
!             break;
!         }
!         else if (rc == PLPGSQL_RC_CONTINUE)
!         {
!             if (estate->exitlabel == NULL)
!                 /* unlabelled continue, so re-run the current loop */
!                 rc = PLPGSQL_RC_OK;
!             else if (stmt->label != NULL &&
!                      strcmp(stmt->label, estate->exitlabel) == 0)
!             {
!                 /* label matches named continue, so re-run loop */
!                 estate->exitlabel = NULL;
!                 rc = PLPGSQL_RC_OK;
!             }
!             else
!             {
!                 /*
!                  * otherwise, this is a named continue that does not match the
!                  * current statement's label, if any: return RC_CONTINUE so
!                  * that the CONTINUE will propagate up the stack.
!                  */
!                 break;
!             }
!         }

          /*
           * Increase/decrease loop value, unless it would overflow, in which
--- 2173,2179 ----
           */
          rc = exec_stmts(estate, stmt->body);

!         LOOP_RC_PROCESSING(stmt->label);

          /*
           * Increase/decrease loop value, unless it would overflow, in which
*************** exec_stmt_foreach_a(PLpgSQL_execstate *e
*** 2536,2586 ****
           */
          rc = exec_stmts(estate, stmt->body);

!         /* Handle the return code */
!         if (rc == PLPGSQL_RC_RETURN)
!             break;                /* break out of the loop */
!         else if (rc == PLPGSQL_RC_EXIT)
!         {
!             if (estate->exitlabel == NULL)
!                 /* unlabelled exit, finish the current loop */
!                 rc = PLPGSQL_RC_OK;
!             else if (stmt->label != NULL &&
!                      strcmp(stmt->label, estate->exitlabel) == 0)
!             {
!                 /* labelled exit, matches the current stmt's label */
!                 estate->exitlabel = NULL;
!                 rc = PLPGSQL_RC_OK;
!             }
!
!             /*
!              * otherwise, this is a labelled exit that does not match the
!              * current statement's label, if any: return RC_EXIT so that the
!              * EXIT continues to propagate up the stack.
!              */
!             break;
!         }
!         else if (rc == PLPGSQL_RC_CONTINUE)
!         {
!             if (estate->exitlabel == NULL)
!                 /* unlabelled continue, so re-run the current loop */
!                 rc = PLPGSQL_RC_OK;
!             else if (stmt->label != NULL &&
!                      strcmp(stmt->label, estate->exitlabel) == 0)
!             {
!                 /* label matches named continue, so re-run loop */
!                 estate->exitlabel = NULL;
!                 rc = PLPGSQL_RC_OK;
!             }
!             else
!             {
!                 /*
!                  * otherwise, this is a named continue that does not match the
!                  * current statement's label, if any: return RC_CONTINUE so
!                  * that the CONTINUE will propagate up the stack.
!                  */
!                 break;
!             }
!         }

          MemoryContextSwitchTo(stmt_mcontext);
      }
--- 2503,2509 ----
           */
          rc = exec_stmts(estate, stmt->body);

!         LOOP_RC_PROCESSING(stmt->label);

          MemoryContextSwitchTo(stmt_mcontext);
      }
*************** exec_for_query(PLpgSQL_execstate *estate
*** 5330,5335 ****
--- 5253,5259 ----
      bool        found = false;
      int            rc = PLPGSQL_RC_OK;
      uint64        n;
+     uint64        i;

      /* Fetch loop variable's datum entry */
      var = (PLpgSQL_variable *) estate->datums[stmt->var->dno];
*************** exec_for_query(PLpgSQL_execstate *estate
*** 5364,5454 ****
      /*
       * Now do the loop
       */
!     while (n > 0)
      {
!         uint64        i;
!
!         for (i = 0; i < n; i++)
!         {
!             /*
!              * Assign the tuple to the target
!              */
!             exec_move_row(estate, var, tuptab->vals[i], tuptab->tupdesc);
!             exec_eval_cleanup(estate);
!
!             /*
!              * Execute the statements
!              */
!             rc = exec_stmts(estate, stmt->body);
!
!             if (rc != PLPGSQL_RC_OK)
!             {
!                 if (rc == PLPGSQL_RC_EXIT)
!                 {
!                     if (estate->exitlabel == NULL)
!                     {
!                         /* unlabelled exit, so exit the current loop */
!                         rc = PLPGSQL_RC_OK;
!                     }
!                     else if (stmt->label != NULL &&
!                              strcmp(stmt->label, estate->exitlabel) == 0)
!                     {
!                         /* label matches this loop, so exit loop */
!                         estate->exitlabel = NULL;
!                         rc = PLPGSQL_RC_OK;
!                     }
!
!                     /*
!                      * otherwise, we processed a labelled exit that does not
!                      * match the current statement's label, if any; return
!                      * RC_EXIT so that the EXIT continues to recurse upward.
!                      */
!                 }
!                 else if (rc == PLPGSQL_RC_CONTINUE)
!                 {
!                     if (estate->exitlabel == NULL)
!                     {
!                         /* unlabelled continue, so re-run the current loop */
!                         rc = PLPGSQL_RC_OK;
!                         continue;
!                     }
!                     else if (stmt->label != NULL &&
!                              strcmp(stmt->label, estate->exitlabel) == 0)
!                     {
!                         /* label matches this loop, so re-run loop */
!                         estate->exitlabel = NULL;
!                         rc = PLPGSQL_RC_OK;
!                         continue;
!                     }
!
!                     /*
!                      * otherwise, we process a labelled continue that does not
!                      * match the current statement's label, if any; return
!                      * RC_CONTINUE so that the CONTINUE will propagate up the
!                      * stack.
!                      */
!                 }

!                 /*
!                  * We're aborting the loop.  Need a goto to get out of two
!                  * levels of loop...
!                  */
!                 goto loop_exit;
!             }
!         }

!         SPI_freetuptable(tuptab);

          /*
!          * Fetch more tuples.  If prefetching is allowed, grab 50 at a time.
           */
!         SPI_cursor_fetch(portal, true, prefetch_ok ? 50 : 1);
!         tuptab = SPI_tuptable;
!         n = SPI_processed;
      }

- loop_exit:
-
      /*
       * Release last group of tuples (if any)
       */
--- 5288,5324 ----
      /*
       * Now do the loop
       */
!     i = 0;
!     while (i < n)
      {
!         /*
!          * Assign the tuple to the target
!          */
!         exec_move_row(estate, var, tuptab->vals[i], tuptab->tupdesc);
!         exec_eval_cleanup(estate);

!         /*
!          * Execute the statements
!          */
!         rc = exec_stmts(estate, stmt->body);

!         LOOP_RC_PROCESSING(stmt->label);

          /*
!          * Advance to next tuple in current tuptab, if there is one.
!          * Otherwise, fetch more tuples.
           */
!         if (++i >= n)
!         {
!             SPI_freetuptable(tuptab);
!             /* If prefetching is allowed, grab 50 at a time. */
!             SPI_cursor_fetch(portal, true, prefetch_ok ? 50 : 1);
!             tuptab = SPI_tuptable;
!             n = SPI_processed;
!             i = 0;
!         }
      }

      /*
       * Release last group of tuples (if any)
       */
diff --git a/src/pl/plpgsql/src/sql/plpgsql_control.sql b/src/pl/plpgsql/src/sql/plpgsql_control.sql
index 2a9dfa9..61d6ca6 100644
*** a/src/pl/plpgsql/src/sql/plpgsql_control.sql
--- b/src/pl/plpgsql/src/sql/plpgsql_control.sql
*************** begin
*** 311,344 ****
    raise notice 'should get here';
  end$$;

- -- check exit to a matching outer loop (other cases are covered elsewhere)
- do $$
- begin
- <<outerloop>>
- for i in 1..10 loop
-   <<innerloop>>
-   for j in 1..10 loop
-     exit outerloop;
-     raise notice 'should not get here';
-   end loop;
-   raise notice 'should not get here, either';
- end loop;
- raise notice 'should get here';
- end$$;
-
- -- return out of a loop
- create function return_from_loop() returns int language plpgsql as $$
- begin
-   for i in 1..10 loop
-     if i > 3 then
-       return i;
-     end if;
-   end loop;
-   return null;
- end$$;
-
- select return_from_loop();
-
  -- unlabeled exit does match a while loop
  do $$
  begin
--- 311,316 ----
*************** end$$;
*** 411,532 ****

  select return_from_while();

- -- exit out of a foreach
- do $$
- declare i int; j int; k int;
- begin
-   <<outermostforeach>>
-   foreach i in array array[1,2,3] loop
-     <<outerforeach>>
-     foreach j in array array[1,2,3] loop
-       <<innerforeach>>
-       foreach k in array array[1,2,3] loop
-         exit;
-         raise notice 'should not get here';
-       end loop;
-       raise notice 'should get here';
-       exit outermostforeach;
-       raise notice 'should not get here, either';
-     end loop;
-     raise notice 'nor here';
-   end loop;
-   raise notice 'should get here, too';
- end$$;
-
- -- continue out of a foreach
- do $$
- declare i int; j int; k int;
- begin
-   <<outermostforeach>>
-   foreach i in array array[1,2,3] loop
-     <<outerforeach>>
-     foreach j in array array[1,2,3] loop
-       <<innerforeach>>
-       foreach k in array array[1,2,3] loop
-         continue;
-         raise notice 'should not get here';
-       end loop;
-       raise notice 'should get here, k = %', k;
-       continue outermostforeach;
-       raise notice 'should not get here, either';
-     end loop;
-     raise notice 'nor here';
-   end loop;
-   raise notice 'should get here, i = %, j = %, k = %', i, j, k;
- end$$;
-
- -- return out of a foreach
- create function return_from_foreach() returns int language plpgsql as $$
- declare i int := 0;
- begin
-   foreach i in array array[1,2,3] loop
-     if i > 1 then
-       return i;
-     end if;
-   end loop;
-   return null;
- end$$;
-
- select return_from_foreach();
-
- -- exit out of a for-over-query
- do $$
- declare i int; j int; k int;
- begin
-   <<outermostfor>>
-   for i in select generate_series(1,3) loop
-     <<outerfor>>
-     for j in select generate_series(1,3) loop
-       <<innerfor>>
-       for k in select generate_series(1,3) loop
-         exit;
-         raise notice 'should not get here';
-       end loop;
-       raise notice 'should get here';
-       exit outermostfor;
-       raise notice 'should not get here, either';
-     end loop;
-     raise notice 'nor here';
-   end loop;
-   raise notice 'should get here, too';
- end$$;
-
- -- continue out of a for-over-query
- do $$
- declare i int; j int; k int;
- begin
-   <<outermostfor>>
-   for i in select generate_series(1,3) loop
-     <<outerfor>>
-     for j in select generate_series(1,3) loop
-       <<innerfor>>
-       for k in select generate_series(1,3) loop
-         continue;
-         raise notice 'should not get here';
-       end loop;
-       raise notice 'should get here, k = %', k;
-       continue outermostfor;
-       raise notice 'should not get here, either';
-     end loop;
-     raise notice 'nor here';
-   end loop;
-   raise notice 'should get here, i = %, j = %, k = %', i, j, k;
- end$$;
-
- -- return out of a for-over-query
- create function return_from_for_query() returns int language plpgsql as $$
- declare i int := 0;
- begin
-   for i in select generate_series(1,3) loop
-     if i > 1 then
-       return i;
-     end if;
-   end loop;
-   return null;
- end$$;
-
- select return_from_for_query();
-
  -- using list of scalars in fori and fore stmts
  create function for_vect() returns void as $proc$
  <<lbl>>declare a integer; b varchar; c varchar; r record;
--- 383,388 ----

Re: Better testing coverage and unified coding for plpgsql loops

From
Pavel Stehule
Date:


2017-12-30 22:46 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
While I've been fooling around with plpgsql, I've been paying close
attention to code coverage reports to make sure that the regression tests
exercise all that new code.  It started to bug me that there were some
serious gaps in the test coverage for existing code in pl_exec.c.
One thing I noticed in particular was that coverage for the
PLPGSQL_RC_EXIT/PLPGSQL_RC_RETURN/PLPGSQL_RC_CONTINUE management code
in the various looping statements was nearly nonexistent, and coverage
for integer FOR loops was pretty awful too (eg, not a single BY clause
in the whole test corpus :-().  So I said to myself "let's fix that",
and wrote up a new regression test file plpgsql_control.sql with a
charter to test plpgsql's control structures.  I moved a few existing
tests that seemed to fall into that charter into the new file, and
added tests until I was happier with the state of the coverage report.
The result is attached as plpgsql-better-code-coverage.patch.

However, while I was doing that, it seemed like the tests I was adding
were mighty repetitive, as many of them were just exactly the same thing
adjusted for a different kind of loop statement.  And so I began to wonder
why it was that we had five copies of the RC_FOO management logic, no two
quite alike.  If we only had *one* copy then it would not seem necessary
to have such duplicative test cases for it.  A bit of hacking later, and
I had the management logic expressed as a macro, with only one copy for
all five kinds of loop.  I verified it still passes the previous set of
tests and then removed the ones that seemed redundant, yielding
plpgsql-unify-loop-rc-code.patch below.  So what I propose actually
committing is the combination of these two patches.

Anyone feel like reviewing this, or should I just push it?  It seems
pretty noncontroversial to me, but maybe I'm wrong about that.

I don't think any issue there. This macro is little bit massive, but similar is used elsewhere.

+1

Pavel



                        regards, tom lane


Re: Better testing coverage and unified coding for plpgsql loops

From
Alvaro Herrera
Date:
Tom Lane wrote:

> However, while I was doing that, it seemed like the tests I was adding
> were mighty repetitive, as many of them were just exactly the same thing
> adjusted for a different kind of loop statement.  And so I began to wonder
> why it was that we had five copies of the RC_FOO management logic, no two
> quite alike.  If we only had *one* copy then it would not seem necessary
> to have such duplicative test cases for it.  A bit of hacking later, and
> I had the management logic expressed as a macro, with only one copy for
> all five kinds of loop.

I'm not entirely sure about this rationale.  Improving coverage is of
course an important goal, but it's not the only one: each documented
construct and behavior should be tested also explicitly, to avoid any
inadvertant breakage.  You're probably the most careful committer in the
project, but what that means is that some other less careful committer
(present or future) will need to hack this code again in the future and
break some of the cases that you've made to work, because the test cases
only exercise the generic behavior through some specific kind of loop,
and not every individual kind of loop specifically.

In other words, I think testing (the basic behavior of) every construct
separately is worthwhile even if it tests the same code several times.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: Better testing coverage and unified coding for plpgsql loops

From
Darafei "Komяpa" Praliaskouski
Date:
Hello!
However, while I was doing that, it seemed like the tests I was adding
were mighty repetitive, as many of them were just exactly the same thing
adjusted for a different kind of loop statement.  And so I began to wonder
why it was that we had five copies of the RC_FOO management logic, no two
quite alike.  If we only had *one* copy then it would not seem necessary
to have such duplicative test cases for it.  A bit of hacking later, and
I had the management logic expressed as a macro, with only one copy for
all five kinds of loop.  I verified it still passes the previous set of
tests and then removed the ones that seemed redundant, yielding
plpgsql-unify-loop-rc-code.patch below.  So what I propose actually
committing is the combination of these two patches.

I have looked into plpgsql-unify-loop-rc-code.patch. 
I have two questions:

 - how do currently existing coverage tools display coverage for such a large macro? 

I expect DEFINE's to be treated as comments. 

I've looked into https://coverage.postgresql.org/src/port/qsort.c.gcov.html and on line 70 I see a similar multi line define that is yellow in coverage, not counted at all. I think that "higher coverage" effect you are seeing is mostly due to code being hidden from coverage counter, not actually better testing. Another thing I see is that most define's are in .h files, and they're also not in coverage report mostly.

 - can this macro become a function?
 

Re: Better testing coverage and unified coding for plpgsql loops

From
Alvaro Herrera
Date:
Darafei "Komяpa" Praliaskouski wrote:

>  - how do currently existing coverage tools display coverage for such a
> large macro?
> 
> I expect DEFINE's to be treated as comments.

It is, but then it is counted in the callsite where each branch is
displayed separately.  So in
https://coverage.postgresql.org/src/pl/plpgsql/src/pl_exec.c.gcov.html
line 2028 you can see a bunch of "+" and three "-".

>  - can this macro become a function?

The "exit_action" argument makes it tough.  It can probably be done --
it seems to require contorting the one callsite that uses "goto" though.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: Better testing coverage and unified coding for plpgsql loops

From
Tom Lane
Date:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Darafei "Komяpa" Praliaskouski wrote:
>> - can this macro become a function?

> The "exit_action" argument makes it tough.  It can probably be done --
> it seems to require contorting the one callsite that uses "goto" though.

It could be converted into a function returning bool, a la

    if (!loop_rc_processing(...))
        break;

but then the burden is on you to show there's negligible performance
impact, a question that doesn't arise when just macro-izing existing
code.  I suppose the function could be made inline, but then we're
right back to the question of how well lcov will display the actual
code coverage.

            regards, tom lane


Re: Better testing coverage and unified coding for plpgsql loops

From
Tom Lane
Date:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Tom Lane wrote:
>> However, while I was doing that, it seemed like the tests I was adding
>> were mighty repetitive, as many of them were just exactly the same thing
>> adjusted for a different kind of loop statement.  And so I began to wonder
>> why it was that we had five copies of the RC_FOO management logic, no two
>> quite alike.  If we only had *one* copy then it would not seem necessary
>> to have such duplicative test cases for it.  A bit of hacking later, and
>> I had the management logic expressed as a macro, with only one copy for
>> all five kinds of loop.

> I'm not entirely sure about this rationale.  Improving coverage is of
> course an important goal, but it's not the only one: each documented
> construct and behavior should be tested also explicitly, to avoid any
> inadvertant breakage.  You're probably the most careful committer in the
> project, but what that means is that some other less careful committer
> (present or future) will need to hack this code again in the future and
> break some of the cases that you've made to work, because the test cases
> only exercise the generic behavior through some specific kind of loop,
> and not every individual kind of loop specifically.

I don't especially buy this argument, at least not in this case.
I can certainly believe that somebody would add another looping construct
to plpgsql in future, but with the new setup they'd just copy-and-paste
a macro invocation, and there's basically nothing to break.  Fooling
with the RC logic itself is so rare as to be negligible --- looking at
the git history, most of it sprang full grown from Jan Wieck's forehead
in 1998, and the only meaningful change since then was when Neil Conway
added CONTINUE in 2005.  So I think you're proposing to add regression
testing overhead that will burden every developer, every day, for the
foreseeable future, for a really negligible return.  That path leads to
regression tests that nobody runs because they take two hours.

            regards, tom lane


Re: Better testing coverage and unified coding for plpgsql loops

From
Robert Haas
Date:
On Tue, Jan 2, 2018 at 10:08 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>> Darafei "Komяpa" Praliaskouski wrote:
>>> - can this macro become a function?
>
>> The "exit_action" argument makes it tough.  It can probably be done --
>> it seems to require contorting the one callsite that uses "goto" though.
>
> It could be converted into a function returning bool, a la
>
>         if (!loop_rc_processing(...))
>                 break;
>
> but then the burden is on you to show there's negligible performance
> impact, a question that doesn't arise when just macro-izing existing
> code.  I suppose the function could be made inline, but then we're
> right back to the question of how well lcov will display the actual
> code coverage.

I prefer writing this sort of thing using a function call and
dispatching on the return value, as you suggest in the text quoted
here. Long macros that involve a zillion continuation lines are hard
to edit, and often hard to step through properly in a debugger.  It
may be that in this case it doesn't matter much because, as you said
in the other email, this code may have no bugs and never get modified;
if so, we don't care whether it's hard to edit and debug the code.  Of
course, then we also don't really need tests for it, either.

I disagree that Alvaro or Darafei must carry the burden of proving
that using a function rather than a macro doesn't slow anything down.
In general, it's not like performance concerns unilaterally trump
readability.  But in this case, I suspect it would be hard to prove
anything at all.  It seems unlikely to be in the first place that any
change would be anything more than noise, and there's the sort of
broader issues that duplicating the code makes the binary bigger,
which carries a distributed cost of its own.  I'm not sure how you'd
even design a fair test for something like this.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: Better testing coverage and unified coding for plpgsql loops

From
Tom Lane
Date:
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Jan 2, 2018 at 10:08 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It could be converted into a function returning bool, a la
>>     if (!loop_rc_processing(...))
>>         break;

> I prefer writing this sort of thing using a function call and
> dispatching on the return value, as you suggest in the text quoted
> here. Long macros that involve a zillion continuation lines are hard
> to edit, and often hard to step through properly in a debugger.

I thought about this a bit harder and realized that if we make it
a function, we will have to pass "rc" by reference since the function
needs to change it in some cases.  That might have no impact if the
compiler is smart enough, but I expect on at least some compilers
it would end up forcing rc into memory with an attendant speed hit.

I really think we should stick with the macro implementation, unless
somebody wants to do some actual investigation to prove that a
function implementation imposes negligible cost.  I'm not prepared
to just assume that, especially not after the work I just did on
plpgsql record processing --- I initially thought that an extra
function call or three wouldn't matter in those code paths either,
but I found out differently.

            regards, tom lane


Re: Better testing coverage and unified coding for plpgsql loops

From
Robert Haas
Date:
On Wed, Jan 3, 2018 at 1:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I thought about this a bit harder and realized that if we make it
> a function, we will have to pass "rc" by reference since the function
> needs to change it in some cases.  That might have no impact if the
> compiler is smart enough, but I expect on at least some compilers
> it would end up forcing rc into memory with an attendant speed hit.
>
> I really think we should stick with the macro implementation, unless
> somebody wants to do some actual investigation to prove that a
> function implementation imposes negligible cost.  I'm not prepared
> to just assume that, especially not after the work I just did on
> plpgsql record processing --- I initially thought that an extra
> function call or three wouldn't matter in those code paths either,
> but I found out differently.

OK.  I'm not really exercised about it, so I'll leave it to others to
decide whether they want to spend time on it.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: Better testing coverage and unified coding for plpgsql loops

From
Alvaro Herrera
Date:
Tom Lane wrote:

> I really think we should stick with the macro implementation, unless
> somebody wants to do some actual investigation to prove that a
> function implementation imposes negligible cost.  I'm not prepared
> to just assume that, especially not after the work I just did on
> plpgsql record processing --- I initially thought that an extra
> function call or three wouldn't matter in those code paths either,
> but I found out differently.

I don't really care too much about the macro-or-function side of this,
but if you wanted to improve debuggability avoiding the performance cost
of a function call, you could use a static inline function, which is
supposed (AFAIK) to have performance characteristics equivalent to those
of a macro.  But again I'm not voting either way and I'm not in a
position to do the legwork either.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: Better testing coverage and unified coding for plpgsql loops

From
Tom Lane
Date:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Tom Lane wrote:
>> I really think we should stick with the macro implementation, unless
>> somebody wants to do some actual investigation to prove that a
>> function implementation imposes negligible cost.

> I don't really care too much about the macro-or-function side of this,
> but if you wanted to improve debuggability avoiding the performance cost
> of a function call, you could use a static inline function, which is
> supposed (AFAIK) to have performance characteristics equivalent to those
> of a macro.

I'm not sure whether inlining the function can be relied on to get rid
of the side effects of taking rc's address.  It wouldn't take all that
much work to establish the point, probably, but it's work I don't care
to put into it.

            regards, tom lane