chore: drop the redundant width_bucket shim registrations and guard serde uniqueness

[dwsmith1983]

Which issue does this PR close?

Closes #4485.

Rationale for this change

width_bucket has gone through a registered serde since #4538, but the Spark 3.5 and 4.x expression shims still returned the same WidthBucket -> CometWidthBucket entry from their version-specific math map. It merged over the same key with the same value, so it changed nothing, but nothing would have said so if the two had ever drifted apart. The combined serde map is built by merging sixteen groups in order, so a class registered twice silently takes whichever serde merges last.

What changes are included in this PR?

• The two stale shim entries are removed; both version-specific math maps are empty now.
• The shared math, map, string and misc maps are hoisted out of their merge expressions into base*Expressions values, with the version shim still merged last, and the combined map is assembled from a named group list instead of a chain of ++. Same entries, same order.
• A SerdeRegistrationSuite asserts that every version shim registers only classes the shared map does not, that no class is registered in more than one group, and that every group entry reaches the combined map by identity with the sizes adding up. It is registered in both PR build workflows.

How are these changes tested?

The first test failed before the dedupe with exactly math: WidthBucket. Planting WidthBucket in the hash group fails the second test with WidthBucket: math, hash and the third on the size equality, so both guards are load-bearing; the plant is not part of the change. The width_bucket SQL fixture still runs natively on Spark 3.5 and 4.0, the suite passes on both, the 3.4 and 4.1 profiles compile, and spotless is clean.

[sunchao]

Reviewed 8311fcca against base 3810936b. No verified P1/P2 findings.

Correctness

The shared math registry already maps WidthBucket to the same CometWidthBucket singleton that the Spark 3.5 and 4.x shims redundantly supplied. Removing those shim entries preserves the effective handler; Spark 3.4 already uses an empty math shim, and Spark 4.0/4.1 inherit the shared 4.x registration. The 272 shared entries and the order of all 16 merged groups are unchanged.

CometWidthBucket still delegates to Spark's code-generation dispatcher, with the same expression enablement and fallback checks. The available Spark 3.5/4.0 implementations agree on null propagation, invalid bounds and bucket counts, ascending/descending boundary handling, and interval inputs. This PR does not change that evaluation path.

The new suite checks shared/shim key overlap before that merge, checks group ownership before constructing the final map, and checks both serde identity and total size afterward. I compiled and ran the exact suite against the extracted registry with synthetic expression/serde types on Scala 2.12.17, 2.13.16 and 2.13.17: all three tests passed for each selected shim configuration. Restoring the shim duplicate failed the overlap test; inserting the same handler into another group failed both ownership and size checks. These focused checks validate registration and Scala typing, not full Spark/native integration. The repository's workflow-suite check also passed.

At 2026-09-12 20:28:20 UTC, the head has only a successful label check; CI and the other three pull-request workflows report action_required. The author's SQL-test claims are not independently verified by a completed product CI run. The latest synthetic merge uses a newer base; all six authored files match the reviewed head, but no product execution is attributed to that merge. Maintained Spark 3.4/4.1 sources were unavailable for direct comparison.

Performance

The combined map remains an immutable singleton with the same lookup path for each expression. The named group list and its traversal add bounded initialization work; the duplicate scans run only in tests. Hoisting the four base maps retains their map roots for the singleton's lifetime, with immutable structure sharing. There is no new per-row work or repeated registry construction per query.

Removing the redundant shim math maps also removes their unnecessary entry allocation and overwrite. This is a maintenance change, and no measured query speedup is claimed. I found no performance change requiring a benchmark or blocking revision.

Design

Naming the groups makes registry composition inspectable while preserving existing merge precedence. Separating each shared map from its shim contribution is necessary for detecting the stale-registration problem: checking only the completed map would lose that evidence. The cross-group test likewise inspects individual groups, so even a duplicate that points to the same serde is detected.

The suite's three checks cover distinct failure modes: shim shadowing, cross-group duplication, and a lost or replaced entry in the combined registry. Both Linux and macOS workflows include it, and Linux selects the supported Spark profiles. Keeping this invariant in tests avoids adding a new production initialization failure mode.

Abstraction & complexity

The change adds four named base maps and one fixed sequence of existing groups. It keeps the existing map types, version-shim interface, serde implementations and dispatch API. The explicit existential type annotations remain in place, and the focused compilation exercised both Scala collection generations.

This is a small, direct representation of the registry rather than a new registration framework. The comments explain where version-specific classes belong and why overlap is disallowed. No additional abstraction or blocking simplification is needed.

[andygrove]

The premise holds up. WidthBucket is in Spark 3.4.3's catalyst and width_bucket is in its FunctionRegistry, so the shared mathExpressions entry has been the live registration on 3.4 ever since #4538, and dropping the 3.5 and 4.x shim entries is a no-op. I also pulled every classOf[...] per group and got 272 entries with 272 distinct classes and no cross-group duplicates, both at your base and at current main, so the new guard will not fire on a rebase.

What bothers me is that we have no CI evidence for the one profile the argument rests on. width_bucket.sql carries -- MinSparkVersion: 3.5, which was added back when the expression really was wired only through the shims, and CometWidthBucketSuite only exists under the spark-3.5 and spark-4.x test roots. So Spark 3.4 runs nothing for width_bucket at all. Could we drop the MinSparkVersion line as part of this PR? The codegen dispatcher is not version gated and CometCodegenSuite already runs on 3.4, so I would expect it to pass, and then the 3.4 job actually exercises the path this change is about.

The other thing is that this closes #4485, but docs/source/contributor-guide/expression-audits/math_funcs.md still says width_bucket is wired via per-version CometExprShim rather than a CometExpressionSerde and links that issue. After this PR that sentence is false and the link points at a closed issue. The same note says interval input types are not exercised by Comet tests, and width_bucket.sql only has a single double column, so that part is still true. Since closing the issue retires that item, could we fix the line here and either add interval cases or file a follow-up to link from it?

[andygrove]

Correction on my MinSparkVersion point above. That line is still on this branch but it is already gone from main, removed by #3910. So Spark 3.4 does run width_bucket.sql today and a rebase picks that up for free. Ignore that paragraph. CometWidthBucketSuite is still only under the spark-3.5 and spark-4.x test roots, but the SQL fixture is the coverage that matters here, so there is nothing for you to do.

The math_funcs.md point stands.

[rich7420]

@dwsmith1983 thanks for the patch