[Part3] Partition and Sort Enforcement, Enforcement rule implementation #4122

mingmwang · 2022-11-07T06:18:28Z

Which issue does this PR close?

Closes #41
Closes #3854.

Rationale for this change

What changes are included in this PR?

Make EquivalenceProperties schema aware.
Add a top-down process to reorder the Join keys/GroupBy keys ordering.
Add Basic Enforcement rule to ensure the Distribution and Ordering requirements.

Are there any user-facing changes?

mingmwang · 2022-11-07T06:22:36Z

@alamb @yahoNanJing @Dandandan @Jefffrey
The last part of the PR. Please help to take a look.

jackwener · 2022-11-07T08:28:04Z

datafusion/core/src/execution/context.rs

        physical_optimizers.push(Arc::new(Repartition::new()));
-        physical_optimizers.push(Arc::new(AddCoalescePartitionsExec::new()));
+        physical_optimizers.push(Arc::new(BasicEnforcement::new()));
+        // physical_optimizers.push(Arc::new(AddCoalescePartitionsExec::new()));


why remove it?

The new rule BasicEnforcement had already covered the work of AddCoalescePartitionsExec.

When this call is removed, there is no other code that calls AddCoalescePartitionsExec and thus I think we should remove that entire module. We could do so as a follow on PR if you want to keep this one smaller

Sure, I will do it in the following PR.

jackwener · 2022-11-07T08:55:14Z

datafusion/core/src/physical_plan/mod.rs

+        equal_properties: F,
+    ) -> bool {
+        match required {
+            Distribution::UnspecifiedDistribution => true,


we can add some UT for satisfy() to confirm some cases:

UnspecifiedDistribution - UnspecifiedDistribution

Hash - SinglePartition

Hash - Hash

.......

SinglePartition should already be covered by existing UTs.

Perhaps @jackwener was getting at adding additional testing could also help to document the intent of satisfy() as well as making the coverage was clear, even if they did not add additional coverage

I would be interested in helping to write such tests as part of my review of this code (as it would help me gain a better understanding)

alamb · 2022-11-07T20:15:28Z

I plan to review this carefully tomorrow

datafusion/core/src/physical_optimizer/enforcement.rs

Ted-Jiang · 2022-11-08T07:41:49Z

I am not familiar with this part. But this EquivalenceProperties system is amazing 👍 ! Looking forward to see this going on 😄

…Exec

mingmwang · 2022-11-08T08:29:25Z

@yahoNanJing
BTW, after this PR, CrossJoin and HashJoin Collect Left should also work in Ballista. Originally they do not work.

alamb · 2022-11-08T21:39:27Z

I am sorry again, I am struggling to find sufficient contiguous time to review this PR. I will set aside time first thing tomorrow morning

yahoNanJing · 2022-11-09T06:59:23Z

datafusion/core/src/physical_optimizer/enforcement.rs

+    }
+}
+
+fn try_reorder(


It's better to add some comments here for the parameters.

For the top-down case, the join_keys are using the its children's schema; while the expected and equivalence_properties are using the current operator's schema.

For the bottom-up case, the join_keys and expected are using the its children's schema; while the equivalence_properties are using the current operator's schema.

Is it possible to make the schema the same for all of the parameters? Otherwise, for the right side of the bottom-up case, it may miss some reorder chance.

Yes, valid points.

Now, in the latest code, for the bottom-up case, the join_keys, expected and equivalence_properties are all using the its children's schema.

yahoNanJing · 2022-11-09T07:17:51Z

datafusion/core/src/physical_optimizer/enforcement.rs

+/// That might not match with the output partitioning of the join node's children
+/// This method runs a top-down process and try to adjust the output partitioning of the children
+/// if the children themselves are Joins or Aggregations.
+fn adjust_input_keys_down_recursively(


Will the parameters, plan and parent_required share the same schema? If so, it's better to add comments for it.

yahoNanJing · 2022-11-09T07:28:48Z

datafusion/core/src/physical_optimizer/enforcement.rs

+                                match mode {
+                                    AggregateMode::FinalPartitioned => {
+                                        let new_input =
+                                            adjust_input_keys_down_recursively(


Since the input of FinalPartitioned should be the Partial and they should share the same column order, it's safe to call adjust_input_keys_down_recursively here.

yahoNanJing · 2022-11-09T07:31:11Z

datafusion/core/src/physical_optimizer/enforcement.rs

+                        right.clone(),
+                        left.schema().fields().len(),
+                    )?,
+                    JoinType::RightSemi | JoinType::RightAnti => {


For RightSemi and RightAnti join, the current schema should be the same with the right one. Therefore, no need to do schema adjustment.

yahoNanJing · 2022-11-09T07:41:23Z

datafusion/core/src/physical_optimizer/enforcement.rs

+    } else if let Some(CrossJoinExec { left, right, .. }) =
+        plan_any.downcast_ref::<CrossJoinExec>()
+    {
+        let new_left = adjust_input_keys_down_recursively(left.clone(), vec![])?;


Since the current implementation for the CrossJoinExec is similar to the CollectLeft, we don't adjust for the left side.

yahoNanJing · 2022-11-09T07:48:40Z

datafusion/core/src/physical_optimizer/enforcement.rs

+            let new_right =
+                adjust_input_keys_down_recursively(right.clone(), right_keys)?;
+
+            Ok(Arc::new(SortMergeJoinExec::try_new(


Since we changes the sort options here, will it influence upstream operators which may rely on it?

If this rule is called at the very beginning of the rule-based optimization process, this influence may be covered by other rules.

yahoNanJing · 2022-11-09T08:05:15Z

datafusion/core/src/physical_optimizer/enforcement.rs

+
+                    // Check whether the requirements can be satisfied by the Aggregation
+                    if parent_required.len() != out_put_exprs.len()
+                        || expr_list_eq_strict_order(&out_put_exprs, &parent_required)


Here, we don't consider the optimization of join group with sharing the same hash table. Therefore, when the group satisfy the join key, we don't push down the required ordering.

yahoNanJing · 2022-11-09T08:39:01Z

datafusion/core/src/physical_optimizer/enforcement.rs

+/// This method will try to change the ordering of the join keys to match with the
+/// partitioning of the join nodes' children.
+/// If it can not match with both sides, it will try to match with one, either left side or right side.
+fn reorder_join_keys_to_inputs(


It's used for bottom-up

yahoNanJing · 2022-11-09T09:02:25Z

datafusion/core/src/physical_optimizer/enforcement.rs

+            )
+        }
+        (_, Some(Partitioning::Hash(right_exprs, _))) => {
+            try_reorder(join_keys.clone(), right_exprs, equivalence_properties).or_else(


Maybe no need to call or_else

yahoNanJing · 2022-11-09T10:31:37Z

datafusion/core/src/physical_optimizer/enforcement.rs

+                    {
+                        Arc::new(CoalescePartitionsExec::new(child.clone()))
+                    }
+                    _ => {


Here, we are sure that the RepartitionExec in the plan tree should satisfy the required distribution so that we don't need to worry about duplicate RepartitionExec be added.

Isn't that case covered by if child.output_partitioning().satisfy() check above?

yahoNanJing · 2022-11-09T10:43:27Z

datafusion/core/src/physical_plan/planner.rs

-
                        // Use hash partition by default to parallelize hash joins
                        Ok(Arc::new(HashJoinExec::try_new(
-                            Arc::new(RepartitionExec::try_new(


Very excellent refactoring. Now the RepartitionExec will be added by the BasicEnforcement optimization rule rather than be added manually for some specific operators.

I agree 100%

yahoNanJing · 2022-11-09T10:46:13Z

Excellent work 👍

alamb

I got through most of this PR other than datafusion/core/src/physical_optimizer/enforcement.rs which I am hoping to to do later today

So far I think it looks like a great piece of work.

Here is a list of follow on items that would be good to address (though we could do them in follow on PRs):

Remove AddCoalescePartitionsExec
Add more test coverage for Partitioning::satisfy()

alamb · 2022-11-09T11:38:31Z

datafusion/core/src/execution/context.rs

            Arc::new(AggregateStatistics::new()),
            Arc::new(HashBuildProbeOrder::new()),
        ];
+        physical_optimizers.push(Arc::new(BasicEnforcement::new()));


Is there a reason that BasicEnforcement must be run twice?

Yes, this is to cover the case that Repartition rule could add additional RepartitionExec with RoundRobin partitioning, and to make sure the SinglePartition is satisfied, we run the BasicEnforcement again. Originally it was the AddCoalescePartitionsExec here, now I replace AddCoalescePartitionsExec with BasicEnforcement.

Cool -- perhaps you can add a comment to explain the rationale

Sure, I will do it in the following PR.

alamb · 2022-11-09T11:40:15Z

datafusion/core/src/execution/context.rs

        physical_optimizers.push(Arc::new(Repartition::new()));
-        physical_optimizers.push(Arc::new(AddCoalescePartitionsExec::new()));
+        physical_optimizers.push(Arc::new(BasicEnforcement::new()));
+        // physical_optimizers.push(Arc::new(AddCoalescePartitionsExec::new()));


When this call is removed, there is no other code that calls AddCoalescePartitionsExec and thus I think we should remove that entire module. We could do so as a follow on PR if you want to keep this one smaller

alamb · 2022-11-09T11:49:38Z

datafusion/core/src/physical_plan/mod.rs

+                    // then we need to have the partition count and hash functions validation.
+                    Partitioning::Hash(partition_exprs, _) => {
+                        let fast_match =
+                            expr_list_eq_strict_order(&required_exprs, partition_exprs);


You might be able to reduce the indent level of this code with a construct like

if expr_list_eq_strict_order(&required_exprs, partition_exprs) { return true }

alamb · 2022-11-09T11:50:38Z

datafusion/core/src/physical_plan/mod.rs

+                                        )
+                                    })
+                                    .collect::<Vec<_>>();
+                                expr_list_eq_strict_order(


👍 this is a nice formulation

alamb · 2022-11-09T11:51:39Z

datafusion/core/src/physical_plan/planner.rs

                        vec![]
                    };

-                    let input_exec = if can_repartition {


it is nice to avoid adding repartitioning directly in the planner and instead add it as a follow on pass

alamb · 2022-11-09T11:59:08Z

datafusion/physical-expr/src/equivalence.rs

 }

+/// Project Equivalence Properties.
+/// 1) Add Alias, Alias can introduce additional equivalence properties,


alamb · 2022-11-09T12:01:01Z

datafusion/physical-expr/src/equivalence.rs

+
+        project_equivalence_properties(input_properties, &alias_map, &mut out_properties);
+        assert_eq!(out_properties.classes().len(), 1);
+        assert_eq!(out_properties.classes()[0].len(), 4);


I suggest actually validating the contents of these equivalence classes somehow (rather than just their sizes)

Sure, will do.

alamb · 2022-11-09T12:01:11Z

datafusion/core/src/physical_plan/mod.rs

+        equal_properties: F,
+    ) -> bool {
+        match required {
+            Distribution::UnspecifiedDistribution => true,


Perhaps @jackwener was getting at adding additional testing could also help to document the intent of satisfy() as well as making the coverage was clear, even if they did not add additional coverage

I would be interested in helping to write such tests as part of my review of this code (as it would help me gain a better understanding)

alamb · 2022-11-09T12:05:43Z

datafusion/core/src/physical_optimizer/enforcement.rs

+/// in the strictest way. It might add additional [[RepartitionExec]] to the plan tree
+/// and give a non-optimal plan, but it can avoid the possible data skew in joins


I don't understand this comment about 'non optimal plan' -- I think the point of this pass to ensure that the plan gets the correct answer :) If the plan doesn't get the correct answer it can't be optimal

Yes, correctness is always the P1 priority. Here the non-optimal plan means if the optimization target is to find out a best plan which can have the least number of Repartitions and Sorts, then the current implement can't reach the goal. And need more complex algorithms.

alamb · 2022-11-09T12:08:39Z

datafusion/core/src/physical_optimizer/enforcement.rs

+    }
+}
+
+/// When the physical planner creates the Joins, the ordering of join keys is from the original query.


Thank you for this comment -- can you explain a little more about why the ordering of join keys in the plan affects the output partitioning? Is it because the output partitioning of a join on (a, b) is always hash(a, b) which might not be the same as hash(b, a)?

Specifically I am trying to figure out under what circumstances join column reordering is needed at all. Like if you have a join on (a, b) and then you have a aggregate grouped on b, a the distributions are compatible aren't they?

Yes, this is to cover the cases that complex plan graphs which include multiple joins or join on aggregations but key orderings are different.
For example:

TopJoin on (a, b, c) bottom left join on(b, a, c) bottom right join on(c, b, a)

Another case:

TopJoin on (a, b, c) Agg1 group by (b, a, c) Agg2 group by (c, b, a)

The PR comes up with two join key reordering implementations:

Top-down approach, see the method fn adjust_input_keys_down_recursively()
The top down approach will adjust the children's key ordering based on parent requirements:

TopJoin on (a, b, c) bottom left join on(b, a, c) bottom right join on(c, b, a)

will be adjusted to

TopJoin on (a, b, c) bottom left join on(a, b, c) bottom right join on(a, b, c)

TopJoin on (a, b, c) Agg1 group by (b, a, c) Agg2 group by (c, b, a)

will be adjusted to:

TopJoin on (a, b, c) Projection(b, a, c) Agg1 group by (a, b, c) Projection(c, b, a) Agg2 group by (a, b, c)

Bottom-up approach, see the method fn reorder_join_keys_to_inputs()
The Bottom-up approach will just adjust the parent's key ordering based on children's, either align with left or right, sometimes can not align with both. The Bottom up approach is much simpler than the top-down approach, but might not reach a best result.

TopJoin on (a, b, c) bottom left join on(b, a, c) bottom right join on(c, b, a)

will be adjusted to

TopJoin on (b, a, c) bottom left join on(b, a, c) bottom right join on(c, b, a)

And by default, the PR go with the top-down approach, and there is a session level configuration config.top_down_join_key_reordering to disable it.
The bottom-up approach will be useful in future if we plan to support storage partition-wised joins. In that case, the data sources/tables might be pre-partitioned and we can't adjust the order, so we would prefer to adjust parent's key ordering based on children's.

This makes sense to me -- thank you for the explanation @mingmwang -- this would also be great content to put into the comments somewhere so future readers can get a little more understanding of the rationale. We could do that as part of a follow on PR too

I think this PR could be merged as is (and do the follow up items as follow on PRs). Thanks again @mingmwang

Please let me know what you would prefer (merge and follow up PRs or modify this one).

Sure, thanks, I prefer to do the changes in the follow up PRs.

alamb

I think this PR looks great -- thank you so much @mingmwang . It is a real step forward

The only thing I would like resolved prior to approving this PR is understanding the need to reorder join keys -- everything else makes sense to me and seems reasonably tested.

Again, really nice work 🏆

alamb · 2022-11-09T12:42:09Z

datafusion/core/src/physical_optimizer/enforcement.rs

+                            "RepartitionExec: partitioning=Hash([Column { name: \"c\", index: 2 }], 10)",
+                            "ParquetExec: limit=None, partitions=[x], projection=[a, b, c, d, e]",
+                        ],
+                        // Should include 4 RepartitionExecs


why does a right outer join (and full join) need the extra repartition? Mostly I don't understand why you don't need the same repartition for the inner join as well

For full join, the output partitioning is UnknownPartitioning, so need the extra repartition.
For right outer join, the output partitioning is the adjusted right output partitioning(shifted column index).
In this UT, different cases are covered

Case1: right out join case

Top RightOutJoin on (a == c) bottom RightOutJoin on(a == b) ParquetScan ParquetScan ParquetScan

The expected physical plan should be:
need 4 Repartitions

Top RightOutJoin on (a == c) Repartition(a) bottom RightOutJoin on(a == b) -- output partition is Partition(b), can not meet requirements. Repartition(a) ParquetScan Repartition(b) ParquetScan Repartition(c) ParquetScan

Case2: an inner join case, original plan:

Top InnerJoin on (a == c) bottom InnerJoin on(a == b) ParquetScan ParquetScan ParquetScan

The expected physical plan should be:
need 3 Repartitions.

Top InnerJoin on (a == c) bottom InnerJoin on(a == b) -- output partition is Partition(a), can meet the requirements. Repartition(a) ParquetScan Repartition(b) ParquetScan Repartition(c) ParquetScan

Case3: another inner join case, original plan:

Top InnerJoin on (b == c) bottom InnerJoin on(a == b) ParquetScan ParquetScan ParquetScan

The expected physical plan should be:
need 3 Repartitions.

Top InnerJoin on (b == c) bottom InnerJoin on(a == b) -- output partition is Partition(a), but Partition(a) == Partition(b), EquivalenceClass takes Repartition(a) -- place, can meet the requirements. ParquetScan Repartition(b) ParquetScan Repartition(c) ParquetScan

/// Calculate the OutputPartitioning for Partitioned Join pub fn partitioned_join_output_partitioning( join_type: JoinType, left_partitioning: Partitioning, right_partitioning: Partitioning, left_columns_len: usize, ) -> Partitioning { match join_type { JoinType::Inner | JoinType::Left | JoinType::LeftSemi | JoinType::LeftAnti => { left_partitioning } JoinType::RightSemi | JoinType::RightAnti => right_partitioning, JoinType::Right => { adjust_right_output_partitioning(right_partitioning, left_columns_len) } JoinType::Full => { Partitioning::UnknownPartitioning(right_partitioning.partition_count()) } } }

This is a wonderful explanation -- thank you @mingmwang

alamb · 2022-11-09T12:42:32Z

datafusion/core/src/physical_optimizer/enforcement.rs

+            &JoinType::Inner,
+        );
+
+        // Output partition need to respect the Alias and should not introduce additional RepartitionExec


alamb · 2022-11-09T12:45:19Z

datafusion/core/src/physical_optimizer/enforcement.rs

+    }
+
+    #[test]
+    fn multi_smj_joins() -> Result<()> {


this is very clever

alamb · 2022-11-09T12:49:12Z

datafusion/core/src/physical_optimizer/enforcement.rs

+            let expected = &[
+                top_join_plan.as_str(),
+                "ProjectionExec: expr=[a@0 as A, a@0 as AA, b@1 as B, c@2 as C]",
+                "HashJoinExec: mode=Partitioned, join_type=Inner, on=[(Column { name: \"a\", index: 0 }, Column { name: \"a1\", index: 0 }), (Column { name: \"b\", index: 1 }, Column { name: \"b1\", index: 1 }), (Column { name: \"c\", index: 2 }, Column { name: \"c1\", index: 2 })]",


I don't understand the need to reorder the join keys here -- if the input is partitioned on a, b, c the correct answer will still happen if you join on b, c, a

My rationale is that any particular values of a, b and c will always be the same partition if it partitioned by hash(a, b, c) or if it is partitioned by hash(b, c, a) although the specific partitions would be different it shouldn't matter

For Aggregations, yes, the ordering doesn't matter. But for Partitioned Joins, it matters.
For Partitioned Joins, there are two inputs, if the left is partitioned on (B, C, A) and right is partitioned on (A, C, B), for the same particular value pairs(a , b, c), the two inputs will not co-locate to the same partition, because the hashed values are different.

alamb · 2022-11-09T12:56:21Z

datafusion/core/src/physical_optimizer/enforcement.rs

+                &top_join_on,
+                &join_type,
+            );
+            let top_join_plan =


Something I don't see in these plans (or in the code) is the fact that if you take several sorted streams and run them through RepartitionExec I don't think the output remains sorted. I think these plans are ok, as they sort after the repartition, but in general that is something to watch out for

alamb

I think this PR could be merged as is (and do the follow up items as follow on PRs). Thanks again @mingmwang

Please let me know what you would prefer (merge and follow up PRs or modify this one).

alamb · 2022-11-09T16:36:47Z

datafusion/core/src/physical_optimizer/enforcement.rs

+                            "RepartitionExec: partitioning=Hash([Column { name: \"c\", index: 2 }], 10)",
+                            "ParquetExec: limit=None, partitions=[x], projection=[a, b, c, d, e]",
+                        ],
+                        // Should include 4 RepartitionExecs


This is a wonderful explanation -- thank you @mingmwang

alamb · 2022-11-10T12:02:30Z

Thanks again @mingmwang -- can you please file tickets for the follow on items? Or if you don't have time let me know so I can do so?

ursabot · 2022-11-10T12:12:18Z

Benchmark runs are scheduled for baseline = 30813dc and contender = 9c24a79. 9c24a79 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

jackwener · 2022-11-11T12:39:59Z

Good job👍.
Although the work has been merged, I'm going to take a closer look over the weekend.

[Part3] Partition and Sort Enforcement, Enforcement rule implementation

a5bad1d

github-actions bot added core Core DataFusion crate physical-expr Changes to the physical-expr crates labels Nov 7, 2022

jackwener reviewed Nov 7, 2022

View reviewed changes

Ted-Jiang reviewed Nov 8, 2022

View reviewed changes

datafusion/core/src/physical_optimizer/enforcement.rs Outdated Show resolved Hide resolved

Avoid unncessary CoalescePartitionsExec in HashJoinExec and CrossJoin…

5cef4ec

…Exec

mingmwang added 2 commits November 9, 2022 00:40

Fix join key ordering

6b2a677

Fix join key reordering

881db1b

join key reordering, handle more operators explicitly

40e2021

yahoNanJing reviewed Nov 9, 2022

View reviewed changes

Resolve review comments, add more UT to test reorder_join_keys_to_inputs

d09d049

add length check in fn expected_expr_positions()

50dc5f6

alamb reviewed Nov 9, 2022

View reviewed changes

alamb approved these changes Nov 9, 2022

View reviewed changes

alamb merged commit 9c24a79 into apache:master Nov 10, 2022

alamb mentioned this pull request Nov 10, 2022

Add ability to specify external sort information for ParquetExec #4169

Closed

alamb mentioned this pull request Nov 22, 2022

CoalesceBatchesExec reports no ordering #4331

Closed

ozankabak mentioned this pull request Dec 1, 2022

WindowAggExec output ordering is fixed #4439

Closed

		/// in the strictest way. It might add additional [[RepartitionExec]] to the plan tree
		/// and give a non-optimal plan, but it can avoid the possible data skew in joins

[Part3] Partition and Sort Enforcement, Enforcement rule implementation #4122

[Part3] Partition and Sort Enforcement, Enforcement rule implementation #4122

Uh oh!

Conversation

mingmwang commented Nov 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Uh oh!

mingmwang commented Nov 7, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb commented Nov 7, 2022

Uh oh!

Uh oh!

Ted-Jiang commented Nov 8, 2022

Uh oh!

mingmwang commented Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alamb commented Nov 8, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yahoNanJing Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yahoNanJing commented Nov 9, 2022

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mingmwang Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mingmwang commented Nov 7, 2022 •

edited

Loading

mingmwang commented Nov 8, 2022 •

edited

Loading

yahoNanJing Nov 9, 2022 •

edited

Loading

mingmwang Nov 9, 2022 •

edited

Loading

mingmwang Nov 9, 2022 •

edited

Loading