ClickHouse / ClickHouse/clickhouse-go

bind: sub-second DateNamed values render as bare tick integers (toDateTime64('1641999600123', 3)) and lose precision near the epoch

Open
#2,014 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
3.3k
Forks
680
Avg merge
2d 3h
Merged PRs (30d)
14

Description

## Description

`formatTime` in `bind.go` renders a `time.Time` bound through `clickhouse.DateNamed(...)` into SQL text. When the value's location has no server-resolvable name — `"Local"` (i.e. `time.Now()` when `TZ` is unset, the default in most containers/CI images) or `""` (any zone produced by `time.Parse` from a numeric offset, or `time.FixedZone("", …)`) — it emits the timestamp as a **bare integer count of ticks passed as a string**:

```go
case MilliSeconds:
return fmt.Sprintf("toDateTime64('%d', 3)", value.UnixMilli()), nil
case MicroSeconds:
return fmt.Sprintf("toDateTime64('%d', 6)", value.UnixMicro()), nil
case NanoSeconds:
return fmt.Sprintf("toDateTime64('%d', 9)", value.UnixNano()), nil
```

This round-trips only because the server has historically parsed an integer-looking `DateTime64` input as the *raw underlying decimal value* (ticks), while a decimal is parsed as seconds — the behaviour documented for `DateTime64` (`INSERT INTO dt64 VALUES (1546300800123)` → `2019-01-01 00:00:00.123` for scale 3). The client is therefore relying on scale-dependent integer semantics rather than on an unambiguous literal.

That exact dependency has just broken for clickhouse-java's v1 client on ClickHouse 26.8, where a `DateTime64(3)` tick value of `1` now comes back as `1970-01-01 00:00:01` instead of `1970-01-01 00:00:00.001` — the integer is being read as **seconds** (see https://github.com/ClickHouse/clickhouse-java/issues/3114). If the server now reads these integers as seconds, `toDateTime64('1641999600123', 3)` no longer means `2022-01-12 15:00:00.123` — it means 1.6e12 seconds, which overflows the `DateTime64` range. Because this appears in `WHERE` predicates, the failure is silent: no error, just wrong rows.

The two rendering branches are already inconsistent with each other for the same instant, which is the smell: the named-zone branch emits an unambiguous wall-clock literal (`toDateTime64('2022-01-12 10:00:00.123', 3, 'America/New_York')`), the unnamed-zone branch emits the tick integer.

Secondly, and verifiable entirely client-side, the `value.Unix() == 0` guard in the same function discards the sub-second part **and** the scale for any instant in `[1970-01-01 00:00:00, 00:00:01)`:

```go
if value.Unix() == 0 {
return "toDateTime(0)", nil
}
```

So `1970-01-01 00:00:00.001` bound with `MilliSeconds` becomes `toDateTime(0)` — literally the same class of symptom as the java report.

## ClickHouse server version

Code analysis only; not verified against a running server (no ClickHouse instance was reachable in the investigation environment, so the 26.8-side semantics change was not re-confirmed here). The client-side SQL rendering below **was** executed and is reproduced verbatim.

## Reproduction

`bind` is unexported, so this test goes in the root package (`clickhouse`), next to `bind_test.go`. Run with `TZ` unset, as CI images typically are:

```go
package clickhouse

import (
"fmt"
"testing"
"time"

"github.com/stretchr/testify/require"
)

func TestDateNamedUnnamedZoneRendering(t *testing.T) {
utc, err := time.LoadLocation("UTC")
require.NoError(t, err)

// 2022-01-12 15:00:00.123 UTC, held in the local zone like time.Now() is.
base := time.Date(2022, 1, 12, 15, 0, 0, 123000000, utc)
local := base.In(time.Local)

for _, scale := range []TimeUnit{Seconds, MilliSeconds, MicroSeconds, NanoSeconds} {
q, err := bind(utc, "SELECT * FROM t WHERE ts = @TS", DateNamed("TS", local, scale))
require.NoError(t, err)
fmt.Printf("local %d -> %s\n", scale, q)
}

// A zone with no name: exactly what time.Parse yields for a numeric offset.
parsed, err := time.Parse(time.RFC3339Nano, "2022-01-12T15:00:00.123+02:00")
require.NoError(t, err)
for _, scale := range []TimeUnit{Seconds, MilliSeconds, MicroSeconds, NanoSeconds} {
q, err := bind(utc, "SELECT * FROM t WHERE ts = @TS", DateNamed("TS", parsed, scale))
require.NoError(t, err)
fmt.Printf("offset %d -> %s\n", scale, q)
}

// The epoch-adjacent value from the java report: 1970-01-01 00:00:00.001
epochMilli := time.Date(1970, 1, 1, 0, 0, 0, int(time.Millisecond), time.FixedZone("", 0))
q, err := bind(utc, "SELECT * FROM t WHERE ts = @TS", DateNamed("TS", epochMilli, MilliSeconds))
require.NoError(t, err)
fmt.Printf("epoch+1ms -> %s\n", q)
}
```

Actual output (`go test -count=1 -run TestDateNamedUnnamedZoneRendering -v ./`):

```
local 0 -> SELECT * FROM t WHERE ts = toDateTime('1641999600')
local 1 -> SELECT * FROM t WHERE ts = toDateTime64('1641999600123', 3)
local 2 -> SELECT * FROM t WHERE ts = toDateTime64('1641999600123000', 6)
local 3 -> SELECT * FROM t WHERE ts = toDateTime64('1641999600123000000', 9)
offset 0 -> SELECT * FROM t WHERE ts = toDateTime('1641992400')
offset 1 -> SELECT * FROM t WHERE ts = toDateTime64('1641992400123', 3)
offset 2 -> SELECT * FROM t WHERE ts = toDateTime64('1641992400123000', 6)
offset 3 -> SELECT * FROM t WHERE ts = toDateTime64('1641992400123000000', 9)
epoch+1ms -> SELECT * FROM t WHERE ts = toDateTime(0)
```

Expected: an unambiguous literal that does not depend on how the server reads a bare integer for a given scale, e.g. `toDateTime64('2022-01-12 15:00:00.123', 3, 'UTC')`, and a millisecond-preserving rendering for `epoch+1ms` rather than `toDateTime(0)`.

For the same reason, `tests/issues/615_test.go` exercises the tick-integer branch today (`ts1 := time.Now()` → `"Local"`, `DateNamed(..., clickhouse.NanoSeconds)`), so it is the integration test most likely to start failing on a server that reads these integers as seconds.

## Suggested fix

`target-repo/bind.go`, `formatTime` (the `case "Local", "":` branch, roughly lines 447–470):

- Emit an unambiguous literal instead of a tick integer — the wall-clock text plus an explicit timezone, the way the named-zone branch already does (e.g. convert to UTC and render `toDateTime64('2006-01-02 15:04:05.000', 3, 'UTC')`). The original reason for the integer form (per the in-code comment, decimal overflow at high precision) does not apply to a wall-clock string.
- Drop or narrow the `value.Unix() == 0` special case so sub-second values in the first second of the epoch keep their fraction and their scale; if a bare `toDateTime(0)` is still needed for the true zero value, gate it on the whole value being zero, not just the seconds part.
- The server-side parameter path in `query_parameters.go` (`formatEpoch`) already sidesteps this by emitting decimal epoch seconds (`0.001`), which is scale-independent; the same reasoning applies here.

## Link

Relayed from https://github.com/ClickHouse/clickhouse-java/issues/3114

Contributor guide

Open the contributing guide

Research direction

Start in bind.go at formatTime, especially the case "Local", "" branch and the value.Unix() == 0 guard; compare it with the named-zone branch and query_parameters.go's formatEpoch. Reproduce the cases in root-package bind_test.go, then check tests/issues/615_test.go; done means unnamed-zone DateTime64 values use unambiguous, precision-preserving SQL and epoch+subsecond values retain their scale.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
database
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.