Add BLOCK_AND_POSITION calling convention to annotation framework by gerlou · Pull Request #10961 · prestodb/presto

gerlou · 2018-07-02T19:31:44Z

Move @BlockPosition and @BlockIndex to SPI

haozhun · 2018-07-02T19:34:36Z

Change commit message to Move @BlockPosition and @BlockIndex to SPI

You don't need a separate PR for this. One PR in GitHub can have multiple commits. This PR is a precursor step of a larger PR you are going to put up. As a result, we'll have this commit as part of that PR, instead of creating a standalone PR for this.

findepi · 2018-07-05T20:13:06Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

Why Move @BlockPosition and @BlockIndex to SPI commit moves other classes around as well?

@findepi : Separated into a different commit now.

findepi · 2018-07-05T20:13:12Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

findepi · 2018-07-05T20:13:48Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

Is this code change directly related to moving BlockPosition and BlockIndex annotations to SPI?

wenleix · 2018-07-13T19:23:47Z

A few high-level comments:

The commits are a bit commingled and the commit message doesn't reflect what it does. For example, the first commit doesn't only move @BlockPosition and @BlockIndex to SPI, but also did some renaming and some support of block_and_position convention in ParmatericScalarImplementation.

We should rebase the PR into the 5 following commits:
1. Move @BlockPosition and @BlockIndex to SPI (a straightforward move)
  2. Rename SFC -> SFIC, SI -> PSI. (SFC: ScalarFunctionChoice, SFIC: ScalarFunctionImplementationChoice, SI: ScalarImplementation, PSI: ParametricScalarImplementation.
  3. Add type parameter specialization inference based on return native container type.
  4. Add return native container type into ParametricScalarImplementation
  5. Add BLOCK_AND_POSITION calling convention for annotation framework.

Let's work together offline on this rebase.

The PR title is not accurate.
return native container type should be added into ParametricScalarImplementation. Let's put them in a separate commit. This will resolve the test failures in Travis.
There are test failures in Travis.

[ERROR] testBlockPosition(com.facebook.presto.type.TestBlockAndPositionNullConvention)  Time elapsed: 0.018 s  <<< FAILURE!
java.lang.invoke.WrongMethodTypeException: cannot convert MethodHandle(Block,int)long to (Object)Object
	at com.facebook.presto.type.TestBlockAndPositionNullConvention.testBlockPosition(TestBlockAndPositionNullConvention.java:44)

[ERROR] testRound(com.facebook.presto.operator.scalar.TestMathFunctions)  Time elapsed: 1.072 s  <<< FAILURE!
com.facebook.presto.spi.PrestoException: Unsupported type parameters (BoundVariables{typeVariables={}, longVariables={p=1, s=0, rp=2}}) for round<rp:min(38, p + 1)>(decimal(p,s),integer):decimal(rp,s)
	at com.facebook.presto.operator.scalar.TestMathFunctions.testRound(TestMathFunctions.java:891)

wenleix

Some quick comments after glancing. Let's first separate the commits.

wenleix · 2018-07-13T19:26:02Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

Move this fix to a separate commit (the third commit noted in #10961 (comment))

wenleix · 2018-07-13T19:31:07Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

nit: need to update this comment

wenleix · 2018-07-13T20:20:35Z

...c/main/java/com/facebook/presto/operator/scalar/annotations/ScalarFromAnnotationsParser.java

Originally, we thought Signature itself can distinguish the different PSI. Thus we only use Signature as the hash key.

Now since Signature itself is not enough to distinguish PSI, and argument native container + specialize native container type are required. The current implementation is incorrect since only one ParametricScalarImplementation will be put into the signature.

Lets put Signature, argument and return value native container type, type parameter specialization native container type into an inner class SpecializedSignature, and use that as key.

haozhun

I took a quick pass.

haozhun · 2018-07-16T01:42:33Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

assertEquals is for assertion in test only. Here, you can use checkArgument. In addition, you would want to provide an error messages. The audience of the error message is people who are implementing custom SQL functions through the annotation message.

haozhun · 2018-07-16T01:46:51Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

This if should not be in if (method.getParameterCount() > (i + 1)). Instead, you would want to move it out so that you check for this annotation regardless of whether a next argument exists. In addition, you would want to assert that 1) the next argument exists, 2) the next argument has @BlockIndex annotation.

haozhun · 2018-07-16T01:53:03Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

With these 3 boolean flags nullableArgument, hasNullFlag, and isBlockAndPositionNullConvention, there exists 8 possible combinations. However, only 4 of them is legal.

Previously, there were 4 possible combinations, but only 3 of them is legal. That was not ideal. However, It was not too hard to 1) conclude that the illegal one is impossible; 2) parser fails loudly when both @IsNull and @SqlNullable is present.

With the addition, reasoning about that just became pretty much intractable.

There are two alternatives ways to address this:

This code is organized in such a way that consumption of these variables and production of these variables are clearly split. At the split, add assertion.

Use an enum instead of 3 flags.

I highly recommend taking the 2nd alternative.

haozhun · 2018-07-16T01:54:39Z

presto-main/src/test/java/com/facebook/presto/type/TestBlockAndPositionNullConvention.java

Make your test contains all 3 of: generic, specialize, and exact. Ideally, there should be at least of specialized ones, and 2 exact ones.

haozhun · 2018-07-16T02:00:43Z

@wenleix When you reference @x, make sure you always put it in a pair of "`". We've previously received complaint from the github user whose username is isnull.

wenleix

The first two commits look good.

Move @BlockPosition and @BlockIndex to SPI
Rename ScalarImplementation to ParametricScalarImplementation

@haozhun : Do we also want to rename ScalarFunctionChoice to ScalarFunctionImplementationChoice ?

findepi · 2018-07-20T06:53:59Z

@gerlou @haozhun @wenleix can you please describe to me the benefits of block and position calling convention?
This looks good at first, because we don't need to extract a value from a block before calling a scalar. However, this looks like preventing composition of scalars. Am I missing something?

haozhun · 2018-07-20T16:39:22Z

@findepi A function that implements @BlockPosition convention is also required to implement the regular one. A function with the exact same signature (signature, native container type, type parameter specialization) can have multiple implementations now.

findepi · 2018-07-20T19:13:34Z

@haozhun that makes sense! i was fooled by "migrate..." PR (#11085) and i admit, i didn't investigate the code

wenleix

Some minor comments plus a major one in ParametricScalarImplementation.specialize. Let's discuss in person on this.

wenleix · 2018-07-19T23:27:03Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

        this.constructorDependencies = ImmutableList.copyOf(requireNonNull(constructorDependencies, "constructorDependencies is null"));
        this.argumentNativeContainerTypes = ImmutableList.copyOf(requireNonNull(argumentNativeContainerTypes, "argumentNativeContainerTypes is null"));
        this.specializedTypeParameters = ImmutableMap.copyOf(requireNonNull(specializedTypeParameters, "specializedTypeParameters is null"));
+        this.choices = choices;


nit: Guard these by requireNonNull, do this for returnContainerType and specializedSignature as well.

wenleix · 2018-07-19T23:34:35Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

                return Optional.empty();
            }
        }
-        Class<?> returnContainerType = getNullAwareReturnType(typeManager.getType(boundSignature.getReturnType()).getJavaType(), nullable);


nit: Remove unused method getNullAwareReturnType

wenleix · 2018-07-20T19:26:28Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

-            else {
-                if (argumentProperty.getArgumentType() != VALUE_TYPE) {
-                    return Optional.empty();
+        for (ParametricScalarImplementationChoice choice : choices) {


At high level, whether the ParametricScalarImplementation (PSI) suited for the specialization should be a decision at PSI level without consulting underlying ParametricScalarImplementationChoice (PSIC).

I understand currently some information is only in PSIC. Let's discuss offline about how to reorg fields between PSI / PSIC.

wenleix · 2018-07-20T19:50:52Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java


            this.methodHandle = getMethodHandle(method);
+
+            ParametricScalarImplementationChoice choice = new ParametricScalarImplementationChoice(nullable, argumentProperties, methodHandle, constructorMethodHandle, specializedTypeParameters, argumentNativeContainerTypes, numberOfBlockPositionArguments);


numberOfBlockPositionArguments should be calculated in the constructor instead of passed in by caller.

wenleix · 2018-07-20T19:52:38Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

+        private final List<ArgumentProperty> argumentProperties;
+        private final MethodHandle methodHandle;
+        private final Optional<MethodHandle> constructor;
+        private final Map<String, Class<?>> specializedTypeParameters;


specializedTypeParameters should be in ParametricScalarImplementation

wenleix · 2018-07-20T20:02:21Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

+                        argumentNativeContainerTypes.add(type.nativeContainerType());
+                    }
+                    else {
+                        checkArgument(type.nativeContainerType().equals(Object.class), "Native container type for parameter needs to be an Object");


I don't understand this check?

I am also thinking, when parameterType from the method itself is Object, we should enforce nativeContainerType is annotated.

So we always have the Java type for the scalar function in PSIC.

cc @haozhun

wenleix

Some quick comments.

Let's talk about the comments about nullConventionFlag part in person.

wenleix · 2018-07-24T00:21:44Z

.../main/java/com/facebook/presto/operator/aggregation/ApproximateCountDistinctAggregation.java

nit: I squash the fixup commit to the wrong commit. This two lines should go to the first commit...

wenleix · 2018-07-24T00:21:55Z

presto-main/src/main/java/com/facebook/presto/operator/scalar/ParametricScalar.java

nit: Cleanup these . Same for other debugging comments :)

wenleix · 2018-07-24T00:26:55Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

Also check

constructorDependencies

constructor

wenleix · 2018-07-24T00:34:33Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

Since we have done unwrap for argumentNativeContainerTypes already, I think we can do

if (!(argumentNativeContainerTypes.get(i).get() == Object.class || argumentNativeContainerTypes.get(i).get() == argumentType)) { ... }

wenleix · 2018-07-24T00:35:55Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

This should never happen. Either remove it or make it to be an checkState statement.

wenleix · 2018-07-24T00:38:59Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

nit: I would put an numberOfBlockPositionArguments before the for loop

wenleix · 2018-07-24T00:44:58Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

It's really one-to-one mapping with NullConvention, let's not create this new enum.

wenleix · 2018-07-24T01:02:10Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

Use a local variable for nullConventionFlag here (instead of using the field)

wenleix · 2018-07-24T01:04:44Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

Here NullConventionFlag is indicating the return convention, which is not a natural usage.

facebook-github-bot · 2018-07-25T20:40:07Z

Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours has expired.

Before we can review or merge your code, we need you to email cla@fb.com with your details so we can update your status.

wenleix

Generally looks good. I will take a look at tests later.

Remember to reorg changes to BlockIndex / BlockPosition to the first commit. (in ApproximateCountDistinctAggregation.java and TestAnnotationEngineForAggregates.java

@haozhun can you also start to review ? :)

wenleix · 2018-07-25T18:13:41Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

                Class<?> argumentType = typeManager.getType(boundSignature.getArgumentTypes().get(i)).getJavaType();
-                Class<?> argumentContainerType = getNullAwareContainerType(argumentType, argumentProperty.getNullConvention());
-                if (!argumentNativeContainerTypes.get(i).isAssignableFrom(argumentContainerType)) {
+                if (!(argumentNativeContainerTypes.get(i).get().isAssignableFrom(argumentType))) {


How does this work when argumentNativeContainerTypes is Object.class and argumentType is primitive type? isAssignableFrom will return false.

Does that mean such case never exists?

nit: remove the redundant brackets !(...) -> !....

@wenleix There is indeed no code that depends on it. TypeOfFunction is an interesting example that demonstrates the deficiency here.

Supporting it is not hard, but also nontrivial.

Boxing will be necessary for primitives.

Downcasting will be necessary for return types.

Once the support is added, this place can be changed to argumentNativeContainerType != Object.class && argumentNativeContainerType == argumentType (NullConvention is no longer available here.)

wenleix · 2018-07-25T20:07:50Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

+    public SpecializedSignature getSpecializedSignature()
    {
-        return dependencies;
+        SpecializedSignature specializedSignature = new SpecializedSignature(


nit: You can create the SpecializedSignature and return it in one step.

return new SpecializedSignature( signature, argumentNativeContainerTypes, specializedTypeParameters, returnNativeContainerType);

wenleix · 2018-07-25T20:46:53Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

+                            // USE_NULL_FLAG or RETURN_NULL_ON_NULL
+                            checkState(parameterType == Void.class || !Primitives.isWrapperType(parameterType), "Method [%s] has parameter expected to use USE_NULL_FLAG or RETURN_NULL_ON_NULL null convention. Parameter type is unexpected: %s", method, parameterType.getSimpleName());
+
+                            boolean useNullFlag = false;


@haozhun Any comments about this auxiliary boolean flag?

facebook-github-bot · 2018-07-26T19:52:23Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

haozhun · 2018-07-27T23:04:50Z

Add type parameter specialization inference based on return type

A better commit message would be something like

Fix specializedTypeParameters in ParametricScalarImplementation

The computation of specializedTypeParameters was incorrect.
Inference was only applied to argument types, but not the return type.

The original message did not provide a high level context. As a result, a reader wouldn't know what it is about.

haozhun

Add BLOCK_AND_POSITION calling convention to annotation framework

haozhun · 2018-07-28T01:15:53Z

presto-main/src/main/java/com/facebook/presto/operator/scalar/ParametricScalar.java

Don't remove this assertion:

checkCondition(methodHandleAndConstructor.isPresent(), FUNCTION_IMPLEMENTATION_ERROR, String.format("Exact implementation of %s do not match expected java types.", boundSignature.getName()));

haozhun · 2018-07-28T01:33:47Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

For all 3,

checkCondition(..., FUNCTION_IMPLEMENTATION_ERROR, "Some descriptive message");

haozhun · 2018-07-28T01:41:03Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

                Class<?> argumentType = typeManager.getType(boundSignature.getArgumentTypes().get(i)).getJavaType();
-                Class<?> argumentContainerType = getNullAwareContainerType(argumentType, argumentProperty.getNullConvention());
-                if (!argumentNativeContainerTypes.get(i).isAssignableFrom(argumentContainerType)) {
+                if (!(argumentNativeContainerTypes.get(i).get().isAssignableFrom(argumentType))) {


nit: remove the redundant brackets !(...) -> !....

@wenleix There is indeed no code that depends on it. TypeOfFunction is an interesting example that demonstrates the deficiency here.

Supporting it is not hard, but also nontrivial.

Boxing will be necessary for primitives.

Downcasting will be necessary for return types.

Once the support is added, this place can be changed to argumentNativeContainerType != Object.class && argumentNativeContainerType == argumentType (NullConvention is no longer available here.)

haozhun · 2018-07-28T01:45:36Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

Remove. This is no longer used.

haozhun · 2018-07-28T03:31:35Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

Add comment that all choices are required to have the same constructor dependencies at this time. And point out that this is asserted in the constructor.

haozhun · 2018-07-28T03:33:41Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

Use a Builder if necessary. Making this class mutable is:

surprising and unintuitive

undermines your validations in the constructor.

haozhun · 2018-07-28T03:36:23Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

@SqlType can only contain an explicitly specified nativeContainerType when using @BlockPosition

haozhun · 2018-07-30T02:54:16Z

presto-main/src/test/java/com/facebook/presto/operator/scalar/TestScalarValidation.java

There is a bug here. The message is wrong.

The return type in PrimitiveParameterWithNullable is long. But that's correct because there is no @SqlNullable annotation on the method.

haozhun · 2018-07-30T02:58:38Z

presto-main/src/test/java/com/facebook/presto/operator/scalar/TestScalarValidation.java

I can't read this sentence. Please change the error message and this assertion to

A parameter with USE_NULL_FLAG or RETURN_NULL_ON_NULL convention must not use wrapper type. Found in method ...

haozhun · 2018-07-30T03:05:02Z

presto-main/src/test/java/com/facebook/presto/operator/scalar/TestScalarValidation.java

The assertions should have use

checkCondition(..., FUNCTION_IMPLEMENTATION_ERROR, ...)

Fix all in ScalarFromAnnotationsParser and ParametricScalarImplementation after this PR is merged.

haozhun · 2018-08-01T01:17:08Z

...ain/java/com/facebook/presto/operator/scalar/annotations/ParametricScalarImplementation.java

Implementations for the same function signature must have matching dependencies: <signature>

Ditto for the next two.

The computation of specializedTypeParameters was incorrect. Inference was only applied to argument types, but not the return type.

facebook-github-bot added the CLA Signed label Jul 2, 2018

gerlou force-pushed the annotation_block_position branch from 697a3c0 to 802ad6b Compare July 2, 2018 19:59

gerlou changed the title ~~Move @BlockPosition, @BlockIndex to SPI~~ Move @BlockPosition and @BlockIndex to SPI Jul 2, 2018

findepi reviewed Jul 5, 2018

View reviewed changes

wenleix assigned wenleix and unassigned wenleix Jul 9, 2018

gerlou force-pushed the annotation_block_position branch from 8d5f43c to 119588c Compare July 12, 2018 20:43

wenleix reviewed Jul 13, 2018

View reviewed changes

haozhun reviewed Jul 16, 2018

View reviewed changes

gerlou force-pushed the annotation_block_position branch from 119588c to 03b003c Compare July 19, 2018 17:51

gerlou changed the title ~~Move @BlockPosition and @BlockIndex to SPI~~ Support @BlockPosition and @BlockIndex annotation in scalar function Jul 19, 2018

gerlou force-pushed the annotation_block_position branch 6 times, most recently from 9fa7194 to 4c67825 Compare July 19, 2018 19:20

gerlou changed the title ~~Support @BlockPosition and @BlockIndex annotation in scalar function~~ Add BLOCK_AND_POSITION calling convention to annotation framework Jul 19, 2018

gerlou force-pushed the annotation_block_position branch from 4c67825 to ec298ad Compare July 19, 2018 22:53

gerlou mentioned this pull request Jul 19, 2018

Migrate IS_DISTINCT_FROM operator to use BLOCK_AND_POSITION convention #11085

Closed

wenleix reviewed Jul 19, 2018

View reviewed changes

wenleix reviewed Jul 20, 2018

View reviewed changes

gerlou force-pushed the annotation_block_position branch 2 times, most recently from 2dba166 to f2e53d3 Compare July 23, 2018 23:58

gerlou force-pushed the annotation_block_position branch 2 times, most recently from 93877c1 to 58c77e9 Compare July 24, 2018 00:01

wenleix reviewed Jul 24, 2018

View reviewed changes

gerlou force-pushed the annotation_block_position branch 4 times, most recently from 2591ffe to 39a5fd3 Compare July 25, 2018 00:09

wenleix mentioned this pull request Jul 25, 2018

Change java type of UNKNOWN from void to boolean #11116

Merged

wenleix reviewed Jul 25, 2018

View reviewed changes

wenleix assigned haozhun Jul 25, 2018

gerlou force-pushed the annotation_block_position branch from 39a5fd3 to 4706596 Compare July 26, 2018 19:52

haozhun reviewed Jul 30, 2018

View reviewed changes

gerlou force-pushed the annotation_block_position branch 6 times, most recently from 5273ef1 to eaf0399 Compare July 31, 2018 05:36

haozhun reviewed Aug 1, 2018

View reviewed changes

gerlou force-pushed the annotation_block_position branch from eaf0399 to 7c121d0 Compare August 1, 2018 16:58

haozhun approved these changes Aug 2, 2018

View reviewed changes

Gerlou Shyy added 4 commits August 2, 2018 11:23

Move @BlockPosition and @BlockIndex to SPI

d6f7548

Rename ScalarImplementation to ParametricScalarImplementation

20a3276

Fix specializedTypeParameters in ParametricScalarImplementation

91e535d

The computation of specializedTypeParameters was incorrect. Inference was only applied to argument types, but not the return type.

Add BLOCK_AND_POSITION calling convention to annotation framework

0284f25

gerlou force-pushed the annotation_block_position branch from 7c121d0 to 0284f25 Compare August 2, 2018 18:23

gerlou merged commit 0284f25 into prestodb:master Aug 2, 2018


		this.methodHandle = getMethodHandle(method);

		ParametricScalarImplementationChoice choice = new ParametricScalarImplementationChoice(nullable, argumentProperties, methodHandle, constructorMethodHandle, specializedTypeParameters, argumentNativeContainerTypes, numberOfBlockPositionArguments);

Conversation

gerlou commented Jul 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haozhun commented Jul 2, 2018

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!

wenleix commented Jul 13, 2018 • edited by gerlou Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wenleix left a comment

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!

wenleix Jul 13, 2018 • 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!

haozhun left a comment

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!

haozhun commented Jul 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wenleix left a comment

Choose a reason for hiding this comment

Uh oh!

findepi commented Jul 20, 2018

Uh oh!

haozhun commented Jul 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

findepi commented Jul 20, 2018

Uh oh!

wenleix left a comment • 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!

wenleix Jul 20, 2018 • 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!

wenleix left a comment

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

gerlou commented Jul 2, 2018 •

edited

Loading

wenleix commented Jul 13, 2018 •

edited by gerlou

Loading

wenleix Jul 13, 2018 •

edited

Loading

haozhun commented Jul 16, 2018 •

edited

Loading

haozhun commented Jul 20, 2018 •

edited

Loading

wenleix left a comment •

edited

Loading

wenleix Jul 20, 2018 •

edited

Loading