davidm-db opened a new pull request, #56566: URL: https://github.com/apache/spark/pull/56566
### What changes were proposed in this pull request? Follow-up to SPARK-57372 (#56439). That PR removed `spark.sql.types.framework.enabled` and made the Types Framework the unconditional integration path for `TimeType`, so `TypeApiOps.apply` / `TypeOps.apply` / `ConnectTypeOps` now always return `Some(...)` for `TimeType`. As a result, the legacy `TimeType` arms that remained in the `...Default` fallbacks behind those dispatches (`Ops(dt).map(_.foo).getOrElse(fooDefault(dt))`) are unreachable for `TimeType`. This removes that dead code throughout the framework's integration points -- physical type, row accessors, literals, encoders, type conversion, the interpreted cast-to-string path, Row JSON, Hive result, Python conversion, Arrow (both directions), the Spark Connect proto/Arrow/literal converters, and the Thrift type-id mapping -- along with the imports and flag-era comments that only those arms needed. Arms that are still reachable are deliberately left in place: - the cast-to-string **codegen** path (`ToStringBase.castToStringCode`) is a direct match, not a framework `getOrElse`, so its `TimeType` branch stays; - `ArrowWriter.TimeWriter` remains -- it is what `TimeTypeOps.createArrowFieldWriter` instantiates; - `CatalystTypeConverters.TimeConverter` remains -- it is used by the value-keyed `convertToCatalyst`; - the class/encoder reverse-lookups and all nanosecond-timestamp arms are untouched. ### Why are the changes needed? These `TimeType` branches can no longer execute after the flag removal, so they are dead code: redundant with the framework `Ops` implementations and a hazard for future readers who might assume they are still reached. Removing them collapses each integration point to the single framework path the flag removal established. ### Does this PR introduce _any_ user-facing change? No. This is a dead-code removal; the framework path produces results identical to the removed (unreachable) arms, and `spark.sql.timeType.enabled` and the other feature gates are unaffected. ### How was this patch tested? Pure dead-code removal, verified behavior-preserving by existing suites that already run with the framework unconditionally enabled. The following all pass: `TimeTypeOpsSuite`, `TimeExpressionsSuite`, `TimeFormatterSuite`, `RowJsonSuite`, `CatalystTypeConvertersSuite`, `RowEncoderSuite`, `ExpressionEncoderSuite`, `LiteralExpressionSuite`, `ArrowUtilsSuite`, `ArrowWriterSuite`, `HiveResultSuite`, `SparkExecuteStatementOperationSuite`, and the Spark Connect `ArrowEncoderSuite`. The `time.sql` and `cast.sql` golden files are unchanged, confirming TIME cast-to-string output is byte-identical. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Opus 4.8 (claude-opus-4-8) 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]
