sebastianhillig-db opened a new pull request, #59320:
URL: https://github.com/apache/spark/pull/59320
### What changes were proposed in this pull request?
This is a follow-up to SPARK-59997 (#59254). That change stopped
`Analyzer.retainResolutionConfigsForAnalysis` from scanning every registered
config entry. The method still runs once per view and SQL function body conf
build, though, and each run copies every explicitly set session config and
looks up the binding policy of each key.
This PR computes the retained configs once per existing conf and memoizes
them on that conf:
- `SQLConf.settings` is now a `ModificationCountingMap` instead of a
`Collections.synchronizedMap`. As before, every map method synchronizes on the
map itself. In addition:
- it counts every mutating call;
- `snapshotWithModificationCount` returns a copy of the entries together
with the count they correspond to;
- the key, value, and entry views are read-only, so no mutation can bypass
the count.
- `object SQLConf` keeps a version of the registered SQL config entries,
which every `register` and `unregister` advances. This covers configs from
lazily loaded modules (e.g. sql/hive) that register after a memo was computed.
- `SQLConf.retainedResolutionConfigs` returns the retained subset of the
settings: `spark.sql.catalog.*` keys and configs bound `SESSION` or
`NOT_APPLICABLE`, the same rule as SPARK-59997.
- It reuses the memo while both the settings count and the entries version
are unchanged.
- Nested view and function bodies install their own conf, so they get
their own memo.
- `ReadOnlySQLConf`, whose settings are task local properties, keeps the
unmemoized path.
- `retainResolutionConfigsForAnalysis` now overlays the memoized map with
`putAll` and then calls `trySetAnsiValue`, in the same order and with the same
precedence as before.
- A new internal, SESSION-bound conf,
`spark.sql.analyzer.memoizeRetainedResolutionConfigs.enabled` (default `true`),
turns the memo off and restores the per-build recomputation.
### Why are the changes needed?
One query can rebuild many body confs from the same session conf. Every
reference to a view or SQL function, including nested ones, builds a new conf
and overlays the session's retained configs onto it. Before this PR, each build
iterates over all explicitly set session configs and looks up the binding
policy of each. With the memo, a build copies the already filtered map, and the
filter runs again only after the session conf or the set of registered configs
changes. Queries that reference many views or SQL functions benefit the most.
### Does this PR introduce _any_ user-facing change?
No. The resulting view and SQL function body confs are unchanged.
Two internal details of `SQLConf.settings`, which is `protected[spark]`, do
change:
- Its key, value, and entry views are now read-only, so mutating the
settings through them throws `UnsupportedOperationException`. No code in this
repository does that.
- The views no longer lock on each call. As with iteration, callers must
hold the map's monitor while using them.
The new conf turns off only the memo, not these changes.
### How was this patch tested?
New tests:
- `ModificationCountingMapSuite`:
- the count advances on every mutating call, including the `java.util.Map`
default methods and the mutators of the `asScala` wrapper;
- the snapshot pairs a copy of the entries with their count;
- the views are read-only.
- `RetainedResolutionConfigsSuite`:
- view and SQL function body confs are identical with and without the
memo, across captured ANSI values, creation versions, and `alwaysSetAnsiValue`;
- the memo is recomputed after each kind of settings change, and after a
config is registered or unregistered;
- a nested conf gets its own memo and leaves the outer one untouched;
- turning the conf off recomputes on every call;
- `ReadOnlySQLConf` filters without memoizing.
- `AnalysisConfOverrideSuite`: a temporary view, a persistent view, and a
SQL function each follow
`spark.sql.analyzer.lowerEmptyGroupingSetToGlobalAggregate.enabled`, a
SESSION-bound config, when it changes with `SET` between queries.
I also checked that the tests catch a stale memo: with the memo's settings
check forced to always match, this test and the recompute test in
`RetainedResolutionConfigsSuite` both fail.
Existing suites, run locally:
- catalyst `*AnalysisSuite`;
- `AnalysisConfOverrideSuite`, `SimpleSQLViewSuite`,
`LocalTempViewTestSuite`, `GlobalTempViewTestSuite`, `PersistedViewTestSuite`,
`SQLFunctionSuite`, `SQLConfSuite`, `ExecutorSideSQLConfSuite`;
- `SQLQueryTestSuite -z grouping_set`;
- `SparkConfigBindingPolicySuite`.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.5)
This pull request and its description were written by Isaac.
--
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]
