Skip to content

Negative sub-second durations lose their sign: Serial(-500ms) equals Serial(500ms) #203

Description

@slachiewicz

Any time.Duration strictly between -1s and 0 is serialised with its sign lost, so a negative interval silently becomes positive.

Serial(-500ms ) -> "INTERVAL '0.5' SECOND"
Serial(-999ms ) -> "INTERVAL '0.999' SECOND"
Serial(-100ms ) -> "INTERVAL '0.1' SECOND"
Serial( 500ms ) -> "INTERVAL '0.5' SECOND"    <- identical
Serial(-1.5s  ) -> "INTERVAL '-1.5' SECOND"   <- correct

Cause

serialMillisecondsInterval (serial.go:253):

seconds := int64(dur / time.Second)
millisInSecond := dur.Abs().Milliseconds() % 1000
intervalNr := strings.TrimRight(fmt.Sprintf("%d.%03d", seconds, millisInSecond), "0")

Integer division truncates toward zero, so for -500ms seconds is 0, not -0, and the sign disappears. The fractional part then comes from dur.Abs(), so nothing restores it. The sign only survives when the magnitude is at least one second, which is why -1.5s is fine.

Why it has not been noticed

Every duration case in serial_test.go is at whole-second magnitude or larger, so none of them enters the broken range, and there is no integration coverage for intervals.

This is on the default parameter path — Serial runs for every argument regardless of useExplicitPrepare (trino.go:1260) — so it is silent wrong data rather than a rejected query.

Fix

Take the sign from the duration rather than from the truncated seconds, and apply it once to the formatted value. A test at sub-second negative magnitudes would have caught this and should come with the fix.

This work was created with AI assistance.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions