-
Notifications
You must be signed in to change notification settings - Fork 28.3k
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
[SQL] SPARK-6981: Factor out SparkPlanner and QueryExecution from SQLContext #6122
Conversation
Conflicts: sql/core/src/main/scala/org/apache/spark/sql/SQLContext.scala
* access to the intermediate phases of query execution for developers. | ||
*/ | ||
@DeveloperApi | ||
protected[sql] class QueryExecution(val sqlContext: SQLContext, val logical: LogicalPlan) { |
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.
Shouldn't this be private[sql] now?
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.
It is protected[sql]
in master.
BTW, this PR is geared towards making it easier for third parties (and HiveContext) to add new processing rules without requiring to subclass (see PR #5556 and SPARK-6320)
I would actually advise making these classes public (or at least protected
, without the [sql]
qualifier)
Conflicts: sql/core/src/main/scala/org/apache/spark/sql/SQLContext.scala
…refactoring Conflicts: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
If everybody agrees, I think we can restart the Jenkins build (this is just the same as the other PR, after all) |
Jenkins, test this please. |
Test build #33237 has finished for PR 6122 at commit
|
Build fails because of this:
this is obvious, since that's the purpose of the PR. How should we proceed? |
Possible solution: add shims to preserve binary compatibility as follows: class SQLContext {
...
@deprecated
class SparkPlanner extends org.apache.spark.sql.SparkPlanner
@deprecated
class QueryExecution extends org.apache.spark.sql.QueryExecution
@deprecated
lazy val prepareForExecution = ...
...
}
class QueryExecution(sqlContext: SQLContext) {
...
lazy val prepareForExecution = sqlContext.prepareForExecution
...
} etc. However, I wouldn't do that unless it is really necessary, because it really makes extending the non-deprecated classes a bit awkward (must be careful with imports) |
…refactoring Conflicts: sql/core/src/main/scala/org/apache/spark/sql/SQLContext.scala
addressed in PR #6356 |
Can one of the admins verify this patch? |
…LContext Alternative to PR #6122; in this case the refactored out classes are replaced by inner classes with the same name for backwards binary compatibility * process in a lighter-weight, backwards-compatible way Author: Edoardo Vacchi <uncommonnonsense@gmail.com> Closes #6356 from evacchi/sqlctx-refactoring-lite.
Cleaned-up version of PR #5556