[SPARK-19084][SQL] Implement expression field #16476

gczsjdy · 2017-01-05T10:50:35Z

What changes were proposed in this pull request?

This is an implementation of expression field which is implemented as built-in function by Hive and MySQL.

field(expr, expr1, expr2, ... ) is a variable-length(>=2) function that returns the index of expr in (expr1, expr2, ...) list or 0 if not found.

It takes at least 2 parameters, and all parameters can be of any type.
Implicit cast will be done when at least 2 parameters have different types, and it will be based on the first parameter's type.
If the first parameter is of NumericType, all parameters will be implicitly cast to DoubleType, and those that can't be cast to DoubleType will be regarded as NULL.
If the first parameter is of any other type, all parameters will be implicitly cast to StringType and the comparison will follow String's comparing rules.
If the search expression is NULL, the return value is 0 because NULL fails equality comparison with any value.

How was this patch tested?

Unit tests are in ConditionalExpressionSuite & ColumnExpressionSuite.

gczsjdy · 2017-01-05T13:10:07Z

cc @chenghao-intel @adrian-wang

adrian-wang · 2017-01-05T16:11:51Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

 }
+
+/**
+  * A function that returns the index of str in (str1, str2, ...) list or 0 if not found.


Nit: delete a space before *

adrian-wang · 2017-01-05T16:16:17Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+    if(target == null)
+      0
+    else
+     findEqual(target, children.tail, 1)


Nit: one more space before findEqual

adrian-wang · 2017-01-05T16:19:57Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+       """
+    }
+
+    def dataTypeEqualsTarget(evalWithIndex: Tuple2[Tuple2[ExprCode, DataType], Int]): Boolean = {


def dataTypeEqualsTarget(evalWithIndex: ((ExprCode, DataType), Int)): Boolean

rxin · 2017-01-06T21:30:30Z

python/pyspark/sql/functions.py



+@since(2.2)
+def field(*cols):


can we remove this?

rxin · 2017-01-06T21:30:37Z

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

+    * @group normal_funcs
+    * @since 2.2.0
+    */
+  @scala.annotation.varargs


can we remove this?

Sure, I have removed this.
Still curious, is it inappropriate to be in Dataset API?

@rxin can you explain a little bit why we remove this?

rxin · 2017-01-06T21:30:49Z

...st/src/test/scala/org/apache/spark/sql/catalyst/expressions/ConditionalExpressionSuite.scala

    checkEvaluation(CaseKeyWhen(literalNull, Seq(c2, c5, c1, c6)), null, row)
  }
