sijms / go-ora

Pure go oracle client
MIT License
795 stars 176 forks source link

Fix local timezone #550

Closed rocksnow1942 closed 5 months ago

rocksnow1942 commented 5 months ago

A small followup to fix https://github.com/sijms/go-ora/issues/546

sijms commented 5 months ago

can you explain

case DATE, TIMESTAMP, TIMESTAMP_DTY:
    par.oPrimValue = tempTime.In(conn.dbTimeZone)

all this type store datetime without timezone information. so you now returning the value in timezone? the original code return the value as it is and just append dbTimeZone (suggested by someone). before I just leave timezone information empty (default is UTC)

rocksnow1942 commented 5 months ago

Thanks for pointing out. Yes, you are right, I thought about it more, and changed to:

    par.oPrimValue = time.Date(tempTime.Year(), tempTime.Month(), tempTime.Day(),
        tempTime.Hour(), tempTime.Minute(), tempTime.Second(), tempTime.Nanosecond(), time.Local)

This way, the DATE, TIMESTAMP, TimeStampDTY are set to the system time zone. This is the same behavior as before commit bbeff0e.

Here is why:

Before commit bbeff0e, getDBTimeZone query SELECT SYSTIMESTAMP FROM DUAL to get conn.dbTimeLoc. "SYSTIMESTAMP" uses system timezone. (see oracle doc here)

SYSTIMESTAMP returns the system date, including fractional seconds and time zone, of the system on which the database resides. The return type is TIMESTAMP WITH TIME ZONE.

So in v2/parameter.go, under case DATE, TIMESTAMP, TimeStampDTY, the time stamp appended with system timezone (conn.dbTimeLoc).

After commit bbeff0e, that is no longer the case. conn.dbTimeZone is the DBTIMEZONE, which can be different from system timezone. So this PR changes to append the time with time.Local, which is the system local time zone.

sijms commented 5 months ago

ok time.Local will use client timezone so better to return the previous variable dbTimeZone in new name dbServerTimeZone and restore its function

sijms commented 5 months ago

I make these changes in last commit

sijms commented 5 months ago

database server timezone +03:00 database timezone: +00:00 the result of examplels/time_issue

DATE:                     2024-04-28 06:27:23 +0300 +03:00
Timestamp:                2024-04-28 06:27:23.9229 +0300 +03:00
Timestamp TZ:             2024-04-28 11:27:23.9229 +0800 CST
Timestamp with local TZ:  2024-04-28 03:27:23.922901 +0000 +00:00

this confirmed by

select SYSTIMESTAMP AT TIME ZONE DBTIMEZONE from dual;

the result: 28/04/24 03:22:23.563000000 AM GMT from sql developer

rocksnow1942 commented 5 months ago

Thanks, will you make a new release anytime soon? I can test your new version.

rocksnow1942 commented 5 months ago

I noticed in your latest commit you didn't fix TimeStampeLTZ, I think the following is still needed.

case TimeStampeLTZ, TimeStampLTZ_DTY:
    tempTime, err := converters.DecodeDate(par.BValue)
    if err != nil {
        return err
    }
    par.oPrimValue = tempTime
    if conn.dbTimeZone != time.UTC {
        par.oPrimValue = time.Date(tempTime.Year(), tempTime.Month(), tempTime.Day(),
            tempTime.Hour(), tempTime.Minute(), tempTime.Second(), tempTime.Nanosecond(), conn.dbTimeZone)
    }

Your test result look correct because the database timezone is +00:00.

rocksnow1942 commented 5 months ago

Thanks! When should I expect a new release?

sijms commented 5 months ago

within days. I will fix other issues and make the release