Skip to content

[SPARK-9393] [SQL] Fix several error-handling bugs in ScriptTransform operator #7710

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.

Sign up for GitHub

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
JoshRosen wants to merge 12 commits into from
Closed

[SPARK-9393] [SQL] Fix several error-handling bugs in ScriptTransform operator #7710

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
fa18d26
Add basic unit test for script transform with 'cat' command.
JoshRosen Jul 27, 2015
b43e4ec
Clean up nullability in ScriptTransformation
JoshRosen Jul 27, 2015
bd4c948
Skip launching of external command for empty partitions.
JoshRosen Jul 27, 2015
4ee36a2
Kill script transform subprocess when error occurs in input writer.
JoshRosen Jul 27, 2015
8b162b6
Add failing test which demonstrates exception masking issue
JoshRosen Jul 28, 2015
88278de
Split ScriptTransformation writer thread into own class.
JoshRosen Jul 28, 2015
b31258d
Rename iterator variables to disambiguate.
JoshRosen Jul 28, 2015
323bb2b
Fix error-swallowing bug
JoshRosen Jul 28, 2015
494cde0
Propagate TaskContext to writer thread
JoshRosen Jul 28, 2015
6a06a8c
Clean up handling of quotes in serde class name
JoshRosen Jul 28, 2015
983f200
Use unescapeSQLString instead of stripQuotes
JoshRosen Jul 28, 2015
16c44e2
Update some comments
JoshRosen Jul 28, 2015
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
Prev Previous commit
Next Next commit
Use unescapeSQLString instead of stripQuotes
  • Loading branch information
@JoshRosen
JoshRosen committed Jul 28, 2015
commit 983f200b73b9c04432c3cf086586e5bcaef0658f
4 changes: 2 additions & 2 deletions sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveQl.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,7 @@ https://cwiki.apache.org/confluence/display/Hive/Enhanced+Aggregation%2C+Cube%2C
(rowFormat, None, Nil)
Copy link
Contributor Author

@JoshRosen JoshRosen Jul 28, 2015

Choose a reason for hiding this comment

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

I thought it was confusing to use an empty string to represent a missing value, hence this change.

Copy link
Contributor

@davies davies Jul 28, 2015

Choose a reason for hiding this comment

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

Could we use null here without changing the type?

Copy link
Contributor Author

@JoshRosen JoshRosen Jul 28, 2015

Choose a reason for hiding this comment

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

We could but I feel that's a bit less clear and more error-prone.

Copy link
Contributor

@marmbrus marmbrus Jul 28, 2015

Choose a reason for hiding this comment

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

+1 to this change.


case Token("TOK_SERDENAME", Token(serdeClass, Nil) :: Nil) :: Nil =>
(Nil, Some(PlanUtils.stripQuotes(serdeClass)), Nil)
(Nil, Some(BaseSemanticAnalyzer.unescapeSQLString(serdeClass)), Nil)

case Token("TOK_SERDENAME", Token(serdeClass, Nil) ::
Token("TOK_TABLEPROPERTIES",
Expand All @@ -891,7 +891,7 @@ https://cwiki.apache.org/confluence/display/Hive/Enhanced+Aggregation%2C+Cube%2C
case Token("TOK_TABLEPROPERTY", Token(name, Nil) :: Token(value, Nil) :: Nil) =>
(name, value)
}
(Nil, Some(PlanUtils.stripQuotes(serdeClass)), serdeProps)
(Nil, Some(BaseSemanticAnalyzer.unescapeSQLString(serdeClass)), serdeProps)

case Nil => (Nil, None, Nil)
}
Expand Down