Skip to content

Statistics of TIMESTAMP column is stored as datetime string without timezone information #52429

Open

Description

@winoros

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

image

As you can see, the type of this column is TIMESTAMP, but we store its histogram with datetime string without timezone information. And the timezone is not UTC, it's decided by the session executing the ANALYZE command.

This is not correct and will introduce an estimation error.

Activity

  1. added
    type/bugThe issue is confirmed as a bug.
    affects-6.1This bug affects the 6.1.x(LTS) versions.
    affects-6.5This bug affects the 6.5.x(LTS) versions.
    affects-7.1This bug affects the 7.1.x(LTS) versions.
    affects-7.5This bug affects the 7.5.x(LTS) versions.
    on Apr 8, 2024
  2. winoros commented on Apr 8, 2024

    @winoros
    MemberAuthor

    This is the root cause of #41985

  3. winoros commented on Apr 8, 2024

    @winoros
    MemberAuthor

    We've already stored the histogram of TIMESTAMP to string value. So it's not easy to store it at its original value for compatibility issues.

    A possible fix is that we always store the datetime string with UTC timezone, and do conversion when doing row count estimation.

  4. winoros commented on Apr 9, 2024

    @winoros
    MemberAuthor

    TopN or index's histogram is correct. They're following the normal encoding&decoding procedure.

  5. winoros commented on Apr 9, 2024

    @winoros
    MemberAuthor

    It's a long-existing issue. I think i would not solve it before 8.1.0 is released. I'll fix it in the minor version.

  6. self-assigned this
    on Jul 3, 2024
  7. mjonss commented on Sep 14, 2026

    @mjonss
    Contributor

    While reviewing #71008 (pre-split index keys encoded with the wrong session's context) I measured a
    consequence of statistics not being stored in UTC that is worth recording here, because the fix for
    this issue removes it as a side effect.

    The round trip through a session zone has no inverse during a DST fall-back.

    Stats-derived TIMESTAMP values are decoded into some session zone and re-encoded from it. For TopN
    that is DecodeColumnTopNValue(encoded, ft, loc) followed by codec.EncodeKey(loc, ...), which
    looks like it cancels out, and does for any fixed-offset zone. In a zone with DST it does not: during
    the autumn fall-back one local wall clock covers two UTC instants, and re-encoding resolves the
    ambiguity to the later one.

    Measured with Europe/Amsterdam, where clocks go back from 03:00 CEST to 02:00 CET on 2024-10-27:

    stored UTC instant decodes to local re-encodes to
    2024-10-27 00:30 (first pass, CEST +02:00) 02:30 2024-10-27 01:30 UTC
    2024-10-27 01:30 (second pass, CET +01:00) 02:30 2024-10-27 01:30 UTC
    2024-10-27 05:30 (ordinary) 06:30 unchanged
    2024-03-31 00:30 / 01:30 (spring forward) 01:30 / 03:30 unchanged

    So two distinct stored values collapse onto one, and a value in the repeated hour is silently moved
    forward by an hour. Nothing errors: CoreTime.GoTime (pkg/types/core_time.go:170-185) validates
    only that the wall-clock fields survive, and for an ambiguous local time they do. Spring forward is
    safe in this direction, since a UTC to local conversion never produces a nonexistent local time.

    Why it belongs here rather than in its own issue. It exists only because a stats value is passed
    through a non-UTC zone at all. Keeping statistics in UTC end to end means no ambiguous local time is
    ever constructed, so this disappears without a separate fix. Filing it separately would just create
    something that closes as a duplicate of this one.

    Suggested addition to the fix for this issue: a test that pins the inverse, rather than trusting
    that it holds. Something that takes a TIMESTAMP value inside a fall-back repeated hour, for example
    2024-10-27 00:30 UTC with Europe/Amsterdam in play, runs it through collection and read-back, and
    requires the instant to be preserved. Under the current code that fails; under UTC end to end it
    should pass, which makes it a direct verification of the fix rather than a restatement of it.

    For reference, #71008's PR has a subtest pinning the current behavior
    (TestPlanAutoPreSplitIndexRegionsTimestampTimeZone, subtest DST fall-back collapses the repeated hour), so when this issue is fixed that assertion is expected to need updating, and that is the
    signal the collapse is gone.

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

Metadata

Metadata

Assignees

Labels

affects-6.1This bug affects the 6.1.x(LTS) versions.affects-6.5This bug affects the 6.5.x(LTS) versions.affects-7.1This bug affects the 7.1.x(LTS) versions.affects-7.5This bug affects the 7.5.x(LTS) versions.affects-8.1This bug affects the 8.1.x(LTS) versions.component/statisticsseverity/moderatesig/plannerSIG: Plannertype/bugThe issue is confirmed as a bug.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions