Conversation
added 2 commits
September 18, 2026 22:13
…r nanosecond-precision timestamps
CatalogColumnStat.toExternalString/fromExternalString only handled
TimestampType/TimestampNTZType, so ANALYZE TABLE ... FOR COLUMNS on a
TIMESTAMP_LTZ(p)/TIMESTAMP_NTZ(p) column threw
columnStatisticsSerializationNotSupportedError. Add nanosecond-aware
formatter cases mirroring the existing microsecond path, reusing the
already-shipped TimestampFormatter.{formatNanos,parseNanos,
formatWithoutTimeZoneNanos,parseWithoutTimeZoneNanos} API.
Unblocking stats collection surfaced two real crashes in code that
receives those stats and had never seen a nanos timestamp before:
- EstimationUtils.toDouble/fromDouble had no case for the nanos types
(or, pre-existing, for TimestampNTZType), so JoinEstimation,
FilterEstimation.evaluateEquality/evaluateBinaryForTwoColumns, and
ValueInterval all threw MatchError once CBO stats existed for such a
column. FilterEstimation.evaluateBinary/evaluateInSet have their own
independent type dispatch and needed the same types added directly.
- CommandUtils.supportsHistogram used a broad `_: DatetimeType` match
that already covered the nanos types, so ANALYZE with histograms
enabled tried to run ApproximatePercentile/
ApproxCountDistinctForIntervals on a TimestampNanosVal and failed
their type checks. Excluded nanos types from histogram collection
instead of teaching percentile/histogram math a new composite value
type; basic min/max/ndv stats are unaffected.
Also fixes DESCRIBE TABLE EXTENDED showing nanosecond LTZ column
stats in raw UTC instead of the session time zone, unlike its
microsecond sibling.
Tests: new CatalogColumnStatSuite (formatter round-trip/truncation),
and new StatisticsCollectionSuite cases for the DESC round-trip, the
histogram skip, and CBO estimation over nanosecond predicates
(join key, equality, IN-list, two-column, and range comparisons).
…imation, extend UNION support A second-pass review of the previous commit found that EstimationUtils .toDouble's nanosecond-timestamp case projected TimestampNanosVal down to epochMicros only, silently dropping the nanosWithinMicro remainder. Two distinct nanosecond values sharing an epochMicros would collapse to the same Double, corrupting CBO selectivity/min-max estimation (wrong evaluateBinaryForNumeric range checks, wrong evaluateInSet maxBy/minBy tie-breaks, wrong ValueInterval.intersect bounds) without crashing. Encode nanosWithinMicro as a fractional component instead (and decode it back with floor-based, sign-correct reconstruction in fromDouble) so distinct nanosecond values compare and round-trip correctly. Also: - UnionEstimation.isTypeSupported never gained the nanos types, so UNION ALL silently dropped min/max for them -- not a crash, but bad enough estimation input to make a downstream join look empty. PhysicalTimestampLTZNanosType/PhysicalTimestampNTZNanosType already define a full-precision Ordering[TimestampNanosVal], so this needed no lossy Double conversion, just widening the type match. - JoinEstimation.computeByHistogram bypassed EstimationUtils.toDouble with its own value.toString.toDouble, which isn't valid for TimestampNanosVal; switched it to the shared conversion. Tests: new EstimationUtilsSuite covering the toDouble/fromDouble precision fix (including pre-1970 dates), a new StatisticsCollection Suite case for the UNION fix, and extended the existing CBO test to also exercise TIMESTAMP_NTZ range/IN-list predicates (previously only equality was covered, leaving the incidental TimestampNTZType widening in evaluateBinary/evaluateInSet from the prior commit untested for those shapes).
Author
|
Picked some tasks that dont have any PR from https://issues.apache.org/jira/browse/SPARK-56822 and working on them. Thanks Uros for sharing the uber jira tickets. |
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
1 participant
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?
This PR extends
CatalogColumnStat.toExternalString/fromExternalStringto supportTimestampLTZNanosType/TimestampNTZNanosType(nanosecond-precisionTIMESTAMP_LTZ(p)/TIMESTAMP_NTZ(p)columns), reusing the existingTimestampFormatter.{formatNanos, parseNanos, formatWithoutTimeZoneNanos, parseWithoutTimeZoneNanos}API added for the nanosecond-timestamp SPIP (SPARK-56822).DESCRIBE TABLE EXTENDEDis updated to render nanosecond LTZ column stats in the session time zone, matching the existing microsecond behavior.Making these stats collectible immediately exposes them to CBO code paths (
EstimationUtils,FilterEstimation,JoinEstimation,UnionEstimation,CommandUtils.supportsHistogram) that previously never saw a nanosecond-timestamp value and would crash (MatchError) or silently drop stats onceANALYZEcould produce them. This PR adds the minimal handling needed so those paths don't crash, then (self-review follow-up) fixes a precision bug in that handling: the initialtoDoubleconversion projected a nanosecond value down to its epoch-microseconds component only, which could make two distinct nanosecond values collapse to the sameDoubleand corrupt CBO min/max/selectivity estimation. The follow-up encodes the sub-microsecond remainder as a fractional component instead, reconstructing it losslessly infromDouble, and extendsUnionEstimation.isTypeSupportedandJoinEstimation.computeByHistogramto the same types. Histogram collection is explicitly excluded for these types rather than taught a new composite value type; basic min/max/ndv stats are unaffected.This also touches SPARK-57839 (CBO filter/selectivity estimation for nanosecond timestamps) and SPARK-57805 (CBO
MatchErrorforTimestampNTZ/interval/TIMEcolumns) -- the crash-prevention and precision work here was a prerequisite for SPARK-57812 to be safely mergeable, but does not claim to fully resolve either of those broader tickets.Why are the changes needed?
Before this change,
ANALYZE TABLE ... COMPUTE STATISTICS FOR COLUMNSon aTIMESTAMP_LTZ(p)/TIMESTAMP_NTZ(p)column threwcolumnStatisticsSerializationNotSupportedError, so these newer nanosecond-precision types (SPARK-56822) couldn't get column statistics at all.Does this PR introduce any user-facing change?
Yes.
ANALYZE TABLE ... FOR COLUMNSandDESCRIBE TABLE EXTENDEDnow work forTIMESTAMP_LTZ(p)/TIMESTAMP_NTZ(p)columns instead of throwingcolumnStatisticsSerializationNotSupportedError.spark.sql.cbo.enabled=truethat filter, join, orANALYZEon such columns no longer throwMatchError, and estimate cardinality using full nanosecond precision rather than crashing or silently dropping min/max onUNION ALL.ANALYZE ... FOR COLUMNSwith histograms enabled skips histogram collection for these columns (min/max/ndv/null-count stats still collected) instead of failing.How was this patch tested?
Added
CatalogColumnStatSuite(new),EstimationUtilsSuite(new), and new cases inStatisticsCollectionSuitecovering DESC round-trip, histogram-skip, CBO estimation over nanosecond predicates (LTZ and NTZ), and UNION ALL min/max propagation.Ran locally with JDK 17 (build/sbt):
catalyst/testOnly CatalogColumnStatSuite EstimationUtilsSuite FilterEstimationSuite JoinEstimationSuite UnionEstimationSuite-- 117/117 passed.sql/testOnly StatisticsCollectionSuite CommandUtilsSuite-- 47/47 passed.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code claude-sonnet-5