[SPARK-39887][SQL] RemoveRedundantAliases should keep aliases that make the output of projection nodes unique #37334

peter-toth · 2022-07-28T19:03:37Z

What changes were proposed in this pull request?

Keep the output attributes of a Union node's first child in the RemoveRedundantAliases rule to avoid correctness issues.

Why are the changes needed?

To fix the result of the following query:

SELECT a, b AS a FROM (
  SELECT a, a AS b FROM (SELECT a FROM VALUES (1) AS t(a))
  UNION ALL
  SELECT a, b FROM (SELECT a, b FROM VALUES (1, 2) AS t(a, b))
)

Before this PR the query returns the incorrect result:

+---+---+
|  a|  a|
+---+---+
|  1|  1|
|  2|  2|
+---+---+

After this PR it returns the expected result:

+---+---+
|  a|  a|
+---+---+
|  1|  1|
|  1|  2|
+---+---+

Does this PR introduce any user-facing change?

Yes, fixes a correctness issue.

How was this patch tested?

Added new UTs.

… Union's first child

cfmcgrady · 2022-07-29T01:57:06Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

@@ -559,6 +559,17 @@ object RemoveRedundantAliases extends Rule[LogicalPlan] {
        })
        Join(newLeft, newRight, joinType, newCondition, hint)

+      case _: Union =>


Shall we update the method comment as well?

Thanks, fixed in db6c73f

cloud-fan · 2022-08-01T04:53:10Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

@@ -568,9 +581,6 @@ object RemoveRedundantAliases extends Rule[LogicalPlan] {
          newChild
        }

-        // Create the attribute mapping. Note that the currentNextAttrPairs can contain duplicate
-        // keys in case of Union (this is caused by the PushProjectionThroughUnion rule); in this
-        // case we use the first mapping (which should be provided by the first child).


since we remove this comment, shall we add an assert to make sure there is no duplicate keys?

added in 11020ee

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

cloud-fan · 2022-08-01T09:32:18Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

