Repository: spark
Updated Branches:
  refs/heads/branch-1.5 7460e4309 -> 4f07a590c


[SPARK-11352][SQL][BRANCH-1.5] Escape */ in the generated comments.

https://issues.apache.org/jira/browse/SPARK-11352

This one backports https://github.com/apache/spark/pull/10072 to branch 1.5.

Author: Yin Huai <yh...@databricks.com>

Closes #10084 from yhuai/SPARK-11352-branch-1.5.


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/4f07a590
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/4f07a590
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/4f07a590

Branch: refs/heads/branch-1.5
Commit: 4f07a590c5d7b9e187f446c077357b00df93ee27
Parents: 7460e43
Author: Yin Huai <yh...@databricks.com>
Authored: Tue Dec 1 20:33:50 2015 -0800
Committer: Yin Huai <yh...@databricks.com>
Committed: Tue Dec 1 20:33:50 2015 -0800

----------------------------------------------------------------------
 .../apache/spark/sql/catalyst/expressions/Expression.scala  | 8 +++++++-
 .../sql/catalyst/expressions/codegen/CodegenFallback.scala  | 2 +-
 .../sql/catalyst/expressions/CodeGenerationSuite.scala      | 9 +++++++++
 3 files changed, 17 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/spark/blob/4f07a590/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
----------------------------------------------------------------------
diff --git 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
index 0b98f55..4ef3b93 100644
--- 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
+++ 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
@@ -97,7 +97,7 @@ abstract class Expression extends TreeNode[Expression] {
     val ve = GeneratedExpressionCode("", isNull, primitive)
     ve.code = genCode(ctx, ve)
     // Add `this` in the comment.
-    ve.copy(s"/* $this */\n" + ve.code)
+    ve.copy(s"/* ${this.toCommentSafeString} */\n" + ve.code)
   }
 
   /**
@@ -175,6 +175,12 @@ abstract class Expression extends TreeNode[Expression] {
   }
 
   override def toString: String = prettyName + children.mkString("(", ",", ")")
+
+  /**
+   * Returns the string representation of this expression that is safe to be 
put in
+   * code comments of generated code.
+   */
+  protected def toCommentSafeString: String = this.toString.replace("*/", 
"\\*\\/")
 }
 
 

http://git-wip-us.apache.org/repos/asf/spark/blob/4f07a590/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodegenFallback.scala
----------------------------------------------------------------------
diff --git 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodegenFallback.scala
 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodegenFallback.scala
index 3492d2c..5061451 100644
--- 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodegenFallback.scala
+++ 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodegenFallback.scala
@@ -33,7 +33,7 @@ trait CodegenFallback extends Expression {
     ctx.references += this
     val objectTerm = ctx.freshName("obj")
     s"""
-      /* expression: ${this} */
+      /* expression: ${this.toCommentSafeString} */
       Object $objectTerm = expressions[${ctx.references.size - 1}].eval(i);
       boolean ${ev.isNull} = $objectTerm == null;
       ${ctx.javaType(this.dataType)} ${ev.primitive} = 
${ctx.defaultValue(this.dataType)};

http://git-wip-us.apache.org/repos/asf/spark/blob/4f07a590/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala
----------------------------------------------------------------------
diff --git 
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala
 
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala
index e323467..fcb0c84 100644
--- 
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala
+++ 
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CodeGenerationSuite.scala
@@ -134,4 +134,13 @@ class CodeGenerationSuite extends SparkFunSuite with 
ExpressionEvalHelper {
     unsafeRow.getStruct(3, 1).getStruct(0, 2).setInt(1, 4)
     assert(internalRow === internalRow2)
   }
+
+  test("*/ in the data") {
+    // When */ appears in a comment block (i.e. in /**/), code gen will break.
+    // So, in Expression and CodegenFallback, we escape */ to \*\/.
+    checkEvaluation(
+      EqualTo(BoundReference(0, StringType, false), Literal.create("*/", 
StringType)),
+      true,
+      InternalRow(UTF8String.fromString("*/")))
+  }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org

Reply via email to