diff --git a/.gitignore b/.gitignore index 1328b5cfd6..28e71e2a0f 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,5 @@ venv benchmark/perf_tools/dolt-builds/dolt benchmark/perf_tools/dolt-builds/working benchmark/perf_tools/output + +test.sh \ No newline at end of file diff --git a/go/libraries/doltcore/schema/alterschema/addpk.go b/go/libraries/doltcore/schema/alterschema/addpk.go index 740e8d768b..7fd2a8db55 100644 --- a/go/libraries/doltcore/schema/alterschema/addpk.go +++ b/go/libraries/doltcore/schema/alterschema/addpk.go @@ -51,6 +51,45 @@ func AddPrimaryKeyToTable(ctx context.Context, table *doltdb.Table, tableName st return col }) + // Get Row Data out of Table + rowData, err := table.GetRowData(ctx) + if err != nil { + return nil, err + } + + // Go through every row + err = rowData.Iter(ctx, func(key, value types.Value) (stop bool, err error) { + r, err := row.FromNoms(sch, key.(types.Tuple), value.(types.Tuple)) + if err != nil { + return false, err + } + + // Go through every column of row + err = newCollection.Iter(func(tag uint64, col schema.Column) (stop bool, err error) { + // Skip if they are not part of primary key + if !col.IsPartOfPK { + return false, nil + } + + // Check if column value is null + val, ok := r.GetColVal(tag) + if !ok || val == nil || val == types.NullValue { + return true, fmt.Errorf("primary key cannot have NULL values") + } + return false, nil + }) + + if err != nil { + return true, err + } + + return false, nil + }) + + if err != nil { + return nil, err + } + newSchema, err := schema.SchemaFromCols(newCollection) if err != nil { return nil, err diff --git a/go/libraries/doltcore/schema/alterschema/addpk_test.go b/go/libraries/doltcore/schema/alterschema/addpk_test.go index 669157cb22..857ade9a96 100644 --- a/go/libraries/doltcore/schema/alterschema/addpk_test.go +++ b/go/libraries/doltcore/schema/alterschema/addpk_test.go @@ -144,4 +144,28 @@ func TestAddPk(t *testing.T) { assert.NoError(t, err) assert.True(t, ok) }) + + t.Run("Add primary key when one more cells contain NULL", func(t *testing.T) { + dEnv := dtestutils.CreateTestEnv() + ctx := context.Background() + + for _, c := range setupAdd { + c.exec(t, ctx, dEnv) + } + + _, err := getTable(ctx, dEnv, "test") + assert.NoError(t, err) + + exitCode := commands.SqlCmd{}.Exec(ctx, "sql", []string{"-q", "ALTER TABLE test ADD PRIMARY KEY (c1)"}, dEnv) + require.Equal(t, 0, exitCode) + + exitCode = commands.SqlCmd{}.Exec(ctx, "sql", []string{"-q", "ALTER TABLE test ADD COLUMN (c2 INT NULL)"}, dEnv) + require.Equal(t, 0, exitCode) + + exitCode = commands.SqlCmd{}.Exec(ctx, "sql", []string{"-q", "ALTER TABLE test DROP PRIMARY KEY"}, dEnv) + require.Equal(t, 0, exitCode) + + exitCode = commands.SqlCmd{}.Exec(ctx, "sql", []string{"-q", "ALTER TABLE test ADD PRIMARY KEY (id, c1, c2)"}, dEnv) + require.Equal(t, 1, exitCode) + }) } diff --git a/integration-tests/bats/primary-key-changes.bats b/integration-tests/bats/primary-key-changes.bats index f8bce650f5..ca0db7cf06 100644 --- a/integration-tests/bats/primary-key-changes.bats +++ b/integration-tests/bats/primary-key-changes.bats @@ -10,6 +10,13 @@ teardown() { teardown_common } +@test "primary-key-changes: add primary key using null values" { + dolt sql -q "create table t(pk int, val int)" + dolt sql -q "INSERT INTO t (val) VALUES (1)" + run dolt sql -q "ALTER TABLE t ADD PRIMARY KEY (pk)" + [ "$status" -eq 1 ] +} + @test "primary-key-changes: add single primary key" { dolt sql -q "create table t(pk int, val int)" run dolt sql -q "ALTER TABLE t ADD PRIMARY KEY (pk)" @@ -573,11 +580,5 @@ SQL dolt sql -q "create table t (pk int, c1 int)" dolt sql -q "insert into t values (NULL, NULL)" run dolt sql -q "alter table t add primary key(pk)" - skip "This should fail on some sort of constraint error" [ $status -eq 1 ] - - # This is the current failure mode - run dolt sql -q "update t set c1=1" - [ $status -eq 1 ] - [[ "$output" =~ "received nil" ]] || false }