Skip to content

Commit 889ab33

Browse files
authored
fix: exclude generated columns from DUMP INSERT statements (#469)
Fixes DUMP statement failures when tables contain generated columns by dynamically querying INFORMATION_SCHEMA.COLUMNS to identify and exclude non-writable columns. Key changes: - Queries INFORMATION_SCHEMA.COLUMNS to get only writable columns (IS_GENERATED = 'NEVER') - Generates explicit column SELECT queries instead of SELECT * to exclude generated columns - Handles both simple and schema-qualified table names correctly - Gracefully skips tables with no writable columns with informative comments - Maintains transaction consistency by reusing read-only transactions across operations - Preserves query options (PROFILE mode, RPC priority) throughout execution Technical improvements: - Refactored dependency resolver to use more efficient spanner.SelectAll - Enhanced metrics handling for accurate DUMP profiling - Added comprehensive integration tests for generated column scenarios This ensures DUMP-generated INSERT statements are always valid and can be successfully reimported, even when tables contain STORED/virtual generated columns or TOKENLIST columns. Fixes #467
1 parent 19e4af0 commit 889ab33

File tree

6 files changed

+473
-216
lines changed

6 files changed

+473
-216
lines changed

dependency_resolver.go

Lines changed: 70 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,13 @@ import (
88
"strings"
99

1010
"cloud.google.com/go/spanner"
11-
"google.golang.org/api/iterator"
1211
)
1312

14-
// FKReference represents a foreign key relationship
13+
// FKReference represents a foreign key relationship between tables
1514
type FKReference struct {
16-
ConstraintName string
17-
ChildTable string
18-
ChildColumn string
19-
ParentTable string
20-
ParentColumn string
15+
ConstraintName string `spanner:"CONSTRAINT_NAME"`
16+
ChildTable string `spanner:"CHILD_TABLE"`
17+
ParentTable string `spanner:"PARENT_TABLE"`
2118
}
2219

2320
// TableDependency represents a table with all its dependencies
@@ -45,15 +42,22 @@ func NewDependencyResolver() *DependencyResolver {
4542
}
4643
}
4744

48-
// BuildDependencyGraph queries the database and builds the complete dependency graph
49-
func (dr *DependencyResolver) BuildDependencyGraph(ctx context.Context, session *Session) error {
45+
// interleaveRow represents a row from INFORMATION_SCHEMA.TABLES for interleave relationships
46+
type interleaveRow struct {
47+
TableName string `spanner:"TABLE_NAME"`
48+
ParentTable *string `spanner:"PARENT_TABLE_NAME"`
49+
OnDeleteAction *string `spanner:"ON_DELETE_ACTION"`
50+
}
51+
52+
// BuildDependencyGraphWithTxn queries the database and builds the complete dependency graph using a transaction
53+
func (dr *DependencyResolver) BuildDependencyGraphWithTxn(ctx context.Context, txn *spanner.ReadOnlyTransaction) error {
5054
// First, query INTERLEAVE relationships
51-
if err := dr.queryInterleaveRelationships(ctx, session); err != nil {
55+
if err := dr.queryInterleaveRelationshipsWithTxn(ctx, txn); err != nil {
5256
return fmt.Errorf("failed to query interleave relationships: %w", err)
5357
}
5458

5559
// Then, query foreign key relationships
56-
if err := dr.queryForeignKeyRelationships(ctx, session); err != nil {
60+
if err := dr.queryForeignKeyRelationshipsWithTxn(ctx, txn); err != nil {
5761
return fmt.Errorf("failed to query foreign key relationships: %w", err)
5862
}
5963

@@ -63,8 +67,30 @@ func (dr *DependencyResolver) BuildDependencyGraph(ctx context.Context, session
6367
return nil
6468
}
6569

66-
// queryInterleaveRelationships queries and builds INTERLEAVE parent-child relationships
67-
func (dr *DependencyResolver) queryInterleaveRelationships(ctx context.Context, session *Session) error {
70+
// processInterleaveRows processes all interleave relationship rows and updates the dependency graph.
71+
func (dr *DependencyResolver) processInterleaveRows(rows []interleaveRow) {
72+
for _, data := range rows {
73+
// Get or create table dependency
74+
table := dr.getOrCreateTable(data.TableName)
75+
76+
// Set INTERLEAVE parent information
77+
if data.ParentTable != nil {
78+
table.ParentTable = *data.ParentTable
79+
if data.OnDeleteAction != nil {
80+
table.OnDeleteAction = *data.OnDeleteAction
81+
}
82+
83+
// Update parent's children list
84+
parent := dr.getOrCreateTable(*data.ParentTable)
85+
if !slices.Contains(parent.ChildrenTables, data.TableName) {
86+
parent.ChildrenTables = append(parent.ChildrenTables, data.TableName)
87+
}
88+
}
89+
}
90+
}
91+
92+
// queryInterleaveRelationshipsWithTxn queries and builds INTERLEAVE parent-child relationships using a transaction
93+
func (dr *DependencyResolver) queryInterleaveRelationshipsWithTxn(ctx context.Context, txn *spanner.ReadOnlyTransaction) error {
6894
query := `
6995
SELECT
7096
TABLE_NAME,
@@ -75,122 +101,54 @@ func (dr *DependencyResolver) queryInterleaveRelationships(ctx context.Context,
75101
ORDER BY TABLE_NAME
76102
`
77103

78-
iter := session.client.Single().Query(ctx, spanner.Statement{SQL: query})
79-
defer iter.Stop()
80-
81-
for {
82-
row, err := iter.Next()
83-
if err == iterator.Done {
84-
break
85-
}
86-
if err != nil {
87-
return err
88-
}
89-
90-
var tableName, parentTable, onDeleteAction spanner.NullString
91-
if err := row.Columns(&tableName, &parentTable, &onDeleteAction); err != nil {
92-
return err
93-
}
94-
95-
if !tableName.Valid {
96-
continue
97-
}
98-
99-
name := tableName.StringVal
100-
101-
// Get or create table dependency
102-
table := dr.getOrCreateTable(name)
103-
104-
// Set INTERLEAVE parent information
105-
if parentTable.Valid {
106-
table.ParentTable = parentTable.StringVal
107-
table.OnDeleteAction = onDeleteAction.StringVal
108-
109-
// Update parent's children list
110-
parent := dr.getOrCreateTable(parentTable.StringVal)
111-
if !slices.Contains(parent.ChildrenTables, name) {
112-
parent.ChildrenTables = append(parent.ChildrenTables, name)
113-
}
114-
}
104+
var rows []interleaveRow
105+
if err := spanner.SelectAll(txn.Query(ctx, spanner.Statement{SQL: query}), &rows); err != nil {
106+
return fmt.Errorf("failed to query interleave relationships: %w", err)
115107
}
116108

109+
dr.processInterleaveRows(rows)
117110
return nil
118111
}
119112

120-
// queryForeignKeyRelationships queries and builds foreign key relationships
121-
func (dr *DependencyResolver) queryForeignKeyRelationships(ctx context.Context, session *Session) error {
122-
// Query using REFERENTIAL_CONSTRAINTS and KEY_COLUMN_USAGE tables
123-
// For multi-column FKs, POSITION_IN_UNIQUE_CONSTRAINT on the child side
124-
// matches ORDINAL_POSITION on the parent side
113+
// processForeignKeyRows processes all foreign key rows and updates the dependency graph.
114+
func (dr *DependencyResolver) processForeignKeyRows(rows []FKReference) {
115+
// No need for deduplication since the query now returns unique constraints only
116+
for _, fkRef := range rows {
117+
// Update child table's foreign keys
118+
childDep := dr.getOrCreateTable(fkRef.ChildTable)
119+
childDep.ForeignKeys = append(childDep.ForeignKeys, fkRef)
120+
121+
// Update parent table's referenced by list
122+
parentDep := dr.getOrCreateTable(fkRef.ParentTable)
123+
parentDep.ReferencedBy = append(parentDep.ReferencedBy, fkRef)
124+
}
125+
}
126+
127+
// queryForeignKeyRelationshipsWithTxn queries and builds foreign key relationships using a transaction
128+
func (dr *DependencyResolver) queryForeignKeyRelationshipsWithTxn(ctx context.Context, txn *spanner.ReadOnlyTransaction) error {
129+
// Query foreign key relationships at the table level
130+
// KEY_COLUMN_USAGE is needed because REFERENTIAL_CONSTRAINTS doesn't contain table names
125131
query := `
126-
SELECT
132+
SELECT DISTINCT
127133
rc.CONSTRAINT_NAME,
128-
kcu_child.TABLE_NAME AS child_table,
129-
kcu_child.COLUMN_NAME AS child_column,
130-
kcu_parent.TABLE_NAME AS parent_table,
131-
kcu_parent.COLUMN_NAME AS parent_column
134+
kcu_child.TABLE_NAME AS CHILD_TABLE,
135+
kcu_parent.TABLE_NAME AS PARENT_TABLE
132136
FROM information_schema.referential_constraints rc
133137
JOIN information_schema.key_column_usage kcu_child
134-
ON rc.CONSTRAINT_SCHEMA = kcu_child.CONSTRAINT_SCHEMA
135-
AND rc.CONSTRAINT_NAME = kcu_child.CONSTRAINT_NAME
138+
USING (CONSTRAINT_SCHEMA, CONSTRAINT_NAME)
136139
JOIN information_schema.key_column_usage kcu_parent
137140
ON rc.UNIQUE_CONSTRAINT_SCHEMA = kcu_parent.CONSTRAINT_SCHEMA
138141
AND rc.UNIQUE_CONSTRAINT_NAME = kcu_parent.CONSTRAINT_NAME
139-
AND kcu_child.POSITION_IN_UNIQUE_CONSTRAINT = kcu_parent.ORDINAL_POSITION
140142
WHERE rc.CONSTRAINT_SCHEMA = ''
141-
ORDER BY rc.CONSTRAINT_NAME, kcu_child.ORDINAL_POSITION
143+
ORDER BY rc.CONSTRAINT_NAME
142144
`
143145

144-
iter := session.client.Single().Query(ctx, spanner.Statement{SQL: query})
145-
defer iter.Stop()
146-
147-
// Track which constraints we've already processed (for multi-column FKs)
148-
processedConstraints := make(map[string]bool)
149-
150-
for {
151-
row, err := iter.Next()
152-
if err == iterator.Done {
153-
break
154-
}
155-
if err != nil {
156-
return err
157-
}
158-
159-
var constraintName, childTable, childColumn, parentTable, parentColumn spanner.NullString
160-
if err := row.Columns(&constraintName, &childTable, &childColumn, &parentTable, &parentColumn); err != nil {
161-
return err
162-
}
163-
164-
if !childTable.Valid || !parentTable.Valid || !constraintName.Valid {
165-
continue
166-
}
167-
168-
// For dependency resolution, we only need one entry per constraint
169-
// (table-to-table relationship), not per column
170-
constraintKey := constraintName.StringVal
171-
if processedConstraints[constraintKey] {
172-
// Already processed this constraint (multi-column FK)
173-
continue
174-
}
175-
processedConstraints[constraintKey] = true
176-
177-
fkRef := FKReference{
178-
ConstraintName: constraintName.StringVal,
179-
ChildTable: childTable.StringVal,
180-
ChildColumn: childColumn.StringVal, // Note: This will be the first column for multi-column FKs
181-
ParentTable: parentTable.StringVal,
182-
ParentColumn: parentColumn.StringVal, // Note: This will be the first column for multi-column FKs
183-
}
184-
185-
// Add FK to child table's foreign keys
186-
child := dr.getOrCreateTable(childTable.StringVal)
187-
child.ForeignKeys = append(child.ForeignKeys, fkRef)
188-
189-
// Add FK to parent table's referenced by list
190-
parent := dr.getOrCreateTable(parentTable.StringVal)
191-
parent.ReferencedBy = append(parent.ReferencedBy, fkRef)
146+
var rows []FKReference
147+
if err := spanner.SelectAll(txn.Query(ctx, spanner.Statement{SQL: query}), &rows); err != nil {
148+
return fmt.Errorf("failed to query foreign key relationships: %w", err)
192149
}
193150

151+
dr.processForeignKeyRows(rows)
194152
return nil
195153
}
196154

dependency_resolver_test.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func TestDependencyResolver_TopologicalSort(t *testing.T) {
116116
"Employees": {
117117
TableName: "Employees",
118118
ForeignKeys: []FKReference{
119-
{ParentTable: "Employees", ChildTable: "Employees", ChildColumn: "ManagerId"},
119+
{ConstraintName: "FK_Employees_Manager", ParentTable: "Employees", ChildTable: "Employees"},
120120
},
121121
Level: 0,
122122
},
@@ -421,8 +421,8 @@ func TestDependencyResolver_ComplexScenarios(t *testing.T) {
421421
"Posts": {
422422
TableName: "Posts",
423423
ForeignKeys: []FKReference{
424-
{ParentTable: "Users", ChildTable: "Posts", ChildColumn: "AuthorId"},
425-
{ParentTable: "Users", ChildTable: "Posts", ChildColumn: "EditorId"},
424+
{ConstraintName: "FK_Posts_Author", ParentTable: "Users", ChildTable: "Posts"},
425+
{ConstraintName: "FK_Posts_Editor", ParentTable: "Users", ChildTable: "Posts"},
426426
},
427427
Level: 0,
428428
},
@@ -557,8 +557,6 @@ func TestDependencyResolver_MultiColumnFK(t *testing.T) {
557557
ConstraintName: "FK_User",
558558
ParentTable: "Users",
559559
ChildTable: "UserPreferences",
560-
ChildColumn: "UserId", // Would be first column of multi-column FK
561-
ParentColumn: "Id",
562560
},
563561
},
564562
Level: 0,

0 commit comments

Comments
 (0)