[SPARK-24121][SQL] Add API for handling expression code generation #21193

viirya · 2018-04-30T13:36:09Z

What changes were proposed in this pull request?

This patch tries to implement this proposal to add an API for handling expression code generation. It should allow us to manipulate how to generate codes for expressions.

In details, this adds an new abstraction CodeBlock to JavaCode. CodeBlock holds the code snippet and inputs for generating actual java code.

For example, in following java code:

  int ${variable} = 1;
  boolean ${isNull} = ${CodeGenerator.defaultValue(BooleanType)};

variable, isNull are two VariableValue and CodeGenerator.defaultValue(BooleanType) is a string. They are all inputs to this code block and held by CodeBlock representing this code.

For codegen, we provide a specified string interpolator code, so you can define a code like this:

  val codeBlock =
    code"""
         |int ${variable} = 1;
         |boolean ${isNull} = ${CodeGenerator.defaultValue(BooleanType)};
        """.stripMargin
  // Generates actual java code.
  codeBlock.toString

Because those inputs are held separately in CodeBlock before generating code, we can safely manipulate them, e.g., replacing statements to aliased variables, etc..

How was this patch tested?

Added tests.

viirya · 2018-04-30T13:38:31Z

cc @cloud-fan @kiszk @maropu @hvanhovell

hvanhovell · 2018-04-30T13:56:36Z

@viirya I just glanced over this. We currently use a lot of mutable code for in code generation with - TBH - way too complex side effects. Your proposal adds another layer of side effects.

Why don't we add a bit more structure to the JavaCode abstraction, and figure out that we need splitting from that? It will probably be more involved than this because you probably cannot fix the names upfront.

viirya · 2018-05-01T05:03:04Z

retest this please.

viirya · 2018-05-01T15:12:14Z

@hvanhovell Thanks for your comment!

I tried to add new JavaCode abstraction Block which holds the code to generate. I hope this approach can be less side effect. Please comment if you have time to take a look. I'll be in vacation in next few days, but I will try to follow your comment if any. Thanks!

viirya · 2018-05-01T21:14:59Z

retest this please.

SparkQA · 2018-05-01T21:24:38Z

Test build #90004 has finished for PR 21193 at commit 369242a.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class ExprCode(var code: Block, var isNull: ExprValue, var value: ExprValue)
trait Block extends JavaCode
implicit class BlockHelper(val sc: StringContext) extends AnyVal
case class CodeBlock(codeParts: Seq[String], exprValues: Seq[Any]) extends Block
case class Blocks(blocks: Seq[Block]) extends Block
trait ExprValue extends JavaCode

SparkQA · 2018-05-01T22:02:46Z

Test build #90009 has finished for PR 21193 at commit 8e2db8b.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class ExprCode(var code: Block, var isNull: ExprValue, var value: ExprValue)
trait Block extends JavaCode
implicit class BlockHelper(val sc: StringContext) extends AnyVal
case class CodeBlock(codeParts: Seq[String], exprValues: Seq[Any]) extends Block
case class Blocks(blocks: Seq[Block]) extends Block
trait ExprValue extends JavaCode

SparkQA · 2018-05-02T18:21:46Z

Test build #90059 has finished for PR 21193 at commit 5fe425c.

This patch fails SparkR unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class ExprCode(var code: Block, var isNull: ExprValue, var value: ExprValue)
trait Block extends JavaCode
implicit class BlockHelper(val sc: StringContext) extends AnyVal
case class CodeBlock(codeParts: Seq[String], exprValues: Seq[Any]) extends Block
case class Blocks(blocks: Seq[Block]) extends Block
trait ExprValue extends JavaCode

viirya · 2018-05-02T23:22:16Z

retest this please.

SparkQA · 2018-05-03T02:51:20Z

Test build #90083 has finished for PR 21193 at commit 5fe425c.

This patch fails SparkR unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class ExprCode(var code: Block, var isNull: ExprValue, var value: ExprValue)
trait Block extends JavaCode
implicit class BlockHelper(val sc: StringContext) extends AnyVal
case class CodeBlock(codeParts: Seq[String], exprValues: Seq[Any]) extends Block
case class Blocks(blocks: Seq[Block]) extends Block
trait ExprValue extends JavaCode

maropu · 2018-05-03T04:19:58Z

retest this please.

SparkQA · 2018-05-03T07:05:01Z

