Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Date/Date32 is wrong when written with time.Time parsed from local timezone. #693

Closed
taiyang-li opened this issue Jul 26, 2022 · 6 comments · Fixed by ClickHouse/ch-go#165
Assignees
Labels
Milestone

Comments

@taiyang-li
Copy link

taiyang-li commented Jul 26, 2022

Issue description

Date/Date32 is wrong when written with time.Time parsed from local timezone.

Example code

package std

import (
	"database/sql"
	"testing"
	"time"

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

func TestStdDate(t *testing.T) {
	if conn, err := sql.Open("clickhouse", "clickhouse://127.0.0.1:9000"); assert.NoError(t, err) {
		const ddl = `
			CREATE TABLE test_date (
				  ID   UInt8
				, Col1 Date
				, Col2 Nullable(Date)
				, Col3 Array(Date)
				, Col4 Array(Nullable(Date))
			) Engine Memory
		`
		type result struct {
			ColID uint8 `ch:"ID"`
			Col1  time.Time
			Col2  *time.Time
			Col3  []time.Time
			Col4  []*time.Time
		}
		if _, err := conn.Exec(ddl); assert.NoError(t, err) {
			scope, err := conn.Begin()
			if !assert.NoError(t, err) {
				return
			}
			if batch, err := scope.Prepare("INSERT INTO test_date"); assert.NoError(t, err) {
				// date, err := time.Parse("2006-01-02 15:04:05", "2022-01-12 00:00:00")
				CurrentLoc, _ := time.LoadLocation("Asia/Shanghai")
				date, err := time.ParseInLocation("2006-01-02 15:04:05", "2022-01-12 00:00:00", CurrentLoc)
				if !assert.NoError(t, err) {
					return
				}
				if _, err := batch.Exec(uint8(1), date, &date, []time.Time{date}, []*time.Time{&date, nil, &date}); !assert.NoError(t, err) {
					return
				}
				if _, err := batch.Exec(uint8(2), date, nil, []time.Time{date}, []*time.Time{nil, nil, &date}); !assert.NoError(t, err) {
					return
				}
				if err := scope.Commit(); assert.NoError(t, err) {
					var (
						result1 result
						result2 result
					)
					if err := conn.QueryRow("SELECT * FROM test_date WHERE ID = $1", 1).Scan(
						&result1.ColID,
						&result1.Col1,
						&result1.Col2,
						&result1.Col3,
						&result1.Col4,
					); assert.NoError(t, err) {
						if assert.Equal(t, date, result1.Col1) {
							assert.Equal(t, "UTC", result1.Col1.Location().String())
							assert.Equal(t, date, *result1.Col2)
							assert.Equal(t, []time.Time{date}, result1.Col3)
							assert.Equal(t, []*time.Time{&date, nil, &date}, result1.Col4)
						}
					}
					if err := conn.QueryRow("SELECT * FROM test_date WHERE ID = $1", 2).Scan(
						&result2.ColID,
						&result2.Col1,
						&result2.Col2,
						&result2.Col3,
						&result2.Col4,
					); assert.NoError(t, err) {
						if assert.Equal(t, date, result2.Col1) {
							assert.Equal(t, "UTC", result2.Col1.Location().String())
							if assert.Nil(t, result2.Col2) {
								assert.Equal(t, []time.Time{date}, result2.Col3)
								assert.Equal(t, []*time.Time{nil, nil, &date}, result2.Col4)
							}
						}
					}
				}
			}
		}
	}
}

Error log

My local timezone is Asia/Shanghai, so is ClickHouse and Linux System. After insertion, the Date in CH is expected to be 2022-01-12, but it is 2022-01-11 actually.

:) select * from test_date;

SELECT *
FROM test_date

Query id: 4d4e60e9-0267-4ffa-9c70-676b3c8f504d

┌─ID─┬───────Col1─┬───────Col2─┬─Col3───────────┬─Col4─────────────────────────────┐
│  1 │ 2022-01-11 │ 2022-01-11 │ ['2022-01-11'] │ ['2022-01-11',NULL,'2022-01-11'] │
│  2 │ 2022-01-11 │       ᴺᵁᴸᴸ │ ['2022-01-11'] │ [NULL,NULL,'2022-01-11']         │
└────┴────────────┴────────────┴────────────────┴──────────────────────────────────┘

2 rows in set. Elapsed: 0.002 sec. 

Configuration

OS: Docker, Ubuntu20.4

Interface: database/sql

Driver version: v2.2.0

Go version: go version go1.18.1 linux/amd64

ClickHouse Server version: v21.10

@gingerwizard
Copy link
Collaborator

Will take a look but Date is not locale aware - its just days since 1970. Think you want https://clickhouse.com/docs/en/sql-reference/data-types/datetime

@taiyang-li
Copy link
Author

taiyang-li commented Jul 26, 2022

The behavior of v2 version is not consistent with lower versions of clickhouse-go. In v1 versions, CH date will be the same with time.Time in golang even if the zone of time.Time is not UTC.

@gingerwizard
Copy link
Collaborator

The behavior of v2 version is not consistent with lower versions of clickhouse-go.

Many differences with v1. Complete rewrite. No effort to have the same behaviour - since a lot of what it did was wrong e.g. loss of precision on types.

The argument here is should the date be persisted in the tz it was sent i.e. should the date be interpreted as-is or its true UTC time stored (since the column has no notion of timezone). In my mind, both are fine and this is a ch-go decision. Either way, we need to be consistent with our date types and document.

@gingerwizard gingerwizard added this to the 2.3.0 milestone Jul 26, 2022
@ernado
Copy link
Collaborator

ernado commented Jul 26, 2022

Fixed in ch-go@v0.47.2

@gingerwizard
Copy link
Collaborator

I'll update, add test and PR. thanks @ernado

@gingerwizard
Copy link
Collaborator

gingerwizard commented Jul 26, 2022

@taiyang-li

your test on 0.47.2 ch-go (albeit simplified)

The following will fail with

Expected :time.Date(2022, time.January, 12, 0, 0, 0, 0, time.Location("Asia/Shanghai"))
Actual   :time.Date(2022, time.January, 12, 0, 0, 0, 0, time.UTC)

The correct date is now stored but strictly, the inserted date and retrieved date are different @ernado This might be the behavior we intend. Obviously if I change the test to require.Equal(t, date.Format("2006-01-02 15:04:05"), result1.Col1.Format("2006-01-02 15:04:05")) it passes. The previous approach would have passed had the user called In and converted the received date to the original tz i.e. the dates would be strictly the same.

func Test693(t *testing.T) {
	conn, err := sql.Open("clickhouse", "clickhouse://127.0.0.1:9000")
	require.NoError(t, err)
	const ddl = `
			CREATE TABLE test_date (
				  ID   UInt8
				, Col1 Date
			) Engine Memory
		`
	type result struct {
		ColID uint8 `ch:"ID"`
		Col1  time.Time
	}
	conn.Exec("DROP TABLE test_date")
	defer func() {
		conn.Exec("DROP TABLE test_date")
	}()
	_, err = conn.Exec(ddl)
	require.NoError(t, err)
	scope, err := conn.Begin()
	require.NoError(t, err)
	batch, err := scope.Prepare("INSERT INTO test_date")
	require.NoError(t, err)
	// date, err := time.Parse("2006-01-02 15:04:05", "2022-01-12 00:00:00")
	CurrentLoc, _ := time.LoadLocation("Asia/Shanghai")
	date, err := time.ParseInLocation("2006-01-02 15:04:05", "2022-01-12 00:00:00", CurrentLoc)
	require.NoError(t, err)
	_, err = batch.Exec(uint8(1), date)
	require.NoError(t, err)
	require.NoError(t, scope.Commit())
	var (
		result1 result
	)
	require.NoError(t, conn.QueryRow("SELECT * FROM test_date WHERE ID = $1", 1).Scan(
		&result1.ColID,
		&result1.Col1,
	))
	require.Equal(t, date, result1.Col1)
	assert.Equal(t, "UTC", result1.Col1.Location().String())
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants