sql: panic in yet undefined circumstances

[knz]

So I was trying to investigate why I couldn't see any result in my table using a 2nd client despite INSERT working as intended in the 1st client. Since my 1st client deletes the test table at the end of its execution, I paused the 1st client while running the 2nd.

Then at some point I try to resume the 1st client and the connection is immediately dropped; and I see the following panic in the server:

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x38 pc=0xc22132]

goroutine 6210 [running]:
panic(0x1592aa0, 0xc82000e150)
        /home/kena/go1.6/src/runtime/panic.go:464 +0x3e6
github.com/cockroachdb/cockroach/sql/parser.WalkExpr(0x7f068616f520, 0xc820c89940, 0x7f06861701e8, 0xc820ba6f80, 0x0, 0x0, 0xc8201ce800)
        /home/kena/go/src/github.com/cockroachdb/cockroach/sql/parser/walk.go:387 +0x92
github.com/cockroachdb/cockroach/sql/parser.(*ComparisonExpr).Walk(0xc8207cf5c0, 0x7f068616f520, 0xc820c89940, 0x0, 0x0)
        /home/kena/go/src/github.com/cockroachdb/cockroach/sql/parser/walk.go:161 +0xce
github.com/cockroachdb/cockroach/sql/parser.WalkExpr(0x7f068616f520, 0xc820c89940, 0x7f0686170228, 0xc8207cf5c0, 0x7f0686170228, 0xc8207cf5c0, 0x0)
        /home/kena/go/src/github.com/cockroachdb/cockroach/sql/parser/walk.go:387 +0x98
github.com/cockroachdb/cockroach/sql/parser.(*Select).WalkStmt(0xc8201ec140, 0x7f068616f520, 0xc820c89940, 0x0, 0x0)
        /home/kena/go/src/github.com/cockroachdb/cockroach/sql/parser/walk.go:543 +0x26e
github.com/cockroachdb/cockroach/sql/parser.WalkStmt(0x7f068616f520, 0xc820c89940, 0x7f068616f8a0, 0xc8201ec140, 0x0, 0x0, 0x126)
        /home/kena/go/src/github.com/cockroachdb/cockroach/sql/parser/walk.go:710 +0xbc
github.com/cockroachdb/cockroach/sql/parser.FillArgs(0x7f068616f8a0, 0xc8201ec140, 0x7f068616f4f8, 0xc820089330, 0x0, 0x0, 0x0, 0x0)
        /home/kena/go/src/github.com/cockroachdb/cockroach/sql/parser/walk.go:781 +0xf3
github.com/cockroachdb/cockroach/sql.(*Executor).execStmt(0xc8200ac400, 0x7f068616f8a0, 0xc8201ec140, 0xc820088d00, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/kena/go/src/github.com/cockroachdb/cockroach/sql/executor.go:473 +0x501
github.com/cockroachdb/cockroach/sql.(*Executor).execStmts(0xc8200ac400, 0xc820d69ace, 0x21, 0xc820088d00, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/kena/go/src/github.com/cockroachdb/cockroach/sql/executor.go:378 +0x327
github.com/cockroachdb/cockroach/sql.(*Executor).ExecuteStatements(0xc8200ac400, 0xc820cb6009, 0x4, 0xc820cb6017, 0x6, 0x0, 0xc8201cf6c0, 0x0, 0x0, 0x0, ...)
        /home/kena/go/src/github.com/cockroachdb/cockroach/sql/executor.go:325 +0x6ec
github.com/cockroachdb/cockroach/sql/pgwire.(*v3Conn).executeStatements(0xc820cbd880, 0xc820d69ace, 0x21, 0xc820ba6ef0, 0x1, 0x1, 0xc820ba6f00, 0x4, 0x4, 0x0, ...)
        /home/kena/go/src/github.com/cockroachdb/cockroach/sql/pgwire/v3.go:555 +0xf1
github.com/cockroachdb/cockroach/sql/pgwire.(*v3Conn).handleExecute(0xc820cbd880, 0xc820cbd8b8, 0x0, 0x0)
        /home/kena/go/src/github.com/cockroachdb/cockroach/sql/pgwire/v3.go:547 +0x329
github.com/cockroachdb/cockroach/sql/pgwire.(*v3Conn).serve(0xc820cbd880, 0x0, 0x0, 0x0)
        /home/kena/go/src/github.com/cockroachdb/cockroach/sql/pgwire/v3.go:267 +0xec3

I can reproduce this panic reliably using the scenario outlined above.

[maddyblue]

What rev is this on?

[knz]

Good question. That is the rev you get by merging your branch sql-fix-param-returning on top of Radu's fix for RETURNING *.

[knz]

i.e.

git checkout master
git pull https://github.com/RaduBerinde/cockroach.git returning-star
git pull https://github.com/mjibson/cockroach.git sql-fix-param-returning

then changing return result into return rh.getResults()

[RaduBerinde]

383 func WalkExpr(v Visitor, expr Expr) (newExpr Expr, changed bool) {
384     recurse, newExpr := v.VisitPre(expr)
385 
386     if recurse {
387         newExpr = newExpr.Walk(v)       <----------

Looks like argVisitor.VisitPre (used by FillArgs) is returning nil. I don't think it's ever supposed to return nil. Visitors definitely can't return nil and recurse = true

[knz]

(While looking at #4036 )

[andreimatei]

Hey @mjibson do you think you're the guy to look at this?

[maddyblue]

sure

[maddyblue]

I think this will be fixed by #4767. @knz could you pull the branch in that PR and see if it works?

[maddyblue]

@knz That PR got merged. Could you check to see if this error is still happening on master?

[knz]

Nope, can't reproduce any more. Looks like this was fixed! 👍