[GitHub] [beam] kennknowles commented on a change in pull request #13118: [BEAM-11094] Only work with fully expanded PCollections in TransformHierarchy and PTransformReplacements

[GitBox <gi...@apache.org>]

kennknowles commented on a change in pull request #13118:
URL: https://github.com/apache/beam/pull/13118#discussion_r514578489



##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/runners/TransformHierarchy.java
##########
@@ -483,12 +402,12 @@ private void setOutput(POutput output) {
       checkState(
           this.outputs == null, "Tried to specify more than one output for %s", getFullName());
       checkNotNull(output, "Tried to set the output of %s to null", getFullName());
-      this.outputs = output.expand();
+      this.outputs = PValues.fullyExpand(output.expand());

Review comment:
       I think this line and the similar one below below made it so a `PCollectionView` is not in `outputs`, so topological traversal does not eagerly process `PCollectionView` as a value, and PCollectionViews also do not have a producer (which is correct for the Beam model). This seems to affect only Dataflow and only because it has special logic which treats PCollectionView as a value in its v1beta3 translation.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org