diff options
author | Dongjoon Hyun <dongjoon@apache.org> | 2016-03-30 16:15:37 -0700 |
---|---|---|
committer | Reynold Xin <rxin@databricks.com> | 2016-03-30 16:15:37 -0700 |
commit | 258a2434193aae62999102a8df73ca70bf0cb9f1 (patch) | |
tree | 5fd50f31ae026cf197393ab4a01653c7989e9ab1 | |
parent | dadf0138b3f6fd618677a2c26f40ab66b7a1139d (diff) | |
download | spark-258a2434193aae62999102a8df73ca70bf0cb9f1.tar.gz spark-258a2434193aae62999102a8df73ca70bf0cb9f1.tar.bz2 spark-258a2434193aae62999102a8df73ca70bf0cb9f1.zip |
[SPARK-14282][SQL] CodeFormatter should handle oneline comment with /* */ properly
## What changes were proposed in this pull request?
This PR improves `CodeFormatter` to fix the following malformed indentations.
```java
/* 019 */ public java.lang.Object apply(java.lang.Object _i) {
/* 020 */ InternalRow i = (InternalRow) _i;
/* 021 */ /* createexternalrow(if (isnull(input[0, double])) null else input[0, double], if (isnull(input[1, int])) null else input[1, int], ... */
/* 022 */ boolean isNull = false;
/* 023 */ final Object[] values = new Object[2];
/* 024 */ /* if (isnull(input[0, double])) null else input[0, double] */
/* 025 */ /* isnull(input[0, double]) */
...
/* 053 */ if (!false && false) {
/* 054 */ /* null */
/* 055 */ final int value9 = -1;
/* 056 */ isNull6 = true;
/* 057 */ value6 = value9;
/* 058 */ } else {
...
/* 077 */ return mutableRow;
/* 078 */ }
/* 079 */ }
/* 080 */
```
After this PR, the code will be formatted like the following.
```java
/* 019 */ public java.lang.Object apply(java.lang.Object _i) {
/* 020 */ InternalRow i = (InternalRow) _i;
/* 021 */ /* createexternalrow(if (isnull(input[0, double])) null else input[0, double], if (isnull(input[1, int])) null else input[1, int], ... */
/* 022 */ boolean isNull = false;
/* 023 */ final Object[] values = new Object[2];
/* 024 */ /* if (isnull(input[0, double])) null else input[0, double] */
/* 025 */ /* isnull(input[0, double]) */
...
/* 053 */ if (!false && false) {
/* 054 */ /* null */
/* 055 */ final int value9 = -1;
/* 056 */ isNull6 = true;
/* 057 */ value6 = value9;
/* 058 */ } else {
...
/* 077 */ return mutableRow;
/* 078 */ }
/* 079 */ }
/* 080 */
```
Also, this issue fixes the following too. (Similar with [SPARK-14185](https://issues.apache.org/jira/browse/SPARK-14185))
```java
16/03/30 12:39:24 DEBUG WholeStageCodegen: /* 001 */ public Object generate(Object[] references) {
/* 002 */ return new GeneratedIterator(references);
/* 003 */ }
```
```java
16/03/30 12:46:32 DEBUG WholeStageCodegen:
/* 001 */ public Object generate(Object[] references) {
/* 002 */ return new GeneratedIterator(references);
/* 003 */ }
```
## How was this patch tested?
Pass the Jenkins tests (including new CodeFormatterSuite testcases.)
Author: Dongjoon Hyun <dongjoon@apache.org>
Closes #12072 from dongjoon-hyun/SPARK-14282.
3 files changed, 17 insertions, 2 deletions
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeFormatter.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeFormatter.scala index 8e40754dc3..ab4831f7ab 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeFormatter.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeFormatter.scala @@ -74,7 +74,8 @@ private class CodeFormatter { // Handle single line comments newIndentLevel = indentLevel } - } else { + } + if (inCommentBlock) { if (line.endsWith("*/")) { inCommentBlock = false newIndentLevel = indentLevelOutsideCommentBlock diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeFormatterSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeFormatterSuite.scala index d7836aa3b2..f57b82bb96 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeFormatterSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeFormatterSuite.scala @@ -115,6 +115,20 @@ class CodeFormatterSuite extends SparkFunSuite { """.stripMargin } + testCase("single line comments /* */ ") { + """/** This is a comment about class A { { { ( ( */ + |class A { + |class body; + |}""".stripMargin + }{ + """ + |/* 001 */ /** This is a comment about class A { { { ( ( */ + |/* 002 */ class A { + |/* 003 */ class body; + |/* 004 */ } + """.stripMargin + } + testCase("multi-line comments") { """ /* This is a comment about |class A { diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegen.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegen.scala index da3ee46b7d..6a779abd40 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegen.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/WholeStageCodegen.scala @@ -337,7 +337,7 @@ case class WholeStageCodegen(child: SparkPlan) extends UnaryNode with CodegenSup // try to compile, helpful for debug val cleanedSource = CodeFormatter.stripExtraNewLines(source) - logDebug(s"${CodeFormatter.format(cleanedSource)}") + logDebug(s"\n${CodeFormatter.format(cleanedSource)}") CodeGenerator.compile(cleanedSource) (ctx, cleanedSource) } |