compute: decode a LetRec binding's value once

The rendered value was decoded twice, by two independent operators over the same
stream: once where the binding is fed back, and again where it is extracted into
the outer scope. Each decode allocates an owned row per record per iteration.
Keep the first decode and reuse it.

Also correct the comment above, which said the recursive value flows as `Vec`
through the loop. It does not: the read edge is encoded for the readers and the
value is decoded again where it is fed back, so each iteration crosses the
container boundary twice.

Author: antiguru

src/compute/src/render.rs

@@ -953,10 +953,11 @@ impl<'scope> Context<'scope, Product<mz_repr::Timestamp, PointStamp<u64>>> {
                 // Re-encode the read-edge to columnar so `Get`s on this rec
                 // binding (e.g. as a Union input) see a columnar edge. The
                 // feedback `Variable` itself stays `Vec` (set at `oks_v.set`
-                // below). The recursive value flows `Vec` through the loop, and
-                // only the externally-visible collection is re-containered. The
-                // re-encode is a stateless, timestamp-agnostic pass-through, so
-                // it does not alter the iterative frontier or fixpoint behavior.
+                // below), so each iteration crosses the container boundary
+                // twice: encoded here for the readers, decoded once per binding
+                // where the value is fed back. The re-encode is a stateless,
+                // timestamp-agnostic pass-through, so it does not alter the
+                // iterative frontier or fixpoint behavior.
                 self.insert_id(
                     Id::Local(*id),
                     CollectionBundle::from_edge(
@@ -966,7 +967,10 @@ impl<'scope> Context<'scope, Product<mz_repr::Timestamp, PointStamp<u64>>> {
                 );
                 variables.insert(Id::Local(*id), (oks_v, err_v));
             }
-            // Now render each of the rec bindings.
+            // Now render each of the rec bindings. The decoded value is kept so
+            // the extraction below reuses it instead of decoding the same stream
+            // a second time.
+            let mut decoded_oks = BTreeMap::new();
             let mut rec_iter = recs.into_iter().peekable();
             while let Some(RecBind { id, value, limit }) = rec_iter.next() {
                 let last = rec_iter.peek().is_none();
@@ -976,6 +980,7 @@ impl<'scope> Context<'scope, Product<mz_repr::Timestamp, PointStamp<u64>>> {
                 // here to cause that to happen.
                 let (oks, mut err) = bundle.collection.clone().unwrap();
                 let oks = oks.into_vec();
+                decoded_oks.insert(id, oks.clone());
                 // Collapses what forward reads see. `err_v` below feeds reads rendered before this
                 // binding and is collapsed separately; without this, a `Get` in a later rec binding
                 // or in the body resolves to the bundle stored here and compounds level over level,
@@ -1035,8 +1040,10 @@ impl<'scope> Context<'scope, Product<mz_repr::Timestamp, PointStamp<u64>>> {
             // Now extract each of the rec bindings into the outer scope.
             for id in rec_ids.into_iter() {
                 let bundle = self.remove_id(Id::Local(id)).unwrap();
-                let (oks, err) = bundle.collection.unwrap();
-                let oks = oks.into_vec();
+                let (_, err) = bundle.collection.unwrap();
+                let oks = decoded_oks
+                    .remove(&id)
+                    .expect("rec binding decoded while rendering above");
                 // Extract into the outer scope and re-encode the read-edge to
                 // columnar, so `Get`s on the extracted binding see a columnar
                 // edge. `leave_dynamic` has already stripped the iteration