lib / pq

Pure Go Postgres driver for database/sql
https://pkg.go.dev/github.com/lib/pq
MIT License
9.09k stars 911 forks source link

better error messaging for query parameters exceeding 2^16 values? #295

Closed bfallik closed 10 years ago

bfallik commented 10 years ago

We recently encountered an issue from a pathological query that used >100k query parameters. The result from the driver was an error message like:

runAssignment(): pq: got 386795 parameters but the statement requires 59115

which comes from https://github.com/lib/pq/blob/master/conn.go#L917. It looks like st.paramTyps is at most 2^16 in length from https://github.com/lib/pq/blob/master/conn.go#L523.

I imagine the 2^16 limit is a limitation of the wire format but I'm wondering if pq could provide better error messaging? The "statement requires" value (59115 above) is wrong but it would have been even more helpful to have an error like "got 386795 parameters but the statement is limited to 65536".

Thanks!

johto commented 10 years ago

I imagine the 2^16 limit is a limitation of the wire format but I'm wondering if pq could provide better error messaging? The "statement requires" value (59115 above) is wrong but it would have been even more helpful to have an error like "got 386795 parameters but the statement is limited to 65536".

I was expecting postgres to throw an error message in this case, but it looks like it doesn't (even in HEAD). I won't object to displaying a better error message, though.

johto commented 10 years ago

I've committed a fix for this in d88e7934ab23861f5d79c398887550e0093e7b0a. Thanks for the report!

Here's the test case I used, but didn't commit:

diff --git a/conn_test.go b/conn_test.go
index 2120e2e..f8eb4c9 100644
--- a/conn_test.go
+++ b/conn_test.go
@@ -176,6 +176,43 @@ func TestExec(t *testing.T) {
    }
 }

+// Test behaviour with tons of parameters
+func TestTonsOfParameters(t *testing.T) {
+   db := openTestConn(t)
+   defer db.Close()
+
+   createStmt := func(n int) string {
+       sql := "SELECT 1 IN ("
+       for i := 1; i <= n; i++ {
+           if (i > 1) {
+               sql += ", "
+           }
+           sql += fmt.Sprintf("$%d", i)
+       }
+       sql += ")"
+       return sql
+   }
+
+   shouldWork := func (n int) {
+       _, err := db.Exec(createStmt(n), make([]interface{}, n)...)
+       if err != nil {
+           t.Fatalf("could not execute a statement with %d parameters: %s", n, err)
+       }
+   }
+
+   shouldWork(32767)
+   shouldWork(32768)
+   shouldWork(65535)
+   // shouldn't work
+   _, err := db.Exec(createStmt(65536), make([]interface{}, 65536)...)
+   if err == nil {
+       t.Fatal("expected error")
+   }
+   if err.Error() != "pq: got 65536 parameters but PostgreSQL only supports 65535 parameters" {
+       t.Fatalf("unexpected error with 65536 parameters: %s", err)
+   }
+}
+
 func TestStatment(t *testing.T) {
    db := openTestConn(t)
    defer db.Close()
bfallik commented 10 years ago

Thanks @johto

galileo-pkm commented 6 years ago

Got the same type of error with a prepared statement. Maybe it would be a good idea to apply the same fix to prepared statements?

pureugong commented 2 years ago

it would be a good idea to apply the same fix to prepared statements?

I have faced the similar issue with a prepared statement and wasted time to look under the hood. I agree, @galileo-pkm, it would be really helpful if we can apply the same error message on it.