-
Notifications
You must be signed in to change notification settings - Fork 73
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
Rework SET IDENTITY_INSERT for Go 1.10 #156
Conversation
9ac7d4d
to
a8bc4bd
Compare
base_test.go
Outdated
_, err := q.Exec(sql) | ||
require.NoError(t, err) | ||
t := suite.T() | ||
sqlTemplate := fmt.Sprintf("SET IDENTITY_INSERT %s %%s", q.QuoteIdentifier(table)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
G201: SQL string formatting
Codecov Report
@@ Coverage Diff @@
## v1-3 #156 +/- ##
=======================================
Coverage 69.56% 69.56%
=======================================
Files 19 19
Lines 1531 1531
=======================================
Hits 1065 1065
Misses 416 416
Partials 50 50 Continue to review full report at Codecov.
|
} | ||
sql := fmt.Sprintf("SET IDENTITY_INSERT %s %s", q.QuoteIdentifier(table), allowString) | ||
_, err := q.Exec(sql) | ||
query := fmt.Sprintf("SET IDENTITY_INSERT %s %%s", q.QuoteIdentifier(table)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
G201: SQL string formatting
db_test.go
Outdated
s.Require().Error(err) | ||
if err.Error() != `pq: Could not complete operation in a failed transaction` && err.Error() != `commit unexpectedly resulted in rollback` { | ||
s.Failf("unexpected error", "actual: %q", err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AlekseyMartynov What is the reason for that change? What version of PostgreSQL were you using?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My original changeset was based on the mssql-update
branch. So was the original PR (according to AppVeyor logs). As I understand, later you've changed the PR to target v1-3
and something went wrong.
Now, in my opinion, it would be cleaner to cherry-pick the original commit on top of the current v1-3
. There'll be a conflict revealing the source of the unwanted change:
<<<<<<< HEAD
s.NoError(tx.Insert(person1))
s.EqualError(tx.Insert(person1), `pq: duplicate key value violates unique constraint "people_pkey"`)
s.EqualError(tx.Insert(person2), `pq: current transaction is aborted, commands ignored until end of transaction block`)
s.EqualError(tx.Commit(), `pq: Could not complete operation in a failed transaction`)
=======
s.NoError(insertPersonWithID(s, tx.Querier, person1))
s.Contains(insertPersonWithID(s, tx.Querier, person1).Error(), `duplicate key value violates unique constraint "people_pkey"`)
s.Contains(insertPersonWithID(s, tx.Querier, person2).Error(), `current transaction is aborted, commands ignored until end of transaction block`)
err = tx.Commit()
s.Require().Error(err)
if err.Error() != `pq: Could not complete operation in a failed transaction` && err.Error() != `commit unexpectedly resulted in rollback` {
s.Failf("unexpected error", "actual: %q", err)
}
>>>>>>> bea984f... Rework SET IDENTITY_INSERT for Go 1.10
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolves #151.