mirror of
https://github.com/dolthub/dolt.git
synced 2026-02-04 10:25:17 -06:00
Merge pull request #2413 from dolthub/james/fix-2329-prevent-pk-on-null-cells
Prevent creating Primary Key using columns that contain null values
This commit is contained in:
2
.gitignore
vendored
2
.gitignore
vendored
@@ -9,3 +9,5 @@ venv
|
||||
benchmark/perf_tools/dolt-builds/dolt
|
||||
benchmark/perf_tools/dolt-builds/working
|
||||
benchmark/perf_tools/output
|
||||
|
||||
test.sh
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user