search

gobuffalo / pop

A Tasty Treat For All Your Database Needs
MIT License
1.44k stars 243 forks source link

Running an empty RawQuery causes a panic #560

Closed aeneasr closed 2 years ago

aeneasr commented 4 years ago

Description

Running an empty RawQuery causes a panic.

Steps to Reproduce the Problem

sqlite, _ := pop.NewConnection(&pop.ConnectionDetails{
    URL: "sqlite3://" + filepath.Join(os.TempDir(), x.NewUUID().String()) + ".sql?mode=memory&_fk=true",
})
_ = sqlite.Open()
sqlite.RawQuery("").Exec()

Expected Behavior

No panic.

Actual Behavior

A panic happens:

% go test -tags sqlite .
--- FAIL: TestMigrations (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x46c74f2]

goroutine 10 [running]:
testing.tRunner.func1.1(0x4d86060, 0x5973aa0)
        /usr/local/Cellar/go/1.14.3/libexec/src/testing/testing.go:940 +0x2f5
testing.tRunner.func1(0xc0001c58c0)
        /usr/local/Cellar/go/1.14.3/libexec/src/testing/testing.go:943 +0x3f9
panic(0x4d86060, 0x5973aa0)
        /usr/local/Cellar/go/1.14.3/libexec/src/runtime/panic.go:969 +0x166
github.com/gobuffalo/pop/v5.(*Model).TableName(0x0, 0x0, 0x0)
        /Users/foobar/go/src/github.com/gobuffalo/pop/model.go:79 +0x52
github.com/gobuffalo/pop/v5.(*sqlBuilder).buildColumns(0xc0001fb868, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
        /Users/foobar/go/src/github.com/gobuffalo/pop/sql_builder.go:218 +0x65
github.com/gobuffalo/pop/v5.(*sqlBuilder).buildSelectSQL(0xc0001fb868, 0x0, 0x0)
        /Users/foobar/go/src/github.com/gobuffalo/pop/sql_builder.go:102 +0x40
github.com/gobuffalo/pop/v5.(*sqlBuilder).compile(0xc0001fb868)
        /Users/foobar/go/src/github.com/gobuffalo/pop/sql_builder.go:87 +0x369
github.com/gobuffalo/pop/v5.(*sqlBuilder).String(...)
        /Users/foobar/go/src/github.com/gobuffalo/pop/sql_builder.go:57
github.com/gobuffalo/pop/v5.Query.ToSQL(0xc0000333b0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /Users/foobar/go/src/github.com/gobuffalo/pop/query.go:206 +0x1db
github.com/gobuffalo/pop/v5.(*Query).Exec.func1(0xbfaffeb74d0bc090, 0xcc6278)
        /Users/foobar/go/src/github.com/gobuffalo/pop/executors.go:25 +0x91
github.com/gobuffalo/pop/v5.(*Connection).timeFunc(0xc0007a4900, 0x4f07ee0, 0x4, 0xc000079cd0, 0x30, 0x4ded6e0)
        /Users/foobar/go/src/github.com/gobuffalo/pop/connection.go:231 +0x50
github.com/gobuffalo/pop/v5.(*Query).Exec(0xc000079e70, 0xc0000333b0, 0x0)
        /Users/foobar/go/src/github.com/gobuffalo/pop/executors.go:24 +0x6a
github.com/ory/kratos/persistence/sql/migratest.TestMigrations(0xc0001c58c0)
        /Users/foobar/go/src/github.com/ory/kratos/persistence/sql/migratest/migration_test.go:74 +0x2f7
testing.tRunner(0xc0001c58c0, 0x4f7c148)
        /usr/local/Cellar/go/1.14.3/libexec/src/testing/testing.go:991 +0xdc
created by testing.(*T).Run
        /usr/local/Cellar/go/1.14.3/libexec/src/testing/testing.go:1042 +0x357
FAIL    github.com/ory/kratos/persistence/sql/migratest 0.429s

Info

github.com/gobuffalo/pop/v5 v5.1.3
sio4 commented 2 years ago

This situation is almost the same as executing an empty Query.

github.com/gobuffalo/pop/v5.(*Model).TableName(0x0, 0x0, 0x0)
        /Users/foobar/go/src/github.com/gobuffalo/pop/model.go:79 +0x52

Even though the panic point is in Model,

github.com/gobuffalo/pop/v5.(*sqlBuilder).buildColumns(0xc0001fb868, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
        /Users/foobar/go/src/github.com/gobuffalo/pop/sql_builder.go:218 +0x65
github.com/gobuffalo/pop/v5.(*sqlBuilder).buildSelectSQL(0xc0001fb868, 0x0, 0x0)
        /Users/foobar/go/src/github.com/gobuffalo/pop/sql_builder.go:102 +0x40
github.com/gobuffalo/pop/v5.(*sqlBuilder).compile(0xc0001fb868)
        /Users/foobar/go/src/github.com/gobuffalo/pop/sql_builder.go:87 +0x369
github.com/gobuffalo/pop/v5.(*sqlBuilder).String(...)
        /Users/foobar/go/src/github.com/gobuffalo/pop/sql_builder.go:57

within a call chain from buildSelectSQL() called by the empty sqlBuilder for the empty Query,

github.com/gobuffalo/pop/v5.Query.ToSQL(0xc0000333b0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /Users/foobar/go/src/github.com/gobuffalo/pop/query.go:206 +0x1db
github.com/gobuffalo/pop/v5.(*Query).Exec.func1(0xbfaffeb74d0bc090, 0xcc6278)
        /Users/foobar/go/src/github.com/gobuffalo/pop/executors.go:25 +0x91
github.com/gobuffalo/pop/v5.(*Connection).timeFunc(0xc0007a4900, 0x4f07ee0, 0x4, 0xc000079cd0, 0x30, 0x4ded6e0)
        /Users/foobar/go/src/github.com/gobuffalo/pop/connection.go:231 +0x50
github.com/gobuffalo/pop/v5.(*Query).Exec(0xc000079e70, 0xc0000333b0, 0x0)
        /Users/foobar/go/src/github.com/gobuffalo/pop/executors.go:24 +0x6a

executing the Exec() on an empty Query is problematic in the first place.

Filing a PR.