-
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
[SPARK-19157][SQL] should be able to change spark.sql.runSQLOnFiles at runtime #16531
Conversation
cc @gatorsmile |
@@ -45,7 +45,7 @@ import org.apache.spark.unsafe.types.UTF8String | |||
* Replaces generic operations with specific variants that are designed to work with Spark | |||
* SQL Data Sources. | |||
*/ | |||
case class DataSourceAnalysis(conf: CatalystConf) extends Rule[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.
I looked at other places in the diff and not clear why you changed this to NOT be a case class.
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.
The same question. What is the reason why we made this change?
Test build #71136 has finished for PR 16531 at commit
|
@@ -116,8 +116,8 @@ private[sql] class SessionState(sparkSession: SparkSession) { | |||
AnalyzeCreateTable(sparkSession) :: | |||
PreprocessTableInsertion(conf) :: | |||
new FindDataSourceTable(sparkSession) :: | |||
DataSourceAnalysis(conf) :: | |||
(if (conf.runSQLonFile) new ResolveDataSource(sparkSession) :: Nil else Nil) |
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.
+1
This was the root cause why we were unable to change the conf at runtime.
LGTM except the question |
963b66f
to
20b2d95
Compare
LGTM pending test |
Test build #71174 has finished for PR 16531 at commit
|
Thanks! Merging to master. |
…t runtime ## What changes were proposed in this pull request? The analyzer rule that supports to query files directly will be added to `Analyzer.extendedResolutionRules` when SparkSession is created, according to the `spark.sql.runSQLOnFiles` flag. If the flag is off when we create `SparkSession`, this rule is not added and we can not query files directly even we turn on the flag later. This PR fixes this bug by always adding that rule to `Analyzer.extendedResolutionRules`. ## How was this patch tested? new regression test Author: Wenchen Fan <wenchen@databricks.com> Closes apache#16531 from cloud-fan/sql-on-files.
…t runtime ## What changes were proposed in this pull request? The analyzer rule that supports to query files directly will be added to `Analyzer.extendedResolutionRules` when SparkSession is created, according to the `spark.sql.runSQLOnFiles` flag. If the flag is off when we create `SparkSession`, this rule is not added and we can not query files directly even we turn on the flag later. This PR fixes this bug by always adding that rule to `Analyzer.extendedResolutionRules`. ## How was this patch tested? new regression test Author: Wenchen Fan <wenchen@databricks.com> Closes apache#16531 from cloud-fan/sql-on-files.
What changes were proposed in this pull request?
The analyzer rule that supports to query files directly will be added to
Analyzer.extendedResolutionRules
when SparkSession is created, according to thespark.sql.runSQLOnFiles
flag. If the flag is off when we createSparkSession
, this rule is not added and we can not query files directly even we turn on the flag later.This PR fixes this bug by always adding that rule to
Analyzer.extendedResolutionRules
.How was this patch tested?
new regression test