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

bugfix: literalTime use t.UTC() , This behavior is different from the original sql.DB #106

Merged
merged 3 commits into from
Jul 24, 2019

Conversation

chen56
Copy link
Contributor

@chen56 chen56 commented Jul 19, 2019

bugfix: literalTime use t.UTC() , This behavior is different from the original sql.DB.
it change the origin location, so , time insert to database be changed and error

@@ -3,6 +3,7 @@ package goqu
import (
"database/sql/driver"
"fmt"
"github.com/stretchr/testify/require"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File is not goimports-ed (from goimports)

Suggested change
"github.com/stretchr/testify/require"


d.Literal(b.Clear(), &now)
dts.assertNotPreparedSQL(t, b, "'"+now.Format(time.RFC3339Nano)+"'")
d.Literal(b.Clear(), &now)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a reference for the variable on range scope now (from scopelint)


d.Literal(b.Clear(), &now)
dts.assertPreparedSQL(t, b, "?", []interface{}{now})
d.Literal(b.Clear(), &now)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a reference for the variable on range scope now (from scopelint)

@codecov
Copy link

codecov bot commented Jul 19, 2019

Codecov Report

Merging #106 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #106   +/-   ##
=======================================
  Coverage   86.35%   86.35%           
=======================================
  Files          38       38           
  Lines        2800     2800           
=======================================
  Hits         2418     2418           
  Misses        330      330           
  Partials       52       52
Impacted Files Coverage Δ
sql_dialect.go 86.49% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7d8e67...e437c14. Read the comment docs.

@doug-martin doug-martin merged commit d8c6c7a into doug-martin:master Jul 24, 2019
doug-martin added a commit that referenced this pull request Jul 24, 2019
* [FIXED] literalTime use t.UTC() , This behavior is different from the original sql.DB #106 - @chen56
* [ADDED] Add new method WithTx for Database #108 - @Xuyuanp
@doug-martin doug-martin mentioned this pull request Jul 24, 2019
@doug-martin
Copy link
Owner

@chen56 I had to change how this worked.

#163 Shed some light on where the strategy used in this PR breaks. After adding more integration tests the code it turns out this PR made some assumption on about the TZ that the db is running in. In both mysql and postgres the time was stored in UTC silently ignoring the timezone information.

In order to support your use case I added a new method goqu.SetTimeLocation which should allow you to force a timezone incase your database is running in a different timezone.

@doug-martin
Copy link
Owner

Take a look at 9e05f02 I added more documentation about working with timezones in `goqu.

@chen56
Copy link
Contributor Author

chen56 commented Sep 25, 2019

i like it

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

Successfully merging this pull request may close these issues.

3 participants