Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,9 @@ private[hive] class SparkSQLCLIDriver extends CliDriver with Logging {
}

// Adapted splitSemiColon from Hive 2.3's CliDriver.splitSemiColon.
// Note: [SPARK-31595] if there is a `'` in a double quoted string, or a `"` in a single quoted
// string, the origin implementation from Hive will not drop the trailing semicolon as expected,
// hence we refined this function a little bit.
private def splitSemiColon(line: String): JList[String] = {
var insideSingleQuote = false
var insideDoubleQuote = false
Expand All @@ -519,13 +522,15 @@ private[hive] class SparkSQLCLIDriver extends CliDriver with Logging {
for (index <- 0 until line.length) {
if (line.charAt(index) == '\'' && !insideComment) {
// take a look to see if it is escaped
Copy link
Contributor

Choose a reason for hiding this comment

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

@adrian-wang Should we update the comment to reflect the newly added condition?

Copy link
Member

Choose a reason for hiding this comment

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

Yep, maybe we can rephrase this comment here.

if (!escape) {
// See the comment above about SPARK-31595
if (!escape && !insideDoubleQuote) {
// flip the boolean variable
insideSingleQuote = !insideSingleQuote
}
} else if (line.charAt(index) == '\"' && !insideComment) {
// take a look to see if it is escaped
Copy link
Contributor

Choose a reason for hiding this comment

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

@adrian-wang Same.

if (!escape) {
// See the comment above about SPARK-31595
if (!escape && !insideSingleQuote) {
// flip the boolean variable
insideDoubleQuote = !insideDoubleQuote
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -500,4 +500,13 @@ class CliSuite extends SparkFunSuite with BeforeAndAfterAll with BeforeAndAfterE
| ;""".stripMargin -> "testcomment"
)
}

test("SPARK-31595 Should allow unescaped quote mark in quoted string") {
runCliWithin(1.minute)(
Copy link
Member

Choose a reason for hiding this comment

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

Could you explicitly set false at spark.sql.parser.escapedStringLiterals for the tests below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this has nothing to do with spark.sql.parser.escapedStringLiterals, even if the config is set to true, the parser should accept this string.

Copy link
Member

@maropu maropu May 2, 2020

Choose a reason for hiding this comment

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

Ur, I got it. I think the option is misleading... Could you remove it from the PR descritpion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done that, thanks!

"SELECT '\"legal string a';select 1 + 234;".stripMargin -> "235"
)
runCliWithin(1.minute)(
"SELECT \"legal 'string b\";select 22222 + 1;".stripMargin -> "22223"
)
}
}