+
+  test("case field") {


what's "case"?

I feel like you have comprehensive testing but at the same time feels like there is overlap amongst the tests in terms of coverage. There is room of reducing the tests while still having same coverage. eg you don't need 5 strings and use lesser.

@rxin My bad, will take it out

@tejasapatil The first 3 strings are base test strings, the 5th is for null of string type. Probably I can remove the 4th one which is not useful. What do you think?
About the tests' role, could you please check another thread where I @ you?

rxin · 2017-01-06T21:31:19Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+  usage = "_FUNC_(str, str1, str2, ...) - Returns the index of str in the str1,str2,... or 0 if not found.",
+  extended = """
+    Examples:
+      > SELECT _FUNC_(10, 9, 3, 10, 4);


can we use strings as examples rather than integer literals?

This UDF accepts any mix atomic types so one can even get fancy with the inputs. Would recommend mentioning that in the doc (given that you have tests for that below)

I have given 3 examples, one for Integer, one for String, one for mixed types.

gatorsmile · 2017-01-07T07:42:30Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+ * It takes at least 2 parameters, and all parameters' types should be subtypes of AtomicType.
+ */
+@ExpressionDescription(
+  usage = "_FUNC_(str, str1, str2, ...) - Returns the index of str in the str1,str2,... or 0 if not found.",


Can we use expr1, expr2, expr3 here? The type can be any atomic type. right?

Yeah, it's more reasonable to use expr(n), thx.
Probably it should be AtomicType or NullType to support user's writing of null.

gatorsmile · 2017-01-07T07:47:29Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+  extended = """
+    Examples:
+      > SELECT _FUNC_(10, 9, 3, 10, 4);
+       3


More examples please?

gatorsmile · 2017-01-07T07:51:08Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+    if(target == null)
+      0
+    else
+      findEqual(target, children.tail, 1)


Could you fix the style, based on https://github.com/databricks/scala-style-guide#curly?

findEqual(target, children.tail, index = 1)

gatorsmile · 2017-01-07T07:53:49Z

...st/src/test/scala/org/apache/spark/sql/catalyst/expressions/ConditionalExpressionSuite.scala

+    checkEvaluation(Field(Seq(str5, str1, str2, str4)), 0)
+    checkEvaluation(Field(Seq(int4, double3, str5, bool1, date1, timeStamp2, int3)), 0)
+    checkEvaluation(Field(Seq(int1, strNull, intNull, bool1, date1, timeStamp2, int3)), 0)
+    checkEvaluation(Field(Seq(strNull, int1, str1, str2, str3)), 0)


This is to test null. Could you add the description?

If the search string is NULL, the return value is 0 because NULL fails equality comparison with any value

This fails with the patch.

hc.sql("""SELECT FIELD("tejas", 34, "patil", true, null, "tejas") FROM src LIMIT 1""").collect.foreach(println)

Removing null makes it work. Can you check on your side ? It worked with Hive.

@gatorsmile Sure.

@tejasapatil It doesn't work with my code because it only support AtomicType. While user's writing of 'null' is NullType, I will add NullType support to be consistent with Hive.

gatorsmile · 2017-01-07T07:55:57Z

...st/src/test/scala/org/apache/spark/sql/catalyst/expressions/ConditionalExpressionSuite.scala

+    checkEvaluation(Field(Seq(int4, double3, str5, bool1, date1, timeStamp2, int4)), 6)
+    checkEvaluation(Field(Seq(str5, str1, str2, str4)), 0)
+    checkEvaluation(Field(Seq(int4, double3, str5, bool1, date1, timeStamp2, int3)), 0)
+    checkEvaluation(Field(Seq(int1, strNull, intNull, bool1, date1, timeStamp2, int3)), 0)


What is the purpose of these checks?

Based on MySQL's field function, the type casting rules is described as

If all arguments to FIELD() are strings, all arguments are compared as strings. If all arguments are numbers, they are compared as numbers. Otherwise, the arguments are compared as double.

Line 180 is to test multi types of parameters;
Line 181 is to test not found case;
Line 182 is to test not found case when parameters are of multi types;
Line 183 is to test null in parameter which has >=1 index
I think maybe we should refer to Hive's field? In Hive, when not all arguments are numbers && not all arguments are strings, they are not compared as double.

Also @tejasapatil , here's some lines' explanation.

I have removed line 181, since line 182 actually covers not found case.

gatorsmile · 2017-01-07T08:02:02Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+    val target = children.head.eval(input)
+    val targetDataType = children.head.dataType
+    def findEqual(target: Any, params: Seq[Expression], index: Int): Int = {
+      params.toList match {


Can we avoid toList for each recursive call?

Any reason to not do this iteratively ? I would suggest avoiding recursion to get better perf.

@gatorsmile Actually, the case:
checkAnswer(testData.selectExpr("field('花花世界', 'a', 1.23, true, '花花世界')"), Row(4))
will produce a child: Seq[Expression] an ArrayBuffer, it's not a list, so can't use head::tail.
So there are 2 ways:

remove the toList and do another pattern match for ArrayBuffer, which I think is not neat.

keep the toList.

@tejasapatil Actually, I think it's tail recursion, so the compiler will do the optimization, then it has the same performance with iteration edition.

you can add the annotation @tailrec for explicitly declare that.

toList probably causes performance overhead, I don't think we have to sacrifice the performance for using the pattern match. In the meantime, I still believe we don't have to check the data type during the runtime. It's supposed to be done during the compile time or only done once for the first time in eval.

The Field evaluation is quite confusing, as @gatorsmile suggested, we need to describe how to evaluate the value when sub expressions' data type are different.

@chenghao-intel I have removed the toList, replaced the pattern match by if & else,
and also reduced the type check time to 1 for the same table.
I have added line 353-354 as comments for sub expressions' multiple data types, could you please have a look?

tejasapatil · 2017-01-07T08:18:22Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

 }
+
+/**
+ * A function that returns the index of str in (str1, str2, ...) list or 0 if not found.


change str to expr here as well.

tejasapatil · 2017-01-07T08:27:38Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+  usage = "_FUNC_(str, str1, str2, ...) - Returns the index of str in the str1,str2,... or 0 if not found.",
+  extended = """
+    Examples:
+      > SELECT _FUNC_(10, 9, 3, 10, 4);


This UDF accepts any mix atomic types so one can even get fancy with the inputs. Would recommend mentioning that in the doc (given that you have tests for that below)

tejasapatil · 2017-01-07T08:31:35Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+    val target = children.head.eval(input)
+    val targetDataType = children.head.dataType
+    def findEqual(target: Any, params: Seq[Expression], index: Int): Int = {
+      params.toList match {


Any reason to not do this iteratively ? I would suggest avoiding recursion to get better perf.

tejasapatil · 2017-01-07T08:31:52Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+        case _ => findEqual(target, params.tail, index + 1)
+      }
+    }
+    if(target == null)


nit: space after if

checkstyle will fail saying if-else needs to use braces

tejasapatil · 2017-01-07T08:35:51Z

...st/src/test/scala/org/apache/spark/sql/catalyst/expressions/ConditionalExpressionSuite.scala

+
+    checkEvaluation(Field(Seq(str1, str2, str3, str1)), 3)
+    checkEvaluation(Field(Seq(str2, str2, str2, str1)), 1)
+    checkEvaluation(Field(Seq(str4, str4, str4, str1)), 1)


same as previous ?

tejasapatil · 2017-01-07T08:42:21Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+      """
+    }
+
+    def genIfElseStructure(code1: String, code2: String): String = {


From the name it felt like this method would put code1 in if block and code2 in else block but turns out thats not the case. That floating else looks weird.

Maybe I can change this function's name? But actually I can't think of a better name. : )

Nit: Maybe we can use foldLeft to replace current approach to get rid of the floating else.

Sorry I don't understand, how to use foldLeft approach? I think we can only use foldRight or reduceRight, because the code for latter children should be nested inner.

Oh, yeah, right, if use foldLeft, there is still a floating else. We can only use foldRight to remove it.

How to use foldRight to remove it?
My thought: If I understand your meaning of floating else right(could you please explain it a little bit?), foldRight and reduceRight both can't avoid floating else, because we need nested else in else block, like this:
if (xxx) else { if (xxx) else { ... } } , so if we avoid floating else in genIfElseStructure, else should be in updateEval, which will make the code unclear and complicated.

I think it looks like:

${evalChildren.zip(dataTypes).zipWithIndex.tail.filter { x => dataTypeMatchIndex.contains(x._2) }.foldRight("") { (code: String, evalWithIndex: ((ExprCode, DataType), Int)) => val ((eval, _), index) = evalWithIndex val condition = ctx.genEqual(targetDataType, eval.value, target.value) s""" ${eval.code} if ($condition) { ${ev.value} = ${index}; } else { $code } """ }

You can do this with a function like you did before. It will have a empty "else" block at the end.

However this doesn't affect the functionality, just dealing with how the code looks. I don't have strong option about this.

I think whats already present in the code is ok. Given that there is no better option without adding more complexity, lets stick with it.

@viirya Maybe the order of code and evalWithIndex parameters should be changed.
@tejasapatil I agree with your opinion.

Current code is ok.

tejasapatil · 2017-01-07T08:45:05Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+      findEqual(target, children.tail, 1)
+  }
+
+  protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {


Do your unit tests cover generated code ?

Yes, because in checkEvaluation function there is : checkEvaluationWithGeneratedMutableProjection

tejasapatil · 2017-01-07T09:19:04Z

...st/src/test/scala/org/apache/spark/sql/catalyst/expressions/ConditionalExpressionSuite.scala

+    checkEvaluation(Field(Seq(str5, str1, str2, str4)), 0)
+    checkEvaluation(Field(Seq(int4, double3, str5, bool1, date1, timeStamp2, int3)), 0)
+    checkEvaluation(Field(Seq(int1, strNull, intNull, bool1, date1, timeStamp2, int3)), 0)
+    checkEvaluation(Field(Seq(strNull, int1, str1, str2, str3)), 0)


This fails with the patch.

hc.sql("""SELECT FIELD("tejas", 34, "patil", true, null, "tejas") FROM src LIMIT 1""").collect.foreach(println)

Removing null makes it work. Can you check on your side ? It worked with Hive.

tejasapatil · 2017-01-07T09:34:02Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+      val ((eval, dataType), index) = evalWithIndex
+      s"""
+        ${eval.code}
+        if (${dataType.equals(targetDataType)}


At this point the code at lines 427-428 (ie. .filter(dataTypeEqualsTarget)) have ensured that this will always be true. The generated code will have this as true and you might as well get rid of the check here.

tejasapatil · 2017-01-07T09:51:24Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+      ${target.code}
+      boolean ${ev.isNull} = false;
+      int ${ev.value} = 0;
+      ${rest.zip(restDataType).zipWithIndex.map(x => (x._1, x._2 + 1)).filter(


.zipWithIndex.map(x => (x._1, x._2 + 1)) can be simplified as .zip(Stream from 1)

chenghao-intel · 2017-01-09T13:06:59Z

@gczsjdy can you please add [WIP] in the title, until you feel the code is ready for review.

chenghao-intel · 2017-01-10T00:51:34Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+
+  private lazy val ordering = TypeUtils.getInterpretedOrdering(children(0).dataType)
+
+  private val dataTypeMatchIndex: Seq[Int] = children.tail.zip(Stream from 1).filter(


Array[Int] instead? Seq[Int] probably a LinkedList in its concrete implementation.

Won't it be inconsistent with children? Since children's type is Seq[Expression].

chenghao-intel · 2017-01-10T00:52:29Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+  private lazy val ordering = TypeUtils.getInterpretedOrdering(children(0).dataType)
+
+  private val dataTypeMatchIndex: Seq[Int] = children.tail.zip(Stream from 1).filter(
+    _._1.dataType == children.head.dataType).map(_._2)


_._1.dataType.sameType(children.head.dataType)?

Good idea, thx.

chenghao-intel · 2017-01-10T00:53:12Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+
+  private lazy val ordering = TypeUtils.getInterpretedOrdering(children(0).dataType)
+
+  private val dataTypeMatchIndex: Seq[Int] = children.tail.zip(Stream from 1).filter(


zip(Stream from 1), do we really need it?

Actually, it's short for zipWithIndex.map(x => (x._1, x._2 + 1)).
I realized it makes people confused, and have changed it.

chenghao-intel · 2017-01-10T01:02:12Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+  override def dataType: DataType = IntegerType
+  override def eval(input: InternalRow): Any = {
+    val target = children.head.eval(input)
+    val targetDataType = children.head.dataType


Unused code.

chenghao-intel · 2017-01-10T01:04:02Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+    val target = children.head.eval(input)
+    val targetDataType = children.head.dataType
+    @tailrec def findEqual(index: Int): Int = {
+      if (index == dataTypeMatchIndex.size) {


if dataTypeMatchIndex is Array[Int], then we'd better use dataTypeMatchIndex.length instead.

chenghao-intel · 2017-01-10T01:06:06Z

...talyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala

+        0
+      } else {
+        val value = children(dataTypeMatchIndex(index)).eval(input)
+        if (value != null && ordering.equiv(target, value))


use braces. see https://github.com/databricks/scala-style-guide#curly

chenghao-intel · 2017-01-10T01:13:18Z

Since the different data type will be simply ignored, I think we'd better also add the optimization rule in Optimizer.

As well as the python/scala API support, but need to confirm with @rxin, why we don't need the API field.

…ern match

…es match

gczsjdy · 2018-07-16T08:30:33Z

@HyukjinKwon Done, thanks : )
Gentle ping @maropu. About the implicit cast part, seems this is the part difficult to agree with, do you have any suggestions?

HyukjinKwon · 2018-07-16T08:32:19Z

ok to test

SparkQA · 2018-07-16T08:35:15Z

Test build #93100 has finished for PR 16476 at commit 4b63e94.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-07-26T11:17:47Z

Test build #93594 has finished for PR 16476 at commit c75d786.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2019-09-16T18:26:51Z

Can one of the admins verify this patch?

github-actions · 2020-01-19T00:06:43Z

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

gczsjdy changed the title ~~Implement expression field~~ [SPARK-19084][SQL] Implement expression field Jan 5, 2017

adrian-wang reviewed Jan 5, 2017

View reviewed changes

gczsjdy force-pushed the udffield branch from 288fd9f to e8e492e Compare January 6, 2017 06:29

rxin reviewed Jan 6, 2017

View reviewed changes

python/pyspark/sql/functions.py Outdated

@since(2.2)

def field(*cols):

Copy link

Contributor

rxin Jan 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove this?

rxin requested changes Jan 6, 2017

View reviewed changes

gatorsmile reviewed Jan 7, 2017

View reviewed changes

tejasapatil suggested changes Jan 7, 2017

View reviewed changes

gczsjdy force-pushed the udffield branch 2 times, most recently from 44941ab to b09f446 Compare January 9, 2017 13:04

gczsjdy force-pushed the udffield branch from 513c4ed to e782906 Compare January 9, 2017 13:41

gczsjdy changed the title ~~[SPARK-19084][SQL] Implement expression field~~ [SPARK-19084][SQL][WIP] Implement expression field Jan 9, 2017

chenghao-intel reviewed Jan 10, 2017

View reviewed changes

gczsjdy force-pushed the udffield branch 2 times, most recently from ff9a0be to 08e9f0c Compare January 10, 2017 07:10

gczsjdy changed the title ~~[SPARK-19084][SQL][WIP] Implement expression field~~ [SPARK-19084][SQL] Implement expression field Jan 10, 2017

GuoChenzhao added 19 commits July 16, 2018 15:58

Remove Dataset API, simplify the test cases, and add some comments

7d347c2

Tune the code

46c3986

Improve the performance and add the tail-rec annotation

4761687

Add description in comment for strNull match

6954b27

Improve the performance by reducing the type check and giving up patt…

c565e6c

…ern match

Improve the performance by only looping through columns where dataTyp…

7f0a7e9

…es match

Add explicit NULL support

e1914ea

Add more comments/docs for parameters with multi types

321c373

Remove unused lines and tune the code

ce776ec

Change size to length

c8ed3d4

Remove unused import and tune the doc

1c61252

Tune the doc further

76278fc

Optimization done and align with scala style requirement

ab990fd

Switch off scalastyle check in test to allow non-ascii characters

d71245f

Remove whitespace

e89ae0f

Modify type check failure hint

082189a

Support implicit cast

32fa22b

Not do implicit cast while all have the same type

b5d1675

Update docs

4b63e94

gczsjdy force-pushed the udffield branch from 9b4155a to 4b63e94 Compare July 16, 2018 08:26

Fix

c75d786

dongjoon-hyun added the SQL label Jun 14, 2019

github-actions bot added the Stale label Jan 19, 2020

github-actions bot closed this Jan 20, 2020


		private lazy val ordering = TypeUtils.getInterpretedOrdering(children(0).dataType)

		private val dataTypeMatchIndex: Seq[Int] = children.tail.zip(Stream from 1).filter(

[SPARK-19084][SQL] Implement expression field #16476

[SPARK-19084][SQL] Implement expression field #16476

Uh oh!

Conversation

gczsjdy commented Jan 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

gczsjdy commented Jan 5, 2017

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!

gczsjdy Jan 7, 2017 • 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!

gczsjdy Jan 7, 2017 • 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!

gatorsmile Jan 7, 2017 • 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!

gatorsmile Jan 7, 2017 • 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!

gatorsmile Jan 7, 2017 • 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!

gczsjdy Jan 7, 2017 • 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

gczsjdy commented Jan 5, 2017 •

edited

Loading

gczsjdy Jan 7, 2017 •

edited

Loading

gczsjdy Jan 7, 2017 •

edited

Loading

gatorsmile Jan 7, 2017 •

edited

Loading

gatorsmile Jan 7, 2017 •

edited

Loading

gatorsmile Jan 7, 2017 •

edited

Loading

gczsjdy Jan 7, 2017 •

edited

Loading

gczsjdy Jan 9, 2017 •

edited

Loading