longvu-db opened a new pull request, #57136: URL: https://github.com/apache/spark/pull/57136
### What changes were proposed in this pull request? Derive the default value of `QueryExecution.refreshPhaseEnabled` from the session's `spark.api.mode` configuration instead of hardcoding it to `true`. 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)` reads `spark.api.mode` (keyed off `SparkSessionBuilder.API_MODE_KEY`, case-insensitively) from the session's `SQLConf` and returns `false` only when the mode is `connect`. This helper becomes the default argument for `refreshPhaseEnabled` at the three sites that previously defaulted to `true`: - the `QueryExecution` primary constructor, - `QueryExecution.create`, - `QueryExecution.runCommand`. Because `SessionState.executePlan` builds its `QueryExecution` via `createQueryExecution` without passing `refreshPhaseEnabled` explicitly, and `Dataset.ofRows` goes through `executePlan`, 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 `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. The refresh then issues redundant `catalog.loadTable` calls for tables that were just resolved in the same `QueryExecution`. 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 `V2TableReference` analyzer rule), so disabling it does not regress view or table freshness. Deriving the default from `spark.api.mode` keeps the switch in one place rather than threading `refreshPhaseEnabled = false` through 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 Connect` verifies the default both through the `refreshPhaseEnabledDefault` helper and through an actual `sessionState.executePlan(...)`, for the unset (Classic) default, `spark.api.mode=connect`, and `spark.api.mode=classic`. - `refreshPhaseEnabled default is derived from spark.api.mode case-insensitively` verifies `CONNECT` and `" 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 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
