Skip to content

[SPARK-57812][SQL] Support catalog column-statistics serialization for nanosecond-precision timestamps - #58946

Open
vrjdev wants to merge 3 commits into
apache:masterfrom
vrjdev:SPARK-57812
Open

vrjdev wants to merge 3 commits into
apache:masterfrom
vrjdev:SPARK-57812

Conversation

@vrjdev

@vrjdev vrjdev commented Sep 21, 2026 •

Copy link
Copy Markdown

What changes were proposed in this pull request?

This PR extends CatalogColumnStat.toExternalString/fromExternalString to support TimestampLTZNanosType/TimestampNTZNanosType (nanosecond-precision TIMESTAMP_LTZ(p)/TIMESTAMP_NTZ(p) columns), reusing the existing TimestampFormatter.{formatNanos, parseNanos, formatWithoutTimeZoneNanos, parseWithoutTimeZoneNanos} API added for the nanosecond-timestamp SPIP (SPARK-56822). DESCRIBE TABLE EXTENDED is updated to render nanosecond LTZ column stats in the session time zone, matching the existing microsecond behavior. The catalog string form keeps all nine fractional digits in both directions -- this part is exact at every precision.

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) once ANALYZE could produce them. This PR adds the minimal handling needed so those paths don't crash: EstimationUtils.toDouble/fromDouble convert a nanosecond value through its epochMicros component, the same way the existing TimestampType/TimestampNTZType case already does, giving CBO estimation for these types the same microsecond resolution (and the same |epochMicros| <= 2^53 exactness domain) as those types. Histogram collection is explicitly excluded for these types rather than taught a new composite value type; basic min/max/ndv stats are unaffected. UnionEstimation.isTypeSupported is extended to the same types, comparing values directly through their own ordering rather than through toDouble, so UNION ALL keeps full nanosecond precision for min/max propagation. JoinEstimation.computeByHistogram is switched to the shared toDouble conversion instead of its own value.toString.toDouble.

Edit: an earlier revision of this PR attempted full nanosecond precision through toDouble/fromDouble via a fractional encoding. @tdcmeehan's review showed that encoding only works within about 102 days of the epoch (epochMicros < 2^43), because a Double's 52-bit mantissa can't hold a 1/1000 fraction at realistic magnitudes -- at a 2022 timestamp only 4 of 1000 sub-microsecond values remain distinguishable, and round-tripping could land in the wrong microsecond entirely. True nanosecond-resolution CBO estimation isn't achievable through a Double at these magnitudes, so this PR now documents and tests the honest microsecond-resolution behavior instead, and leaves full nanosecond-resolution CBO estimation to SPARK-57839.

This also touches SPARK-57839 (CBO filter/selectivity estimation for nanosecond timestamps) and SPARK-57805 (CBO MatchError for TimestampNTZ/interval/TIME columns) -- the crash-prevention 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 COLUMNS on a TIMESTAMP_LTZ(p)/TIMESTAMP_NTZ(p) column threw columnStatisticsSerializationNotSupportedError, 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 COLUMNS and DESCRIBE TABLE EXTENDED now work for TIMESTAMP_LTZ(p)/TIMESTAMP_NTZ(p) columns instead of throwing columnStatisticsSerializationNotSupportedError.
  • Queries with spark.sql.cbo.enabled=true that filter, join, or ANALYZE on such columns no longer throw MatchError, and estimate cardinality at microsecond resolution instead of crashing; UNION ALL propagates min/max at full nanosecond precision instead of silently dropping them.
  • ANALYZE ... FOR COLUMNS with 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 in StatisticsCollectionSuite covering DESC round-trip, histogram-skip, CBO estimation over nanosecond predicates (LTZ and NTZ), and UNION ALL min/max propagation. EstimationUtilsSuite covers the microsecond-resolution round-trip and collision behavior at both epoch-adjacent and realistic (2022-scale) magnitudes, and monotonicity across microseconds.

Ran locally with JDK 17 (build/sbt):

  • catalyst/testOnly CatalogColumnStatSuite EstimationUtilsSuite FilterEstimationSuite JoinEstimationSuite UnionEstimationSuite -- 119/119 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

Rajesh Vakkalagadda 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).
@vrjdev

vrjdev commented Sep 21, 2026

Copy link
Copy Markdown
Author

cc : @MaxGekk / @uros-b PTAL.

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.

@tdcmeehan tdcmeehan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review summary

The serialization side of this change is sound and worth landing, but the cost-based-estimation goal is not achievable in the representation it uses. EstimationUtils.toDouble must return a single Double, and a Double holds nanosecond resolution for only about 104 days past the epoch, so the fractional encoding delivers full precision for no timestamp anyone will store and about four distinct sub-microsecond values per microsecond at present-day magnitudes. Three statements in the change therefore describe behaviour the code cannot provide -- the comment at the encoding, the description's lossless-reconstruction and full-nanosecond-precision claims, and the name of the new test -- and the tests were placed where the difference is invisible. The decision this needs is which resolution nanosecond estimation should declare, not a correction to the formula: either state and implement microsecond resolution here, or defer the estimation half to the wider work in SPARK-57839 that the PR already says it does not claim to resolve.

That is the review's one finding, and it is not a blocker: it is confined to CBO estimate quality for two datatypes no released Spark can hold, behind spark.sql.cbo.enabled, which defaults to false. Query results stay correct and nothing inaccurate is persisted.

The rest of the change held up under review. The catalog string form is exact in both directions and keeps all nine fractional digits, with the formatter's UTC default zone used symmetrically on write and read, so the DESCRIBE TABLE EXTENDED output is correct at present-day magnitudes -- including the 2022-01-01 00:00:01.123456789 +0000 the new end-to-end test asserts. The histogram exclusion works as described: the AnyTimestampNanoType arm precedes the DatetimeType arm it has to shadow, and the predicate gates both collection sites, which also keeps JoinEstimation.computeByHistogram unreachable for these types and makes the widened conversion there behaviour-neutral. Union estimation merges nanosecond min/max at full precision rather than through the Double encoding, because both physical types define their own ordering. The missing NTZ nanosecond arm in DescribeColumnCommand matches how TimestampNTZType already behaves, so it is consistent rather than an omission. No serialization version bump is owed, and the quantisation cannot throw: TimestampNanosVal.fromParts' [0, 999] precondition cannot be violated by the inverse as written, and FilterEstimation's assert(max > min) is unreachable at an interval the encoding has collapsed.

Findings

1 total: 0 P0, 0 P1, 1 P2, 0 P3.

Non-blocking (P2)

  • Nanosecond CBO encoding is lossy past 1970-04-12, and the new tests only cover where it works — sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/EstimationUtils.scala:148
    toDouble's nanosecond arm encodes TimestampNanosVal as epochMicros + nanosWithinMicro / 1000.0 and fromDouble inverts it with math.floor plus math.round((double - epochMicros) * 1000). A Double carries a 52-bit mantissa, so for a value in [2^n, 2^(n+1)) the spacing is 2^(n-52), and math.round(fraction * 1000) recovers nanosWithinMicro only while that spacing stays below 1/1000 -- that is, while epochMicros < 2^43 = 8796093022208, or 1970-04-12T19:21:33Z. Past that point the sub-microsecond component is quantised away.

    At 2022-01-01T00:00:00Z, epochMicros = 1640995200000000 falls in [2^50, 2^51), so the spacing is 0.25 us = 250 ns and four of the thousand sub-microsecond values survive. Writing k = 1640995200000000:

    • toDouble(fromParts(k, 1)) == toDouble(fromParts(k, 2)) -- distinct nanosecond values still collapse to a single Double, which is the specific outcome the comment says the fractional encoding prevents. Their relative order does not break: toDouble stays monotone non-decreasing at every magnitude, because rounding to nearest preserves order. What is lost is the ability to tell the two values apart;
    • toDouble(fromParts(k, 789)) comes back as nanos 750;
    • toDouble(fromParts(k, 900)) rounds up into the next microsecond, so fromDouble returns fromParts(k + 1, 0) -- a value strictly greater than the one it was given, and fromDouble(toDouble(fromParts(k, 999))) != fromParts(k, 999), so the round-trip property fails as well.

    This is what the comment at the encoding, the description's "reconstructing it losslessly in fromDouble" and "estimate cardinality using full nanosecond precision", and the new test's name all assert holds. It holds only within about 102 days of the epoch, and the two new EstimationUtilsSuite cases that assert it sit at epochMicros 100 and -100, inside that window; CatalogColumnStatSuite uses 123456. The four new StatisticsCollectionSuite cases do use 2022 timestamps, but on the estimation path they assert only that min and max are defined and that nothing throws, and the one place exact nanosecond strings are asserted at a 2022 magnitude is the DESCRIBE render path, which is produced by CatalogColumnStat.toExternalString and never reaches toDouble. So no new test fails at a realistic timestamp, and the reported green catalyst run is consistent with the encoding being lossy rather than evidence against it.

    The effect is confined to cost-based estimation for these two datatypes, behind spark.sql.cbo.enabled, which is off by default. Within that: ValueInterval.contains rounds the literal and the stored endpoints the same way, so a literal up to one quantum outside the column's real range is judged inside it, and evaluateEquality then returns 1/ndv where it owes the exact Some(0.0). The error runs in that direction only -- a literal that is genuinely in range is never judged out, because toDouble is monotone and the endpoint rounds with it, and that holds even for an endpoint already shifted by an intersect write-back. evaluateBinaryForNumeric computes its (literal - min) / (max - min) ratio on quantised endpoints; evaluateInSet's maxBy/minBy over toDouble can pick a non-extremal element when several IN-list values share a quantum and write it back as the new min or max; and ValueInterval.intersect, the only production fromDouble caller, can shift a written endpoint, which EXPLAIN COST then displays. Query results stay correct, and ANALYZE computes the persisted statistics from the data rather than through fromDouble, so nothing inaccurate reaches the metastore -- the serialized string form keeps the full nanosecond value in both directions.

    Recommended change: Decide the resolution the conversion actually provides for nanosecond timestamps, implement it uniformly, and make every statement of it true. Concretely: replace the fractional sub-microsecond encoding with a documented microsecond-resolution conversion, so that toDouble is exact and monotone wherever integral microseconds are exactly representable in a Double -- |epochMicros| <= 2^53, the domain the existing TimestampType arm of these same two methods already depends on -- and fromDouble can never land in a different microsecond than it started in on that domain; then align the comment at the encoding, the PR description, and the estimation-conversion test names to that resolution. Include estimation-conversion test support that pins the stated resolution at a timestamp magnitude in current use, not only near the epoch. Leave the catalog string form at full nanosecond precision -- that part of the change is correct and is what the PR title promises.

    Why this works: The invariant is that a declared conversion property must hold across the range the type admits. The current arithmetic satisfies it on a 102-day window and silently fails outside, because recovering a 1/1000 fractional step needs a representable spacing below 0.001 and the spacing grows with magnitude. Dropping the fractional term restores the invariant by making the declared property one the representation already keeps: integral microseconds are exact in a Double up to 2^53 microseconds, or 2255-06-05, which is the same domain the existing TimestampType and TimestampNTZType arm of these two methods already depends on, so the nanosecond arm would match the guarantee its neighbours provide rather than ask anything new of the representation. Ordering itself is not the defect: toDouble is monotone non-decreasing today, because round-to-nearest of a monotone function is monotone, and it stays so under flooring. What changes is that the values it collapses onto one Double become an explicit tie inside a stated microsecond resolution instead of a magnitude-dependent one -- at a 2022 magnitude, 24,073 of 400,000 distinct ordered pairs already compare equal. The round trip also becomes exact to the documented microsecond instead of landing in a neighbouring one: today, over all 1000 sub-microsecond values at that magnitude, 498 return a later microsecond, 498 an earlier one, and only 4 exactly, because math.floor paired with math.round is asymmetric. That displacement cannot flip a containment decision: a re-converted endpoint maps back to the same Double, and toDouble is monotone, so a literal that is genuinely in range is never judged out -- checked exhaustively over a strided sweep of minima, maxima and literals, including endpoints already written back by ValueInterval.intersect, it never happens. It surfaces instead wherever a fromDouble result is read as a timestamp rather than re-converted: FilterEstimation writes the narrowed min and max back into the per-estimation column statistics, so EXPLAIN COST can name a microsecond the column's values never occupied. The error the comparison does admit runs the other way -- a literal up to one quantum outside the real range, 250 ns at a 2022 magnitude, is judged inside it, and evaluateEquality then returns 1/ndv where it owes the exact Some(0.0). Flooring makes both bounded and stated: the round trip cannot leave the microsecond it started in, and the containment slack becomes exactly the documented microsecond instead of a quantum no reader can predict without knowing the magnitude. That slack is wider at a 2022 magnitude, where the quantum is a quarter of a microsecond, and identical from 2112 onward, where the quantum reaches a microsecond; the trade is a bound that is the same everywhere and written down. Uniform and stated beats better-but-magnitude-dependent here, because every consumer reasons about the converted value without knowing which regime it came from.

    Scope: A bounded change to one conversion pair plus the statements that describe it, with test support that spans the magnitude dimension. No plan, catalog or serialization behaviour changes, and no configuration is involved.

    Compatibility: Full nanosecond fidelity of the catalog statistic string in both directions, and of the DESCRIBE TABLE EXTENDED rendering built from it, must be preserved -- that is the change's headline behaviour and it is correct today. The deliberate exclusion of histograms for these types, and the estimation behaviour of every other data type, must also be preserved exactly.

    Risks: Estimation for nanosecond columns becomes explicitly microsecond-resolution, so two timestamps inside the same microsecond are indistinguishable to the CBO. Past 1970-04-12 the encoding already collapses most such pairs, but not all of them -- values either side of a quantum boundary still compare distinctly -- so the change does give up the accidental resolution that survives at a 2022 magnitude, and widens the containment slack from a quarter of a microsecond there to a full one. It buys a bound that holds at every magnitude, and it must be written down rather than implied, since a later reader could otherwise reintroduce the fractional encoding. Ties become reachable where the current encoding made them accidental, so any consumer that assumed strictly increasing conversion output would be affected. Checked against the tree: NumericValueInterval.contains, isIntersected and intersect use non-strict comparisons or endpoint selection, evaluateInSet uses maxBy/minBy which are total, and evaluateBinaryForNumeric's assert(max > min) is unreachable at min == max because its noOverlap/completeOverlap tests are exhaustive for all four operators. No consumer needs changing, but that should be re-confirmed rather than assumed if the chosen resolution differs. Narrowing a claim in the PR description is a scope statement as much as a code change, so the author may prefer to defer the whole estimation half to SPARK-57839 and land only the serialization work. That is a legitimate outcome of this finding and would resolve it too.

    Constraints: The DateType, numeric, TimestampType and TimestampNTZType arms of both conversions must stay byte-identical; nothing outside the nanosecond arms may change behaviour. TimestampNanosVal.fromParts' precondition that nanosWithinMicro lies in [0, 999] must continue to hold on every value fromDouble constructs. The histogram exclusion for nanosecond types in CommandUtils.supportsHistogram must stay, including its placement before the DatetimeType arm that it must shadow. The catalog string form must keep all nine fractional digits at every precision, and CatalogColumnStat.VERSION must not be bumped: no released Spark can hold a statistic for these types, so there is nothing to version.

    Success: The nanosecond arms of toDouble and fromDouble state the resolution they provide, and that statement, including the domain over which it holds, is true wherever the conversion is used, not only near the epoch. fromDouble(toDouble(v)) never returns a value in a different microsecond than v, on the domain where integral microseconds are exactly representable in a Double -- |epochMicros| <= 2^53, which is 1970 plus or minus about 285 years -- and which is the same domain the existing TimestampType and TimestampNTZType arm of these two methods already requires. Past 2^53 the spacing between adjacent Double values exceeds one microsecond, so no Double-based encoding can hold the guarantee there and the postcondition is not stated over the full Long range. toDouble is monotone non-decreasing over nanosecond values, and any two distinct values it maps to the same Double are documented as indistinguishable to estimation rather than claimed to compare distinctly. No statement in the change claims lossless reconstruction of the sub-microsecond remainder or full-nanosecond cardinality estimation unless the code delivers it at realistic timestamp magnitudes -- this covers the code comment, the PR description and the test names together. The containment slack is the stated resolution rather than a magnitude-dependent quantum: a literal outside the column's real range is judged inside it only when it shares a microsecond with an endpoint, and a literal inside the range is never judged out -- the direction that is already unreachable today, and must stay so. The estimation-conversion tests assert the stated resolution at a timestamp magnitude in current use as well as near the epoch and across the pre-1970 sign boundary, and at least one of those assertions fails against the current encoding.

Verification

  • The persisted statistic and the DESCRIBE TABLE EXTENDED rendering built from it are exact at present-day magnitudes: CatalogColumnStat.toExternalString/fromExternalString use a nine-digit fractional pattern and the formatter's UTC default zone symmetrically on write and read, so the nanosecond value survives a full round trip through the metastore and the new end-to-end assertion of 2022-01-01 00:00:01.123456789 +0000 is genuinely exact rather than incidentally passing.
  • The histogram exclusion is effective, not merely declared: CommandUtils.supportsHistogram's case _: AnyTimestampNanoType => false precedes the case _: DatetimeType => true arm that would otherwise match these subtypes, and the predicate gates both collection sites, which keeps JoinEstimation.computeByHistogram unreachable for nanosecond columns and makes the conversion widened there behaviour-neutral.
  • Union estimation is unaffected by the encoding: UnionEstimation merges nanosecond min/max by comparing the values directly through the ordering both physical types define, not through EstimationUtils.toDouble, so propagated union statistics keep full nanosecond precision.
  • The quantisation degrades estimates but cannot throw: math.floor leaves a fraction in [0, 1) and the nearest representable value to em + 0.999 never rounds to 1000, so TimestampNanosVal.fromParts' [0, 999] precondition holds at every magnitude, and FilterEstimation.evaluateBinaryForNumeric's assert(max > min) is unreachable when the encoding collapses an interval, because the preceding no-overlap and complete-overlap tests are exhaustive at min == max for all four comparison operators.
  • The degradation runs one way at the containment test: NumericValueInterval.contains converts the literal through the same toDouble as the stored endpoints, so a literal up to one quantum outside the real range is judged inside it and evaluateEquality returns 1/ndv instead of Some(0.0), while a literal genuinely inside the range is never judged out -- monotonicity of toDouble rules that out, and it still holds when the endpoint has been shifted by ValueInterval.intersect, the one production EstimationUtils.fromDouble caller.

