Comments (27)
Just for the record, here's some code from @chris-baynes to make this a little bit more comfortable, reproduced here incase the Gist disappears:
// similar to sql.Row but without the error field
type DbRowFromInsert struct {
rows *sql.Rows
}
// Using sql.Row.Scan() as a template for this method, but modifying the errors.
func (row *DbRowFromInsert) Scan(dest ...interface{}) error {
defer row.rows.Close()
row.rows.Next()
// There may be no rows, but Scan anyway to get the errors...
// If there are no rows because of a db constraint error this is when those errors will be returned.
err := row.rows.Scan(dest...)
if err != nil {
return err
}
return nil
}
// Usage:
func InsertReturning(query string, args ...interface{}) (*DbRowFromInsert, error) {
rows, err := dbConn.Query(query, args...)
if err != nil {
return nil, err
}
return &DbRowFromInsert{rows: rows}, nil
}
from pq.
I've had a look through the code and, from my understanding, think this would be possible in the following way.
Exec
would allow INSERT ... RETURNING
queries. Either the query returns a single row and single column (which is then set on result lastInsertId), or no result rows are returned, or an error is set.
This would mean some changes to the result
struct, and require handling of result rows in Exec
. Do you think this is a feasible/sensible approach?
from pq.
Interesting. I'll have to do some digging, I wonder if some database/sql
mandated behavior is masking over any errors propagated from Postgres, since no rows in result set
doesn't look like a Postgres-derived error to me.
from pq.
@chris-baynes What's wrong with using Exec()? The Result type has a LastInsertId() method that should give you the result from the INSERT ... RETURNING queries, or am I not understanding?
The Result type or Exec signature in the go standard library is not going to change because of the Go API guarantee.
from pq.
I've figured out a solution to this. Instead of using QueryRow(), I'm using Query() and getting the first row. I actually use a struct similar to sql.Row to do the work for me. Here's how it looks:
https://gist.github.com/chris-baynes/5361728
@kisielk As I already mentioned, Exec() does not return any rows, and hence no id from the insert if everything went well. LastInsertId() is not implemented in this package.
from pq.
I'm having the same issue.
@kisielk LastInsertId
doesn't appear to be implemented in pq. I can use Exec
and it'll give me the correct error when one occurs, but when everything goes ok LastInsertId
errors with Error: no LastInsertId available
.
If you look at the various Exec
methods, none of them seem to set the Result
return value.
func (st *stmt) Exec(v []driver.Value) (res driver.Result, err error) {
defer errRecover(&err)
if len(v) == 0 {
return st.cn.simpleQuery(st.query)
}
st.exec(v)
for {
t, r := st.cn.recv1()
switch t {
case 'E':
err = parseError(r)
case 'C':
res = parseComplete(r.string())
case 'Z':
// done
return
case 'T', 'N', 'S', 'D':
// Ignore
default:
errorf("unknown exec response: %q", t)
}
}
panic("not reached")
}
from pq.
@jagregory Yes, that was a misunderstanding on my part. There is no reasonable way to implement LastInsertId in pq.
from pq.
I think I see why this may not work with QueryRow correctly.
The high level logic going on in database/sql is something like:
rows, err := db.Query("...")
if err != nil {
return err
}
if !rows.Next() {
return ErrNoRows
}
err := rows.Scan(val)
return err
I haven't verified yet, but I think in the case of an InsertError pq will not tell you about it until you call rows.Err, which happens after rows.Next returns false, by which point QueryRow has already returned ErrNoRows.
from pq.
Have you tried go tip? I can't reproduce this there.
func TestInsertError(t *testing.T) {
db := openTestConn(t)
defer db.Close()
_, err := db.Exec("CREATE TEMP TABLE test (i SERIAL PRIMARY KEY, v text)")
if err != nil {
t.Fatal(err)
}
r := db.QueryRow("INSERT INTO test VALUES (DEFAULT, 'hello') RETURNING i")
var i int
err = r.Scan(&i)
if err != nil {
t.Fatal(err)
}
if i != 1 {
t.Fatalf("bad value for i: got %d, want 1", i)
}
r = db.QueryRow("INSERT INTO test VALUES (1, 'world') RETURNING i")
err = r.Scan(&i)
if err == nil {
t.Fatal("expected an error")
}
_, ok := err.(PGError)
if !ok {
t.Fatalf("expected PGError, got: %#v", err)
}
t.Log(err)
}
The log message at the bottom gives:
conn_test.go:444: pq: S:"ERROR" C:"23505" M:"duplicate key value violates unique constraint \"test_pkey\"" D:"Key (i)=(1) already exists." F:"nbtinsert.c" L:"396" R:"_bt_check_unique"
which is as expected.
from pq.
Actually even looking at the code for Row.Scan in tip, it hasn't changed much since 1.1. Do you have some code you can share that reproduces the problem? It seems to work fine in the example I pasted above.
from pq.
I'll investigate some more, thanks for taking the time to look into it. I'm on Go 1.1.2. When I tried it earlier, I was getting sql.ErrNoRows
instead of the constraint error.
from pq.
Ah, this was my mistake. I was running a version of pq from a few weeks ago. It looks like somebody has fixed it up since then! Thanks for the help.
from pq.
Cool. I added a test in 6cc5689 that should catch any regressions in this.
from pq.
I'm currently using master at 7876026 and seeing a regression here, I think.
I know there's a test case added that should guard against this regression, but this raises on the second insert for me, returning sql.ErrNoRows
// Inserting a row with RETURNING and violating a unique, or foreign key
// constraint should return the *pq.Error, not sql.ErrNoRows, I believe
// the root cause of this issue is inside the database/sql package itself
// as it counts the returned rows before checking for errors returned.
//
// http://golang.org/src/pkg/database/sql/sql.go around line #1424
//
// I had seen mention of this issue somewhere in the lib/pq issue tracker
// and a link to a Gist with a workaround, as well as some discussion about
// how eaigner\s hood ORM had worked around this.
func Test_RaisingPqErrorOnQueryRowConstraintViolation(t *testing.T) {
var returned string
db := openTestConn(t)
defer db.Close()
_, err := db.Exec("CREATE TEMP TABLE pqerronviolateconstraint (a varchar(3) not null UNIQUE)")
if err != nil {
t.Fatal(err)
}
err = db.QueryRow("INSERT INTO pqerronviolateconstraint(a) values($1) RETURNING a", "abc").Scan(&returned)
if err != nil {
t.Fatal(err)
}
err = db.QueryRow("INSERT INTO pqerronviolateconstraint(a) values($1) RETURNING a", "abc").Scan(&returned)
if err == sql.ErrNoRows {
t.Fatal(err)
}
}
from pq.
The full results of $ go test [-v]
included below for completeness:
➜ pq git:(master) ✗ go test
Skipping failing test: see https://github.com/lib/pq/issues/148
--- FAIL: Test_RaisingPqErrorOnQueryRowConstraintViolation (0.01 seconds)
known_issues_test.go:66: sql: no rows in result set
FAIL
exit status 1
FAIL _/Users/leehambley/Projects/leehambley/pq 0.304s
➜ pq git:(master) ✗ go test -v
=== RUN TestDecodeBool
--- PASS: TestDecodeBool (0.01 seconds)
=== RUN TestReconnect
--- PASS: TestReconnect (0.04 seconds)
=== RUN TestOpenURL
--- PASS: TestOpenURL (0.01 seconds)
=== RUN TestExec
--- PASS: TestExec (0.02 seconds)
=== RUN TestStatment
--- PASS: TestStatment (0.04 seconds)
=== RUN TestRowsCloseBeforeDone
--- PASS: TestRowsCloseBeforeDone (0.01 seconds)
=== RUN TestEncodeDecode
--- PASS: TestEncodeDecode (0.01 seconds)
=== RUN TestNoData
--- PASS: TestNoData (0.01 seconds)
=== RUN TestError
--- PASS: TestError (0.05 seconds)
=== RUN TestBadConn
--- PASS: TestBadConn (0.00 seconds)
=== RUN TestErrorOnExec
--- PASS: TestErrorOnExec (0.01 seconds)
=== RUN TestErrorOnQuery
--- PASS: TestErrorOnQuery (0.02 seconds)
=== RUN TestErrorOnQueryRow
--- PASS: TestErrorOnQueryRow (0.02 seconds)
=== RUN TestSimpleQuery
--- PASS: TestSimpleQuery (0.02 seconds)
=== RUN TestBindError
--- PASS: TestBindError (0.02 seconds)
=== RUN TestReturning
--- PASS: TestReturning (0.03 seconds)
=== RUN TestParseEnviron
--- PASS: TestParseEnviron (0.00 seconds)
=== RUN TestExecerInterface
--- PASS: TestExecerInterface (0.00 seconds)
=== RUN TestNullAfterNonNull
--- PASS: TestNullAfterNonNull (0.01 seconds)
=== RUN Test64BitErrorChecking
--- PASS: Test64BitErrorChecking (0.02 seconds)
=== RUN TestRollback
--- PASS: TestRollback (0.03 seconds)
=== RUN TestParseOpts
--- PASS: TestParseOpts (0.00 seconds)
=== RUN TestRuntimeParameters
--- PASS: TestRuntimeParameters (0.14 seconds)
=== RUN TestIsUTF8
--- PASS: TestIsUTF8 (0.00 seconds)
=== RUN TestXactMultiStmt
--- SKIP: TestXactMultiStmt (0.00 seconds)
conn_xact_test.go:12: Skipping failing test
=== RUN TestScanTimestamp
--- PASS: TestScanTimestamp (0.00 seconds)
=== RUN TestScanNilTimestamp
--- PASS: TestScanNilTimestamp (0.00 seconds)
=== RUN TestParseTs
--- PASS: TestParseTs (0.00 seconds)
=== RUN TestTimestampWithTimeZone
--- PASS: TestTimestampWithTimeZone (0.05 seconds)
=== RUN TestTimestampWithOutTimezone
--- PASS: TestTimestampWithOutTimezone (0.02 seconds)
=== RUN TestStringWithNul
--- PASS: TestStringWithNul (0.01 seconds)
=== RUN TestByteToText
--- PASS: TestByteToText (0.01 seconds)
=== RUN TestNo148
Skipping failing test: see https://github.com/lib/pq/issues/148
--- PASS: TestNo148 (0.00 seconds)
=== RUN Test_RaisingPqErrorOnQueryRowConstraintViolation
--- FAIL: Test_RaisingPqErrorOnQueryRowConstraintViolation (0.01 seconds)
known_issues_test.go:66: sql: no rows in result set
=== RUN TestSimpleParseURL
--- PASS: TestSimpleParseURL (0.00 seconds)
=== RUN TestFullParseURL
--- PASS: TestFullParseURL (0.00 seconds)
=== RUN TestInvalidProtocolParseURL
--- PASS: TestInvalidProtocolParseURL (0.00 seconds)
=== RUN TestMinimalURL
--- PASS: TestMinimalURL (0.00 seconds)
FAIL
exit status 1
FAIL _/Users/leehambley/Projects/leehambley/pq 0.639s
from pq.
I think ErrNoRows is correct in this case, from the Postgres docs:
If the INSERT command contains a RETURNING clause, the result will be similar to that of a SELECT statement containing the columns and values defined in the RETURNING list, computed over the row(s) inserted by the command.
Since the query doesn't actually insert anything because of the unique constraint, there are no rows over which RETURNING is computed.
from pq.
Sure, but when implemented in terms of an *[sql,tx].Exec
, with a *sql.Result
returned, there's an error returned by the database, specifically, I think the case in database/sql
should be:
- Check for errors, return if any.
- Try and read a row. (or return
sql.ErrNoRows
, which is where this process starts, currently) - Return error if there's a problem reading a row.
It seems like the Go driver checks for no rows, before checking if there was an error with the query, which in theory, is backwards, as they get an error result, but don't check it until after examining the (possibly bogus) rows object they get back... at least that's my take. Probably the subtle database semantics between MySQL and PostgreSQL etc don't really apply here, and I will just have to wrap this in my code to make it less ugly.
Given the complexity, I was really hoping to lean on QueryRow
rather than having to do all the complex error checking (err on query?, check for rows, error on checking rows?)
I'm also curious as to why the TestErrorOnQueryRow
test case passes, as I suppose that also returns no rows, but you are explicitly raising the error there, whereas the error is implicit in my example code.
from pq.
Hm yes, if Query is returning an error in this case, then QueryRow probably should too. It's probably too late to fix in Go 1.2 but you could file an issue in the Go tracker.
from pq.
Right, if I do open an issue, I'll mention it here for posterity, thanks for the second opinion. Time to code a workaround!
from pq.
I'm also curious as to why the TestErrorOnQueryRow test case passes, as I suppose that also returns no rows, but you are explicitly raising the error there, whereas the error is implicit in my example code.
The difference is probably that TestErrorOnQueryRow uses the simpleExec path, whereas your code does a full prepare/execute.
from pq.
FWIW, this looks like a bug in database/sql to me. What it does it this:
if !r.rows.Next() {
return ErrNoRows
}
And Rows.Next() looks like this:
// Next prepares the next result row for reading with the Scan method.
// It returns true on success, false if there is no next result row.
// Every call to Scan, even the first one, must be preceded by a call
// to Next.
func (rs *Rows) Next() bool {
if rs.closed {
return false
}
if rs.lasterr != nil {
return false
}
if rs.lastcols == nil {
rs.lastcols = make([]driver.Value, len(rs.rowsi.Columns()))
}
rs.lasterr = rs.rowsi.Next(rs.lastcols)
if rs.lasterr == io.EOF {
rs.Close()
}
return rs.lasterr == nil
}
So not checking rs.lasterr after calling Next() looks to be the cause of this, and a pretty obvious bug if you ask me.
from pq.
Do either of you guys have any experience filing bugs with the Go issue tracker, I wonder how they'd respond to being told that that there's extensive discussion about it at Github?!
You're right, though they should switch on the result of rs.lasterr = rs.rowsi.Next(rs.lastcols)
6 lines from the bottom, I guess, otherwise they miss when the database has thrown a constraint violation error.
from pq.
I've filed lots of issues there. It's best to just put the relevant points in the issue description.
from pq.
I've filed lots of issues there. It's best to just put the relevant points in the issue description.
Would you mind filing a report? It would be really great to get this fixed in 1.2 if possible, and I think you're in a better time zone for that than I am.
from pq.
Maybe @leehambley can do it? He seems to be most familiar with the details and I'd rather not have to act as an intermediary :)
from pq.
Done, issue filed at https://code.google.com/p/go/issues/detail?id=6651&thanks=6651&ts=1382563078
from pq.
I've just faced the same problem. Solution described here https://gist.github.com/chris-baynes/5361728 is nice, but currently I'm trying to write SQL-transaction with RETURNING query and I'm stuck with it.
This is an example of SQL-transactions which I tried to use https://pseudomuto.com/2018/01/clean-sql-transactions-in-golang/ and an Exec() method was used there. So same story, but my knowledge doesn't allow me to implement something like in the first link. And I'm looking for help, please.
from pq.
Related Issues (20)
- Tx.Rollback returns ErrBadConn after context is cancelled
- Connection string parsing fails randomly HOT 2
- 1 Listener for 2 channel HOT 1
- SonarCloud Vulnerability: Make sure this PostgreSQL database password gets changed and removed from the code.
- Usage of COPY TO STDOUT WITH CSV
- RFC 9266: Channel Bindings for TLS 1.3 support
- Query extra_float_digits problem
- Is Postgres15.4 is compatible with the lib/pq package?
- What kind of mistake is this?
- how to deal with "cached plan must not change result type" error?
- Looking for a security contact to disclose a vulnerability HOT 1
- Exectuing pq.CopyIn fails with one of two errors depending on usage
- Why do I have errors in local language?
- pq: invalid array literal.
- pq: invalid input syntax for type integer HOT 2
- Private keys cause Trivy HIGH vulnerability alerts HOT 4
- Nil byte array doesn't get saved as JSONB field
- nil byte array `[]byte(nil)` is encoded as empty byte array `[]byte([])` instead of nil
- Extra DNS name verification is made in `verify-ca` mode, making it effectively `verify-full` HOT 1
- Got error pq: syntax error at the end of ... when doing custom dialer
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from pq.