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.

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-37786][SQL] StreamingQueryListener support use SQLConf.get to get corresponding SessionState's SQLConf #35092

Closed
wants to merge 8 commits into from

Conversation

AngersZhuuuu
Copy link
Contributor

What changes were proposed in this pull request?

Current StreamingQueryListener only support add constructor parameter of SparkConf, but if we start a SparkContext first, many SQL conf won't be applied to SparkContext's SparkConf.
Also it's a SQL component, related configuration we defined it as SQLConf. But it's hard to use SQLConf in this plugin listener.

This patch, we use pass SQLConf to StreamingQueryManager and use SQLConf.withExistingConf to wrap the passed SQLConf. Then we can use SQLConf.get to get the same SQLConf of SessionState when initializing StreamingQueryListener

Why are the changes needed?

Pass corresponding SessionState's SQLConf to StreamingQueryListener. Then user can use SQLConf.get to use the SQLConf.

Does this PR introduce any user-facing change?

User can use StreamingQueryListener with corresponding SessionState's SQLConf. For example

private class SQLConfStreamingQueryListener extends StreamingQueryListener {
  val sqlConf = SQLConf.get
  override def onSuccess(funcName: String, qe: QueryExecution, durationNs: Long): Unit = {
  }
  override def onFailure(funcName: String, qe: QueryExecution, exception: Exception): Unit = {
  }
}

How was this patch tested?

Added UT

AngersZhuuuu and others added 2 commits January 4, 2022 17:17
…ngQueryListenersConfSuite.scala

Co-authored-by: Hyukjin Kwon <gurwls223@gmail.com>
…/StreamingQueryListenersConfSuite.scala"

This reverts commit 5ba1290.
@AngersZhuuuu
Copy link
Contributor Author

GA passed, can merge this.

@HyukjinKwon
Copy link
Member

Merged to master.

@AngersZhuuuu
Copy link
Contributor Author

@HyukjinKwon Should we add a migration guide to mention this in the migration guide? If need I will raise a follow up for this two prs.

@HyukjinKwon
Copy link
Member

what do we need for migration?

@AngersZhuuuu
Copy link
Contributor Author

AngersZhuuuu commented Jan 7, 2022

what do we need for migration?

Right, it's not a migration. But we should make user know this change. Maybe we can mention this in their configuration doc? Such as

spark.sql.queryExecutionListeners

@AngersZhuuuu
Copy link
Contributor Author

what do we need for migration?

See #35122

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants