Skip to content

[SPARK-11352] [SQL] Escape */ in the generated comments. #10072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[SPARK-11352] [SQL] Escape */ in the generated comments. #10072

wants to merge 1 commit into from

Conversation

yhuai
Copy link
Contributor

@yhuai yhuai commented Dec 1, 2015

@@ -95,7 +95,7 @@ abstract class Expression extends TreeNode[Expression] {
ctx.subExprEliminationExprs.get(this).map { subExprState =>
// This expression is repeated meaning the code to evaluated has already been added
// as a function and called in advance. Just use it.
val code = s"/* $this */"
val code = s"/* ${this.toString.replace("*/", "\\*\\/")} */"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we put this as a utility in ctx?

Maybe. def escapeComment(< thing to put inside /* */>)

This seems easy to get wrong in the future too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. Let me do that.

@SparkQA
Copy link

SparkQA commented Dec 1, 2015

Test build #46981 has finished for PR 10072 at commit 16a6bbe.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * * For example, we build an encoder forcase class Data(a: Int, b: String)and the real type\n * case class UpCast(child: Expression, dataType: DataType, walkedTypePath: Seq[String])\n * case class InitializeJavaBean(beanInstance: Expression, setters: Map[String, Expression])\n

@rxin
Copy link
Contributor

rxin commented Dec 1, 2015

We should backport this into 1.5 as well.

@yhuai
Copy link
Contributor Author

yhuai commented Dec 1, 2015

sure

@nongli
Copy link
Contributor

nongli commented Dec 1, 2015

LGTM

@SparkQA
Copy link

SparkQA commented Dec 2, 2015

Test build #46997 has finished for PR 10072 at commit 1ee6378.

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

@yhuai
Copy link
Contributor Author

yhuai commented Dec 2, 2015

Merging to master, branch 1.6, and branch 1.5.

asfgit pushed a commit that referenced this pull request Dec 2, 2015
https://issues.apache.org/jira/browse/SPARK-11352

Author: Yin Huai <yhuai@databricks.com>

Closes #10072 from yhuai/SPARK-11352.

(cherry picked from commit 5872a9d)
Signed-off-by: Yin Huai <yhuai@databricks.com>
@yhuai
Copy link
Contributor Author

yhuai commented Dec 2, 2015

I will have a pr for branch 1.5. The current on conflicts a lot with branch 1.5.

@asfgit asfgit closed this in 5872a9d Dec 2, 2015
asfgit pushed a commit that referenced this pull request Dec 2, 2015
https://issues.apache.org/jira/browse/SPARK-11352

This one backports #10072 to branch 1.5.

Author: Yin Huai <yhuai@databricks.com>

Closes #10084 from yhuai/SPARK-11352-branch-1.5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants