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

formatTime sends empty timezone #759

Closed
mayamika opened this issue Sep 21, 2022 · 3 comments · Fixed by #766
Closed

formatTime sends empty timezone #759

mayamika opened this issue Sep 21, 2022 · 3 comments · Fixed by #766

Comments

@mayamika
Copy link

Issue description

formatTime
toDateTime doc

Can't we just send Unix time for all cases?

When i use time.Parse with RFC3339Nano format with explicit timezone, the time.Location's String method returns empty string, then clickhouse-go uses this empty string and sends to clickhouse server empty timezone parameter. As a result i got wrong time in query parameter.

Example code

package main

import (
	"context"
	"fmt"
	"os"
	"time"

	"github.com/ClickHouse/clickhouse-go/v2"
)

func main() {
	// Parse time where +04:00 is not local time.
	timeWant, err := time.Parse(time.RFC3339Nano, "2022-09-15T17:06:31.81718722+04:00")
	if err != nil {
		panic(err)
	}
	// You must get time difference, because clickhouse-server parses this as UTC time.
	fmt.Println("With timezone:")
	testWith(timeWant)

	// Now test with t.Local()
	// No time difference, because clickhouse-go sends unix time.
	fmt.Println("With local time:")
	testWith(timeWant.Local())
}

func testWith(timeWant time.Time) {
	fmt.Printf("Location string: %v\n",
		timeWant.Location().String(),
	)

	conn, err := clickhouse.Open(&clickhouse.Options{
		Addr: []string{os.Getenv("CLICKHOUSE_ADDR")},
		Auth: clickhouse.Auth{
			Database: os.Getenv("CLICKHOUSE_DATABASE"),
			Username: os.Getenv("CLICKHOUSE_USERNAME"),
			Password: os.Getenv("CLICKHOUSE_PASSWORD"),
		},
	})
	if err != nil {
		panic(err)
	}
	defer func() { _ = conn.Close() }()

	date := clickhouse.DateNamed("Time", timeWant, clickhouse.NanoSeconds)
	r := conn.QueryRow(context.TODO(), "SELECT @Time", date)

	var timeGot time.Time
	if err := r.Scan(&timeGot); err != nil {
		panic(fmt.Errorf("scan: %w", err))
	}

	if timeGot.Unix() != timeWant.Unix() {
		fmt.Printf("wrong: got = %v, want = %v\n", timeGot.Local(), timeWant.Local())
	} else {
		fmt.Println("ok")
	}
}

Configuration

OS:

Interface: native

Driver version: latest

Go version: 1.19

ClickHouse Server version: 22.3

@gingerwizard
Copy link
Collaborator

#719

@gingerwizard gingerwizard changed the title formatTime sends empty timzeone formatTime sends empty timezone Sep 23, 2022
@gingerwizard
Copy link
Collaborator

Couple of things here @mayamika

  • We should consider the tz if its in the string see DateTime handling - consistency with client specification #719 item 2. We need to fix.
  • If no tz is set for a time.Time{} (which in your case it isn't because you've parsed with time.RFC3339Nano) we will use the tz of the client. Not sure this addresses your problem but we wont just assume its UTC if that's what you hoped for.

@gingerwizard
Copy link
Collaborator

Trival fix will include in PR

func formatTime(tz *time.Location, scale TimeUnit, value time.Time) (string, error) {
	switch value.Location().String() {
	case "Local", "":

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

Successfully merging a pull request may close this issue.

2 participants