Skip to content

Commit 34ecaea

Browse files
fix(ctid-load): fix for mixed case partitioned tables (#4234)
### Why When the CTID based partitioning initial load setting is enabled, initial loads for mixed case table names do not sync data to target, because the input we pass to `to_regclass` in queries was not quoted, and Postgres assumes lowercase, making those queries not return anything ### What - Fix `to_regclass` usage in estimated table size function - Make `classifyTableName` return OID of the table as well - Replace usage of `to_regclass` with comparing with OID directly - Adapt E2E test to cover mixed case usage Fixes: DBI-718
1 parent e457c60 commit 34ecaea

3 files changed

Lines changed: 92 additions & 48 deletions

File tree

flow/connectors/postgres/postgres.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,9 +360,13 @@ func (c *PostgresConnector) GetDatabaseVariant(ctx context.Context) (protos.Data
360360
}
361361

362362
func (c *PostgresConnector) GetTableSizeEstimatedBytes(ctx context.Context, tableIdentifier string) (int64, error) {
363+
parsed, err := common.ParseTableIdentifier(tableIdentifier)
364+
if err != nil {
365+
return 0, fmt.Errorf("failed to parse table identifier %s: %w", tableIdentifier, err)
366+
}
363367
tableSizeQuery := "SELECT pg_relation_size(to_regclass($1))"
364368
var tableSizeBytes pgtype.Int8
365-
if err := c.conn.QueryRow(ctx, tableSizeQuery, tableIdentifier).Scan(&tableSizeBytes); err != nil {
369+
if err := c.conn.QueryRow(ctx, tableSizeQuery, parsed.String()).Scan(&tableSizeBytes); err != nil {
366370
return 0, err
367371
}
368372
if !tableSizeBytes.Valid {

flow/connectors/postgres/qrep_partition.go

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,13 @@ func CTIDBlockPartitioningFunc(ctx context.Context, pp PartitionParams) ([]*prot
130130
}
131131
switch {
132132
case tc.relkind == "p":
133-
blocksPerTable, err := getPartitionedTables(ctx, pp.tx, tc.qualifiedName)
133+
blocksPerTable, err := getPartitionedTables(ctx, pp.tx, tc.oid)
134134
if err != nil {
135135
return nil, fmt.Errorf("failed to get child partitioned tables: %w", err)
136136
}
137137
return ctidPartitionsForChildTables(pp, blocksPerTable)
138138
case tc.relhassubclass:
139-
blocksPerTable, err := getInheritedTables(ctx, pp.tx, tc.qualifiedName)
139+
blocksPerTable, err := getInheritedTables(ctx, pp.tx, tc.oid, tc.qualifiedName)
140140
if err != nil {
141141
return nil, fmt.Errorf("failed to get child inherited tables: %w", err)
142142
}
@@ -305,19 +305,22 @@ func getTableBlockCount(ctx context.Context, tx pgx.Tx, table string) (int64, er
305305
type tableClassification struct {
306306
qualifiedName string
307307
relkind string
308+
oid uint32
308309
relhassubclass bool
309310
}
310311

311312
func classifyTable(ctx context.Context, tx pgx.Tx, table string) (tableClassification, error) {
312313
var classification tableClassification
313314
err := tx.QueryRow(ctx,
314315
// We fetch the qualified name from pg_class instead of using the input directly because
315-
// the input table name is quoted; and we want the unquoted canonical form for uniformity
316-
`SELECT format('%s.%s', n.nspname, c.relname), c.relkind::text, c.relhassubclass
316+
// the input table name is quoted; and we want the unquoted canonical form for uniformity.
317+
// We also fetch the OID so downstream queries can join on it directly, avoiding
318+
// to_regclass which lowercases unquoted identifiers and breaks mixed-case table names.
319+
`SELECT c.oid, format('%s.%s', n.nspname, c.relname), c.relkind::text, c.relhassubclass
317320
FROM pg_class c
318321
JOIN pg_namespace n ON c.relnamespace = n.oid
319322
WHERE c.oid = to_regclass($1)`,
320-
table).Scan(&classification.qualifiedName, &classification.relkind, &classification.relhassubclass)
323+
table).Scan(&classification.oid, &classification.qualifiedName, &classification.relkind, &classification.relhassubclass)
321324
if err != nil {
322325
if errors.Is(err, pgx.ErrNoRows) {
323326
return classification, fmt.Errorf("table %s not found in pg_class", table)
@@ -329,30 +332,32 @@ func classifyTable(ctx context.Context, tx pgx.Tx, table string) (tableClassific
329332

330333
// getPartitionedTables returns a map of partitioned child table names to their block counts,
331334
// recursively handle multi-level partitioned tables.
332-
func getPartitionedTables(ctx context.Context, tx pgx.Tx, table string) (map[string]int64, error) {
335+
// Uses OID-based join to avoid to_regclass which lowercases unquoted mixed-case identifiers.
336+
func getPartitionedTables(ctx context.Context, tx pgx.Tx, parentOID uint32) (map[string]int64, error) {
333337
rows, err := tx.Query(ctx, `
334-
SELECT format('%s.%s', n.nspname, c.relname), c.relkind::text,
338+
SELECT c.oid, format('%s.%s', n.nspname, c.relname), c.relkind::text,
335339
(pg_relation_size(c.oid) / current_setting('block_size')::int)::bigint
336340
FROM pg_inherits i
337341
JOIN pg_class c ON i.inhrelid = c.oid
338342
JOIN pg_namespace n ON c.relnamespace = n.oid
339-
WHERE i.inhparent = to_regclass($1)
343+
WHERE i.inhparent = $1
340344
ORDER BY c.relname
341-
`, table)
345+
`, parentOID)
342346
if err != nil {
343-
return nil, fmt.Errorf("failed to query child tables for %s: %w", table, err)
347+
return nil, fmt.Errorf("failed to query child tables for oid %d: %w", parentOID, err)
344348
}
345349
defer rows.Close()
346350

347351
type tableInfo struct {
348352
name string
349353
kind string
350354
blocks int64
355+
oid uint32
351356
}
352357
var tableInfos []tableInfo
353358
for rows.Next() {
354359
var info tableInfo
355-
if err := rows.Scan(&info.name, &info.kind, &info.blocks); err != nil {
360+
if err := rows.Scan(&info.oid, &info.name, &info.kind, &info.blocks); err != nil {
356361
return nil, fmt.Errorf("failed to scan child table: %w", err)
357362
}
358363
tableInfos = append(tableInfos, info)
@@ -364,7 +369,7 @@ func getPartitionedTables(ctx context.Context, tx pgx.Tx, table string) (map[str
364369
blocksPerPartitionedTable := make(map[string]int64)
365370
for _, info := range tableInfos {
366371
if info.kind == "p" {
367-
leaveTables, err := getPartitionedTables(ctx, tx, info.name)
372+
leaveTables, err := getPartitionedTables(ctx, tx, info.oid)
368373
if err != nil {
369374
return nil, fmt.Errorf("failed to get partitions of %s: %w", info.name, err)
370375
}
@@ -378,40 +383,44 @@ func getPartitionedTables(ctx context.Context, tx pgx.Tx, table string) (map[str
378383

379384
// getInheritedTables returns a map of table names to their block counts for an inherited
380385
// table hierarchy. Unlike partitioned tables, the parent itself stores data and is included.
381-
func getInheritedTables(ctx context.Context, tx pgx.Tx, table string) (map[string]int64, error) {
386+
// Uses OID-based join to avoid to_regclass which lowercases unquoted mixed-case identifiers.
387+
func getInheritedTables(ctx context.Context, tx pgx.Tx, parentOID uint32, parentName string) (map[string]int64, error) {
382388
blocksPerTable := make(map[string]int64)
383389

384-
numBlocks, err := getTableBlockCount(ctx, tx, table)
385-
if err != nil {
386-
return nil, err
390+
var numBlocks int64
391+
if err := tx.QueryRow(ctx,
392+
"SELECT (pg_relation_size($1) / current_setting('block_size')::int)::bigint",
393+
parentOID).Scan(&numBlocks); err != nil {
394+
return nil, fmt.Errorf("failed to get block count for %s: %w", parentName, err)
387395
}
388396
if numBlocks > 0 {
389-
blocksPerTable[table] = numBlocks
397+
blocksPerTable[parentName] = numBlocks
390398
}
391399

392400
rows, err := tx.Query(ctx, `
393-
SELECT format('%s.%s', n.nspname, c.relname), c.relhassubclass,
401+
SELECT c.oid, format('%s.%s', n.nspname, c.relname), c.relhassubclass,
394402
(pg_relation_size(c.oid) / current_setting('block_size')::int)::bigint
395403
FROM pg_inherits i
396404
JOIN pg_class c ON i.inhrelid = c.oid
397405
JOIN pg_namespace n ON c.relnamespace = n.oid
398-
WHERE i.inhparent = to_regclass($1)
406+
WHERE i.inhparent = $1
399407
ORDER BY c.relname
400-
`, table)
408+
`, parentOID)
401409
if err != nil {
402-
return nil, fmt.Errorf("failed to query child tables for %s: %w", table, err)
410+
return nil, fmt.Errorf("failed to query child tables for %s: %w", parentName, err)
403411
}
404412
defer rows.Close()
405413

406414
type tableInfo struct {
407415
name string
408-
hasSubclass bool
409416
blocks int64
417+
oid uint32
418+
hasSubclass bool
410419
}
411420
var children []tableInfo
412421
for rows.Next() {
413422
var info tableInfo
414-
if err := rows.Scan(&info.name, &info.hasSubclass, &info.blocks); err != nil {
423+
if err := rows.Scan(&info.oid, &info.name, &info.hasSubclass, &info.blocks); err != nil {
415424
return nil, fmt.Errorf("failed to scan child table: %w", err)
416425
}
417426
children = append(children, info)
@@ -425,7 +434,7 @@ func getInheritedTables(ctx context.Context, tx pgx.Tx, table string) (map[strin
425434
blocksPerTable[child.name] = child.blocks
426435
}
427436
if child.hasSubclass {
428-
childTables, err := getInheritedTables(ctx, tx, child.name)
437+
childTables, err := getInheritedTables(ctx, tx, child.oid, child.name)
429438
if err != nil {
430439
return nil, fmt.Errorf("failed to get inherited tables of %s: %w", child.name, err)
431440
}

flow/connectors/postgres/qrep_partition_test.go

Lines changed: 54 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -346,19 +346,33 @@ func TestCTIDPartitioningOnPartitionedTable(t *testing.T) {
346346

347347
func TestCTIDPartitioningOnMultiLevelPartitionedTable(t *testing.T) {
348348
t.Parallel()
349-
schemaName, conn, connStr := setupTestSchema(t)
349+
_, conn, connStr := setupTestSchema(t)
350350

351-
// Root table partitioned by region
352-
rootTable := schemaName + ".multi_level"
353-
_, err := conn.Exec(t.Context(), fmt.Sprintf(`
351+
// Use a mixed-case schema name as well to verify OID-based lookups work
352+
// for both schema and table identifiers (to_regclass lowercases unquoted identifiers).
353+
schemaName := "Test_" + shared.RandomString(8)
354+
schemaNameDDL := `"` + schemaName + `"`
355+
_, err := conn.Exec(t.Context(), `CREATE SCHEMA `+schemaNameDDL)
356+
require.NoError(t, err)
357+
t.Cleanup(func() {
358+
if _, err := conn.Exec(t.Context(), `DROP SCHEMA `+schemaNameDDL+` CASCADE`); err != nil {
359+
t.Logf("Failed to drop schema: %v", err)
360+
}
361+
})
362+
363+
// Mixed-case names to verify OID-based lookups work (to_regclass lowercases unquoted identifiers).
364+
// rootTable is the unquoted form passed to PeerDB; rootTableDDL is the quoted form for SQL DDL.
365+
rootTable := schemaName + ".MultiLevel"
366+
rootTableDDL := schemaNameDDL + `."MultiLevel"`
367+
_, err = conn.Exec(t.Context(), fmt.Sprintf(`
354368
CREATE TABLE %s (
355369
id SERIAL,
356370
region INT NOT NULL,
357371
category INT NOT NULL,
358372
value TEXT,
359373
PRIMARY KEY (region, category, id)
360374
) PARTITION BY RANGE (region)
361-
`, rootTable))
375+
`, rootTableDDL))
362376
require.NoError(t, err)
363377

364378
rowsPerPartition := 20
@@ -367,15 +381,15 @@ func TestCTIDPartitioningOnMultiLevelPartitionedTable(t *testing.T) {
367381

368382
// Two mid-level partitions, each sub-partitioned by category
369383
for r := range numMidLevelTables {
370-
mid := fmt.Sprintf("%s.region_%d", schemaName, r)
384+
mid := fmt.Sprintf(`%s."Region_%d"`, schemaNameDDL, r)
371385
_, err = conn.Exec(t.Context(), fmt.Sprintf(
372386
`CREATE TABLE %s PARTITION OF %s FOR VALUES FROM (%d) TO (%d) PARTITION BY RANGE (category)`,
373-
mid, rootTable, r*50, (r+1)*50))
387+
mid, rootTableDDL, r*50, (r+1)*50))
374388
require.NoError(t, err)
375389

376390
// Three leaf partitions per mid-level
377391
for c := range numLeafChildTablesPerMidLevelTable {
378-
leaf := fmt.Sprintf("%s.region_%d_cat_%d", schemaName, r, c)
392+
leaf := fmt.Sprintf(`%s."Region_%d_Cat_%d"`, schemaNameDDL, r, c)
379393
_, err = conn.Exec(t.Context(), fmt.Sprintf(
380394
`CREATE TABLE %s PARTITION OF %s FOR VALUES FROM (%d) TO (%d)`,
381395
leaf, mid, c*33, (c+1)*33))
@@ -384,14 +398,15 @@ func TestCTIDPartitioningOnMultiLevelPartitionedTable(t *testing.T) {
384398
}
385399

386400
// 6 leaf tables total, insert 20 rows into each
401+
// expectedLeaves uses unquoted names since that's what format('%s.%s', ...) in pg_class returns
387402
expectedLeaves := make([]string, 0, numLeafChildTablesPerMidLevelTable*numMidLevelTables)
388403
for r := range numMidLevelTables {
389404
for c := range numLeafChildTablesPerMidLevelTable {
390-
leaf := fmt.Sprintf("%s.region_%d_cat_%d", schemaName, r, c)
405+
leaf := fmt.Sprintf("%s.Region_%d_Cat_%d", schemaName, r, c)
391406
expectedLeaves = append(expectedLeaves, leaf)
392407
for j := range rowsPerPartition {
393408
_, err = conn.Exec(t.Context(), fmt.Sprintf(
394-
`INSERT INTO %s (region, category, value) VALUES ($1, $2, $3)`, rootTable),
409+
`INSERT INTO %s (region, category, value) VALUES ($1, $2, $3)`, rootTableDDL),
395410
r*50, c*33+j%33, fmt.Sprintf("v_%d_%d_%d", r, c, j))
396411
require.NoError(t, err)
397412
}
@@ -404,7 +419,7 @@ func TestCTIDPartitioningOnMultiLevelPartitionedTable(t *testing.T) {
404419
conn: conn,
405420
logger: log.NewStructuredLogger(slog.With(slog.String(string(shared.FlowNameKey), "testMultiLevel"))),
406421
}
407-
query := fmt.Sprintf(`SELECT * FROM %s WHERE ctid BETWEEN {{.start}} AND {{.end}}`, rootTable)
422+
query := fmt.Sprintf(`SELECT * FROM %s WHERE ctid BETWEEN {{.start}} AND {{.end}}`, rootTableDDL)
408423
partitions, err := c.GetQRepPartitions(t.Context(), &protos.QRepConfig{
409424
FlowJobName: "test_ctid_multi_level",
410425
NumRowsPerPartition: 10,
@@ -431,7 +446,7 @@ func TestCTIDPartitioningOnMultiLevelPartitionedTable(t *testing.T) {
431446
require.Positive(t, childTableCounts[leaf])
432447
}
433448
for r := range numMidLevelTables {
434-
mid := fmt.Sprintf("%s.region_%d", schemaName, r)
449+
mid := fmt.Sprintf("%s.Region_%d", schemaName, r)
435450
require.Zero(t, childTableCounts[mid], mid)
436451
}
437452
}
@@ -597,27 +612,42 @@ func TestCtidPartitionsForChildTablesOffsetNumberBounds(t *testing.T) {
597612

598613
func TestCTIDPartitioningOnInheritedTable(t *testing.T) {
599614
t.Parallel()
600-
schemaName, conn, connStr := setupTestSchema(t)
615+
_, conn, connStr := setupTestSchema(t)
601616

602-
parentTable := schemaName + ".parent_inh"
603-
_, err := conn.Exec(t.Context(), fmt.Sprintf(`CREATE TABLE %s (id SERIAL PRIMARY KEY, value TEXT)`, parentTable))
617+
// Use a mixed-case schema name as well to verify OID-based lookups work
618+
// for both schema and table identifiers (to_regclass lowercases unquoted identifiers).
619+
schemaName := "Test_" + shared.RandomString(8)
620+
schemaNameDDL := `"` + schemaName + `"`
621+
_, err := conn.Exec(t.Context(), `CREATE SCHEMA `+schemaNameDDL)
622+
require.NoError(t, err)
623+
t.Cleanup(func() {
624+
if _, err := conn.Exec(t.Context(), `DROP SCHEMA `+schemaNameDDL+` CASCADE`); err != nil {
625+
t.Logf("Failed to drop schema: %v", err)
626+
}
627+
})
628+
629+
// Mixed-case names to verify OID-based lookups work (to_regclass lowercases unquoted identifiers).
630+
// parentTable is the unquoted form passed to PeerDB; parentTableDDL is the quoted form for SQL DDL.
631+
parentTable := schemaName + ".ParentInh"
632+
parentTableDDL := schemaNameDDL + `."ParentInh"`
633+
_, err = conn.Exec(t.Context(), fmt.Sprintf(`CREATE TABLE %s (id SERIAL PRIMARY KEY, value TEXT)`, parentTableDDL))
604634
require.NoError(t, err)
605635

606636
numChildren := 3
607637
rowsPerTable := 20
608-
childTables := make([]string, numChildren)
638+
childTablesDDL := make([]string, numChildren)
609639
for i := range numChildren {
610-
childTables[i] = fmt.Sprintf("%s.child_inh_%d", schemaName, i)
611-
_, err = conn.Exec(t.Context(), fmt.Sprintf(`CREATE TABLE %s () INHERITS (%s)`, childTables[i], parentTable))
640+
childTablesDDL[i] = fmt.Sprintf(`%s."ChildInh_%d"`, schemaNameDDL, i)
641+
_, err = conn.Exec(t.Context(), fmt.Sprintf(`CREATE TABLE %s () INHERITS (%s)`, childTablesDDL[i], parentTableDDL))
612642
require.NoError(t, err)
613643
}
614644

615645
for j := range rowsPerTable {
616646
_, err = conn.Exec(t.Context(), fmt.Sprintf(
617-
`INSERT INTO %s (value) VALUES ($1)`, parentTable), fmt.Sprintf("parent_%d", j))
647+
`INSERT INTO %s (value) VALUES ($1)`, parentTableDDL), fmt.Sprintf("parent_%d", j))
618648
require.NoError(t, err)
619649
}
620-
for i, child := range childTables {
650+
for i, child := range childTablesDDL {
621651
for j := range rowsPerTable {
622652
_, err = conn.Exec(t.Context(), fmt.Sprintf(
623653
`INSERT INTO %s (value) VALUES ($1)`, child), fmt.Sprintf("child_%d_%d", i, j))
@@ -648,10 +678,11 @@ func TestCTIDPartitioningOnInheritedTable(t *testing.T) {
648678
childTablesCovered[ctr.Table] = true
649679
}
650680
}
681+
// pg_class returns unquoted names from format('%s.%s', ...) so assertions use the unquoted form
651682
require.Len(t, childTablesCovered, 1+numChildren)
652-
require.True(t, childTablesCovered[parentTable])
653-
for _, child := range childTables {
654-
require.True(t, childTablesCovered[child])
683+
require.True(t, childTablesCovered[schemaName+".ParentInh"])
684+
for i := range numChildren {
685+
require.True(t, childTablesCovered[fmt.Sprintf("%s.ChildInh_%d", schemaName, i)])
655686
}
656687
}
657688

0 commit comments

Comments
 (0)