Skip to content

Commit c7b6576

Browse files
authored
[python] Fix output pcollections of composite transforms that return DoOutputsTuple (#36220)
* add test and potential fix * Simplify test * add some comments
1 parent cf55fee commit c7b6576

2 files changed

Lines changed: 56 additions & 3 deletions

File tree

sdks/python/apache_beam/pipeline.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -841,10 +841,10 @@ def apply(
841841
self._infer_result_type(transform, tuple(inputs.values()), result)
842842

843843
assert isinstance(result.producer.inputs, tuple)
844-
# The DoOutputsTuple adds the PCollection to the outputs when accessed
845-
# except for the main tag. Add the main tag here.
846844
if isinstance(result, pvalue.DoOutputsTuple):
847-
current.add_output(result, result._main_tag)
845+
for tag, pc in list(result._pcolls.items()):
846+
if tag not in current.outputs:
847+
current.add_output(pc, tag)
848848
continue
849849

850850
# If there is already a tag with the same name, increase a counter for

sdks/python/apache_beam/pipeline_test.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1564,6 +1564,59 @@ def file_artifact(path, hash, staged_name):
15641564

15651565
self.assertEqual(len(proto.components.environments), 6)
15661566

1567+
def test_multiple_outputs_composite_ptransform(self):
1568+
"""
1569+
Test that a composite PTransform with multiple outputs is represented
1570+
correctly in the pipeline proto.
1571+
"""
1572+
class SalesSplitter(beam.DoFn):
1573+
def process(self, element):
1574+
price = element['price']
1575+
if price > 100:
1576+
yield beam.pvalue.TaggedOutput('premium_sales', element)
1577+
else:
1578+
yield beam.pvalue.TaggedOutput('standard_sales', element)
1579+
1580+
class ParentSalesSplitter(beam.PTransform):
1581+
def expand(self, pcoll):
1582+
return pcoll | beam.ParDo(SalesSplitter()).with_outputs(
1583+
'premium_sales', 'standard_sales')
1584+
1585+
sales_data = [
1586+
{
1587+
'item': 'Laptop', 'price': 1200
1588+
},
1589+
{
1590+
'item': 'Mouse', 'price': 25
1591+
},
1592+
{
1593+
'item': 'Keyboard', 'price': 75
1594+
},
1595+
{
1596+
'item': 'Monitor', 'price': 350
1597+
},
1598+
{
1599+
'item': 'Headphones', 'price': 90
1600+
},
1601+
]
1602+
1603+
with beam.Pipeline() as pipeline:
1604+
sales_records = pipeline | 'Create Sales' >> beam.Create(sales_data)
1605+
_ = sales_records | 'Split Sales' >> ParentSalesSplitter()
1606+
current_transforms = list(pipeline.transforms_stack)
1607+
all_applied_transforms = {
1608+
xform.full_label: xform
1609+
for xform in current_transforms
1610+
}
1611+
while current_transforms:
1612+
xform = current_transforms.pop()
1613+
all_applied_transforms[xform.full_label] = xform
1614+
current_transforms.extend(xform.parts)
1615+
xform = all_applied_transforms['Split Sales']
1616+
# Confirm that Split Sales correctly has two outputs as specified by
1617+
# ParDo.with_outputs in ParentSalesSplitter.
1618+
assert len(xform.outputs) == 2
1619+
15671620

15681621
if __name__ == '__main__':
15691622
unittest.main()

0 commit comments

Comments
 (0)