+            child match {
+              case p: Project =>
+                var passThroughAttributes =
+                  AttributeSet(p.projectList.filter(_.isInstanceOf[Attribute]))


I'm not sure this works. The Project may not be the direct child of Union. I think we should add a new bool flag to removeRedundantAliases which indicates that removing alias should not introduce conflicting expr IDs.

or we should always guarantee it. the plan looks quite weird if a plan node has output columns with conflicting expr IDs.

I'm not sure this works. The Project may not be the direct child of Union. I think we should add a new bool flag to removeRedundantAliases which indicates that removing alias should not introduce conflicting expr IDs.

I wanted to handle this case with line 585: case _ => removeRedundantAliases(child, excluded ++ child.outputSet) to keep all attributes if we have intermediate nodes between Union and Project. But I see your point, I can further refine this fix...

608f83a would be the change if we always wanted to guarantee it.

But it looks like an extra exchange is inserted into q70: d212dab:

(24) HashAggregate [codegen id : 5] Input [2]: [s_state#13, sum#16] Keys [1]: [s_state#13] Functions [1]: [sum(UnscaledValue(ss_net_profit#10))] Aggregate Attributes [1]: [sum(UnscaledValue(ss_net_profit#10))#17] -Results [3]: [s_state#13, s_state#13, MakeDecimal(sum(UnscaledValue(ss_net_profit#10))#17,17,2) AS _w2#18] +Results [3]: [s_state#13 AS s_state#18, s_state#13, MakeDecimal(sum(UnscaledValue(ss_net_profit#10))#17,17,2) AS _w2#19] -(25) Sort [codegen id : 5] -Input [3]: [s_state#13, s_state#13, _w2#18] -Arguments: [s_state#13 ASC NULLS FIRST, _w2#18 DESC NULLS LAST], false, 0 +(25) Exchange +Input [3]: [s_state#18, s_state#13, _w2#19] +Arguments: hashpartitioning(s_state#13, 5), ENSURE_REQUIREMENTS, [plan_id=3] -(26) Window -Input [3]: [s_state#13, s_state#13, _w2#18] -Arguments: [rank(_w2#18) windowspecdefinition(s_state#13, _w2#18 DESC NULLS LAST, specifiedwindowframe(RowFrame, unboundedpreceding$(), currentrow$())) AS ranking#19], [s_state#13], [_w2#18 DESC NULLS LAST] +(26) Sort [codegen id : 6] +Input [3]: [s_state#18, s_state#13, _w2#19] +Arguments: [s_state#13 ASC NULLS FIRST, _w2#19 DESC NULLS LAST], false, 0

and I'm not sure yet why. Tomorrow I can take a deeper look at it...

Hmm, I think the issue is in AliasAwareOutputPartitioning, it doesn't take into account if outputExpressions contain both an attribute and the attribute's alias...

cloud-fan · 2022-08-01T15:40:48Z

sql/core/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q70.sf100/explain.txt

-(25) Sort [codegen id : 5]
-Input [3]: [s_state#14, s_state#14, _w2#18]
-Arguments: [s_state#14 ASC NULLS FIRST, _w2#18 DESC NULLS LAST], false, 0
+(25) Exchange


so we have an extra exchange now?

Yes we have.
I think that's because a HashAggregate in q70 have an output attribute and also have it aliased as a new attribute (we kept that alias with this PR to avoid the same output attribute appear multiple times).

-Results [3]: [s_state#13, s_state#13, MakeDecimal(sum(UnscaledValue(ss_net_profit#10))#17,17,2) AS _w2#18] +Results [3]: [s_state#13 AS s_state#18, s_state#13, MakeDecimal(sum(UnscaledValue(ss_net_profit#10))#17,17,2) AS _w2#19]

IMO that shouldn't be a bad thing and the plan looks ok. But the root cause of the issue is in AliasAwareOutputPartitioning as it generates outputPartitioning based on the aliased output attribute only and doesn't produce a PartitioningCollection with both the base attribute and the aliased version.
Because the parent of the aggregate requires a distribution based on the base attribute EnsureRequirements inserts the extra exchange...

We also have test failures, I'm not sure yet why. Will check tomorrow...

I put the WIP flag into the PR description for now and giving a try to an experiment with b3b7d24 to fix AliasAwareOutputPartitioning and AliasAwareOutputOrdering to handle cases when AliasAwareOutputExpression.outputExpressions contains both an attribute and its aliased version.

b3b7d24 removed the extra exchange. I fixed the indentation of the code and added some comments in a737020.

Very likely 59dcc0d will fix the other test issues, but tests are still running...

cloud-fan · 2022-08-01T15:41:56Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

+          removeRedundantAliases(mapAttributes(a.aggregateExpressions, mapping), excluded),
+          newChild)
+
+      case w: Window =>


why do we need to deal with these 3 nodes separately now?

The removeRedundantAliases() is dealing with the output defining named expression lists as a whole (Project.projectList, Aggregate.aggregateExpressions, ...) to be able to keep those aliases that are required to keep the output attributes unique.

…plans

…d duplicate output attributes

peter-toth · 2022-08-02T15:41:44Z

sql/core/src/test/scala/org/apache/spark/sql/ExplainSuite.scala

@@ -681,7 +681,7 @@ class ExplainSuiteAE extends ExplainSuiteHelper with EnableAdaptiveExecutionSuit
        df.createTempView("df")

        val sqlText = "EXPLAIN CODEGEN SELECT key, MAX(value) FROM df GROUP BY key"
-        val expectedCodegenText = "Found 2 WholeStageCodegen subtrees."
+        val expectedCodegenText = "Found 1 WholeStageCodegen subtrees."


Due to the AliasAwareOutputPartitioning fix the explain codegen output (plan) changed from:

Found 2 WholeStageCodegen subtrees. == Subtree 1 / 2 (maxMethodCodeSize:282; maxConstantPoolSize:348(0.53% used); numInnerClasses:2) == *(1) HashAggregate(keys=[key#xL], functions=[partial_max(value#xL)], output=[key#xL, max#xL]) +- *(1) Project [id#xL AS key#xL, id#xL AS value#xL] +- *(1) Range (0, 5, step=1, splits=2) == Subtree 2 / 2 (maxMethodCodeSize:252; maxConstantPoolSize:214(0.33% used); numInnerClasses:0) == *(2) HashAggregate(keys=[key#xL], functions=[max(value#xL)], output=[key#xL, max(value)#xL]) +- AQEShuffleRead coalesced +- ShuffleQueryStage 0 +- Exchange hashpartitioning(key#xL, 5), ENSURE_REQUIREMENTS, [plan_id=55] +- *(1) HashAggregate(keys=[key#xL], functions=[partial_max(value#xL)], output=[key#xL, max#xL]) +- *(1) Project [id#xL AS key#xL, id#xL AS value#xL] +- *(1) Range (0, 5, step=1, splits=2)

to:

Found 1 WholeStageCodegen subtrees. == Subtree 1 / 1 (maxMethodCodeSize:308; maxConstantPoolSize:374(0.57% used); numInnerClasses:2) == *(1) HashAggregate(keys=[key#xL], functions=[max(value#xL)], output=[key#xL, max(value)#xL]) +- *(1) HashAggregate(keys=[key#xL], functions=[partial_max(value#xL)], output=[key#xL, max#xL]) +- *(1) Project [id#xL AS key#xL, id#xL AS value#xL] +- *(1) Range (0, 5, step=1, splits=2)

peter-toth · 2022-08-02T15:45:49Z

sql/core/src/test/scala/org/apache/spark/sql/execution/CoalesceShufflePartitionsSuite.scala

@@ -339,12 +339,12 @@ class CoalesceShufflePartitionsSuite extends SparkFunSuite {
      //     ShuffleQueryStage 0
      //   ShuffleQueryStage 2
      //     ReusedQueryStage 0
-      val grouped = df.groupBy("key").agg(max("value").as("value"))
+      val grouped = df.groupBy((col("key") + 1).as("key")).agg(max("value").as("value"))


I had to modify the test because the AliasAwareOutputPartitioning fix modified the the explain plan of the original query from:

Union :- *(5) HashAggregate(keys=[_groupingexpression#79L], functions=[max(value#38L)], output=[(key + 1)#44L, max(value)#45L]) : +- AQEShuffleRead coalesced : +- ShuffleQueryStage 3 : +- Exchange hashpartitioning(_groupingexpression#79L, 5), ENSURE_REQUIREMENTS, [plan_id=693] : +- *(3) HashAggregate(keys=[_groupingexpression#79L], functions=[partial_max(value#38L)], output=[_groupingexpression#79L, max#62L]) : +- *(3) HashAggregate(keys=[key#12L], functions=[max(value#13L)], output=[value#38L, _groupingexpression#79L]) : +- AQEShuffleRead coalesced : +- ShuffleQueryStage 0 : +- Exchange hashpartitioning(key#12L, 5), ENSURE_REQUIREMENTS, [plan_id=623] : +- *(1) HashAggregate(keys=[key#12L], functions=[partial_max(value#13L)], output=[key#12L, max#64L]) : +- *(1) Project [id#10L AS key#12L, id#10L AS value#13L] : +- *(1) Range (0, 6, step=1, splits=10) +- *(6) HashAggregate(keys=[_groupingexpression#80L], functions=[max(value#38L)], output=[(key + 2)#51L, max(value)#52L]) +- AQEShuffleRead coalesced +- ShuffleQueryStage 4 +- Exchange hashpartitioning(_groupingexpression#80L, 5), ENSURE_REQUIREMENTS, [plan_id=719] +- *(4) HashAggregate(keys=[_groupingexpression#80L], functions=[partial_max(value#38L)], output=[_groupingexpression#80L, max#66L]) +- *(4) HashAggregate(keys=[key#12L], functions=[max(value#13L)], output=[value#38L, _groupingexpression#80L]) +- AQEShuffleRead coalesced +- ShuffleQueryStage 2 +- ReusedExchange [key#12L, max#64L], Exchange hashpartitioning(key#12L, 5), ENSURE_REQUIREMENTS, [plan_id=623]

to (1 less exchange):

Union :- *(3) HashAggregate(keys=[_groupingexpression#75L], functions=[max(value#38L)], output=[(key + 1)#44L, max(value)#45L]) : +- AQEShuffleRead coalesced : +- ShuffleQueryStage 0 : +- Exchange hashpartitioning(_groupingexpression#75L, 5), ENSURE_REQUIREMENTS, [plan_id=514] : +- *(1) HashAggregate(keys=[_groupingexpression#75L], functions=[partial_max(value#38L)], output=[_groupingexpression#75L, max#62L]) : +- *(1) HashAggregate(keys=[key#12L], functions=[max(value#13L)], output=[value#38L, _groupingexpression#75L]) : +- *(1) HashAggregate(keys=[key#12L], functions=[partial_max(value#13L)], output=[key#12L, max#64L]) : +- *(1) Project [id#10L AS key#12L, id#10L AS value#13L] : +- *(1) Range (0, 6, step=1, splits=10) +- *(4) HashAggregate(keys=[_groupingexpression#76L], functions=[max(value#38L)], output=[(key + 2)#51L, max(value)#52L]) +- AQEShuffleRead coalesced +- ShuffleQueryStage 1 +- Exchange hashpartitioning(_groupingexpression#76L, 5), ENSURE_REQUIREMENTS, [plan_id=532] +- *(2) HashAggregate(keys=[_groupingexpression#76L], functions=[partial_max(value#38L)], output=[_groupingexpression#76L, max#66L]) +- *(2) HashAggregate(keys=[key#12L], functions=[max(value#13L)], output=[value#38L, _groupingexpression#76L]) +- *(2) HashAggregate(keys=[key#12L], functions=[partial_max(value#13L)], output=[key#12L, max#64L]) +- *(2) Project [id#55L AS key#12L, id#55L AS value#13L] +- *(2) Range (0, 6, step=1, splits=10)

and so the query didn't match the test case 2 description.

peter-toth · 2022-08-02T15:48:40Z

sql/core/src/main/scala/org/apache/spark/sql/RelationalGroupedDataset.scala

      case ne: NamedExpression => ne
      case other => Alias(other, other.toString)()
    }

    val rightGroupingNamedExpressions = r.groupingExprs.map {
+      case a: Attribute => Alias(a, a.name)()


These changes are needed to make the output of Projects in line 615/617 unique. Without these the new assert in removeRedundantAliases() would fail.

hmmm, does it mean it's legitimate to have conflicting attribute IDs within one operator? This looks totally fine in FlatMapCoGroupsInPandas

I don't know, this was the only place where I had to modify the code to avoid conflicts.
But I was thinking about that instead of the new assert in the rule, we could fix these conflicting attributes with inserting a Project. How about that?

peter-toth · 2022-08-02T20:18:08Z

@cloud-fan, I've updates this PR that removeRedundantAliases will always keep aliases to avoid duplicate output attributes.

Remaining failures in sparkr/linters tests don't seem related to this change.

cloud-fan · 2022-08-05T16:26:47Z

sql/core/src/test/resources/tpcds-plan-stability/approved-plans-v1_4/q70.sf100/explain.txt

@@ -157,31 +157,31 @@ Input [2]: [s_state#14, sum#16]
 Keys [1]: [s_state#14]
 Functions [1]: [sum(UnscaledValue(ss_net_profit#10))]
 Aggregate Attributes [1]: [sum(UnscaledValue(ss_net_profit#10))#17]
-Results [3]: [s_state#14, s_state#14, MakeDecimal(sum(UnscaledValue(ss_net_profit#10))#17,17,2) AS _w2#18]
+Results [3]: [s_state#14 AS s_state#18, s_state#14, MakeDecimal(sum(UnscaledValue(ss_net_profit#10))#17,17,2) AS _w2#19]


I'm surprised that this query plan works fine today (two s_state#14). Reading the source code a bit more, I think Spark is actually fine with duplicated attribute ids. It assumes columns with the same attr id always output same values, so it can safely bind attributes with the first match. See BindReferences.

So the problem is still at Union. It's not fine to introduce duplicated attr ids in the first child of Union, as it will introduce duplicated attr ids in the output of Union, which causes wrong results.

I think eventually we should make Union use new attr ids to build its output columns. As a bug fix, your first attempt to simply not remove alias in the first child of Union looks the best one: it's simple, and having a bit more unnecessary alias won't impact performance. Sorry for the back and forth!

Ok. No problem, I can revert this PR to the fisrt version next week.

I reverted the PR and its description to the first version in 7062ed8

cloud-fan · 2022-08-05T16:27:50Z

Fixes AliasAwareOutputPartitioning and AliasAwareOutputOrdering as testing discovered that ...

Can we create a new PR for this?

peter-toth · 2022-08-05T17:57:18Z

Fixes AliasAwareOutputPartitioning and AliasAwareOutputOrdering as testing discovered that ...

Can we create a new PR for this?

Sure, I can open a new one for this next week.

cloud-fan · 2022-08-09T15:00:50Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

+        var first = true
+        plan.mapChildren { child =>
+          if (first) {
+            first = false


can we add some comments to explain why union's first child is special?

Sure, added in: db10b5f

cloud-fan · 2022-08-09T15:01:04Z

cc @sigmod

cloud-fan · 2022-08-09T15:01:22Z

@peter-toth do you know which Spark version starts to have this issue?

peter-toth · 2022-08-10T06:52:27Z

@peter-toth do you know which Spark version starts to have this issue?

This rule is quite old so I just ran a quick check on the latest 2.4 branch and even that is affected.

cloud-fan · 2022-08-10T15:11:29Z

The GA failure is unrelated, thanks, merging to master!

Since there are conflicts, can you create backport PRs for 3.3? thanks!

peter-toth · 2022-08-10T17:56:41Z

The GA failure is unrelated, thanks, merging to master!

Since there are conflicts, can you create backport PRs for 3.3? thanks!

Thanks for the review @cloud-fan!
I've opened 3.3 PR here: #37472

[SPARK-39887][SQL] RemoveRedundantAliases should keep attributes of a…

3f1a887

… Union's first child

github-actions bot added the SQL label Jul 28, 2022

peter-toth mentioned this pull request Jul 28, 2022

[SPARK-39887][SQL] PullOutGroupingExpressions should generate different alias names #37319

Closed

regenerate plans

81c600b

cfmcgrady reviewed Jul 29, 2022

View reviewed changes

Peter Toth added 2 commits July 29, 2022 08:11

fix test

ab2b8f9

fix method comment

db6c73f

peter-toth force-pushed the SPARK-39887-keep-attributes-of-unions-first-child branch from 2eb2d6a to db6c73f Compare July 29, 2022 06:25

cloud-fan reviewed Aug 1, 2022

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala Show resolved Hide resolved

peter-toth force-pushed the SPARK-39887-keep-attributes-of-unions-first-child branch from d5193ce to c546757 Compare August 1, 2022 08:39

Peter Toth added 2 commits August 1, 2022 10:43

keep only conflict fixing aliases and revert uneccessary test fixes

6adbd44

add assert

11020ee

peter-toth force-pushed the SPARK-39887-keep-attributes-of-unions-first-child branch from c546757 to 11020ee Compare August 1, 2022 08:43

cloud-fan reviewed Aug 1, 2022

View reviewed changes

Peter Toth added 2 commits August 1, 2022 14:47

keep necessary aliases to always guanatee unique output attributes

608f83a

regenerate plans

d212dab

cloud-fan reviewed Aug 1, 2022

View reviewed changes

peter-toth changed the title ~~[SPARK-39887][SQL] RemoveRedundantAliases should keep attributes of a Union's first child~~ [WIP][SPARK-39887][SQL] RemoveRedundantAliases should keep attributes of a Union's first child Aug 1, 2022

Peter Toth added 4 commits August 1, 2022 21:32

fix AliasAwareOutputPartitioning and AliasAwareOutputOrdering

b3b7d24

revert ansi plan changes as those have nothing to do with this PR

e509053

fix test issues

59dcc0d

fix indent, add comment

a737020

peter-toth force-pushed the SPARK-39887-keep-attributes-of-unions-first-child branch from deac8ce to a737020 Compare August 2, 2022 07:35

Peter Toth added 4 commits August 2, 2022 12:43

fix scala 2.13 compatibility, minor code clean-up

793d613

fix tests as some of the not needed exchanges have been dropped from …

e14354b

…plans

fix flatMapCoGroupsInPandas and flatMapGroupsInPandas aliases to avoi…

c8ba154

…d duplicate output attributes

minor fix

155cf31

peter-toth force-pushed the SPARK-39887-keep-attributes-of-unions-first-child branch from fddd468 to 155cf31 Compare August 2, 2022 15:38

peter-toth commented Aug 2, 2022

View reviewed changes

peter-toth changed the title ~~[WIP][SPARK-39887][SQL] RemoveRedundantAliases should keep attributes of a Union's first child~~ [WIP][SPARK-39887][SQL] RemoveRedundantAliases should keep aliases that make the output of projection nodes unique Aug 2, 2022

peter-toth changed the title ~~[WIP][SPARK-39887][SQL] RemoveRedundantAliases should keep aliases that make the output of projection nodes unique~~ [SPARK-39887][SQL] RemoveRedundantAliases should keep aliases that make the output of projection nodes unique Aug 2, 2022

cloud-fan reviewed Aug 5, 2022

View reviewed changes

revert to first version

7062ed8

cloud-fan approved these changes Aug 9, 2022

View reviewed changes

cloud-fan reviewed Aug 9, 2022

View reviewed changes

add comment

db10b5f

cloud-fan closed this in 36a269d Aug 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-39887][SQL] RemoveRedundantAliases should keep aliases that make the output of projection nodes unique #37334

[SPARK-39887][SQL] RemoveRedundantAliases should keep aliases that make the output of projection nodes unique #37334

peter-toth commented Jul 28, 2022 •

edited

cfmcgrady Jul 29, 2022

peter-toth Jul 29, 2022

cloud-fan Aug 1, 2022

peter-toth Aug 1, 2022

cloud-fan Aug 1, 2022

cloud-fan Aug 1, 2022 •

edited

peter-toth Aug 1, 2022 •

edited

peter-toth Aug 1, 2022

peter-toth Aug 1, 2022

cloud-fan Aug 1, 2022

peter-toth Aug 1, 2022 •

edited

peter-toth Aug 1, 2022

peter-toth Aug 2, 2022 •

edited

cloud-fan Aug 1, 2022

peter-toth Aug 1, 2022

peter-toth Aug 2, 2022

peter-toth Aug 2, 2022

peter-toth Aug 2, 2022

cloud-fan Aug 4, 2022

peter-toth Aug 4, 2022

peter-toth commented Aug 2, 2022

cloud-fan Aug 5, 2022

peter-toth Aug 5, 2022

peter-toth Aug 9, 2022

cloud-fan commented Aug 5, 2022

peter-toth commented Aug 5, 2022

cloud-fan Aug 9, 2022

peter-toth Aug 10, 2022

cloud-fan commented Aug 9, 2022

cloud-fan commented Aug 9, 2022

peter-toth commented Aug 10, 2022

cloud-fan commented Aug 10, 2022

peter-toth commented Aug 10, 2022

[SPARK-39887][SQL] RemoveRedundantAliases should keep aliases that make the output of projection nodes unique #37334

[SPARK-39887][SQL] RemoveRedundantAliases should keep aliases that make the output of projection nodes unique #37334

Conversation

peter-toth commented Jul 28, 2022 • edited

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloud-fan Aug 1, 2022 • edited

Choose a reason for hiding this comment

peter-toth Aug 1, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peter-toth Aug 1, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peter-toth Aug 2, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peter-toth commented Aug 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloud-fan commented Aug 5, 2022

peter-toth commented Aug 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloud-fan commented Aug 9, 2022

cloud-fan commented Aug 9, 2022

peter-toth commented Aug 10, 2022

cloud-fan commented Aug 10, 2022

peter-toth commented Aug 10, 2022

peter-toth commented Jul 28, 2022 •

edited

cloud-fan Aug 1, 2022 •

edited

peter-toth Aug 1, 2022 •

edited

peter-toth Aug 1, 2022 •

edited

peter-toth Aug 2, 2022 •

edited