Skip to content

[SPARK-29530][SQL] Make SQLConf in SQL parse process thread safe #26187

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
AngersZhuuuu wants to merge 2 commits into from
Closed

[SPARK-29530][SQL] Make SQLConf in SQL parse process thread safe #26187

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f456207
Update SparkSession.scala
AngersZhuuuu Oct 21, 2019
ad9dba8
follow comment
AngersZhuuuu Oct 21, 2019
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
1 change: 1 addition & 0 deletions sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,7 @@ class SparkSession private(
* @since 2.0.0
*/
def sql(sqlText: String): DataFrame = {
SparkSession.setActiveSession(this)
Copy link
Member

@zsxwing zsxwing Oct 21, 2019

Choose a reason for hiding this comment

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

Who takes responsibility to set this back if we set the active session in this method? For example, the current active session is spark1 when calling spark2.sql(...), then the active session will become spark2. When we will set it back to spark1, or it will always be spark2?

Copy link
Contributor Author

@AngersZhuuuu AngersZhuuuu Oct 21, 2019

Choose a reason for hiding this comment

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

Who takes responsibility to set this back if we set the active session in this method? For example, the current active session is spark1 when calling spark2.sql(...), then the active session will become spark2. When we will set it back to spark1, or it will always be spark2?

Under normal circumstances， our spark program use one SparkSession, if some one write program use multi sparksession, they should control these things.

And for analyzer, it will set this.

spark/sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala

Line 64 in e99a9f7

SparkSession.setActiveSession(sparkSession)

And only
SparkSession.cleanupAnyExistingSession() will call

  SparkSession.clearActiveSession()
      SparkSession.clearDefaultSession()

val tracker = new QueryPlanningTracker
val plan = tracker.measurePhase(QueryPlanningTracker.PARSING) {
sessionState.sqlParser.parsePlan(sqlText)
Copy link
Contributor

@cloud-fan cloud-fan Oct 21, 2019

Choose a reason for hiding this comment

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

I know that we use SQLConf.get a lot in analyzer/optimizer rules, but does it also true for the parser?

Copy link
Contributor Author

@AngersZhuuuu AngersZhuuuu Oct 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I know that we use SQLConf.get a lot in analyzer/optimizer rules, but does it also true for the parser?

Parser use SQLConf.get too.
It truly make some configuration about parser mess in SparkThriftServer multi thread mode.

spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/ParseDriver.scala

Line 94 in e99a9f7

lexer.legacy_setops_precedence_enbled = SQLConf.get.setOpsPrecedenceEnforced

spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/ParseDriver.scala

Line 95 in e99a9f7

lexer.ansi = SQLConf.get.ansiEnabled

It is all about error happened in #26172

Copy link
Contributor

@cloud-fan cloud-fan Oct 21, 2019

Choose a reason for hiding this comment

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

shall we define a conf: SQLConf method in ParserDriver so that we can avoid calling SQLConf.get in the parser?

Copy link
Contributor Author

@AngersZhuuuu AngersZhuuuu Oct 21, 2019

Choose a reason for hiding this comment

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

shall we define a conf: SQLConf method in ParserDriver so that we can avoid calling SQLConf.get in the parser?

How about new change? Test is ok for #26172 . also cc @juliuszsompolski

Expand Down