[SPARK-55798][SQL] Propagate config data type into config builders#54579
Closed
dtenedor wants to merge 1 commit intoapache:masterfrom
Closed
[SPARK-55798][SQL] Propagate config data type into config builders#54579dtenedor wants to merge 1 commit intoapache:masterfrom
dtenedor wants to merge 1 commit intoapache:masterfrom
Conversation
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
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?
Add a
ConfigEntryTypesealed-trait enum toConfigEntry[T], threaded fromConfigBuilderthroughTypedConfigBuilderand allcreate*methods, so that config entries are tagged with their declared type at construction time without runtime type probing or exception handling.Specifically:
ConfigEntryTypesealed trait (ConfigEntry.scala) with case objectsBooleanEntry,IntEntry,LongEntry,DoubleEntry,StringEntry,EnumEntry,TimeEntry,BytesEntry,RegexEntry, andOtherEntry.ConfigEntry[T]gains a requiredval configEntryType: ConfigEntryTypeconstructor parameter, propagated through all five subclasses (ConfigEntryWithDefault,ConfigEntryWithDefaultFunction,ConfigEntryWithDefaultString,OptionalConfigEntry,FallbackConfigEntry).TypedConfigBuilder[T]gains a requiredval configEntryType: ConfigEntryTypeconstructor parameter, propagated throughtransform,toSequence, and allcreate*methods (createWithDefault,createWithDefaultFunction,createWithDefaultString,createOptional).ConfigBuilder.*Conffactory method (intConf,longConf,doubleConf,booleanConf,stringConf,enumConf,timeConf,bytesConf,regexConf) passes the appropriate enum variant.fallbackConfinherits the variant from the fallback entry.configEntryTypeis a required (non-default) constructor parameter on bothConfigEntryandTypedConfigBuilder, so the compiler forces every new construction site to explicitly specify the type—preventing silent omission.Using an enum instead of a single
isBooleanEntry: Booleanflag makes the design extensible: callers can match on the specific config type (e.g. to optimize access paths differently for boolean vs. numeric entries) without adding new boolean fields for each type.Why are the changes needed?
Pattern matching on config values at runtime (e.g.
case b: Boolean => ...) or usingisInstanceOf[Boolean]type tests causes JVMclass_checkdeoptimizations at megamorphic call sites. By tagging each config entry with its declared type at construction time, hot-path config access code can use a simple field check instead, avoiding these deoptimizations entirely.Does this PR introduce any user-facing change?
No.
ConfigEntryTypeandconfigEntryTypeareprivate[spark]; no public API is affected.How was this patch tested?
New unit test suite
RecordConfigAccessSuite(core/src/test/scala/org/apache/spark/RecordConfigAccessSuite.scala) with 19 tests covering:configEntryTypeassignment for builtin entries of every type (boolean, int, long, double, string, bytes, time).fallbackConfinheritance ofconfigEntryTypefrom the fallback entry.configEntryTypethrough allcreate*variants (createWithDefault,createWithDefaultString,createWithDefaultFunction,createOptional).transform,checkValue, andtoSequence.ConfigBuilder.*Confmethod confirming the correct enum variant.BooleanEntry.Run with:
Was this patch authored or co-authored using generative AI tooling?
Yes,
claude-4.6-opus-high