kennknowles commented on a change in pull request #7316:
URL: https://github.com/apache/beam/pull/7316#discussion_r501248667
##########
File path:
sdks/java/core/src/main/java/org/apache/beam/sdk/runners/TransformHierarchy.java
##########
@@ -629,13 +635,15 @@ private void visit(
if (!isRootNode()) {
// Visit inputs.
for (PValue inputValue : inputs.values()) {
- Node valueProducer = getProducer(inputValue);
- if (!visitedNodes.contains(valueProducer)) {
- valueProducer.visit(visitor, visitedValues, visitedNodes,
skippedComposites);
- }
- if (visitedValues.add(inputValue)) {
- LOG.debug("Visiting input value {}", inputValue);
- visitor.visitValue(inputValue, valueProducer);
+ Node valueProducer = maybeGetProducer(inputValue);
+ if (valueProducer != null) {
+ if (!visitedNodes.contains(valueProducer)) {
+ valueProducer.visit(visitor, visitedValues, visitedNodes,
skippedComposites);
+ }
+ if (visitedValues.add(inputValue)) {
Review comment:
I was just looking into side inputs. I believe that this change means
any value with a direct producer is not visited. So `PCollectionView` is only
visited because it is produced by a transform. But in fact if we remove the
deprecated `WritePCollectionView` primitive they are never visited. I think
they should be visited still. I'm not exactly sure the ramifications. I'll post
a PR if I figure it out but LMK if you know already whether this will break.
----------------------------------------------------------------
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:
[email protected]
