[GitHub] spark pull request #21193: [SPARK-24121][SQL][WIP] Add API for handling expr...
Github user viirya commented on a diff in the pull request: https://github.com/apache/spark/pull/21193#discussion_r185794875 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala --- @@ -112,6 +112,102 @@ object JavaCode { def isNullExpression(code: String): SimpleExprValue = { expression(code, BooleanType) } + + def block(code: String): Block = { +CodeBlock(codeParts = Seq(code), exprValues = Seq.empty) + } +} + +/** + * A block of java code which involves some expressions represented by `ExprValue`. + */ +trait Block extends JavaCode { + def exprValues: Seq[Any] + + // This will be called during string interpolation. + override def toString: String = _marginChar match { +case Some(c) => code.stripMargin(c) +case _ => code + } + + var _marginChar: Option[Char] = None + + def stripMargin(c: Char): this.type = { +_marginChar = Some(c) +this + } + + def stripMargin: this.type = { +_marginChar = Some('|') +this + } + + def + (other: Block): Block +} + +object Block { + implicit def blockToString(block: Block): String = block.toString + + implicit def blocksToBlock(blocks: Seq[Block]): Block = Blocks(blocks) + + implicit class BlockHelper(val sc: StringContext) extends AnyVal { +def code(args: Any*): Block = { --- End diff -- Ok. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21193: [SPARK-24121][SQL][WIP] Add API for handling expr...
Github user viirya commented on a diff in the pull request: https://github.com/apache/spark/pull/21193#discussion_r185794699 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala --- @@ -120,13 +216,15 @@ object JavaCode { trait ExprValue extends JavaCode { def javaType: Class[_] def isPrimitive: Boolean = javaType.isPrimitive + + // This will be called during string interpolation. + override def toString: String = ExprValue.exprValueToString(this) } object ExprValue { - implicit def exprValueToString(exprValue: ExprValue): String = exprValue.toString + implicit def exprValueToString(exprValue: ExprValue): String = exprValue.code --- End diff -- Because `toString` will call `exprValueToString`. We should have only one place to convert an `ExprValue` to string. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21193: [SPARK-24121][SQL][WIP] Add API for handling expr...
Github user mgaido91 commented on a diff in the pull request: https://github.com/apache/spark/pull/21193#discussion_r185761453 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala --- @@ -120,13 +216,15 @@ object JavaCode { trait ExprValue extends JavaCode { def javaType: Class[_] def isPrimitive: Boolean = javaType.isPrimitive + + // This will be called during string interpolation. + override def toString: String = ExprValue.exprValueToString(this) } object ExprValue { - implicit def exprValueToString(exprValue: ExprValue): String = exprValue.toString + implicit def exprValueToString(exprValue: ExprValue): String = exprValue.code --- End diff -- why can't we use `toString`? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21193: [SPARK-24121][SQL][WIP] Add API for handling expr...
Github user mgaido91 commented on a diff in the pull request: https://github.com/apache/spark/pull/21193#discussion_r185761604 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala --- @@ -112,6 +112,102 @@ object JavaCode { def isNullExpression(code: String): SimpleExprValue = { expression(code, BooleanType) } + + def block(code: String): Block = { +CodeBlock(codeParts = Seq(code), exprValues = Seq.empty) + } +} + +/** + * A block of java code which involves some expressions represented by `ExprValue`. + */ +trait Block extends JavaCode { + def exprValues: Seq[Any] + + // This will be called during string interpolation. + override def toString: String = _marginChar match { +case Some(c) => code.stripMargin(c) +case _ => code + } + + var _marginChar: Option[Char] = None + + def stripMargin(c: Char): this.type = { +_marginChar = Some(c) +this + } + + def stripMargin: this.type = { +_marginChar = Some('|') +this + } + + def + (other: Block): Block +} + +object Block { + implicit def blockToString(block: Block): String = block.toString + + implicit def blocksToBlock(blocks: Seq[Block]): Block = Blocks(blocks) + + implicit class BlockHelper(val sc: StringContext) extends AnyVal { +def code(args: Any*): Block = { + if (sc.parts.length == 0) { +EmptyBlock + } else { +args.foreach { + case _: ExprValue => true --- End diff -- why `true`? Can't we just do nothing after the `=>`? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21193: [SPARK-24121][SQL][WIP] Add API for handling expr...
Github user mgaido91 commented on a diff in the pull request: https://github.com/apache/spark/pull/21193#discussion_r185763081 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala --- @@ -112,6 +112,102 @@ object JavaCode { def isNullExpression(code: String): SimpleExprValue = { expression(code, BooleanType) } + + def block(code: String): Block = { +CodeBlock(codeParts = Seq(code), exprValues = Seq.empty) + } +} + +/** + * A block of java code which involves some expressions represented by `ExprValue`. + */ +trait Block extends JavaCode { + def exprValues: Seq[Any] + + // This will be called during string interpolation. + override def toString: String = _marginChar match { +case Some(c) => code.stripMargin(c) +case _ => code + } + + var _marginChar: Option[Char] = None + + def stripMargin(c: Char): this.type = { +_marginChar = Some(c) +this + } + + def stripMargin: this.type = { +_marginChar = Some('|') +this + } + + def + (other: Block): Block +} + +object Block { + implicit def blockToString(block: Block): String = block.toString + + implicit def blocksToBlock(blocks: Seq[Block]): Block = Blocks(blocks) + + implicit class BlockHelper(val sc: StringContext) extends AnyVal { +def code(args: Any*): Block = { + if (sc.parts.length == 0) { +EmptyBlock + } else { +args.foreach { + case _: ExprValue => true + case _: Int | _: Long | _: Float | _: Double | _: String => true + case _: Block => true + case other => throw new IllegalArgumentException( +s"Can not interpolate ${other.getClass} into code block.") +} +CodeBlock(sc.parts, args) + } +} + } +} + +/** + * A block of java code. + */ +case class CodeBlock(codeParts: Seq[String], exprValues: Seq[Any]) extends Block { + override def code: String = { +val strings = codeParts.iterator +val expressions = exprValues.iterator +var buf = new StringBuffer(strings.next) +while (strings.hasNext) { + if (expressions.hasNext) { --- End diff -- this is not needed (especially if we add the `checkLengths`), because if it is not true we are in a bad situation --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21193: [SPARK-24121][SQL][WIP] Add API for handling expr...
Github user mgaido91 commented on a diff in the pull request: https://github.com/apache/spark/pull/21193#discussion_r185762835 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/javaCode.scala --- @@ -112,6 +112,102 @@ object JavaCode { def isNullExpression(code: String): SimpleExprValue = { expression(code, BooleanType) } + + def block(code: String): Block = { +CodeBlock(codeParts = Seq(code), exprValues = Seq.empty) + } +} + +/** + * A block of java code which involves some expressions represented by `ExprValue`. + */ +trait Block extends JavaCode { + def exprValues: Seq[Any] + + // This will be called during string interpolation. + override def toString: String = _marginChar match { +case Some(c) => code.stripMargin(c) +case _ => code + } + + var _marginChar: Option[Char] = None + + def stripMargin(c: Char): this.type = { +_marginChar = Some(c) +this + } + + def stripMargin: this.type = { +_marginChar = Some('|') +this + } + + def + (other: Block): Block +} + +object Block { + implicit def blockToString(block: Block): String = block.toString + + implicit def blocksToBlock(blocks: Seq[Block]): Block = Blocks(blocks) + + implicit class BlockHelper(val sc: StringContext) extends AnyVal { +def code(args: Any*): Block = { --- End diff -- chat about adding `checkLengths(args)`? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org