-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-13432][SQL] add the source file name and line into a generated Java code #11301
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
Changes from all commits
3f99740
ec0c1a9
899bc64
bf72c13
27b13d2
09245eb
9731e4e
305852a
10de448
e68f551
458db22
dfbe2df
7b606ab
d9536d4
9d70b36
05332ed
330521c
607e678
f0b9587
0bf3586
c08d839
7c68e07
0091eb5
bacfcc6
4f8772b
ccdf12d
7279489
a96bc48
597b732
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,7 +98,7 @@ abstract class AbstractSqlParser extends ParserInterface with Logging { | |
| case e: ParseException => | ||
|
||
| throw e.withCommand(command) | ||
| case e: AnalysisException => | ||
| val position = Origin(e.line, e.startPosition) | ||
| val position = Origin(None, e.line, e.startPosition) | ||
| throw new ParseException(Option(command), e.message, position, position) | ||
| } | ||
| } | ||
|
|
@@ -150,7 +150,7 @@ case object ParseErrorListener extends BaseErrorListener { | |
| charPositionInLine: Int, | ||
| msg: String, | ||
| e: RecognitionException): Unit = { | ||
| val position = Origin(Some(line), Some(charPositionInLine)) | ||
| val position = Origin(None, Some(line), Some(charPositionInLine)) | ||
| throw new ParseException(None, msg, position, position) | ||
| } | ||
| } | ||
|
|
@@ -176,7 +176,7 @@ class ParseException( | |
| val builder = new StringBuilder | ||
| builder ++= "\n" ++= message | ||
| start match { | ||
| case Origin(Some(l), Some(p)) => | ||
| case Origin(_, Some(l), Some(p)) => | ||
| builder ++= s"(line $l, pos $p)\n" | ||
| command.foreach { cmd => | ||
| val (above, below) = cmd.split("\n").splitAt(l) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,7 @@ import org.apache.spark.util.Utils | |
| private class MutableInt(var i: Int) | ||
|
|
||
| case class Origin( | ||
| var callSite: Option[String] = None, | ||
| line: Option[Int] = None, | ||
| startPosition: Option[Int] = None) | ||
|
|
||
|
|
@@ -58,15 +59,15 @@ object CurrentOrigin { | |
|
|
||
| def reset(): Unit = value.set(Origin()) | ||
|
|
||
| def setPosition(line: Int, start: Int): Unit = { | ||
| def setPosition(callSite: String, line: Int, start: Int): Unit = { | ||
| value.set( | ||
| value.get.copy(line = Some(line), startPosition = Some(start))) | ||
| value.get.copy(callSite = Some(callSite), line = Some(line), startPosition = Some(start))) | ||
| } | ||
|
|
||
| def withOrigin[A](o: Origin)(f: => A): A = { | ||
| val current = get | ||
| set(o) | ||
| val ret = try f finally { reset() } | ||
| reset() | ||
| val ret = try f finally { set(current) } | ||
|
||
| ret | ||
| } | ||
| } | ||
|
|
@@ -442,6 +443,13 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]] extends Product { | |
|
|
||
| override def toString: String = treeString | ||
|
|
||
| def toOriginString: String = | ||
| if (this.origin.callSite.isDefined) { | ||
| this.toString + " @ " + this.origin.callSite.get | ||
| } else { | ||
| this.toString | ||
| } | ||
|
|
||
| /** Returns a string representation of the nodes in this tree */ | ||
| def treeString: String = generateTreeString(0, Nil, new StringBuilder).toString | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,10 +26,12 @@ import org.apache.spark.sql.catalyst.encoders.{encoderFor, ExpressionEncoder} | |
| import org.apache.spark.sql.catalyst.expressions._ | ||
| import org.apache.spark.sql.catalyst.expressions.aggregate.AggregateExpression | ||
| import org.apache.spark.sql.catalyst.parser.CatalystSqlParser | ||
| import org.apache.spark.sql.catalyst.trees.{CurrentOrigin, Origin} | ||
| import org.apache.spark.sql.catalyst.util.usePrettyExpression | ||
| import org.apache.spark.sql.execution.aggregate.TypedAggregateExpression | ||
| import org.apache.spark.sql.functions.lit | ||
| import org.apache.spark.sql.types._ | ||
| import org.apache.spark.util.Utils.getCallSite | ||
|
|
||
| private[sql] object Column { | ||
|
|
||
|
|
@@ -46,6 +48,16 @@ private[sql] object Column { | |
| case expr => usePrettyExpression(expr).sql | ||
| } | ||
| } | ||
|
|
||
| @scala.annotation.varargs | ||
| def updateExpressionsOrigin(cols: Column*): Unit = { | ||
| // Update Expression.origin using the callSite of an operation | ||
| val callSite = org.apache.spark.util.Utils.getCallSite().shortForm | ||
| cols.map(col => col.expr.foreach(e => e.origin.callSite = Some(callSite))) | ||
| // Update CurrentOrigin for setting origin for LogicalPlan node | ||
| CurrentOrigin.set( | ||
| Origin(Some(callSite), CurrentOrigin.get.line, CurrentOrigin.get.startPosition)) | ||
|
||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes,
constraintsinQueryPlanisExpressionSetandSparkPlanwhich is a subclass ofQueryPlanis serializable soExpressionSetshould be also serializable strictly. Butconstraintsis lazy val and it's not accessed when the receiver object is a instance ofSparkPlan. In other word,constraintsis accessed only when the receiver object is a instance ofLogicalPlan.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your explanation on how
constraintsis accessed. From the implementation view, my understanding is that it is still necessary to declareSerializableforExpressionSet. Is there another good idea to enableSerializableonly forLogicalPlan?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
ExpressionSetis really serialized only in the case ofLogicalPlan, we could moveconstraintsfromQueryPlantoLogicalPlanbut I'm not sure it's correct way.Have you ever got any problem because
ExpressionSetis notSerializable?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I got an exception regarding non-serializable in test suites in
hivewhenExpressionSetis notSerializable. This is why I addedSerialiabletoExpressionSetThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
O.K. I got it.