Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 21 additions & 6 deletions internal/plan/rewrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func generateIndexRewrite(index *ir.Index) []RewriteStep {

// Generate concurrent SQL
concurrentSQL := generateIndexSQL(index, true) // With CONCURRENTLY
waitSQL := generateIndexWaitQueryWithName(index.Name)
waitSQL := generateIndexWaitQueryWithName(index.Schema, index.Name)

return []RewriteStep{
{
Expand Down Expand Up @@ -176,7 +176,7 @@ func generateIndexChangeRewriteFromIndex(index *ir.Index) []RewriteStep {
tempIndex := *index
tempIndex.Name = tempIndexName
concurrentSQL := generateIndexSQL(&tempIndex, true)
waitSQL := generateIndexWaitQueryWithName(tempIndexName)
waitSQL := generateIndexWaitQueryWithName(index.Schema, tempIndexName)

// Drop old index and rename new one. Use IF EXISTS because a same-plan
// DROP COLUMN step may already have implicitly dropped this index
Expand Down Expand Up @@ -227,7 +227,7 @@ func generateIndexChangeRewrite(indexDiff *diff.IndexDiff) []RewriteStep {
tempIndex := *indexDiff.New
tempIndex.Name = tempIndexName
concurrentSQL := generateIndexSQL(&tempIndex, true)
waitSQL := generateIndexWaitQueryWithName(tempIndexName)
waitSQL := generateIndexWaitQueryWithName(indexDiff.New.Schema, tempIndexName)

// Drop old index and rename new one. Use IF EXISTS because a same-plan
// DROP COLUMN step may already have implicitly dropped this index
Expand Down Expand Up @@ -550,18 +550,33 @@ func generateIndexSQL(index *ir.Index, isConcurrent bool) string {
return sql.String()
}

// generateIndexWaitQueryWithName creates a wait query for monitoring concurrent index creation
func generateIndexWaitQueryWithName(indexName string) string {
// generateIndexWaitQueryWithName creates a wait query for monitoring concurrent index creation.
// The query is schema-qualified so an invalid index of the same name in another
// schema (e.g. a failed CREATE INDEX CONCURRENTLY left behind in a sibling
// tenant schema) cannot make apply hang forever. See issue #641.
//
// The schema is matched by pg_namespace.nspname against an escaped string
// literal rather than a '...'::regnamespace cast, which would case-fold
// unquoted mixed-case names and break on embedded quotes.
func generateIndexWaitQueryWithName(schemaName, indexName string) string {
return fmt.Sprintf(`SELECT
COALESCE(i.indisvalid, false) as done,
CASE
WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total
ELSE 0
END as progress
FROM pg_class c
JOIN pg_namespace n ON c.relnamespace = n.oid
LEFT JOIN pg_index i ON c.oid = i.indexrelid
LEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid
WHERE c.relname = '%s';`, indexName)
WHERE c.relname = %s
AND n.nspname = %s;`, quoteSQLLiteral(indexName), quoteSQLLiteral(schemaName))
}

// quoteSQLLiteral returns s as a single-quoted SQL string literal, doubling
// embedded single quotes.
func quoteSQLLiteral(s string) string {
return "'" + strings.ReplaceAll(s, "'", "''") + "'"
}

// Helper functions
Expand Down
70 changes: 70 additions & 0 deletions internal/plan/rewrite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,73 @@ func TestPendingNotNullValidateIsolated(t *testing.T) {
t.Errorf("isolated step SQL = %q, want %q", got, validate.Statements[0].SQL)
}
}

// TestGenerateIndexWaitQuerySchemaQualified verifies the wait query for
// CREATE INDEX CONCURRENTLY is limited to the target schema (issue #641).
// Without the namespace filter, an invalid index of the same name in another
// schema makes apply hang forever because checkWaitStatus reads the first row.
func TestGenerateIndexWaitQuerySchemaQualified(t *testing.T) {
sql := generateIndexWaitQueryWithName("tenant_b", "items_sku_idx")
if !strings.Contains(sql, "c.relname = 'items_sku_idx'") {
t.Fatalf("wait query missing index name filter:\n%s", sql)
}
if !strings.Contains(sql, "n.nspname = 'tenant_b'") {
t.Fatalf("wait query missing schema qualification (issue #641):\n%s", sql)
}
}

// TestGenerateIndexRewriteWaitIsSchemaQualified wires the schema through the
// concurrent rewrite path so apply waits on the right index.
func TestGenerateIndexRewriteWaitIsSchemaQualified(t *testing.T) {
index := &ir.Index{
Schema: "tenant_b",
Table: "items",
Name: "items_sku_idx",
Type: ir.IndexTypeRegular,
Method: "btree",
Columns: []*ir.IndexColumn{{Name: "sku"}},
}
steps := generateIndexRewrite(index)
if len(steps) < 2 {
t.Fatalf("expected concurrent create + wait steps, got %d", len(steps))
}
wait := steps[1].SQL
if !strings.Contains(wait, "n.nspname = 'tenant_b'") {
t.Fatalf("rewrite wait step not schema-qualified:\n%s", wait)
}
if steps[1].Directive == nil || steps[1].Directive.Type != DirectiveTypeWait {
t.Fatalf("expected wait directive on step 1, got %#v", steps[1].Directive)
}
}

// TestGenerateIndexWaitQueryEscapesNames verifies the wait query matches the
// exact (case-sensitive) schema name and escapes embedded single quotes,
// instead of relying on a '...'::regnamespace cast that case-folds unquoted
// names (issue #641).
func TestGenerateIndexWaitQueryEscapesNames(t *testing.T) {
tests := []struct {
name string
schema string
index string
wantSchema string
wantIndex string
}{
{"mixed case schema", "TenantB", "items_sku_idx", "n.nspname = 'TenantB'", "c.relname = 'items_sku_idx'"},
{"apostrophe in schema", "o'brien", "items_sku_idx", "n.nspname = 'o''brien'", "c.relname = 'items_sku_idx'"},
{"apostrophe in index", "public", "it's_idx", "n.nspname = 'public'", "c.relname = 'it''s_idx'"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
sql := generateIndexWaitQueryWithName(tt.schema, tt.index)
if !strings.Contains(sql, tt.wantSchema) {
t.Errorf("wait query missing %q:\n%s", tt.wantSchema, sql)
}
if !strings.Contains(sql, tt.wantIndex) {
t.Errorf("wait query missing %q:\n%s", tt.wantIndex, sql)
}
if strings.Contains(sql, "regnamespace") {
t.Errorf("wait query must not cast to regnamespace:\n%s", sql)
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
{
"steps": [
{
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'idx_orders_status_active';",
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nJOIN pg_namespace n ON c.relnamespace = n.oid\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'idx_orders_status_active'\n AND n.nspname = 'public';",
"directive": {
"type": "wait",
"message": "Creating index idx_orders_status_active"
Expand All @@ -43,7 +43,7 @@
{
"steps": [
{
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'idx_orders_user_id';",
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nJOIN pg_namespace n ON c.relnamespace = n.oid\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'idx_orders_user_id'\n AND n.nspname = 'public';",
"directive": {
"type": "wait",
"message": "Creating index idx_orders_user_id"
Expand Down
8 changes: 6 additions & 2 deletions testdata/diff/create_index/add_index_with_reloptions/plan.sql
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ SELECT
ELSE 0
END as progress
FROM pg_class c
JOIN pg_namespace n ON c.relnamespace = n.oid
LEFT JOIN pg_index i ON c.oid = i.indexrelid
LEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid
WHERE c.relname = 'idx_orders_status_active';
WHERE c.relname = 'idx_orders_status_active'
AND n.nspname = 'public';

CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_orders_user_id ON orders (user_id) WITH (fillfactor=90);

Expand All @@ -22,6 +24,8 @@ SELECT
ELSE 0
END as progress
FROM pg_class c
JOIN pg_namespace n ON c.relnamespace = n.oid
LEFT JOIN pg_index i ON c.oid = i.indexrelid
LEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid
WHERE c.relname = 'idx_orders_user_id';
WHERE c.relname = 'idx_orders_user_id'
AND n.nspname = 'public';
8 changes: 6 additions & 2 deletions testdata/diff/create_index/add_index_with_reloptions/plan.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ SELECT
ELSE 0
END as progress
FROM pg_class c
JOIN pg_namespace n ON c.relnamespace = n.oid
LEFT JOIN pg_index i ON c.oid = i.indexrelid
LEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid
WHERE c.relname = 'idx_orders_status_active';
WHERE c.relname = 'idx_orders_status_active'
AND n.nspname = 'public';

-- Transaction Group #3
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_orders_user_id ON orders (user_id) WITH (fillfactor=90);
Expand All @@ -39,6 +41,8 @@ SELECT
ELSE 0
END as progress
FROM pg_class c
JOIN pg_namespace n ON c.relnamespace = n.oid
LEFT JOIN pg_index i ON c.oid = i.indexrelid
LEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid
WHERE c.relname = 'idx_orders_user_id';
WHERE c.relname = 'idx_orders_user_id'
AND n.nspname = 'public';
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
{
"steps": [
{
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'idx_events_desc_pgschema_new';",
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nJOIN pg_namespace n ON c.relnamespace = n.oid\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'idx_events_desc_pgschema_new'\n AND n.nspname = 'public';",
"directive": {
"type": "wait",
"message": "Creating index idx_events_desc_pgschema_new"
Expand Down
4 changes: 3 additions & 1 deletion testdata/diff/create_index/alter_index_null_order/plan.sql
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ SELECT
ELSE 0
END as progress
FROM pg_class c
JOIN pg_namespace n ON c.relnamespace = n.oid
LEFT JOIN pg_index i ON c.oid = i.indexrelid
LEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid
WHERE c.relname = 'idx_events_desc_pgschema_new';
WHERE c.relname = 'idx_events_desc_pgschema_new'
AND n.nspname = 'public';

DROP INDEX IF EXISTS idx_events_desc;

Expand Down
4 changes: 3 additions & 1 deletion testdata/diff/create_index/alter_index_null_order/plan.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ SELECT
ELSE 0
END as progress
FROM pg_class c
JOIN pg_namespace n ON c.relnamespace = n.oid
LEFT JOIN pg_index i ON c.oid = i.indexrelid
LEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid
WHERE c.relname = 'idx_events_desc_pgschema_new';
WHERE c.relname = 'idx_events_desc_pgschema_new'
AND n.nspname = 'public';

-- Transaction Group #3
DROP INDEX IF EXISTS idx_events_desc;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
{
"steps": [
{
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'activity_embedding_idx';",
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nJOIN pg_namespace n ON c.relnamespace = n.oid\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'activity_embedding_idx'\n AND n.nspname = 'public';",
"directive": {
"type": "wait",
"message": "Creating index activity_embedding_idx"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ SELECT
ELSE 0
END as progress
FROM pg_class c
JOIN pg_namespace n ON c.relnamespace = n.oid
LEFT JOIN pg_index i ON c.oid = i.indexrelid
LEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid
WHERE c.relname = 'activity_embedding_idx';
WHERE c.relname = 'activity_embedding_idx'
AND n.nspname = 'public';
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ SELECT
ELSE 0
END as progress
FROM pg_class c
JOIN pg_namespace n ON c.relnamespace = n.oid
LEFT JOIN pg_index i ON c.oid = i.indexrelid
LEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid
WHERE c.relname = 'activity_embedding_idx';
WHERE c.relname = 'activity_embedding_idx'
AND n.nspname = 'public';
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
{
"steps": [
{
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'parent2_id_tenant_key';",
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nJOIN pg_namespace n ON c.relnamespace = n.oid\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'parent2_id_tenant_key'\n AND n.nspname = 'public';",
"directive": {
"type": "wait",
"message": "Creating index parent2_id_tenant_key"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ SELECT
ELSE 0
END as progress
FROM pg_class c
JOIN pg_namespace n ON c.relnamespace = n.oid
LEFT JOIN pg_index i ON c.oid = i.indexrelid
LEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid
WHERE c.relname = 'parent2_id_tenant_key';
WHERE c.relname = 'parent2_id_tenant_key'
AND n.nspname = 'public';

ALTER TABLE child
ADD CONSTRAINT child_parent_id_tenant_fkey FOREIGN KEY (parent_id, tenant) REFERENCES parent (id, tenant);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@ SELECT
ELSE 0
END as progress
FROM pg_class c
JOIN pg_namespace n ON c.relnamespace = n.oid
LEFT JOIN pg_index i ON c.oid = i.indexrelid
LEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid
WHERE c.relname = 'parent2_id_tenant_key';
WHERE c.relname = 'parent2_id_tenant_key'
AND n.nspname = 'public';

-- Transaction Group #4
ALTER TABLE child
ADD CONSTRAINT child_parent_id_tenant_fkey FOREIGN KEY (parent_id, tenant) REFERENCES parent (id, tenant);

ALTER TABLE child2
ADD CONSTRAINT child2_parent2_id_tenant_fkey FOREIGN KEY (parent2_id, tenant) REFERENCES parent2 (id, tenant);
ADD CONSTRAINT child2_parent2_id_tenant_fkey FOREIGN KEY (parent2_id, tenant) REFERENCES parent2 (id, tenant);
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
{
"steps": [
{
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'metrics_tripled_idx_pgschema_new';",
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nJOIN pg_namespace n ON c.relnamespace = n.oid\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'metrics_tripled_idx_pgschema_new'\n AND n.nspname = 'public';",
"directive": {
"type": "wait",
"message": "Creating index metrics_tripled_idx_pgschema_new"
Expand Down Expand Up @@ -199,7 +199,7 @@
{
"steps": [
{
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'orders_code_key_pgschema_new';",
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nJOIN pg_namespace n ON c.relnamespace = n.oid\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'orders_code_key_pgschema_new'\n AND n.nspname = 'public';",
"directive": {
"type": "wait",
"message": "Creating index orders_code_key_pgschema_new"
Expand Down Expand Up @@ -239,7 +239,7 @@
{
"steps": [
{
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'orders_lookup_idx_pgschema_new';",
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nJOIN pg_namespace n ON c.relnamespace = n.oid\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'orders_lookup_idx_pgschema_new'\n AND n.nspname = 'public';",
"directive": {
"type": "wait",
"message": "Creating index orders_lookup_idx_pgschema_new"
Expand Down Expand Up @@ -279,7 +279,7 @@
{
"steps": [
{
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'orders_total_key';",
"sql": "SELECT \n COALESCE(i.indisvalid, false) as done,\n CASE \n WHEN p.blocks_total > 0 THEN p.blocks_done * 100 / p.blocks_total\n ELSE 0\n END as progress\nFROM pg_class c\nJOIN pg_namespace n ON c.relnamespace = n.oid\nLEFT JOIN pg_index i ON c.oid = i.indexrelid\nLEFT JOIN pg_stat_progress_create_index p ON c.oid = p.index_relid\nWHERE c.relname = 'orders_total_key'\n AND n.nspname = 'public';",
"directive": {
"type": "wait",
"message": "Creating index orders_total_key"
Expand Down
Loading
Loading