From c98dfe8e44e480bd01c2caf8f5fcaf6fe763d786 Mon Sep 17 00:00:00 2001 From: Jeff Swenson Date: Mon, 14 Jul 2025 14:58:03 -0400 Subject: [PATCH] logical: fix creation time check for REFCURSOR This fixes a bug that was uncovered by TestRandomTables. Compound types that include REFCURSORs could sneak by the validation. Epic: CRDB-48647 Release note: none --- .../logical/logical_replication_job_test.go | 2 +- .../tabledesc/logical_replication_helpers.go | 28 +++++++++++++++---- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/pkg/crosscluster/logical/logical_replication_job_test.go b/pkg/crosscluster/logical/logical_replication_job_test.go index c166ee316c80..aef2f7b048aa 100644 --- a/pkg/crosscluster/logical/logical_replication_job_test.go +++ b/pkg/crosscluster/logical/logical_replication_job_test.go @@ -2845,7 +2845,7 @@ func TestLogicalReplicationCreationChecks(t *testing.T) { // Check that REFCURSOR columns are not allowed. dbA.Exec(t, "CREATE TABLE tab_with_refcursor (pk INT PRIMARY KEY, curs REFCURSOR)") dbB.Exec(t, "CREATE TABLE b.tab_with_refcursor (pk INT PRIMARY KEY, curs REFCURSOR)") - expectErr(t, "tab_with_refcursor", "cannot create logical replication stream: column curs is a RefCursor") + expectErr(t, "tab_with_refcursor", "cannot create logical replication stream: RefCursor is not supported by LDR") // Add different default values to to the source and dest, verify the stream // can be created, and that the default value is sent over the wire. diff --git a/pkg/sql/catalog/tabledesc/logical_replication_helpers.go b/pkg/sql/catalog/tabledesc/logical_replication_helpers.go index 2392531be5c3..a56e471c2d61 100644 --- a/pkg/sql/catalog/tabledesc/logical_replication_helpers.go +++ b/pkg/sql/catalog/tabledesc/logical_replication_helpers.go @@ -66,12 +66,30 @@ func CheckLogicalReplicationCompatibility( } func checkForbiddenTypes(dst *descpb.TableDescriptor) error { + var isForbiddenType func(typ *types.T) error + isForbiddenType = func(typ *types.T) error { + switch typ.Family() { + case types.RefCursorFamily: + // RefCursor is not supported by LDR because it has no definition of + // equality. It's a weird type that should never exist in a durable table + // since a cursor is a session scoped entity. + return errors.Newf("RefCursor is not supported by LDR") + case types.ArrayFamily: + return isForbiddenType(typ.ArrayContents()) + case types.TupleFamily: + for _, tupleTyp := range typ.TupleContents() { + if err := isForbiddenType(tupleTyp); err != nil { + return err + } + } + return nil + default: + return nil + } + } for _, col := range dst.Columns { - // RefCursor is not supported by LDR because it has no definition of - // equality. It's a weird type that should never exist in a durable table - // since a cursor is a session scoped entity. - if col.Type.Family() == types.RefCursorFamily { - return errors.Newf("column %s is a RefCursor", col.Name) + if err := isForbiddenType(col.Type); err != nil { + return err } } return nil