@tdcmeehan

Copy link
Copy Markdown

Thanks for picking this up, and for the pointer to the SPARK-56822 umbrella. I have reviewed the change; my comments are in the review. The main one is that the new toDouble/fromDouble encoding in EstimationUtils does not keep the property its comment states -- that distinct nanosecond values stay distinguishable and come back unchanged through fromDouble -- once epochMicros exceeds 2^43 (1970-04-12), which covers every realistic timestamp. Relative order is unaffected, since toDouble stays monotone, so what is lost is resolution rather than sort order. The new EstimationUtilsSuite cases pass because they sit at epochMicros 100 and -100, inside the window where the fraction survives.

@uros-b uros-b left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the ping @vrjdev! Adding @stevomitric to help with review here

…d-resolution

Review on apache#58946 found that the fractional sub-microsecond encoding added by
the previous commit to EstimationUtils.toDouble/fromDouble only preserves
nanosecond distinctions for epochMicros < 2^43 (~1970-04-12): a Double's
52-bit mantissa cannot also hold a 1/1000 fraction at realistic (e.g. 2022)
magnitudes, where only 4 of 1000 sub-microsecond values remain
distinguishable and a round-trip can land in the wrong microsecond entirely.
The code comment, the PR description, and the previous commit's test names
all claimed a lossless/full-precision round-trip the encoding never actually
provided outside that epoch-adjacent window, and the added tests happened to
use epochMicros values (100, -100) that stayed inside it.

Revert the AnyTimestampNanoType case in toDouble/fromDouble to the
epochMicros-only conversion used by the original commit on this branch, and
document it honestly as microsecond resolution with the same |epochMicros|
<= 2^53 exactness domain as the existing TimestampType/TimestampNTZType
case. True nanosecond-resolution CBO estimation cannot be done through a
Double at these magnitudes at all, and is left to SPARK-57839.

Update EstimationUtilsSuite to match: round-trip now asserts truncation to
the microsecond, and a new test pins the documented collision (two distinct
nanosecond values sharing a microsecond compare equal) at a realistic
2022-magnitude epochMicros instead of only near the epoch. Also adds a
monotonicity check across distinct epochMicros, which is the property CBO
range/IN-list estimation actually relies on.

Tests: catalyst/testOnly EstimationUtilsSuite FilterEstimationSuite
JoinEstimationSuite UnionEstimationSuite CatalogColumnStatSuite (119/119),
sql/testOnly StatisticsCollectionSuite CommandUtilsSuite (47/47).
@vrjdev

vrjdev commented Sep 22, 2026

Copy link
Copy Markdown
Author

Thanks for the detailed review, @tdcmeehan and for explaining the math. Fractional encoding only holds up to epochMicros < 2^43 (~1970-04-12); past that a Double's mantissa can't also hold the sub-microsecond fraction, so it silently degrades exactly as you describe, and the tests I'd added happened to sit inside that safe window.

I've pushed a fix taking your first option: toDouble/fromDouble's AnyTimestampNanoType case is back to the epochMicros-only conversion (matching the original commit and the same |epochMicros| <= 2^53 exactness domain as the existing TimestampType/TimestampNTZType case), and the comment now states the microsecond-resolution limit plainly instead of claiming a lossless nanosecond round-trip. EstimationUtilsSuite now has a test at a realistic 2022-magnitude epochMicros (the value from your write-up) that pins the documented collision, plus a monotonicity check across microseconds — instead of only exercising near-epoch values.

JoinEstimation.computeByHistogram's switch to the shared toDouble and UnionEstimation's type-widening are unchanged, since neither depends on the fractional encoding. Agreed that real nanosecond-resolution CBO estimation isn't achievable through a Double at these magnitudes and belongs in SPARK-57839 rather than here.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants