Skip to content
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

[SPARK-17751] [SQL] Remove spark.sql.eagerAnalysis and Output the Plan if Existed in AnalysisException #15316

Closed
wants to merge 8 commits into from
Closed

[SPARK-17751] [SQL] Remove spark.sql.eagerAnalysis and Output the Plan if Existed in AnalysisException #15316

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c71a03e
clean
gatorsmile Sep 30, 2016
ca45217
fix msg
gatorsmile Oct 1, 2016
8fb4f40
fix msg
gatorsmile Oct 1, 2016
ab893a6
clean
gatorsmile Oct 1, 2016
39dc9ce
Merge remote-tracking branch 'upstream/master' into eagerAnalysis
gatorsmile Oct 10, 2016
2c5de3c
add comments
gatorsmile Oct 10, 2016
6f51363
Merge remote-tracking branch 'upstream/master' into eagerAnalysis
gatorsmile Oct 17, 2016
f082643
fix
gatorsmile Oct 17, 2016
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
5 changes: 5 additions & 0 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/AnalysisException.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ class AnalysisException protected[sql] (
}

override def getMessage: String = {
val planAnnotation = plan.map(p => s";\n$p").getOrElse("")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need a separate method here?

Copy link
Member Author

Choose a reason for hiding this comment

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

When hitting an AnalysisException, we called getMessage in SQLQueryTestSuite.scala. That means, we also output the logical plan in the .sql.out when hitting an AnalysisException. The logical plan contains expression IDs. Thus, when we doing the text compare between the query results with the contents in .sql.out, it might fail due to the mismatch of expression IDs. Thus, I added an extra function here. Maybe I need to add a comment to say this is for testing only.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that makes sense.

getSimpleMessage + planAnnotation
}

def getSimpleMessage: String = {
val lineAnnotation = line.map(l => s" line $l").getOrElse("")
val positionAnnotation = startPosition.map(p => s" pos $p").getOrElse("")
s"$message;$lineAnnotation$positionAnnotation"
Expand Down
9 changes: 0 additions & 9 deletions sql/core/src/main/scala/org/apache/spark/sql/execution/debug/package.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,6 @@ package object debug {
output
}

/**
* Augments [[SparkSession]] with debug methods.
*/
implicit class DebugSQLContext(sparkSession: SparkSession) {
def debug(): Unit = {
sparkSession.conf.set(SQLConf.DATAFRAME_EAGER_ANALYSIS.key, false)
}
}

/**
* Augments [[Dataset]]s with debug methods.
*/
Expand Down
10 changes: 0 additions & 10 deletions sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,14 +393,6 @@ object SQLConf {
.intConf
.createWithDefault(32)

// Whether to perform eager analysis when constructing a dataframe.
// Set to false when debugging requires the ability to look at invalid query plans.
val DATAFRAME_EAGER_ANALYSIS = SQLConfigBuilder("spark.sql.eagerAnalysis")
.internal()
.doc("When true, eagerly applies query analysis on DataFrame operations.")
.booleanConf
.createWithDefault(true)

// Whether to automatically resolve ambiguity in join conditions for self-joins.
// See SPARK-6231.
val DATAFRAME_SELF_JOIN_AUTO_RESOLVE_AMBIGUITY =
Expand Down Expand Up @@ -733,8 +725,6 @@ private[sql] class SQLConf extends Serializable with CatalystConf with Logging {
// See the comments of SCHEMA_STRING_LENGTH_THRESHOLD above for more information.
def schemaStringLengthThreshold: Int = getConf(SCHEMA_STRING_LENGTH_THRESHOLD)

def dataFrameEagerAnalysis: Boolean = getConf(DATAFRAME_EAGER_ANALYSIS)

def dataFrameSelfJoinAutoResolveAmbiguity: Boolean =
getConf(DATAFRAME_SELF_JOIN_AUTO_RESOLVE_AMBIGUITY)

Expand Down
3 changes: 3 additions & 0 deletions sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ class SQLQueryTestSuite extends QueryTest with SharedSQLContext {
if (isSorted(df.queryExecution.analyzed)) (schema, answer) else (schema, answer.sorted)

} catch {
case a: AnalysisException if a.plan.nonEmpty =>
// Do not output the logical plan tree which contains expression IDs.
(StructType(Seq.empty), Seq(a.getClass.getName, a.getSimpleMessage))
case NonFatal(e) =>
// If there is an exception, put the exception class followed by the message.
(StructType(Seq.empty), Seq(e.getClass.getName, e.getMessage))
Expand Down