diff --git a/enginetest/enginetests.go b/enginetest/enginetests.go index 9b608e0c94..92b6ccef4a 100644 --- a/enginetest/enginetests.go +++ b/enginetest/enginetests.go @@ -1025,7 +1025,6 @@ func TestInsertIntoErrors(t *testing.T, harness Harness) { func TestBrokenInsertScripts(t *testing.T, harness Harness) { for _, script := range queries.InsertBrokenScripts { - t.Skip() TestScript(t, harness, script) } } diff --git a/enginetest/queries/insert_queries.go b/enginetest/queries/insert_queries.go index 4e577cb3cd..f90f30c9ec 100644 --- a/enginetest/queries/insert_queries.go +++ b/enginetest/queries/insert_queries.go @@ -2988,7 +2988,7 @@ var InsertBrokenScripts = []ScriptTest{ { Query: "SELECT * FROM x", Expected: []sql.Row{ - {1, "one"}, {2, 1}, {3, "three"}, + {1, "one"}, {2, "1"}, {3, "three"}, }, }, { @@ -3123,9 +3123,59 @@ var InsertBrokenScripts = []ScriptTest{ Expected: []sql.Row{{types.NewOkResult(1)}}, }, { - Query: "select * from t2;", + Query: "select * from test;", Expected: []sql.Row{{1, "a"}}, }, }, }, + { + Name: "column count mismatch in insert", + SetUpScript: []string{ + "create table t(i int, j int)", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "insert into t(i, j) values (1, 2, 3)", + ExpectedErr: sql.ErrColValCountMismatch, + }, + { + Query: "insert into t(i, j) values (1)", + ExpectedErr: sql.ErrColValCountMismatch, + }, + { + Query: "insert into t(i, j) values (1, 2), ()", + ExpectedErr: sql.ErrColValCountMismatch, + }, + }, + }, + { + Name: "no cols empty val insert", + SetUpScript: []string{ + "create table t_auto (id int auto_increment primary key, name varchar(10) default null)", + "create table t_default_null (id int default null, name varchar(10) default null)", + "create table t_not_null (id int not null, name varchar(10) not null)", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "insert into t_auto values ()", + Expected: []sql.Row{{types.OkResult{RowsAffected: 1, InsertID: 1}}}, + }, + { + Query: "select * from t_auto", + Expected: []sql.Row{{1, nil}}, + }, + { + Query: "insert into t_default_null values ()", + Expected: []sql.Row{{types.OkResult{RowsAffected: 1, InsertID: 0}}}, + }, + { + Query: "select * from t_default_null", + Expected: []sql.Row{{nil, nil}}, + }, + { + Query: "insert into t_not_null values ()", + ExpectedErr: sql.ErrInsertIntoNonNullableDefaultNullColumn, + }, + }, + }, } diff --git a/enginetest/queries/queries.go b/enginetest/queries/queries.go index 08e08ce57b..a9e7913f82 100644 --- a/enginetest/queries/queries.go +++ b/enginetest/queries/queries.go @@ -10462,28 +10462,28 @@ var KeylessQueries = []QueryTest{ { Query: "DESCRIBE keyless", Expected: []sql.Row{ - {"c0", "bigint", "YES", "", nil, ""}, - {"c1", "bigint", "YES", "", nil, ""}, + {"c0", "bigint", "YES", "", "NULL", ""}, + {"c1", "bigint", "YES", "", "NULL", ""}, }, }, { Query: "SHOW COLUMNS FROM keyless", Expected: []sql.Row{ - {"c0", "bigint", "YES", "", nil, ""}, - {"c1", "bigint", "YES", "", nil, ""}, + {"c0", "bigint", "YES", "", "NULL", ""}, + {"c1", "bigint", "YES", "", "NULL", ""}, }, }, { Query: "SHOW FULL COLUMNS FROM keyless", Expected: []sql.Row{ - {"c0", "bigint", nil, "YES", "", nil, "", "", ""}, - {"c1", "bigint", nil, "YES", "", nil, "", "", ""}, + {"c0", "bigint", nil, "YES", "", "NULL", "", "", ""}, + {"c1", "bigint", nil, "YES", "", "NULL", "", "", ""}, }, }, { Query: "SHOW CREATE TABLE keyless", Expected: []sql.Row{ - {"keyless", "CREATE TABLE `keyless` (\n `c0` bigint,\n `c1` bigint\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}, + {"keyless", "CREATE TABLE `keyless` (\n `c0` bigint DEFAULT NULL,\n `c1` bigint DEFAULT NULL\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin"}, }, }, } diff --git a/enginetest/queries/trigger_queries.go b/enginetest/queries/trigger_queries.go index c18a78ed6e..ef9deb9e75 100644 --- a/enginetest/queries/trigger_queries.go +++ b/enginetest/queries/trigger_queries.go @@ -2574,7 +2574,7 @@ end;`, { Name: "insert into common sequence table (https://github.com/dolthub/dolt/issues/2534)", SetUpScript: []string{ - "create table mytable (id integer PRIMARY KEY DEFAULT 0, sometext text);", + "create table mytable (id integer PRIMARY KEY DEFAULT 0, sometext text DEFAULT NULL);", "create table sequence_table (max_id integer PRIMARY KEY);", "create trigger update_position_id before insert on mytable for each row begin set new.id = (select coalesce(max(max_id),1) from sequence_table); update sequence_table set max_id = max_id + 1; end;", "insert into sequence_table values (1);", @@ -2605,7 +2605,7 @@ end;`, { Name: "insert into common sequence table workaround", SetUpScript: []string{ - "create table mytable (id integer PRIMARY KEY DEFAULT 0, sometext text);", + "create table mytable (id integer PRIMARY KEY DEFAULT 0, sometext text DEFAULT NULL);", "create table sequence_table (max_id integer PRIMARY KEY);", `create trigger update_position_id before insert on mytable for each row begin @@ -4500,7 +4500,7 @@ var BrokenTriggerQueries = []ScriptTest{ { Name: "update common table multiple times in single insert", SetUpScript: []string{ - "create table mytable (id integer PRIMARY KEY DEFAULT 0, sometext text);", + "create table mytable (id integer PRIMARY KEY DEFAULT 0, sometext text DEFAULT NULL);", "create table sequence_table (max_id integer PRIMARY KEY);", "create trigger update_position_id before insert on mytable for each row begin set new.id = (select coalesce(max(max_id),1) from sequence_table); update sequence_table set max_id = max_id + 1; end;", "insert into sequence_table values (1);", diff --git a/enginetest/scriptgen/setup/scripts/keyless b/enginetest/scriptgen/setup/scripts/keyless index a6aa182d9a..7b8beb8b56 100644 --- a/enginetest/scriptgen/setup/scripts/keyless +++ b/enginetest/scriptgen/setup/scripts/keyless @@ -1,8 +1,8 @@ exec CREATE TABLE `unique_keyless` ( - `c0` bigint, - `c1` bigint + `c0` bigint DEFAULT NULL, + `c1` bigint DEFAULT NULL ) ---- @@ -15,8 +15,8 @@ insert into unique_keyless values exec CREATE TABLE `keyless` ( - `c0` bigint, - `c1` bigint + `c0` bigint DEFAULT NULL, + `c1` bigint DEFAULT NULL ) ---- diff --git a/enginetest/scriptgen/setup/setup_data.sg.go b/enginetest/scriptgen/setup/setup_data.sg.go index 2173f99a5b..4378e50cc5 100755 --- a/enginetest/scriptgen/setup/setup_data.sg.go +++ b/enginetest/scriptgen/setup/setup_data.sg.go @@ -2958,12 +2958,12 @@ var JsontableData = []SetupScript{{ }} var KeylessData = []SetupScript{{ - "CREATE TABLE `unique_keyless` ( `c0` bigint, `c1` bigint )", + "CREATE TABLE `unique_keyless` ( `c0` bigint DEFAULT NULL, `c1` bigint DEFAULT NULL )", `insert into unique_keyless values (0,0), (1,1), (2,2)`, - "CREATE TABLE `keyless` ( `c0` bigint, `c1` bigint )", + "CREATE TABLE `keyless` ( `c0` bigint DEFAULT NULL, `c1` bigint DEFAULT NULL )", `insert into keyless values (0,0), (1,1), diff --git a/sql/planbuilder/dml.go b/sql/planbuilder/dml.go index d7ec5053b6..5bbbd39146 100644 --- a/sql/planbuilder/dml.go +++ b/sql/planbuilder/dml.go @@ -75,8 +75,25 @@ func (b *Builder) buildInsert(inScope *scope, i *ast.Insert) (outScope *scope) { if len(columns) == 0 && len(destScope.cols) > 0 && rt != nil { schema := rt.Schema() columns = make([]string, len(schema)) - for i, col := range schema { - columns[i] = col.Name + for index, col := range schema { + columns[index] = col.Name + } + if ir, ok := i.Rows.(*ast.AliasedValues); ok { + // Handle any empty VALUES() tuples when no column list was specified + for valueIdx, valueRow := range ir.Values { + if len(valueRow) == 0 { + // VALUES() clause is empty in conjunction with empty column list, so we need to + // insert default val for respective columns. + ir.Values[valueIdx] = make([]ast.Expr, len(schema)) + for j, col := range schema { + if col.Default == nil && !col.AutoIncrement { + b.handleErr(sql.ErrInsertIntoNonNullableDefaultNullColumn.New(col.Name)) + } + + ir.Values[valueIdx][j] = &ast.Default{} + } + } + } } } } @@ -210,20 +227,20 @@ func (b *Builder) buildInsertValues(inScope *scope, v *ast.AliasedValues, column literalOnly = true exprTuples := make([][]sql.Expression, len(v.Values)) for i, vt := range v.Values { - // noExprs is an edge case where we fill VALUES with nil expressions - noExprs := len(vt) == 0 // triggerUnknownTable is an edge case where we ignored an unresolved // table error and do not have a schema for resolving defaults triggerUnknownTable := (len(columnNames) == 0 && len(vt) > 0) && (len(b.TriggerCtx().UnresolvedTables) > 0) - if len(vt) != len(columnNames) && !noExprs && !triggerUnknownTable { - err := sql.ErrInsertIntoMismatchValueCount.New() + // For empty VALUES with explicit column list, this is an error + // For empty VALUES without column list, it was handled above by filling with defaults + if len(vt) != len(columnNames) && !triggerUnknownTable { + err := sql.ErrColValCountMismatch.New(i + 1) b.handleErr(err) } exprs := make([]sql.Expression, len(columnNames)) exprTuples[i] = exprs for j := range columnNames { - if noExprs || triggerUnknownTable { + if triggerUnknownTable { exprs[j] = expression.WrapExpression(columnDefaultValues[j]) continue }