plpgsql: add support for WHILE loops

[mgartner]

plpgsql/parser: parse WHILE loops

Release note: None

opt: support PL/pgSQL WHILE loops

This commit adds support for PL/pgSQL WHILE loops. A WHILE loop is
syntactic sugar for a LOOP with that exits conditionally, so we build
a WHILE loop with a simple transformation:

WHILE [cond] LOOP
  [body];
END LOOP;
=>
LOOP
  IF [cond] THEN
    [body];
  ELSE
    EXIT;
  END IF;
END LOOP;

This transformation allows us to avoid adding complicated logic
specifically for WHILE loops and instead rely on the existing
implementations for LOOP and IF/ELSE statements.

Epic: CRDB-799

Release note: None

opt: add unimplemented errors for EXIT/CONTINUE WHEN

Release note: None

[cockroach-teamcity]

This change is 

[mgartner]

The first three commits are from #108397.

Originally @rharding6373 and I tried to build a WHILE loop without transforming the AST, but we couldn't quite figure it out. I spent a bit more time trying to figure out the proper incantation, but eventually came the to conclusion that a simple AST transformation is probably the best solution - easy to understand and maintain, and it builds off of existing logic for LOOPs and IF/ELSE statements.

[DrewKimball]

Nice solution!

Reviewed 10 of 10 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 4 of 4 files at r4, 3 of 3 files at r5, 2 of 2 files at r6, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @rharding6373)

---

pkg/sql/plpgsql/parser/plpgsql.y line 920 at r4 (raw file):

stmt_while: opt_loop_label WHILE expr_until_loop LOOP loop_body opt_label ';'
  {
    // TODO(drewk): does the second usage of the label actually

I think this may just be useful for ensuring that a given END LOOP corresponds to the expected block - postgres throws an error if the loop being ended isn't the one that's currently in scope.

[rharding6373]

Although modifying ASTs is undesirable, one has to admit it's an elegant solution. Thanks for driving this!

Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @mgartner)

[mgartner]

Although modifying ASTs is undesirable, one has to admit it's an elegant solution.

Agreed. I think it's less bad in this case than the modification we do during type-checking because 1) we're always creating new nodes here, so the original AST is left unchanged and 2) during this building we're not annotating the original AST with information that needs to be used later. If (2) were to change, we might want to rethink this.

TFTRs!

bors r+

Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @DrewKimball)

---

pkg/sql/plpgsql/parser/plpgsql.y line 920 at r4 (raw file):

Previously, DrewKimball (Drew Kimball) wrote…

I think this may just be useful for ensuring that a given END LOOP corresponds to the expected block - postgres throws an error if the loop being ended isn't the one that's currently in scope.

Ok. We can remove/address the two TODOs in the future I think, if that sounds good to you. I just copied this from above.

[mgartner]

bors r-

[craig]

Canceled.

[mgartner]

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)