dolthub / dolt

Dolt – Git for Data
Apache License 2.0
17.41k stars 488 forks source link

Panic on invalid query #8028

Closed nicktobey closed 3 weeks ago

nicktobey commented 4 weeks ago

The following invalid query will be successfully parsed and then cause a panic during plan building:

CREATE PROCEDURE foo() CREATE PROCEDURE foo() SELECT 0;

Stack trace:

panic: runtime error: slice bounds out of range [:64] with length 36 [recovered]
    panic: runtime error: slice bounds out of range [:64] with length 36

goroutine 1 [running]:
github.com/dolthub/go-mysql-server/sql/planbuilder.(*Builder).Parse.func1()
    /Users/nick/Documents/go-mysql-server/sql/planbuilder/parse.go:59 +0xa8
panic({0x105a68860?, 0x14000d1d7d0?})
    /Users/nick/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.2.darwin-arm64/src/runtime/panic.go:770 +0x124
github.com/dolthub/go-mysql-server/sql/planbuilder.(*Builder).buildCreateProcedure(0x14000d51280, 0x14000e29950, {0x14000632f1c, 0x24}, 0x14000705340)
    /Users/nick/Documents/go-mysql-server/sql/planbuilder/create_ddl.go:197 +0xd0c
github.com/dolthub/go-mysql-server/sql/planbuilder.(*Builder).buildDDL(0x14000d51280, 0x14000e29950, {0x14000632f1c, 0x24}, 0x14000705340)
    /Users/nick/Documents/go-mysql-server/sql/planbuilder/ddl.go:124 +0x628
github.com/dolthub/go-mysql-server/sql/planbuilder.(*Builder).build(0x14000d51280, 0x0?, {0x105c13bf0, 0x14000705340}, {0x14000632f1c?, 0x102d8637c?})
    /Users/nick/Documents/go-mysql-server/sql/planbuilder/builder.go:209 +0x1708
github.com/dolthub/go-mysql-server/sql/planbuilder.(*Builder).buildCreateProcedure(0x14000d51280, 0x14000e29950, {0x14000632f00, 0x40}, 0x14000705500)
    /Users/nick/Documents/go-mysql-server/sql/planbuilder/create_ddl.go:199 +0xad4
github.com/dolthub/go-mysql-server/sql/planbuilder.(*Builder).buildDDL(0x14000d51280, 0x14000e29950, {0x14000632f00, 0x40}, 0x14000705500)
    /Users/nick/Documents/go-mysql-server/sql/planbuilder/ddl.go:124 +0x628
github.com/dolthub/go-mysql-server/sql/planbuilder.(*Builder).build(0x14000d51280, 0x14000632f00?, {0x105c13bf0, 0x14000705500}, {0x14000632f00?, 0x40?})
    /Users/nick/Documents/go-mysql-server/sql/planbuilder/builder.go:209 +0x1708
github.com/dolthub/go-mysql-server/sql/planbuilder.(*Builder).Parse(0x14000d51280, {0x14000632f00, 0x40}, 0x0)
    /Users/nick/Documents/go-mysql-server/sql/planbuilder/parse.go:75 +0x4a0
github.com/dolthub/go-mysql-server.(*Engine).bindQuery(0x140000ee930, 0x14000bf2fa0, {0x14000632f00, 0x40}, {0x0?, 0x0?}, 0x0, {0x0?, 0x0?}, 0x14000d51280)
    /Users/nick/Documents/go-mysql-server/engine.go:581 +0x80
github.com/dolthub/go-mysql-server.(*Engine).QueryWithBindings(0x140000ee930, 0x14000bf2fa0, {0x14000632f00, 0x40}, {0x0, 0x0}, 0x0)
    /Users/nick/Documents/go-mysql-server/engine.go:383 +0xf0
github.com/dolthub/go-mysql-server.(*Engine).Query(...)
    /Users/nick/Documents/go-mysql-server/engine.go:246
github.com/dolthub/dolt/go/cmd/dolt/commands/engine.(*SqlEngine).Query(0x40?, 0x40?, {0x14000632f00?, 0x40?})
    /Users/nick/Documents/dolt/go/cmd/dolt/commands/engine/sqlengine.go:312 +0x30
github.com/dolthub/dolt/go/cmd/dolt/commands.processParsedQuery(0x14000bf2fa0, {0x14000632f00?, 0x14000a16b78?}, {0x105c07480?, 0x14000db1ef0?}, {0x105c13bf0?, 0x14000705180?})
    /Users/nick/Documents/dolt/go/cmd/dolt/commands/sql.go:1103 +0x238