Test build #90092 has finished for PR 21193 at commit 5fe425c.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class ExprCode(var code: Block, var isNull: ExprValue, var value: ExprValue)
trait Block extends JavaCode
implicit class BlockHelper(val sc: StringContext) extends AnyVal
case class CodeBlock(codeParts: Seq[String], exprValues: Seq[Any]) extends Block
case class Blocks(blocks: Seq[Block]) extends Block
trait ExprValue extends JavaCode

viirya · 2018-05-03T07:06:30Z

retest this please.

SparkQA · 2018-05-03T11:03:18Z

Test build #90104 has finished for PR 21193 at commit 5fe425c.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class ExprCode(var code: Block, var isNull: ExprValue, var value: ExprValue)
trait Block extends JavaCode
implicit class BlockHelper(val sc: StringContext) extends AnyVal
case class CodeBlock(codeParts: Seq[String], exprValues: Seq[Any]) extends Block
case class Blocks(blocks: Seq[Block]) extends Block
trait ExprValue extends JavaCode

mgaido91

Thanks for your work @viirya. I do like this approach.

mgaido91 · 2018-05-03T10:58:03Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala


 object ExprValue {
-  implicit def exprValueToString(exprValue: ExprValue): String = exprValue.toString
+  implicit def exprValueToString(exprValue: ExprValue): String = exprValue.code


why can't we use toString?

Because toString will call exprValueToString. We should have only one place to convert an ExprValue to string.

mgaido91 · 2018-05-03T10:58:45Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala

+        EmptyBlock
+      } else {
+        args.foreach {
+          case _: ExprValue => true


why true? Can't we just do nothing after the =>?

mgaido91 · 2018-05-03T11:04:23Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala

+  implicit def blocksToBlock(blocks: Seq[Block]): Block = Blocks(blocks)
+
+  implicit class BlockHelper(val sc: StringContext) extends AnyVal {
+    def code(args: Any*): Block = {


chat about adding checkLengths(args)?

mgaido91 · 2018-05-03T11:05:37Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala

+    val expressions = exprValues.iterator
+    var buf = new StringBuffer(strings.next)
+    while (strings.hasNext) {
+      if (expressions.hasNext) {


this is not needed (especially if we add the checkLengths), because if it is not true we are in a bad situation

SparkQA · 2018-05-03T14:05:49Z

Test build #90120 has finished for PR 21193 at commit 00bef6b.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class CodeBlock(codeParts: Seq[String], blockInputs: Seq[Any]) extends Block

viirya · 2018-05-03T14:25:08Z

retest this please.

SparkQA · 2018-05-03T14:34:56Z

Test build #90135 has finished for PR 21193 at commit 00bef6b.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class CodeBlock(codeParts: Seq[String], blockInputs: Seq[Any]) extends Block

SparkQA · 2018-05-03T18:42:50Z

Test build #90139 has finished for PR 21193 at commit 162deb2.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2018-05-04T01:31:00Z

I think this can be reviewed for now. Thanks. cc @cloud-fan @hvanhovell @kiszk @maropu

cloud-fan

LGTM except some minor comments

cloud-fan · 2018-05-18T08:50:50Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala

      // as a function before. In that case, we just re-use it.
-      ExprCode(ctx.registerComment(this.toString), subExprState.isNull, subExprState.value)
+      ExprCode(ctx.registerComment(this.toString), subExprState.isNull,
+        subExprState.value)


unnecessary change

cloud-fan · 2018-05-18T08:56:58Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala

+      buf append inputs.next
+      buf append StringContext.treatEscapes(strings.next)
+    }
+    buf.toString


shall we add an assert that assert(!inputs.hasNext)?

it is not needed because we have checkLengths at line 159. This is the same approach which is used by the StringContext in the scala library.

cloud-fan · 2018-05-18T09:04:34Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/inputFileBlock.scala

-    ev.copy(code = s"final ${CodeGenerator.javaType(dataType)} ${ev.value} = " +
-      s"$className.getInputFilePath();", isNull = FalseLiteral)
+    ev.copy(code = code"final ${CodeGenerator.javaType(dataType)} ${ev.value} = " +
+      code"$className.getInputFilePath();", isNull = FalseLiteral)


the Blocks.code is defined as override def code: String = blocks.map(_.toString).mkString("\n"), is it OK here?

maybe we can do

val typeDef = s"final ${CodeGenerator.javaType(dataType)}" ev.code(code = code"$typeDef ${ev.value} = $className.getInputFilePath();", ...)

cloud-fan · 2018-05-18T09:05:21Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/inputFileBlock.scala

    val className = InputFileBlockHolder.getClass.getName.stripSuffix("$")
-    ev.copy(code = s"final ${CodeGenerator.javaType(dataType)} ${ev.value} = " +
-      s"$className.getStartOffset();", isNull = FalseLiteral)
+    ev.copy(code = code"final ${CodeGenerator.javaType(dataType)} ${ev.value} = " +


cloud-fan · 2018-05-18T09:05:26Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/inputFileBlock.scala

    val className = InputFileBlockHolder.getClass.getName.stripSuffix("$")
-    ev.copy(code = s"final ${CodeGenerator.javaType(dataType)} ${ev.value} = " +
-      s"$className.getLength();", isNull = FalseLiteral)
+    ev.copy(code = code"final ${CodeGenerator.javaType(dataType)} ${ev.value} = " +


cloud-fan · 2018-05-18T09:07:48Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeBlockSuite.scala

+  }
+
+  test("Throws exception when interpolating unexcepted object in code block") {
+    val obj = TestClass(100)


we can simply use a Tuple2 here, like 1 -> 1

cloud-fan · 2018-05-18T09:10:44Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeBlockSuite.scala

+      case _: SimpleExprValue => aliasedParam
+      case other => other
+    }
+    val aliasedCode = CodeBlock(code.asInstanceOf[CodeBlock].codeParts, aliasedInputs).stripMargin


In the follow up, we may need a better API to do this. The current way is a little hacky.

+1 with @cloud-fan 's comment above. The current demonstrated way of manipulating code is less than ideal. But at least this PR does provide a good carrier for us to build the future code manipulation API so it's good enough for now.

Yes, I agreed. In the follow up, we definitely should have a better API for manipulating code. I think here it is used to show what we can do for CodeBlock right now.

Actually I just think about this and have initial idea. I'd prototype it locally and make follow up for your review after this.

cloud-fan · 2018-05-18T09:11:42Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ColumnarBatchScan.scala

    val str = s"columnVector[$columnVar, $ordinal, ${dataType.simpleString}]"
-    val code = s"${ctx.registerComment(str)}\n" + (if (nullable) {
-      s"""
+    val code = code"${ctx.registerComment(str)}\n" + (if (nullable) {


the \n is not needed now, since we always add \n between code blocks.

cloud-fan · 2018-05-18T09:12:20Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala

+
+case class Blocks(blocks: Seq[Block]) extends Block {
+  override lazy val exprValues: Set[ExprValue] = blocks.flatMap(_.exprValues).toSet
+  override def code: String = blocks.map(_.toString).mkString("\n")


shall we also make it lazy val?

I think one of the goals of this is letting us changing the string representation of an expression, so making it a lazy val may be fine for now, but would impede us from doing that. What do you think?

it depends on how we would implement code rewriting later. If we wanna keep JavaCode immutable, then lazy val is OK here, because once we rewrite the code, we will have a new Blocks.

Yeah, I think so. Ideally I think JavaCode should be immutable. Once we change code in it, we should have a new object instead of chancing current one.

cloud-fan · 2018-05-18T09:13:34Z

sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegenExec.scala

  protected def evaluateVariables(variables: Seq[ExprCode]): String = {
-    val evaluate = variables.filter(_.code != "").map(_.code.trim).mkString("\n")
-    variables.foreach(_.code = "")
+    val evaluate = variables.filter(_.code.toString != "").map(_.code.toString).mkString("\n")


nit: _.code.toString.nonEmpty

rednaxelafx

I love having more structure in the codegen framework. Thanks for tackling this, @viirya !

Left some nitpick comments inline. Overall I'd say this does a good job in laying down the foundations, but there's still a lot to be done for building up a nice API for the actual code manipulation.

@viirya do you have concrete follow-up ideas / tasks that would use this API? If so, it'd be nice to see how you're planning to use the API. I imagine the original intent of #19813 would be the main use case?

rednaxelafx · 2018-05-18T10:17:52Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala

+          case other => throw new IllegalArgumentException(
+            s"Can not interpolate ${other.getClass.getName} into code block.")
+        }
+        CodeBlock(sc.parts, args)


I'm wondering: does it make sense to eagerly fold the literal args into the parts, instead of carrying them over into CodeBlock? That's similar to what you're already doing with CodeBlock.exprValues, but just done a bit more eagerly.

Basically, the only things that are safe to manipulate are the structured ones like ExprValue and Block; none of the others are safe to change because we don't know what's in there. So no need to even carry them as separate argument into CodeBlock.

I think this is a good idea. We should not change literal args. Keeping them in block inputs seems meaningless.

rednaxelafx · 2018-05-18T10:35:15Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeBlockSuite.scala

+  }
+
+  test("replace expr values in code block") {
+    val statement = JavaCode.expression("1 + 1", IntegerType)


Nit: can we rename this statement variable to something like expr or someExpr or something? It's an expression not a statement.

rednaxelafx · 2018-05-18T10:36:49Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeBlockSuite.scala

+      case _: SimpleExprValue => aliasedParam
+      case other => other
+    }
+    val aliasedCode = CodeBlock(code.asInstanceOf[CodeBlock].codeParts, aliasedInputs).stripMargin


+1 with @cloud-fan 's comment above. The current demonstrated way of manipulating code is less than ideal. But at least this PR does provide a good carrier for us to build the future code manipulation API so it's good enough for now.

viirya · 2018-05-18T13:22:35Z

@rednaxelafx Thanks for your comments! Yes, #19813 is should be the first use case. However, I'd like to first have follow up for some points we discussed so far, e.g., preventing string interpolation, manipulating code API, etc.. So this API can be more easily to be used.

SparkQA · 2018-05-19T11:37:31Z

Test build #90833 has finished for PR 21193 at commit 2ca9741.

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

viirya · 2018-05-19T12:54:11Z

retest this please.

SparkQA · 2018-05-19T13:02:53Z

Test build #90835 has finished for PR 21193 at commit 2ca9741.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2018-05-20T12:21:14Z

retest this please.

SparkQA · 2018-05-20T16:15:15Z

Test build #90858 has finished for PR 21193 at commit 2ca9741.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2018-05-21T04:20:37Z

@cloud-fan @rednaxelafx Your last comments are addressed. Please check if you have other comments. Thanks for review.

cloud-fan · 2018-05-21T15:02:30Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala

+  // Folds eagerly the literal args into the code parts.
+  private def foldLiteralArgs(parts: Seq[String], args: Seq[Any]): (Seq[String], Seq[Any]) = {
+    val codeParts = ArrayBuffer.empty[String]
+    val blockInputs = ArrayBuffer.empty[Any]


shall we make the type JavaCode instead of Any?

Ok. JavaCode is better.

cloud-fan · 2018-05-21T15:03:49Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala

+    val inputs = args.iterator
+    val buf = new StringBuilder(Block.CODE_BLOCK_BUFFER_LENGTH)
+
+    buf append strings.next


can we use java style here? buf.append(strings.next).

rednaxelafx

Thanks for addressing my comments! LGTM now, no more comments from my side.
Just anxiously waiting to play with it more and also the polished code manipulation API ;-)

rednaxelafx · 2018-05-22T06:12:18Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeBlockSuite.scala

    assert(code.toString == "boolean expr1_isNull = false;")
  }

+  test("Literals are folded into string code parts instead of block inputs") {


Great, I like it!

rednaxelafx · 2018-05-22T06:13:34Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala

+    val buf = new StringBuilder(Block.CODE_BLOCK_BUFFER_LENGTH)
+
+    buf append strings.next
+    while (strings.hasNext) {


SparkQA · 2018-05-22T07:05:01Z

Test build #90931 has finished for PR 21193 at commit 96c594a.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class CodeBlock(codeParts: Seq[String], blockInputs: Seq[JavaCode]) extends Block

viirya · 2018-05-22T07:09:07Z

retest this please.

SparkQA · 2018-05-22T11:05:21Z

Test build #90941 has finished for PR 21193 at commit 96c594a.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class CodeBlock(codeParts: Seq[String], blockInputs: Seq[JavaCode]) extends Block

cloud-fan · 2018-05-22T12:49:28Z

LGTM, can we fix the conflict?

viirya · 2018-05-22T13:24:37Z

@cloud-fan Thanks. Fixed.

SparkQA · 2018-05-22T17:14:36Z

Test build #90964 has finished for PR 21193 at commit 00cc564.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class TaskKilled(
trait HasValidationIndicatorCol extends Params
trait DivModLike extends BinaryArithmetic
case class Divide(left: Expression, right: Expression) extends DivModLike
case class Remainder(left: Expression, right: Expression) extends DivModLike
case class MapEntries(child: Expression) extends UnaryExpression with ExpectsInputTypes
class ReadOnlySQLConf(context: TaskContext) extends SQLConf
class TaskContextConfigProvider(context: TaskContext) extends ConfigProvider
case class ContinuousShuffleReadPartition(index: Int, queueSize: Int) extends Partition
class ContinuousShuffleReadRDD(
trait ContinuousShuffleReader

cloud-fan · 2018-05-22T17:50:52Z

thanks, merging to master!

viirya · 2018-05-22T22:27:47Z

Thanks for your review @hvanhovell @cloud-fan @kiszk @maropu @mgaido91 @ueshin @rednaxelafx

Add API for handling expression code generation.

1df9943

viirya mentioned this pull request Apr 30, 2018

[SPARK-22600][SQL][WIP] Fix 64kb limit for deeply nested expressions under wholestage codegen #21140

Closed

viirya changed the title ~~[SPARK-24121][SQL] Add API for handling expression code generation~~ [SPARK-24121][SQL][WIP] Add API for handling expression code generation May 1, 2018

viirya force-pushed the SPARK-24121 branch from 369242a to 8e2db8b Compare May 1, 2018 21:39

Add new abstraction for expression codegen.

5fe425c

viirya force-pushed the SPARK-24121 branch from 8e2db8b to 5fe425c Compare May 2, 2018 14:39

mgaido91 reviewed May 3, 2018

View reviewed changes

Add basic tests.

00bef6b

viirya added 2 commits May 3, 2018 14:53

Merge remote-tracking branch 'upstream/master' into SPARK-24121

5d9c454

Deal merging conflict.

162deb2

viirya changed the title ~~[SPARK-24121][SQL][WIP] Add API for handling expression code generation~~ [SPARK-24121][SQL] Add API for handling expression code generation May 4, 2018

cloud-fan reviewed May 18, 2018

View reviewed changes

rednaxelafx reviewed May 18, 2018

View reviewed changes

Address comments.

2ca9741

Merge remote-tracking branch 'upstream/master' into SPARK-24121

d91f111

cloud-fan reviewed May 21, 2018

View reviewed changes

viirya added 2 commits May 22, 2018 03:11

Merge remote-tracking branch 'upstream/master' into SPARK-24121

4b49e8a

Use Java call style and use JavaCode instead of Any.

96c594a

rednaxelafx reviewed May 22, 2018

View reviewed changes

Merge remote-tracking branch 'upstream/master' into SPARK-24121

00cc564

asfgit closed this in f9f055a May 22, 2018

viirya mentioned this pull request Aug 8, 2018

[SPARK-24505][SQL] Forbidding string interpolation in CodeBlock #21520

Closed

mgaido91 mentioned this pull request Aug 15, 2018

[SPARK-24505][SQL] Convert strings in codegen to blocks: Cast and BoundAttribute #21537

Closed

kevinjmh mentioned this pull request Mar 5, 2019

[SPARK-21870][SQL] Split aggregation code into small functions #19082

Closed

viirya deleted the SPARK-24121 branch December 27, 2023 18:21

[SPARK-24121][SQL] Add API for handling expression code generation #21193

[SPARK-24121][SQL] Add API for handling expression code generation #21193

Uh oh!

Conversation

viirya commented Apr 30, 2018 • 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!

viirya commented Apr 30, 2018

Uh oh!

hvanhovell commented Apr 30, 2018

Uh oh!

viirya commented May 1, 2018

Uh oh!

viirya commented May 1, 2018

Uh oh!

viirya commented May 1, 2018

Uh oh!

SparkQA commented May 1, 2018

Uh oh!

SparkQA commented May 1, 2018

Uh oh!

SparkQA commented May 2, 2018

Uh oh!

viirya commented May 2, 2018

Uh oh!

SparkQA commented May 3, 2018

Uh oh!

maropu commented May 3, 2018

Uh oh!

SparkQA commented May 3, 2018

Uh oh!

viirya commented May 3, 2018

Uh oh!

SparkQA commented May 3, 2018

Uh oh!

mgaido91 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 3, 2018

Uh oh!

viirya commented May 3, 2018

Uh oh!

SparkQA commented May 3, 2018

Uh oh!

SparkQA commented May 3, 2018

Uh oh!

viirya commented May 4, 2018

Uh oh!

cloud-fan 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!

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

viirya commented Apr 30, 2018 •

edited

Loading