Vinai/temporary tables #442
Conversation
…lementation for memory now:
| @@ -620,6 +627,12 @@ type TableCreator interface { | |||
| CreateTable(ctx *Context, name string, schema Schema) error | |||
| } | |||
|
|
|||
| type TemporaryTableCreator interface { | |||
There was a problem hiding this comment.
Interface doc: "TemporaryTableCreator is a database that can create temporary tables that persist only as long as the session"
Also embed the Database interface
There was a problem hiding this comment.
Put it in the wrong place, needs to go above the interface. Also document the fact that temp table with the same name as an existing table are permitted and take precedence, here and in GetTableInsensitive
Also CreateTemporaryTable needs method doc
| @@ -540,6 +541,12 @@ type TriggerDatabase interface { | |||
| DropTrigger(ctx *Context, name string) error | |||
| } | |||
|
|
|||
| // TemporaryTableDatabase is a database that can query the session (which manages the temporary table state) to | |||
| // retrieve the name of all temporary tables. | |||
| type TemporaryTableDatabase interface { | |||
There was a problem hiding this comment.
Get rid of this, no need for this interface
Just iterate over all tables and ask each one if it's temp or not
There was a problem hiding this comment.
Resolved in DMs
sql/plan/ddl.go
Outdated
| } | ||
|
|
||
| var _ sql.Databaser = (*CreateTable)(nil) | ||
| var _ sql.Node = (*CreateTable)(nil) | ||
| var _ sql.Expressioner = (*CreateTable)(nil) | ||
|
|
||
| // NewCreateTable creates a new CreateTable node | ||
| func NewCreateTable(db sql.Database, name string, ifNotExists bool, tableSpec *TableSpec) *CreateTable { | ||
| func NewCreateTable(db sql.Database, name string, ifNotExists bool, temporary bool, tableSpec *TableSpec) *CreateTable { |
There was a problem hiding this comment.
Clean this up: use bool alias types instead of bools for these two params, and introduce consts for them
type IfNotExists bool
type TempTable bool
IfNotExists = true
IfNotExistsAbsent = false
IsTempTable = true
IsNotTempTable = false
There was a problem hiding this comment.
I experimented with this and feels little messier tbh. I'll push to review though and we can reassess
sql/plan/ddl.go
Outdated
| if c.temporary { | ||
| creatable, ok := c.db.(sql.TemporaryTableCreator) | ||
| if !ok { | ||
| return sql.RowsToRowIter(), fmt.Errorf("error: database does not support temporary tables") |
sql/plan/ddl.go
Outdated
|
|
||
| //TODO: in the event that foreign keys or indexes aren't supported, you'll be left with a created table and no foreign keys/indexes | ||
| //this also means that if a foreign key or index fails, you'll only have what was declared up to the failure | ||
| if len(c.idxDefs) > 0 || len(c.fkDefs) > 0 || len(c.chDefs) > 0 { |
There was a problem hiding this comment.
Split this up into three separate parts and extract a method for each
| @@ -620,6 +627,12 @@ type TableCreator interface { | |||
| CreateTable(ctx *Context, name string, schema Schema) error | |||
| } | |||
|
|
|||
| type TemporaryTableCreator interface { | |||
There was a problem hiding this comment.
Put it in the wrong place, needs to go above the interface. Also document the fact that temp table with the same name as an existing table are permitted and take precedence, here and in GetTableInsensitive
Also CreateTemporaryTable needs method doc
sql/plan/ddl.go
Outdated
| @@ -42,6 +42,24 @@ var ErrNullDefault = errors.NewKind("column declared not null must have a non-nu | |||
| // ErrTableCreatedNotFound is thrown when a table is created from CREATE TABLE but cannot be found immediately afterward | |||
| var ErrTableCreatedNotFound = errors.NewKind("table was created but could not be found") | |||
|
|
|||
| // ErrTableCreatedNotFound is thrown when an integrator attempts to create a temporary tables without temporary table | |||
| // support. | |||
| var ErrTemporaryTableNotSupported = errors.NewKind("database does not support temporary tables") | |||
sql/plan/ddl.go
Outdated
| // support. | ||
| var ErrTemporaryTableNotSupported = errors.NewKind("database does not support temporary tables") | ||
|
|
||
| type IfNotExistsOperator bool |
There was a problem hiding this comment.
Not an operator
How about IfNotExistsOption?
And TempTableOption
sql/plan/exchange_test.go
Outdated
| @@ -150,6 +150,10 @@ func (partitionable) Schema() sql.Schema { | |||
|
|
|||
| func (partitionable) Name() string { return "partitionable" } | |||
|
|
|||
| func (partitionable) IsTemporary() bool { | |||
There was a problem hiding this comment.
From the dolt PR, you should define a new TemporaryTable interface extension to Table interface so that not every table implementation needs to provide IsTemporary
…server into vinai/temporary-tables
No description provided.