github.com/dolthub/dolt/go/cmd/dolt/commands.processQuery(0x14000bf2fa0, {0x14000632f00, 0x40}, {0x105c07480, 0x14000db1ef0})
    /Users/nick/Documents/dolt/go/cmd/dolt/commands/sql.go:1073 +0xd8
github.com/dolthub/dolt/go/cmd/dolt/commands.execShell.func3.2({0x105c25510, 0x14000f02480}, 0x14000bf21e0, 0x14000a16d20, 0x14000a16d48, 0x14001100080, {0x14000632f00, 0x40}, {0x105c07480, 0x14000db1ef0}, ...)
    /Users/nick/Documents/dolt/go/cmd/dolt/commands/sql.go:808 +0x130
github.com/dolthub/dolt/go/cmd/dolt/commands.execShell.func3(0x14000ce2000?)
    /Users/nick/Documents/dolt/go/cmd/dolt/commands/sql.go:827 +0x46c
github.com/dolthub/ishell.handleUninterpretedInput(0x14000ce2000, {0x14000632f00, 0x41})
    /Users/nick/go/pkg/mod/github.com/dolthub/ishell@v0.0.0-20221214210346-d7db0b066488/ishell.go:294 +0x1e4
github.com/dolthub/ishell.(*Shell).run(0x14000ce2000)
    /Users/nick/go/pkg/mod/github.com/dolthub/ishell@v0.0.0-20221214210346-d7db0b066488/ishell.go:247 +0x280
github.com/dolthub/ishell.(*Shell).Run(0x14000ce2000)
    /Users/nick/go/pkg/mod/github.com/dolthub/ishell@v0.0.0-20221214210346-d7db0b066488/ishell.go:141 +0x28
github.com/dolthub/dolt/go/cmd/dolt/commands.execShell(0x14000bf21e0, {0x105c07480, 0x14000db1ef0}, 0x0, {0x105c1d928, 0x1400004e9c0})
    /Users/nick/Documents/dolt/go/cmd/dolt/commands/sql.go:838 +0x52c
github.com/dolthub/dolt/go/cmd/dolt/commands.SqlCmd.Exec({{0x104bd0412?, 0x105a3a280?}}, {0x105c25510, 0x14000f02480}, {0x140007483e0, 0x8}, {0x14000110030, 0x0, 0x0}, 0x140005903f0, ...)
    /Users/nick/Documents/dolt/go/cmd/dolt/commands/sql.go:264 +0xc9c
github.com/dolthub/dolt/go/cmd/dolt/cli.SubCommandHandler.handleCommand({{0x104bccfc9, 0x4}, {0x104beda6e, 0x11}, {0x0, 0x0}, {0x10717df60, 0x35, 0x35}, 0x0}, ...)
    /Users/nick/Documents/dolt/go/cmd/dolt/cli/command.go:245 +0x4d0
github.com/dolthub/dolt/go/cmd/dolt/cli.SubCommandHandler.Exec({{0x104bccfc9, 0x4}, {0x104beda6e, 0x11}, {0x0, 0x0}, {0x10717df60, 0x35, 0x35}, 0x0}, ...)
    /Users/nick/Documents/dolt/go/cmd/dolt/cli/command.go:194 +0x248
main.runMain()
    /Users/nick/Documents/dolt/go/cmd/dolt/dolt.go:647 +0x2f10
main.main()
    /Users/nick/Documents/dolt/go/cmd/dolt/dolt.go:246 +0x1c
nicktobey commented 3 weeks ago

Notably, while the above statement is invalid, some statements with multiple nested sub-statements are valid. The following statements are valid MySQL:

mysql> create procedure foo() create view v as select 1;
Query OK, 0 rows affected (0.01 sec)
mysql> create procedure foo() create table t as select 1;
Query OK, 0 rows affected (0.00 sec)

But both fail in Dolt. Curiously, neither is a panic:

json/main*> create procedure foo() create view v as select 1;
unable to get sub statement
json/main*> create procedure foo() create table t as select 1;
creating tables in stored procedures is currently unsupported and will be added in a future release

For the case of creating a view inside a procedure, it would be a panic, just like the previous ones, but there's an additional check in the logic for CREATE VIEW that detects the out-of-bounds access and makes it an error instead.