New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic when selecting from table with dropped column and new index #188

Closed
dbrower opened this Issue Aug 31, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@dbrower
Collaborator

dbrower commented Aug 31, 2017

This is similar to but different from #187 since the panic happens in findIndexByColName().

$ rm -f ql.db
$ ql 'CREATE TABLE tbl (a string, b string)'
$ ql 'CREATE INDEX ax ON tbl (a)'
$ ql 'ALTER TABLE tbl DROP COLUMN a'
$ ql 'CREATE INDEX bx ON tbl (b)'
$ ql 'INSERT INTO tbl (b) VALUES ("abc")'
$ ql 'SELECT b FROM tbl WHERE b = "abc"'
panic: runtime error: index out of range

goroutine 1 [running]:
github.com/cznic/ql.(*table).findIndexByColName(0xc42014a4b0, 0xc420154cf5, 0x1, 0x1, 0x43bfa60)
	/Users/dbrower/gocode/src/github.com/cznic/ql/storage.go:333 +0x19b
github.com/cznic/ql.(*tableDefaultPlan).filterBinOp(0xc420158e80, 0xc42015a780, 0xc4200ab208, 0xc420154e40, 0x440b800, 0xc4200ab2e0, 0x42bc0ea, 0xc42015a780, 0xc4200ab200)
	/Users/dbrower/gocode/src/github.com/cznic/ql/plan.go:2269 +0xab
github.com/cznic/ql.(*tableDefaultPlan).filter(0xc420158e80, 0x45cf800, 0xc42015a780, 0x0, 0x0, 0x0, 0x0, 0x0, 0xe022, 0x45cf900)
	/Users/dbrower/gocode/src/github.com/cznic/ql/plan.go:2375 +0x58e
github.com/cznic/ql.(*whereRset).planBinOp(0xc4200ab680, 0xc42015a780, 0x0, 0x0, 0x0, 0x0)
	/Users/dbrower/gocode/src/github.com/cznic/ql/ql.go:398 +0x16d
github.com/cznic/ql.(*whereRset).planExpr(0xc4200ab680, 0xc42014f9c0, 0xc4200ab510, 0x4010f12, 0xc42015a6f0, 0x30)
	/Users/dbrower/gocode/src/github.com/cznic/ql/ql.go:580 +0x5f2
github.com/cznic/ql.(*whereRset).plan(0xc4200ab680, 0xc42014f9c0, 0x45d1120, 0xc420158e80, 0x0, 0x0)
	/Users/dbrower/gocode/src/github.com/cznic/ql/ql.go:561 +0x4f8
github.com/cznic/ql.(*selectStmt).plan(0xc4201526c0, 0xc42014f9c0, 0xc42015a720, 0xc42011eb70, 0xc4200ab710, 0x4011768)
	/Users/dbrower/gocode/src/github.com/cznic/ql/stmt.go:809 +0x891
github.com/cznic/ql.(*selectStmt).exec(0xc4201526c0, 0xc42014f9c0, 0xc42015a720, 0x0, 0xc42015a720, 0xc4200ab7e8)
	/Users/dbrower/gocode/src/github.com/cznic/ql/stmt.go:856 +0x39
github.com/cznic/ql.(*DB).run1(0xc420090480, 0xc420154db0, 0x47ab190, 0xc4201526c0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, ...)
	/Users/dbrower/gocode/src/github.com/cznic/ql/ql.go:1424 +0x9e4
github.com/cznic/ql.(*DB).Execute(0xc420090480, 0xc420154db0, 0xc42014f500, 0x3, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/dbrower/gocode/src/github.com/cznic/ql/ql.go:1259 +0x95e
main.run(0xc4200abea8, 0xc42014f380, 0xc42014f3c0, 0x3c, 0xc420090480, 0x0, 0x0)
	/Users/dbrower/gocode/src/github.com/cznic/ql/ql/main.go:308 +0x724
main.do(0x45cb0c0, 0xc420154e70)
	/Users/dbrower/gocode/src/github.com/cznic/ql/ql/main.go:183 +0x3f7
main.main()
	/Users/dbrower/gocode/src/github.com/cznic/ql/ql/main.go:83 +0x22

My guess would be that the t.cols should be t.cols0 but I don't know enough of the engine to really say anything.

@cznic cznic self-assigned this Aug 31, 2017

@cznic cznic added the bug label Aug 31, 2017

@cznic

This comment has been minimized.

Show comment
Hide comment
@cznic

cznic Aug 31, 2017

Owner

My guess would be that the t.cols should be t.cols0 but I don't know enough of the engine to really say anything.

I today tried to look into #187 and initially it seems to me I don't know probably much more than you do. Next attempt possibly tomorrow.

Owner

cznic commented Aug 31, 2017

My guess would be that the t.cols should be t.cols0 but I don't know enough of the engine to really say anything.

I today tried to look into #187 and initially it seems to me I don't know probably much more than you do. Next attempt possibly tomorrow.

@dbrower

This comment has been minimized.

Show comment
Hide comment
@dbrower

dbrower Sep 1, 2017

Collaborator

It looks like the code in storage.go expects indices and cols0 to match each other, with indices contains an extra entry at the beginning. However, in stmt.go:1137 the column id is resolved using cols instead of cols0, as would be expected.

Collaborator

dbrower commented Sep 1, 2017

It looks like the code in storage.go expects indices and cols0 to match each other, with indices contains an extra entry at the beginning. However, in stmt.go:1137 the column id is resolved using cols instead of cols0, as would be expected.

@cznic

This comment has been minimized.

Show comment
Hide comment
@cznic

cznic Sep 1, 2017

Owner

I haven't yet tried again, but in case your insight is correct, please feel free to try the patch and test it against go test and send a PR if it works ;-)

Owner

cznic commented Sep 1, 2017

I haven't yet tried again, but in case your insight is correct, please feel free to try the patch and test it against go test and send a PR if it works ;-)

dbrower added a commit to dbrower/ql that referenced this issue Sep 3, 2017

dbrower added a commit to dbrower/ql that referenced this issue Sep 3, 2017

Use physical column to find simple index by name
The difference between physical and logical columns only appears after
dropping a column.

Fixing this uncovered another bug where dropped columns are returned
from selects with a WHERE clause using an index. That bug is from
dropped columns not being filtered out after reading a row. The
filtering is done in a simple table scan, that code is replicated in the
path used by index plans. This bug is also fixed.

Fixes #188

@dbrower dbrower closed this in 44c86fb Sep 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment