From 3bdbfc94928eb879863f99b320bf21e94fa76b1e Mon Sep 17 00:00:00 2001 From: Sankalp Thakur Date: Sat, 1 Aug 2026 13:58:05 +0530 Subject: [PATCH 1/2] fix(bind): treat []byte as String and time.Duration as Time Client-side formatValue had no cases for []byte or time.Duration, so []byte fell through to reflect.Slice (Array(UInt8)) and Duration hit fmt.Stringer (Go's "14h30m0s"). Both are unusable against String and Time/Time64 columns respectively. - Quote []byte like string (escape \, ', and NUL as \0) - Format time.Duration / *time.Duration as HH:MM:SS[.frac] - Unit tests for bind + formatDuration Fixes #1942 AI assistance: used for locating formatValue fallthrough and drafting the bind tests; change reviewed and verified with go test. --- bind.go | 41 ++++++++++++++++++++++++++++++++- bind_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/bind.go b/bind.go index 8e8208fd28..bf1593ed9b 100644 --- a/bind.go +++ b/bind.go @@ -483,7 +483,9 @@ func formatTime(tz *time.Location, scale TimeUnit, value time.Time) (string, err return fmt.Sprintf("toDateTime64('%s', %d, '%s')", value.Format(fmt.Sprintf("2006-01-02 15:04:05.%0*d", int(scale*3), 0)), int(scale*3), escapedTimezone), nil } -var stringQuoteReplacer = strings.NewReplacer(`\`, `\\`, `'`, `\'`) +// Escape order: backslash first so later replacements are not re-escaped. +// NUL is written as \0 so binary String values survive client-side bind. +var stringQuoteReplacer = strings.NewReplacer(`\`, `\\`, `'`, `\'`, "\x00", `\0`) // formatMode says which syntax formatValue should produce. A value spliced // into the query text needs SQL syntax; a server-side query parameter needs @@ -512,6 +514,30 @@ func format(tz *time.Location, scale TimeUnit, v any) (string, error) { return formatValue(tz, scale, v, formatSQL) } +// formatDuration renders a time.Duration as a ClickHouse Time/Time64 text +// literal body: HH:MM:SS or HH:MM:SS.frac with trailing fractional zeros +// trimmed. time.Duration is the ScanType for Time/Time64 columns; without a +// dedicated case it would hit fmt.Stringer and emit Go's "14h30m0s" form. +func formatDuration(d time.Duration) string { + sign := "" + if d < 0 { + sign = "-" + d = -d + } + hours := d / time.Hour + d -= hours * time.Hour + mins := d / time.Minute + d -= mins * time.Minute + secs := d / time.Second + frac := d % time.Second + if frac == 0 { + return fmt.Sprintf("%s%02d:%02d:%02d", sign, hours, mins, secs) + } + fracStr := fmt.Sprintf("%09d", frac.Nanoseconds()) + fracStr = strings.TrimRight(fracStr, "0") + return fmt.Sprintf("%s%02d:%02d:%02d.%s", sign, hours, mins, secs, fracStr) +} + // formatValue turns v into a string in the given mode. The mode carries down // into nested values, so a bool or map keeps its formatting at any depth. // @@ -528,6 +554,11 @@ func formatValue(tz *time.Location, scale TimeUnit, v any, mode formatMode) (str return "NULL", nil case string: return quote(v), nil + case []byte: + // []byte is a valid database/sql driver.Value and the natural Go type + // for a ClickHouse String holding arbitrary bytes. Without this case + // it falls through to reflect.Slice and becomes Array(UInt8). + return quote(string(v)), nil case time.Time: if mode == formatParamText { return quote(formatTimeParam(v)), nil @@ -541,6 +572,14 @@ func formatValue(tz *time.Location, scale TimeUnit, v any, mode formatMode) (str return quote(formatTimeParam(*v)), nil } return formatTime(tz, scale, *v) + case time.Duration: + // Must precede fmt.Stringer: Duration.String() is Go's "14h30m0s". + return quote(formatDuration(v)), nil + case *time.Duration: + if v == nil { + return "NULL", nil + } + return quote(formatDuration(*v)), nil case bool: if mode == formatParamText { if v { diff --git a/bind_test.go b/bind_test.go index cabe479b4a..0b28bee6e0 100644 --- a/bind_test.go +++ b/bind_test.go @@ -983,3 +983,68 @@ func BenchmarkBindNamed(b *testing.B) { } } } + +// TestBindDuration checks that time.Duration (the ScanType for ClickHouse +// Time/Time64) binds as a Time-parseable literal, not Go's duration string. +func TestBindDuration(t *testing.T) { + q, err := bind(time.UTC, "SELECT toTime(?)", 14*time.Hour+30*time.Minute) + assert.NoError(t, err) + assert.Equal(t, "SELECT toTime('14:30:00')", q) + + // *time.Duration and zero / fractional / negative values + d := 1*time.Second + 250*time.Millisecond + q, err = bind(time.UTC, "SELECT ?", &d) + assert.NoError(t, err) + assert.Equal(t, "SELECT '00:00:01.25'", q) + + q, err = bind(time.UTC, "SELECT ?", time.Duration(0)) + assert.NoError(t, err) + assert.Equal(t, "SELECT '00:00:00'", q) + + q, err = bind(time.UTC, "SELECT ?", -90*time.Second) + assert.NoError(t, err) + assert.Equal(t, "SELECT '-00:01:30'", q) + + var nilDur *time.Duration + q, err = bind(time.UTC, "SELECT ?", nilDur) + assert.NoError(t, err) + assert.Equal(t, "SELECT NULL", q) +} + +// TestBindBytes checks that []byte binds as a String literal (with the same +// escaping as string), not as Array(UInt8). +func TestBindBytes(t *testing.T) { + q, err := bind(time.UTC, "SELECT ?", []byte("A\x00B")) + assert.NoError(t, err) + assert.Equal(t, "SELECT 'A\\0B'", q) + + q, err = bind(time.UTC, "SELECT ?", []byte(`a'b\c`)) + assert.NoError(t, err) + assert.Equal(t, `SELECT 'a\'b\\c'`, q) + + q, err = bind(time.UTC, "SELECT ?", []byte{}) + assert.NoError(t, err) + assert.Equal(t, "SELECT ''", q) + + // nested: Array(String) of binary strings, not Array(Array(UInt8)) + q, err = bind(time.UTC, "SELECT ?", [][]byte{[]byte("x"), []byte("y")}) + assert.NoError(t, err) + assert.Equal(t, "SELECT ['x', 'y']", q) +} + +func TestFormatDuration(t *testing.T) { + cases := []struct { + in time.Duration + want string + }{ + {0, "00:00:00"}, + {14*time.Hour + 30*time.Minute, "14:30:00"}, + {1*time.Second + 250*time.Millisecond, "00:00:01.25"}, + {123 * time.Nanosecond, "00:00:00.000000123"}, + {-90 * time.Second, "-00:01:30"}, + {25 * time.Hour, "25:00:00"}, + } + for _, tc := range cases { + assert.Equal(t, tc.want, formatDuration(tc.in), "in=%v", tc.in) + } +} From 8393cdca83e3cf65a2f259fe1303b72f31749a8c Mon Sep 17 00:00:00 2001 From: Sankalp Thakur Date: Thu, 6 Aug 2026 02:30:47 +0530 Subject: [PATCH 2/2] fix(bind): make formatDuration return quoted literal and add Time64 precision tests Address review feedback from chernser: - formatDuration now returns quoted string like formatTime, including single quotes in the return value for API consistency. - Add TestBindDuration_Time64Precision covering Time/Time64 with different precisions (seconds, milli, micro, nano) and composite types. Fixes #1942 Signed-off-by: Sankalp Thakur --- bind.go | 24 +++++++++++++--------- bind_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 66 insertions(+), 16 deletions(-) diff --git a/bind.go b/bind.go index bf1593ed9b..740536c18c 100644 --- a/bind.go +++ b/bind.go @@ -514,10 +514,10 @@ func format(tz *time.Location, scale TimeUnit, v any) (string, error) { return formatValue(tz, scale, v, formatSQL) } -// formatDuration renders a time.Duration as a ClickHouse Time/Time64 text -// literal body: HH:MM:SS or HH:MM:SS.frac with trailing fractional zeros -// trimmed. time.Duration is the ScanType for Time/Time64 columns; without a -// dedicated case it would hit fmt.Stringer and emit Go's "14h30m0s" form. +// formatDuration renders a time.Duration as a quoted ClickHouse Time/Time64 +// literal: 'HH:MM:SS' or 'HH:MM:SS.frac' with trailing fractional zeros +// trimmed. It returns the quoted form directly, like formatTime does with +// its toDateTime('...') wrapper, so the API is consistent. func formatDuration(d time.Duration) string { sign := "" if d < 0 { @@ -530,12 +530,15 @@ func formatDuration(d time.Duration) string { d -= mins * time.Minute secs := d / time.Second frac := d % time.Second + var body string if frac == 0 { - return fmt.Sprintf("%s%02d:%02d:%02d", sign, hours, mins, secs) + body = fmt.Sprintf("%s%02d:%02d:%02d", sign, hours, mins, secs) + } else { + fracStr := fmt.Sprintf("%09d", frac.Nanoseconds()) + fracStr = strings.TrimRight(fracStr, "0") + body = fmt.Sprintf("%s%02d:%02d:%02d.%s", sign, hours, mins, secs, fracStr) } - fracStr := fmt.Sprintf("%09d", frac.Nanoseconds()) - fracStr = strings.TrimRight(fracStr, "0") - return fmt.Sprintf("%s%02d:%02d:%02d.%s", sign, hours, mins, secs, fracStr) + return "'" + stringQuoteReplacer.Replace(body) + "'" } // formatValue turns v into a string in the given mode. The mode carries down @@ -574,12 +577,13 @@ func formatValue(tz *time.Location, scale TimeUnit, v any, mode formatMode) (str return formatTime(tz, scale, *v) case time.Duration: // Must precede fmt.Stringer: Duration.String() is Go's "14h30m0s". - return quote(formatDuration(v)), nil + // formatDuration already returns a quoted literal like formatTime. + return formatDuration(v), nil case *time.Duration: if v == nil { return "NULL", nil } - return quote(formatDuration(*v)), nil + return formatDuration(*v), nil case bool: if mode == formatParamText { if v { diff --git a/bind_test.go b/bind_test.go index 0b28bee6e0..3b0ab2a5ce 100644 --- a/bind_test.go +++ b/bind_test.go @@ -1037,14 +1037,60 @@ func TestFormatDuration(t *testing.T) { in time.Duration want string }{ - {0, "00:00:00"}, - {14*time.Hour + 30*time.Minute, "14:30:00"}, - {1*time.Second + 250*time.Millisecond, "00:00:01.25"}, - {123 * time.Nanosecond, "00:00:00.000000123"}, - {-90 * time.Second, "-00:01:30"}, - {25 * time.Hour, "25:00:00"}, + {0, "'00:00:00'"}, + {14*time.Hour + 30*time.Minute, "'14:30:00'"}, + {1*time.Second + 250*time.Millisecond, "'00:00:01.25'"}, + {123 * time.Nanosecond, "'00:00:00.000000123'"}, + {-90 * time.Second, "'-00:01:30'"}, + {25 * time.Hour, "'25:00:00'"}, } for _, tc := range cases { assert.Equal(t, tc.want, formatDuration(tc.in), "in=%v", tc.in) } } + +// TestBindDuration_Time64Precision covers Time/Time64 with different precision +// (seconds, milli, micro, nano) — the server parses each as the corresponding +// Time64 scale. The binder always emits the minimal fractional form via +// formatDuration, which ClickHouse accepts for any Time64 precision. +func TestBindDuration_Time64Precision(t *testing.T) { + cases := []struct { + name string + dur time.Duration + want string + }{ + // Time (seconds, no fraction) + {"Time seconds", 8*time.Hour + 15*time.Minute + 30*time.Second, "'08:15:30'"}, + // Time64(3) — milliseconds + {"Time64(3) milliseconds", 1*time.Second + 123*time.Millisecond, "'00:00:01.123'"}, + {"Time64(3) trimmed", 1*time.Second + 120*time.Millisecond, "'00:00:01.12'"}, + // Time64(6) — microseconds + {"Time64(6) microseconds", 1*time.Second + 123456*time.Microsecond, "'00:00:01.123456'"}, + {"Time64(6) trimmed", 1*time.Second + 123400*time.Microsecond, "'00:00:01.1234'"}, + // Time64(9) — nanoseconds + {"Time64(9) nanoseconds", 1*time.Second + 123456789*time.Nanosecond, "'00:00:01.123456789'"}, + {"Time64(9) trimmed", 1*time.Second + 100000000*time.Nanosecond, "'00:00:01.1'"}, + // Negative with fraction (Time64) + {"negative Time64(3)", -(2*time.Hour + 500*time.Millisecond), "'-02:00:00.5'"}, + // Array(Time) and Array(Time64) via join + {"zero", 0, "'00:00:00'"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + q, err := bind(time.UTC, "SELECT ?", tc.dur) + assert.NoError(t, err) + assert.Equal(t, "SELECT "+tc.want, q) + // formatDuration directly should match the quoted literal + assert.Equal(t, tc.want, formatDuration(tc.dur)) + }) + } + // Array(Time64) — durations inside an array keep quoted form + q, err := bind(time.UTC, "SELECT ?", []time.Duration{1 * time.Second, 2*time.Second + 500*time.Millisecond}) + assert.NoError(t, err) + assert.Equal(t, "SELECT ['00:00:01', '00:00:02.5']", q) + + // Map with Duration values + q, err = bind(time.UTC, "SELECT ?", map[string]time.Duration{"a": 1 * time.Second + 123*time.Millisecond}) + assert.NoError(t, err) + assert.Equal(t, "SELECT map('a', '00:00:01.123')", q) +}