[CONNECT] Default refreshPhaseEnabled from spark.api.mode#57136
Open
longvu-db wants to merge 2 commits into
Open
[CONNECT] Default refreshPhaseEnabled from spark.api.mode#57136longvu-db wants to merge 2 commits into
longvu-db wants to merge 2 commits into
Conversation
Derive the default of `QueryExecution.refreshPhaseEnabled` from the session's `spark.api.mode` config instead of hardcoding `true`: the refresh phase stays on in Classic and is off in Connect. The `QueryExecution` refresh phase (`tableVersionsRefreshed`, backed by `V2TableRefreshUtil.refresh`) reloads every versioned DSv2 relation from the catalog at execution time to account for the delay between analysis and the subsequent phases. Spark Connect re-resolves and re-analyzes each plan on every request, so by the time a plan reaches execution the analyzed plan already references the latest table state, making the reload a redundant `catalog.loadTable` round-trip. Rather than threading `refreshPhaseEnabled = false` through every Connect-side `QueryExecution` / `Dataset.ofRows` / `executePlan` construction site, this makes the flag's default a function of `spark.api.mode` (keyed off `SparkSessionBuilder.API_MODE_KEY`). All the existing construction sites that do not pass the flag explicitly (including `SessionState.executePlan` via `createQueryExecution`, and `QueryExecution.create` / `runCommand`) then pick up the mode-appropriate default with no per-call-site changes. Co-authored-by: Isaac
…ctor body
A default argument expression cannot reference another parameter in the same
parameter list, so `refreshPhaseEnabled: Boolean = refreshPhaseEnabledDefault(
sparkSession)` failed to compile ("not found: value sparkSession").
Make the primary constructor take a private `refreshPhaseEnabledOpt: Option[Boolean]`
(default None) and resolve the public `refreshPhaseEnabled` field in the class body,
where `sparkSession` is in scope: honor an explicit value, otherwise fall back to the
mode-based default (enabled in Classic, disabled in Connect). QueryExecution.create and
runCommand keep a plain Boolean parameter and forward it via refreshPhaseEnabledOpt.
Co-authored-by: Isaac
dongjoon-hyun
requested changes
Jul 9, 2026
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
Member
There was a problem hiding this comment.
Please file a JIRA issue and use it, @longvu-db .
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Derive the default value of
QueryExecution.refreshPhaseEnabledfrom the session'sspark.api.modeconfiguration instead of hardcoding it totrue. The refresh phase stays enabled by default in Spark Classic and is disabled by default in Spark Connect.Concretely, a small helper
QueryExecution.refreshPhaseEnabledDefault(sparkSession)readsspark.api.mode(keyed offSparkSessionBuilder.API_MODE_KEY, case-insensitively) from the session'sSQLConfand returnsfalseonly when the mode isconnect. This helper becomes the default argument forrefreshPhaseEnabledat the three sites that previously defaulted totrue:QueryExecutionprimary constructor,QueryExecution.create,QueryExecution.runCommand.Because
SessionState.executePlanbuilds itsQueryExecutionviacreateQueryExecutionwithout passingrefreshPhaseEnabledexplicitly, andDataset.ofRowsgoes throughexecutePlan, every construction site that does not opt out now picks up the mode-appropriate default with no per-call-site changes.Why are the changes needed?
The
QueryExecutionrefresh phase (tableVersionsRefreshed, backed byV2TableRefreshUtil.refresh) reloads every versioned DSv2 relation from the catalog at execution time to account for the delay between analysis and the subsequent phases. Spark Connect re-resolves and re-analyzes each plan on every request, so by the time a plan reaches execution the analyzed plan already references the latest table state. The refresh then issues redundantcatalog.loadTablecalls for tables that were just resolved in the sameQueryExecution.Disabling the refresh phase for Connect avoids these extra catalog round-trips. Because analysis and execution happen together in Connect, the refresh is redundant there. The refresh phase is not what keeps stored-plan temp views fresh (that is the
V2TableReferenceanalyzer rule), so disabling it does not regress view or table freshness.Deriving the default from
spark.api.modekeeps the switch in one place rather than threadingrefreshPhaseEnabled = falsethrough every Connect-side construction site.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Added unit tests in
QueryExecutionSuite:refreshPhaseEnabled defaults to true in Classic and false in Connectverifies the default both through therefreshPhaseEnabledDefaulthelper and through an actualsessionState.executePlan(...), for the unset (Classic) default,spark.api.mode=connect, andspark.api.mode=classic.refreshPhaseEnabled default is derived from spark.api.mode case-insensitivelyverifiesCONNECTand" Connect "(mixed case and surrounding whitespace) resolve to the refresh-disabled default.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code