Skip to content
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.

Sign up for GitHub

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

Allow any select statement for CREATE TABLE AS SELECT ... #1355

Merged
merged 3 commits into from
Oct 31, 2022
Merged

Allow any select statement for CREATE TABLE AS SELECT ... #1355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bea471f
Mostly fixed create table as select ... but it's slightly too broad
zachmu Oct 28, 2022
6d133ca
Fixed bug in semantics of create table as select *
zachmu Oct 28, 2022
7a9fa5a
[ga-format-pr] Run ./format_repo.sh to fix formatting
zachmu Oct 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions enginetest/memory_engine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,19 +182,23 @@ func TestSingleScript(t *testing.T) {

var scripts = []queries.ScriptTest{
{
Name: "enums with default, case-sensitive collation (utf8mb4_0900_bin)",
Name: "create table as select distinct",
SetUpScript: []string{
"CREATE TABLE enumtest1 (pk int primary key, e enum('abc', 'XYZ'));",
"CREATE TABLE enumtest2 (pk int PRIMARY KEY, e enum('x ', 'X ', 'y', 'Y'));",
"CREATE TABLE t1 (a int, b varchar(10));",
"insert into t1 values (1, 'a'), (2, 'b'), (2, 'b'), (3, 'c');",
},
Assertions: []queries.ScriptTestAssertion{
{
Query: "select data_type, column_type from information_schema.columns where table_name='enumtest1' and column_name='e';",
Expected: []sql.Row{{"enum('abc','XYZ')", "enum('abc','XYZ')"}},
Query: "create table t2 as select distinct b, a from t1;",
Expected: []sql.Row{{sql.OkResult{RowsAffected: 3}}},
},
{
Query: "select data_type, column_type from information_schema.columns where table_name='enumtest2' and column_name='e';",
Expected: []sql.Row{{"enum('x','X','y','Y')", "enum('x','X','y','Y')"}},
Query: "select * from t2 order by a;",
Expected: []sql.Row{
{"a", 1},
{"b", 2},
{"c", 3},
},
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion enginetest/queries/charset_collation_engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ var CharsetCollationEngineTests = []CharsetCollationEngineTest{
{
Query: "SHOW CREATE TABLE test4;",
Expected: []sql.Row{
{"test4", "CREATE TABLE `test4` (\n `pk` bigint NOT NULL,\n `v1` varchar(255) COLLATE utf8mb4_unicode_ci,\n PRIMARY KEY (`pk`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"},
{"test4", "CREATE TABLE `test4` (\n `pk` bigint NOT NULL,\n `v1` varchar(255) COLLATE utf8mb4_unicode_ci\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to double check this one, but it looks like my test was wrong! It doesn't carry over primary keys, which makes sense as it also doesn't carry over indexes, so it would have to special-case it for primary keys, which doesn't really make sense. A small detail that I didn't catch, just assumed it brought those over. I checked this against MySQL the first time too, that makes it even worse lol

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I saw the comment later on regarding stripping the schema. Wrote the above comment before I got to that part in the review.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rare Daylon error, hard to catch you slippin

},
},
{
Expand Down
57 changes: 56 additions & 1 deletion enginetest/queries/create_table_queries.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ var CreateTableQueries = []WriteQueryTest{
WriteQuery: `CREATE TABLE t1 SELECT * from mytable`,
ExpectedWriteResult: []sql.Row{{sql.NewOkResult(3)}},
SelectQuery: "SHOW CREATE TABLE t1",
ExpectedSelect: []sql.Row{sql.Row{"t1", "CREATE TABLE `t1` (\n `i` bigint NOT NULL,\n `s` varchar(20) NOT NULL COMMENT 'column s',\n PRIMARY KEY (`i`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}},
ExpectedSelect: []sql.Row{sql.Row{"t1", "CREATE TABLE `t1` (\n `i` bigint NOT NULL,\n `s` varchar(20) NOT NULL\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}},
},
{
WriteQuery: `CREATE TABLE mydb.t1 (a INTEGER NOT NULL PRIMARY KEY, b VARCHAR(10) NOT NULL)`,
Expand Down Expand Up @@ -150,4 +150,59 @@ var CreateTableQueries = []WriteQueryTest{
SelectQuery: `SHOW CREATE TABLE t1`,
ExpectedSelect: []sql.Row{{"t1", "CREATE TABLE `t1` (\n `i` int NOT NULL,\n `j` varchar(16383),\n PRIMARY KEY (`i`)\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}},
},
{
WriteQuery: `CREATE TABLE t1 as select * from mytable`,
ExpectedWriteResult: []sql.Row{{sql.NewOkResult(3)}},
SelectQuery: `select * from t1 order by i`,
ExpectedSelect: []sql.Row{{1, "first row"}, {2, "second row"}, {3, "third row"}},
},
{
WriteQuery: `CREATE TABLE t1 as select * from mytable`,
ExpectedWriteResult: []sql.Row{{sql.NewOkResult(3)}},
SelectQuery: `show create table t1`,
ExpectedSelect: []sql.Row{{"t1", "CREATE TABLE `t1` (\n `i` bigint NOT NULL,\n `s` varchar(20) NOT NULL\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}},
},
{
WriteQuery: `CREATE TABLE t1 as select s, i from mytable`,
ExpectedWriteResult: []sql.Row{{sql.NewOkResult(3)}},
SelectQuery: `select * from t1 order by i`,
ExpectedSelect: []sql.Row{{"first row", 1}, {"second row", 2}, {"third row", 3}},
},
{
WriteQuery: `CREATE TABLE t1 as select distinct s, i from mytable`,
ExpectedWriteResult: []sql.Row{{sql.NewOkResult(3)}},
SelectQuery: `select * from t1 order by i`,
ExpectedSelect: []sql.Row{{"first row", 1}, {"second row", 2}, {"third row", 3}},
},
{
WriteQuery: `CREATE TABLE t1 as select s, i from mytable order by s`,
ExpectedWriteResult: []sql.Row{{sql.NewOkResult(3)}},
SelectQuery: `select * from t1 order by i`,
ExpectedSelect: []sql.Row{{"first row", 1}, {"second row", 2}, {"third row", 3}},
},
// TODO: the second column should be named `sum(i)` but is `SUM(mytable.i)`
{
WriteQuery: `CREATE TABLE t1 as select s, sum(i) from mytable group by s`,
ExpectedWriteResult: []sql.Row{{sql.NewOkResult(3)}},
SelectQuery: `select * from t1 order by s`, // other column is named `SUM(mytable.i)`
ExpectedSelect: []sql.Row{{"first row", 1}, {"second row", 2}, {"third row", 3}},
},
{
WriteQuery: `CREATE TABLE t1 as select s, sum(i) from mytable group by s having sum(i) > 2`,
ExpectedWriteResult: []sql.Row{{sql.NewOkResult(1)}},
SelectQuery: "select * from t1",
ExpectedSelect: []sql.Row{{"third row", 3}},
},
{
WriteQuery: `CREATE TABLE t1 as select s, i from mytable order by s limit 1`,
ExpectedWriteResult: []sql.Row{{sql.NewOkResult(1)}},
SelectQuery: `select * from t1 order by i`,
ExpectedSelect: []sql.Row{{"first row", 1}},
},
{
WriteQuery: `CREATE TABLE t1 as select concat("new", s), i from mytable`,
ExpectedWriteResult: []sql.Row{{sql.NewOkResult(3)}},
SelectQuery: `select * from t1 order by i`,
ExpectedSelect: []sql.Row{{"newfirst row", 1}, {"newsecond row", 2}, {"newthird row", 3}},
},
}
20 changes: 19 additions & 1 deletion sql/analyzer/resolve_create_select.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ func resolveCreateSelect(ctx *sql.Context, a *Analyzer, n sql.Node, scope *Scope

// Get the correct schema of the CREATE TABLE based on the select query
inputSpec := ct.TableSpec()
selectSchema := analyzedSelect.Schema()

// We don't want to carry any information about keys, constraints, defaults, etc. from a `create table as select`
// statement. When the underlying select node is a table, we must remove all such info from its schema. The only
// exception is NOT NULL constraints, which we leave alone.
selectSchema := stripSchema(analyzedSelect.Schema())
mergedSchema := mergeSchemas(inputSpec.Schema.Schema, selectSchema)
newSch := make(sql.Schema, len(mergedSchema))

Expand Down Expand Up @@ -48,6 +52,20 @@ func resolveCreateSelect(ctx *sql.Context, a *Analyzer, n sql.Node, scope *Scope
return plan.NewTableCopier(ct.Database(), StripPassthroughNodes(analyzedCreate), StripPassthroughNodes(analyzedSelect), plan.CopierProps{}), transform.NewTree, nil
}

// stripSchema removes all non-type information from a schema, such as the key info, default value, etc.
func stripSchema(schema sql.Schema) sql.Schema {
sch := make(sql.Schema, len(schema))
for i := range schema {
sch[i] = schema[i].Copy()
sch[i].Default = nil
sch[i].AutoIncrement = false
sch[i].PrimaryKey = false
sch[i].Source = ""
sch[i].Comment = ""
}
return sch
}

// mergeSchemas takes in the table spec of the CREATE TABLE and merges it with the schema used by the
// select query. The ultimate structure for the new table will be [CREATE TABLE exclusive columns, columns with the same
// name, SELECT exclusive columns]
Expand Down
33 changes: 21 additions & 12 deletions sql/plan/ddl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ func (c *CreateTable) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter, error
if !ok {
return sql.RowsToRowIter(), sql.ErrTemporaryTableNotSupported.New()
}
vd = maybePrivDb.(sql.ViewDatabase)
vd, _ = maybePrivDb.(sql.ViewDatabase)

if err := c.validateDefaultPosition(); err != nil {
return sql.RowsToRowIter(), err
Expand All @@ -272,7 +272,7 @@ func (c *CreateTable) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter, error
if !ok {
return sql.RowsToRowIter(), sql.ErrCreateTableNotSupported.New(c.db.Name())
}
vd = maybePrivDb.(sql.ViewDatabase)
vd, _ = maybePrivDb.(sql.ViewDatabase)

if err := c.validateDefaultPosition(); err != nil {
return sql.RowsToRowIter(), err
Expand All @@ -284,12 +284,15 @@ func (c *CreateTable) RowIter(ctx *sql.Context, row sql.Row) (sql.RowIter, error
return sql.RowsToRowIter(), err
}

_, ok, err := vd.GetView(ctx, c.name)
if err != nil {
return nil, err
}
if ok {
return nil, sql.ErrTableAlreadyExists.New(c.name)
if vd != nil {
_, ok, err := vd.GetView(ctx, c.name)
if err != nil {
return nil, err
}

if ok {
return nil, sql.ErrTableAlreadyExists.New(c.name)
}
}

//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
Expand Down Expand Up @@ -459,11 +462,10 @@ func (c CreateTable) WithChildren(children ...sql.Node) (sql.Node, error) {
} else if len(children) == 1 {
child := children[0]

switch child.(type) {
case *Project, *Limit:
c.selectNode = child
default:
if c.like != nil {
c.like = child
} else {
c.selectNode = child
}

return &c, nil
Expand Down Expand Up @@ -497,6 +499,13 @@ func (c *CreateTable) DebugString() string {
ifNotExists = "if not exists "
}
p := sql.NewTreePrinter()

if c.selectNode != nil {
p.WriteNode("Create table %s%s as", ifNotExists, c.name)
p.WriteChildren(sql.DebugString(c.selectNode))
return p.String()
}

p.WriteNode("Create table %s%s", ifNotExists, c.name)

var children []string
Expand Down