Merge pull request #6 from database-pizza/fix/alter-table-semantics

test(alter-table): pin ADD/DROP/RENAME semantics for client migrations

pkg/executor/alter_table_test.go

@@ -0,0 +1,143 @@
+package executor
+
+import (
+	"sync"
+	"testing"
+)
+
+// ADD COLUMN only appends schema metadata. Existing rows are not rewritten, so
+// a declared DEFAULT applies to later INSERTs only.
+func TestAlterTableAddColumnDoesNotBackfillPopulatedTable(t *testing.T) {
+	_, schema, table := newTestDB(t)
+	e := newExec(schema, table)
+	execMust(t, e, "CREATE TABLE t (id INTEGER PRIMARY KEY)")
+	execMust(t, e, "INSERT INTO t (id) VALUES (1), (2)")
+
+	execMust(t, e, "ALTER TABLE t ADD COLUMN note TEXT DEFAULT 'later'")
+
+	res := execMust(t, e, "SELECT id, note FROM t ORDER BY id")
+	if res.RowCount != 2 {
+		t.Fatalf("row count = %d, want 2", res.RowCount)
+	}
+	for _, row := range res.Rows {
+		if row[1] != nil {
+			t.Fatalf("existing row %v note = %v, want NULL (no backfill)", row[0], row[1])
+		}
+	}
+
+	execMust(t, e, "INSERT INTO t (id) VALUES (3)")
+	res = execMust(t, e, "SELECT note FROM t WHERE id = 3")
+	if got := res.Rows[0][0]; got != "later" {
+		t.Fatalf("new row note = %v, want later", got)
+	}
+}
+
+func TestAlterTableAddColumnIfNotExistsIsIdempotent(t *testing.T) {
+	_, schema, table := newTestDB(t)
+	e := newExec(schema, table)
+	execMust(t, e, "CREATE TABLE t (id INTEGER PRIMARY KEY)")
+
+	execMust(t, e, "ALTER TABLE t ADD COLUMN IF NOT EXISTS c TEXT")
+	execMust(t, e, "ALTER TABLE t ADD COLUMN IF NOT EXISTS c TEXT")
+
+	s, err := schema.GetSchema("t")
+	if err != nil {
+		t.Fatalf("get schema: %v", err)
+	}
+	count := 0
+	for _, col := range s.Columns {
+		if col.Name == "c" {
+			count++
+		}
+	}
+	if count != 1 {
+		t.Fatalf("column c appears %d times, want 1", count)
+	}
+}
+
+// Concurrent IF NOT EXISTS ADD COLUMN on one engine instance must not duplicate
+// metadata; the schema manager serializes the check-and-append.
+func TestAlterTableAddColumnIfNotExistsConcurrent(t *testing.T) {
+	_, schema, table := newTestDB(t)
+	e := newExec(schema, table)
+	execMust(t, e, "CREATE TABLE t (id INTEGER PRIMARY KEY)")
+
+	var wg sync.WaitGroup
+	for i := 0; i < 8; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			if _, err := execSQL(e, "ALTER TABLE t ADD COLUMN IF NOT EXISTS c TEXT"); err != nil {
+				t.Errorf("add column: %v", err)
+			}
+		}()
+	}
+	wg.Wait()
+
+	s, err := schema.GetSchema("t")
+	if err != nil {
+		t.Fatalf("get schema: %v", err)
+	}
+	count := 0
+	for _, col := range s.Columns {
+		if col.Name == "c" {
+			count++
+		}
+	}
+	if count != 1 {
+		t.Fatalf("column c appears %d times, want 1", count)
+	}
+}
+
+// DROP COLUMN rewrites every remaining row so the removed value does not stay
+// in durable storage.
+func TestAlterTableDropColumnRewritesPopulatedRows(t *testing.T) {
+	_, schema, table := newTestDB(t)
+	e := newExec(schema, table)
+	execMust(t, e, "CREATE TABLE t (id INTEGER PRIMARY KEY, keep TEXT, dropme TEXT)")
+	execMust(t, e, "INSERT INTO t (id, keep, dropme) VALUES (1, 'a', 'secret'), (2, 'b', 'secret')")
+
+	execMust(t, e, "ALTER TABLE t DROP COLUMN dropme")
+
+	res := execMust(t, e, "SELECT * FROM t ORDER BY id")
+	if len(res.Columns) != 2 || res.Columns[0] != "id" || res.Columns[1] != "keep" {
+		t.Fatalf("columns after drop = %v, want [id keep]", res.Columns)
+	}
+	if res.RowCount != 2 || res.Rows[0][1] != "a" || res.Rows[1][1] != "b" {
+		t.Fatalf("rows after drop = %v", res.Rows)
+	}
+	if _, err := execSQL(e, "SELECT dropme FROM t"); err == nil {
+		t.Fatal("expected dropped column to be unknown")
+	}
+}
+
+// RENAME COLUMN rewrites row keys but deliberately leaves index metadata
+// unchanged. Documented guidance: drop and recreate indexes after a rename.
+func TestAlterTableRenameColumnLeavesIndexMetadataStale(t *testing.T) {
+	_, schema, table := newTestDB(t)
+	e := newExec(schema, table)
+	execMust(t, e, "CREATE TABLE t (id INTEGER PRIMARY KEY, a TEXT)")
+	execMust(t, e, "INSERT INTO t (id, a) VALUES (1, 'x')")
+	execMust(t, e, "CREATE INDEX idx_a ON t(a)")
+
+	execMust(t, e, "ALTER TABLE t RENAME COLUMN a TO aa")
+
+	res := execMust(t, e, "SELECT aa FROM t WHERE id = 1")
+	if got := res.Rows[0][0]; got != "x" {
+		t.Fatalf("renamed value = %v, want x", got)
+	}
+
+	names, err := schema.ListIndexes()
+	if err != nil {
+		t.Fatalf("list indexes: %v", err)
+	}
+	found := false
+	for _, name := range names {
+		if name == "idx_a" {
+			found = true
+		}
+	}
+	if !found {
+		t.Fatalf("indexes = %v, want idx_a to still exist (rename does not rewrite index metadata)", names)
+	}
+}

pkg/pgserver/alter_table_test.go

@@ -0,0 +1,46 @@
+package pgserver
+
+import (
+	"testing"
+)
+
+func TestAlterTableAddColumnOverSimpleQuery(t *testing.T) {
+	c, client := newDataConnection(t)
+	runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("CREATE TABLE t (id INTEGER PRIMARY KEY)"), 0)})
+	runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("INSERT INTO t (id) VALUES (1)"), 0)})
+
+	runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("ALTER TABLE t ADD COLUMN IF NOT EXISTS note TEXT DEFAULT 'later'"), 0)})
+	// Idempotent second boot-time migration.
+	runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("ALTER TABLE t ADD COLUMN IF NOT EXISTS note TEXT DEFAULT 'later'"), 0)})
+
+	msgs := runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("SELECT id, note FROM t ORDER BY id"), 0)})
+	values := dataRowValues(t, msgs[1])
+	if len(values) != 2 || values[0] != "1" || values[1] != "<null>" {
+		t.Fatalf("existing row after ADD COLUMN = %v, want [1 <null>]", values)
+	}
+
+	runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("INSERT INTO t (id) VALUES (2)"), 0)})
+	msgs = runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("SELECT note FROM t WHERE id = 2"), 0)})
+	if got := dataRowValues(t, msgs[1]); len(got) != 1 || got[0] != "later" {
+		t.Fatalf("new row note = %v, want [later]", got)
+	}
+}
+
+func TestAlterTableDropAndRenameColumnOverSimpleQuery(t *testing.T) {
+	c, client := newDataConnection(t)
+	runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("CREATE TABLE t (id INTEGER PRIMARY KEY, keep TEXT, dropme TEXT)"), 0)})
+	runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("INSERT INTO t (id, keep, dropme) VALUES (1, 'a', 'gone')"), 0)})
+
+	runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("ALTER TABLE t DROP COLUMN dropme"), 0)})
+	runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("ALTER TABLE t RENAME COLUMN keep TO kept"), 0)})
+
+	msgs := runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("SELECT kept FROM t WHERE id = 1"), 0)})
+	if got := dataRowValues(t, msgs[1]); len(got) != 1 || got[0] != "a" {
+		t.Fatalf("renamed column value = %v, want [a]", got)
+	}
+
+	msgs = runQuery(t, c, client, &Message{Type: MsgQuery, Data: append([]byte("SELECT dropme FROM t"), 0)})
+	if code := errorCode(msgs[0]); code == "" {
+		t.Fatalf("expected an error selecting a dropped column, got %v", msgs)
+	}
+}