Incorrect date time value returned

[fineol]

Describe the bug
When an application using this driver is running on a system that is configured for a local time zone that uses daylight savings time, the driver returns incorrect values for certain non-daylight savings times stored in the database.

For example, when this driver is running on a system with the local time zone location set to "America/New_York", and it retrieves the datetime value 2021-03-14 02:30:00 from a database, this driver will incorrectly return 2021-03-14 01:30:00 for the Go time.Time value.

To Reproduce
Open a connection to a SQL database (in my case I used SQL Server) and execute the following:

	// Ensure test is run with local timezone location set to New York (it is normally set via the OS)
	tz, err := time.LoadLocation("America/New_York")
	if err != nil {
		log.Fatalf("Error loading New York time zone: %v", err)
	}
	time.Local = tz
	r, err := db.Query("declare @now datetime;set @now = '2021-03-14 02:30:00';select @now;")
	if err != nil {
		log.Fatalf("Error executing database query: %v", err)
	}
	var t time.Time
	r.Next()
	if err := r.Scan(&t); err != nil {
		log.Fatalf("Error scanning results: %v", err)
	}
	r.Close()
	fmt.Println(t)

Observe that the returned value 2021-03-14 01:30:00 -0500 EST does not match the value 2021-03-14 02:30:00 selected in the embedded SQL query.

Expected behavior
The returned value has the time component 2:30 AM.

Additional information

The problem is caused by the interaction of daylight savings time with the hard-coded use of time.Local in the Value function defined in column.go, as seen here at line 150 but also in other locations in the file:

odbc/column.go

Lines 146 to 151 in f0492df

	case api.SQL_C_TYPE_TIMESTAMP:
	t := (*api.SQL_TIMESTAMP_STRUCT)(p)
	r := time.Date(int(t.Year), time.Month(t.Month), int(t.Day),
	int(t.Hour), int(t.Minute), int(t.Second), int(t.Fraction),
	time.Local)
	return r, nil

The date-time 2021-03-14 02:30:00 used within the code that reproduces this bug is perfectly valid in UTC or any other time zone that does not include a daylight savings time change on 2021-03-14 (e.g. Australia/Perth, Asia/Tokyo, Asia/Seoul, America/Argentina/Buenos_Aires, America/Phoenix, Pacific/Honolulu, and many others).

However, it is not a valid time for a time zone that observes a daylight savings change on 2021-03-14 (e.g. America/New_York). The hour from 2:00 AM to 3:00 AM is skipped in such time zones. Therefore, when this driver attempts to create the time using a local daylight savings time zone, Go's time package adjusts the value to a valid time on that date in that time zone, but a time that does not match the database value.

This is not a bug in Go's time package. It is doing the best it can when presented with invalid/ambiguous time data for the specified time.Location.

This is as bug in this driver, because it prevents a designer from accurately retrieving date-time values stored in the database under some circumstances.

Suggested solutions
One way to solve this would be to substitute time.UTC for time.Local throughout the Value function in column.go. Designers could still face the problem of the returned time.Time values not having the desired time zone setting, but at least the time component of the values would always match what was stored in the database, and the designers could then rezone the values as needed.

Of course that could cause problems for people who have already adapted to the current behavior and don't see the bug because all of their systems are running with the same local time zone. Therefore, another solution would be to make the time zone location configurable, preferably on a connection basis, as suggested here and here.

[alexbrainman]

@fineol thank you for creating this issue.

Describe the bug

I don't consider this a bug.

Since Microsoft SQL server does not store timezone in dates, I had to put some value into time.Time timezone when reading SQL Server data.

The only choices I had were time.Local and time.UTC. Both of these choices are bad. I picked time.Local.

For example, I have both UTC and local time columns in one of my databases. So I have to convert them when reading.

But I agree with you, some people might get away with just having time.UTC, instead of time.Local in this package. Perhaps something like you suggested - #117 (comment)

Maybe we can add new Driver.Loc field

type Driver struct {
        Stats
        Loc *time.Location    // Location used for all time.Time values

        // Has unexported fields.
}

with Loc set to time.Local so we don't break any existing users.

But I don't need this feature. So you have to implement it yourself. And the change will need some test to verify that change actually work. I will accept a PR, if you want to implement this.

Thank you.

Alex

[fineol]

Yes, many database date-time types such as SQL Server's datetime and datetime2 do not store time zone information. And yes, Go forces us to supply a location when creating a time.Time variable. Therefore, we have to pick something, and time.Local and time.UTC are the two most readily usable choices. Neither are ideal for all circumstances, but I wouldn't go so far as to say both are bad.

However, I still contend that use of time.Local creates a bug in some circumstances. It may not show up at all for some people due to the time zones they operate in. And even for those who are susceptible, it may not show up in practice due to the relative rarity of the problematic dates. But it is real, and I will try to demonstrate more clearly.

You mentioned that you have both UTC and local time columns in one of your databases. Let's concentrate on one of the UTC columns and assume that it contains the values 2021-03-14 01:30:00 and 2021-03-14 02:30:00. I'll simulate a query of this data with the following T-SQL code:

declare @utc1 datetime, @utc2 datetime;
set @utc1 = '2021-03-14 01:30:00';
set @utc2 = '2021-03-14 02:30:00';
select @utc1 union all select @utc2;

When you run the above in SQL Server Management Studio, you get the following results, as expected:

2021-03-14 01:30:00.000
2021-03-14 02:30:00.000

Next, let me revise my test code to extract all the rows from the result set. I'll also wrap it in a function that accepts a time zone location string to make it easy to simulate different local times:

func DemoBug(db *sql.DB, tzs string) {

	tz, err := time.LoadLocation(tzs)
	if err != nil {
		log.Fatalf("Error loading time zone: %v", err)
	}
	time.Local = tz
	r, err := db.Query("declare @utc1 datetime, @utc2 datetime;" +
		"set @utc1 = '2021-03-14 01:30:00';" +
		"set @utc2 = '2021-03-14 02:30:00';" +
		"select @utc1 union all select @utc2;")
	if err != nil {
		log.Fatalf("Error executing database query: %v", err)
	}
	var t time.Time
	for r.Next() {
		if err := r.Scan(&t); err != nil {
			log.Fatalf("Error scanning results: %v", err)
		}
		fmt.Println(t)
	}
	r.Close()
}

If I run the code with the local time zone set to UTC, I get the correct results exactly:

DemoBug(db, "UTC")

2021-03-14 01:30:00 +0000 UTC
2021-03-14 02:30:00 +0000 UTC

If I run the code in a local time zone that does not implement daylight savings, such as Perth, Australia, I get the wrong time zone, but the time components are correct and, as you indicate you do in your code, I could convert them to the right time zone if desired:

DemoBug(db, "Australia/Perth")

2021-03-14 01:30:00 +0800 AWST
2021-03-14 02:30:00 +0800 AWST

However, if I run the code in a local time zone that does implement daylight savings, such as New York, USA, I get not only the wrong time zone, but I also get the wrong hour in the second row:

DemoBug(db, "America/New_York")

2021-03-14 01:30:00 -0500 EST
2021-03-14 01:30:00 -0500 EST

That second row is where the bug reveals itself. The value is off by an hour. If you inspect the private fields of the time.Time struct for both values, you will see that they are identical. Both have wall = 0 and ext = 63751300200. Thus, there is no way to distinguish them and convert the second to the correct value.

For completeness, here is a variant that rezones the values to demonstrate that changing the time zones of the returned incorrect value doesn't fix the problem:

func Rezone(t time.Time, loc *time.Location) time.Time {

	y, o, d := t.Date()
	h := t.Hour()
	m := t.Minute()
	s := t.Second()
	n := t.Nanosecond()

	return time.Date(y, o, d, h, m, s, n, loc)
}

func DemoBugWithRezone(db *sql.DB, tzs string) {

	tz, err := time.LoadLocation(tzs)
	if err != nil {
		log.Fatalf("Error loading time zone: %v", err)
	}
	time.Local = tz
	r, err := db.Query("declare @utc1 datetime, @utc2 datetime;" +
		"set @utc1 = '2021-03-14 01:30:00';" +
		"set @utc2 = '2021-03-14 02:30:00';" +
		"select @utc1 union all select @utc2;")
	if err != nil {
		log.Fatalf("Error executing database query: %v", err)
	}
	var t time.Time
	for r.Next() {
		if err := r.Scan(&t); err != nil {
			log.Fatalf("Error scanning results: %v", err)
		}
		fmt.Println(Rezone(t, time.UTC))
	}
	r.Close()
}

With rezoning, the code works perfectly when you are in a non-daylight savings area such as Perth. Both the time and the location are as desired:

DemoBugWithRezone(db, "Australia/Perth")

2021-03-14 01:30:00 +0000 UTC
2021-03-14 02:30:00 +0000 UTC

But the user in New York still has problems:

DemoBugWithRezone(db, "America/New_York")

2021-03-14 01:30:00 +0000 UTC
2021-03-14 01:30:00 +0000 UTC

Maybe we can add new Driver.Loc field

That would be the simplest solution with the least impact to the codebase. It isn't as flexible as making the choice configurable at the connection level as NickTaylor and I suggested, but it might be the best first step.

But I don't need this feature. So you have to implement it yourself. And the change will need some test to verify that change actually work. I will accept a PR, if you want to implement this.

Understood, and I appreciate your willingness to accept a PR. At this point I am not likely to use this driver for the project I was considering, so if I am able to work on it, it will have to be on my own time.

[alexbrainman]

At this point I am not likely to use this driver for the project I was considering, ...

Then this is not a problem for you.

Alex