-
Notifications
You must be signed in to change notification settings - Fork 110
Drop / create table #845
Drop / create table #845
Conversation
Signed-off-by: Zach Musgrave <zach@liquidata.co>
Signed-off-by: Zach Musgrave <zach@liquidata.co>
...rface for create table statements, deprecating Alterable (which only supports Create) Signed-off-by: Zach Musgrave <zach@liquidata.co>
Signed-off-by: Zach Musgrave <zach@liquidata.co>
Signed-off-by: Zach Musgrave <zach@liquidata.co>
...E TABLE with an empty table spec prior to this change. Signed-off-by: Zach Musgrave <zach@liquidata.co>
Signed-off-by: Zach Musgrave <zach@liquidata.co>
Drop and create table support, as well as support for 24-bit integers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! Overall it looks really good, just some minor nitpicks and it's good to merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, thank you for the contribution!
I only miss an end-to-end test for the new Drop table feature. Something simple in engine.go to check that a DROP TABLE statement does indeed drop the specified table.
...omments. Signed-off-by: Zach Musgrave <zach@liquidata.co>
PR feedback: added engine tests, removed Alterable interface, added comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the changes, they look great.
But Travis is failing now because there is still a reference to Alterable in memory/database_test.go. Could you remove it?
Signed-off-by: Zach Musgrave <zach@liquidata.co>
Fixed test error caused by deleting Alterable interface.
Oops, sorry about that. All fixed now, PTAL.
Implemented DROP TABLE. Also: