Skip to content

Commit

Permalink
types: give REFCURSOR its own type family
Browse files Browse the repository at this point in the history
Previously, REFCURSOR shared a type family with the string-like types.
However, since it can't be implicitly casted to and from string types in
all cases, it needs to have its own family (since types within a family
must be compatible). This patch adds the new `RefCursorFamily`, and
follows the example of the PG_LSN type in plumbing it throughout the
SQL layer.

Informs cockroachdb#111560

Release note: None
  • Loading branch information
DrewKimball committed Oct 6, 2023
1 parent 890154d commit 0199252
Show file tree
Hide file tree
Showing 30 changed files with 236 additions and 87 deletions.
10 changes: 10 additions & 0 deletions pkg/ccl/changefeedccl/avro.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,16 @@ func typeToAvroSchema(typ *types.T) (*avroSchemaField, error) {
return tree.ParseDPGLSN(x.(string))
},
)
case types.RefCursorFamily:
setNullable(
avroSchemaString,
func(d tree.Datum, _ interface{}) (interface{}, error) {
return string(tree.MustBeDString(d)), nil
},
func(x interface{}) (tree.Datum, error) {
return tree.NewDRefCursor(x.(string)), nil
},
)
case types.Box2DFamily:
setNullable(
avroSchemaString,
Expand Down
16 changes: 8 additions & 8 deletions pkg/ccl/changefeedccl/encoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1087,22 +1087,22 @@ func TestParquetEncoder(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
sqlDB := sqlutils.MakeSQLRunner(s.DB)
sqlDB.Exec(t, `CREATE TABLE foo (i INT PRIMARY KEY, x STRING, y INT, z FLOAT NOT NULL, a BOOL, c INT[])`)
sqlDB.Exec(t, `CREATE TABLE foo (i INT PRIMARY KEY, x STRING, y INT, z FLOAT NOT NULL, a BOOL, c INT[], d REFCURSOR)`)
defer sqlDB.Exec(t, `DROP TABLE FOO`)
sqlDB.Exec(t, `INSERT INTO foo VALUES (1, 'Alice', 3, 0.5032135844230652, true, ARRAY[]), (2, 'Bob',
2, CAST('nan' AS FLOAT),false, NULL),(3, NULL, NULL, 4.5, NULL, ARRAY[1,NULL,3])`)
sqlDB.Exec(t, `INSERT INTO foo VALUES (1, 'Alice', 3, 0.5032135844230652, true, ARRAY[], 'foo'), (2, 'Bob',
2, CAST('nan' AS FLOAT),false, NULL, 'bar'),(3, NULL, NULL, 4.5, NULL, ARRAY[1,NULL,3], NULL)`)
foo := feed(t, f, test.changefeedStmt)
defer closeFeed(t, foo)

assertPayloads(t, foo, []string{
`foo: [1]->{"after": {"a": true, "c": [], "i": 1, "x": "Alice", "y": 3, "z": 0.5032135844230652}}`,
`foo: [2]->{"after": {"a": false, "c": null, "i": 2, "x": "Bob", "y": 2, "z": "NaN"}}`,
`foo: [3]->{"after": {"a": null, "c": [1, null, 3], "i": 3, "x": null, "y": null, "z": 4.5}}`,
`foo: [1]->{"after": {"a": true, "c": [], "d": "foo", "i": 1, "x": "Alice", "y": 3, "z": 0.5032135844230652}}`,
`foo: [2]->{"after": {"a": false, "c": null, "d": "bar", "i": 2, "x": "Bob", "y": 2, "z": "NaN"}}`,
`foo: [3]->{"after": {"a": null, "c": [1, null, 3], "d": null, "i": 3, "x": null, "y": null, "z": 4.5}}`,
})

sqlDB.Exec(t, `UPDATE foo SET x='wonderland' where i=1`)
assertPayloads(t, foo, []string{
`foo: [1]->{"after": {"a": true, "c": [], "i": 1, "x": "wonderland", "y": 3, "z": 0.5032135844230652}}`,
`foo: [1]->{"after": {"a": true, "c": [], "d": "foo", "i": 1, "x": "wonderland", "y": 3, "z": 0.5032135844230652}}`,
})

sqlDB.Exec(t, `DELETE from foo where i=1`)
Expand Down Expand Up @@ -1151,7 +1151,7 @@ func TestJsonRountrip(t *testing.T) {
switch typ {
case types.Jsonb:
// Unsupported by sql/catalog/colinfo
case types.TSQuery, types.TSVector, types.PGLSN:
case types.TSQuery, types.TSVector:
// Unsupported by pkg/sql/parser
default:
if arrayTyp.InternalType.ArrayContents == typ {
Expand Down
16 changes: 8 additions & 8 deletions pkg/sql/catalog/colinfo/col_type_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,6 @@ func ValidateColumnDefType(ctx context.Context, version clusterversion.Handle, t
return pgerror.Newf(pgcode.Syntax, `invalid locale %s`, t.Locale())
}
}
if t.Oid() == oid.T_refcursor {
if !version.IsActive(ctx, clusterversion.V23_2) {
return pgerror.Newf(
pgcode.FeatureNotSupported,
"refcursor not supported until version 23.2",
)
}
}

case types.DecimalFamily:
switch {
Expand Down Expand Up @@ -140,6 +132,14 @@ func ValidateColumnDefType(ctx context.Context, version clusterversion.Handle, t
)
}

case types.RefCursorFamily:
if !version.IsActive(ctx, clusterversion.V23_2) {
return pgerror.Newf(
pgcode.FeatureNotSupported,
"refcursor not supported until version 23.2",
)
}

default:
return pgerror.Newf(pgcode.InvalidTableDefinition,
"value type %s cannot be used for table columns", t.String())
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/catalog/colinfo/column_type_properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ func CanHaveCompositeKeyEncoding(typ *types.T) bool {
types.EnumFamily,
types.Box2DFamily,
types.PGLSNFamily,
types.RefCursorFamily,
types.VoidFamily,
types.EncodedKeyFamily,
types.TSQueryFamily,
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -2010,6 +2010,7 @@ func checkResultType(typ *types.T) error {
case types.INetFamily:
case types.OidFamily:
case types.PGLSNFamily:
case types.RefCursorFamily:
case types.TupleFamily:
case types.EnumFamily:
case types.VoidFamily:
Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/importer/exportparquet.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,14 @@ func NewParquetColumn(typ *types.T, name string, nullable bool) (ParquetColumn,
col.DecodeFn = func(x interface{}) (tree.Datum, error) {
return tree.ParseDPGLSN(string(x.([]byte)))
}
case types.RefCursorFamily:
populateLogicalStringCol(schemaEl)
col.encodeFn = func(d tree.Datum) (interface{}, error) {
return []byte(tree.MustBeDString(d)), nil
}
col.DecodeFn = func(x interface{}) (tree.Datum, error) {
return tree.NewDRefCursor(string(x.([]byte))), nil
}
case types.Box2DFamily:
populateLogicalStringCol(schemaEl)
col.encodeFn = func(d tree.Datum) (interface{}, error) {
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/pg_catalog
Original file line number Diff line number Diff line change
Expand Up @@ -1999,7 +1999,7 @@ oid typname typcategory typispreferred typisdefined typdel
1562 varbit V false true , 0 0 1563
1563 _varbit A false true , 0 1562 0
1700 numeric N false true , 0 0 1231
1790 refcursor S false true , 0 0 2201
1790 refcursor U false true , 0 0 2201
2201 _refcursor A false true , 0 1790 0
2202 regprocedure N false true , 0 0 2207
2205 regclass N false true , 0 0 2210
Expand Down Expand Up @@ -2344,8 +2344,8 @@ oid typname typndims typcollation typdefaultbin typdefault
1562 varbit 0 0 NULL NULL NULL
1563 _varbit 0 0 NULL NULL NULL
1700 numeric 0 0 NULL NULL NULL
1790 refcursor 0 3403232968 NULL NULL NULL
2201 _refcursor 0 3403232968 NULL NULL NULL
1790 refcursor 0 0 NULL NULL NULL
2201 _refcursor 0 0 NULL NULL NULL
2202 regprocedure 0 0 NULL NULL NULL
2205 regclass 0 0 NULL NULL NULL
2206 regtype 0 0 NULL NULL NULL
Expand Down
30 changes: 8 additions & 22 deletions pkg/sql/logictest/testdata/logic_test/refcursor
Original file line number Diff line number Diff line change
Expand Up @@ -52,34 +52,23 @@ SELECT * FROM xy;
----
1 bar

statement ok
DROP TABLE IF EXISTS xy;
CREATE TABLE xy (x INT, y INT);
INSERT INTO xy VALUES (1, 2);
INSERT INTO xy VALUES (3, 4);

# Create a partial index that uses the REFCURSOR type.
statement ok
CREATE INDEX part ON xy (x) WHERE y::TEXT::REFCURSOR < '3';
CREATE INDEX part ON xy (x) WHERE y::REFCURSOR::TEXT <= 'bar';

query II
SELECT * FROM xy@part WHERE y::TEXT::REFCURSOR < '3';
query IT
SELECT * FROM xy@part WHERE y::REFCURSOR::TEXT <= 'bar';
----
1 2

statement ok
DROP TABLE IF EXISTS xy;
CREATE TABLE xy (x INT, y INT);
INSERT INTO xy VALUES (1, 2);
1 bar

# Add a check constraint that uses the REFCURSOR type.
statement ok
ALTER TABLE xy ADD CONSTRAINT bar CHECK (y::TEXT::REFCURSOR < 'baz');
ALTER TABLE xy ADD CONSTRAINT bar CHECK (y::REFCURSOR::TEXT <= 'bar');

query II
query IT
SELECT * FROM xy;
----
1 2
1 bar

subtest type

Expand Down Expand Up @@ -211,11 +200,8 @@ SELECT f();

subtest error

# TODO(drewk): REFCURSOR should not support collation.
query T
statement error pgcode 42804 pq: incompatible type for COLLATE: refcursor
SELECT 'foo'::REFCURSOR COLLATE en;
----
foo

# REFCURSOR does not have a width.
statement error pgcode 42601 syntax error
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/pg_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -4539,6 +4539,7 @@ var datumToTypeCategory = map[types.Family]*tree.DString{
types.TupleFamily: typCategoryPseudo,
types.OidFamily: typCategoryNumeric,
types.PGLSNFamily: typCategoryUserDefined,
types.RefCursorFamily: typCategoryUserDefined,
types.UuidFamily: typCategoryUserDefined,
types.INetFamily: typCategoryNetworkAddr,
types.UnknownFamily: typCategoryUnknown,
Expand Down
9 changes: 8 additions & 1 deletion pkg/sql/pgwire/pgwirebase/encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -815,9 +815,16 @@ func DecodeDatum(
return nil, err
}
return tree.NewDEnum(e), nil
case types.RefCursorFamily:
if err := validateStringBytes(b); err != nil {
return nil, err
}
// Note: we could use bs here if we were guaranteed all callers never
// mutated b.
return tree.NewDRefCursor(string(b)), nil
}
switch id {
case oid.T_text, oid.T_varchar, oid.T_refcursor, oid.T_unknown:
case oid.T_text, oid.T_varchar, oid.T_unknown:
if err := validateStringBytes(b); err != nil {
return nil, err
}
Expand Down
34 changes: 34 additions & 0 deletions pkg/sql/randgen/datum.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ func RandDatumWithNullChance(
return tree.NewDBox2D(*b)
case types.PGLSNFamily:
return tree.NewDPGLSN(lsn.LSN(rng.Uint64()))
case types.RefCursorFamily:
p := make([]byte, rng.Intn(10))
for i := range p {
p[i] = byte(1 + rng.Intn(127))
}
return tree.NewDRefCursor(string(p))
case types.GeographyFamily:
gm, err := typ.GeoMetadata()
if err != nil {
Expand Down Expand Up @@ -719,6 +725,14 @@ var (
tree.NewDPGLSN(math.MaxInt64 + 1),
tree.NewDPGLSN(math.MaxUint64),
},
types.RefCursorFamily: {
tree.NewDRefCursor(""),
tree.NewDRefCursor("X"),
tree.NewDRefCursor(`"`),
tree.NewDRefCursor(`'`),
tree.NewDRefCursor("\x00"),
tree.NewDRefCursor("\u2603"), // unicode snowman
},
types.JsonFamily: func() []tree.Datum {
var res []tree.Datum
for _, s := range []string{
Expand Down Expand Up @@ -888,6 +902,26 @@ var (
types.PGLSNFamily: {
tree.NewDPGLSN(0x1000),
},
types.RefCursorFamily: {
tree.NewDRefCursor("a"),
tree.NewDRefCursor("a\n"),
tree.NewDRefCursor("aa"),
tree.NewDRefCursor(`Aa`),
tree.NewDRefCursor(`aab`),
tree.NewDRefCursor(`aaaaaa`),
tree.NewDRefCursor("a "),
tree.NewDRefCursor(" a"),
tree.NewDRefCursor(" a"),
tree.NewDRefCursor("a "),
tree.NewDRefCursor("a "),
tree.NewDRefCursor("\u0001"),
tree.NewDRefCursor("\ufffd"),
tree.NewDRefCursor("\u00e1"),
tree.NewDRefCursor("À"),
tree.NewDRefCursor("à"),
tree.NewDRefCursor("àá"),
tree.NewDRefCursor("À1 à\n"),
},
types.IntervalFamily: func() []tree.Datum {
var res []tree.Datum
for _, nanos := range []int64{
Expand Down
10 changes: 10 additions & 0 deletions pkg/sql/rowenc/keyside/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,16 @@ func Decode(
rkey, i, err = encoding.DecodeUvarintDescending(key)
}
return a.NewDPGLSN(tree.DPGLSN{LSN: lsn.LSN(i)}), rkey, err
case types.RefCursorFamily:
var r string
if dir == encoding.Ascending {
// Perform a deep copy so that r would never reference the key's
// memory which might keep the BatchResponse alive.
rkey, r, err = encoding.DecodeUnsafeStringAscendingDeepCopy(key, nil)
} else {
rkey, r, err = encoding.DecodeUnsafeStringDescending(key, nil)
}
return a.NewDRefCursor(tree.DString(r)), rkey, err
case types.FloatFamily:
var f float64
if dir == encoding.Ascending {
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/rowenc/valueside/array.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ func DatumTypeToArrayElementEncodingType(t *types.T) (encoding.Type, error) {
return encoding.Geo, nil
case types.DecimalFamily:
return encoding.Decimal, nil
case types.BytesFamily, types.StringFamily, types.CollatedStringFamily, types.EnumFamily:
case types.BytesFamily, types.StringFamily, types.CollatedStringFamily,
types.EnumFamily, types.RefCursorFamily:
return encoding.Bytes, nil
case types.TimestampFamily, types.TimestampTZFamily:
return encoding.Time, nil
Expand Down
6 changes: 6 additions & 0 deletions pkg/sql/rowenc/valueside/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ func DecodeUntaggedDatum(
return nil, b, err
}
return a.NewDPGLSN(tree.DPGLSN{LSN: lsn.LSN(data)}), b, nil
case types.RefCursorFamily:
b, data, err := encoding.DecodeUntaggedBytesValue(buf)
if err != nil {
return nil, b, err
}
return a.NewDRefCursor(tree.DString(data)), b, nil
case types.Box2DFamily:
b, data, err := encoding.DecodeUntaggedBox2DValue(buf)
if err != nil {
Expand Down
11 changes: 11 additions & 0 deletions pkg/sql/rowenc/valueside/legacy.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ func MarshalLegacy(colType *types.T, val tree.Datum) (roachpb.Value, error) {
r.SetInt(int64(v.LSN))
return r, nil
}
case types.RefCursorFamily:
if v, ok := tree.AsDString(val); ok {
r.SetString(string(v))
return r, nil
}
case types.GeographyFamily:
if v, ok := val.(*tree.DGeography); ok {
err := r.SetGeo(v.SpatialObject())
Expand Down Expand Up @@ -249,6 +254,12 @@ func UnmarshalLegacy(a *tree.DatumAlloc, typ *types.T, value roachpb.Value) (tre
return nil, err
}
return a.NewDPGLSN(tree.DPGLSN{LSN: lsn.LSN(v)}), nil
case types.RefCursorFamily:
v, err := value.GetBytes()
if err != nil {
return nil, err
}
return a.NewDRefCursor(tree.DString(v)), nil
case types.FloatFamily:
v, err := value.GetFloat()
if err != nil {
Expand Down
3 changes: 1 addition & 2 deletions pkg/sql/sem/builtins/pgformat/format_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ func TestFormat(t *testing.T) {
}
}
if typ.Oid() == oid.T_refcursor {
// TODO(drewk): this case is temporarily skipped until we split REFCURSOR
// into its own family.
// REFCURSOR doesn't support comparison operators.
return true
}
return !randgen.IsLegalColumnType(typ)
Expand Down
20 changes: 13 additions & 7 deletions pkg/sql/sem/eval/cast.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,13 +416,6 @@ func performCastWithoutPrecisionTruncation(
case types.StringFamily, types.CollatedStringFamily:
var s string
typ := t
if typ.Oid() == oid.T_refcursor {
if !evalCtx.Settings.Version.IsActive(ctx, clusterversion.V23_2) {
return nil, pgerror.Newf(pgcode.FeatureNotSupported,
"refcursor not supported until version 23.2",
)
}
}
switch t := d.(type) {
case *tree.DBitArray:
s = t.BitArray.String()
Expand Down Expand Up @@ -613,6 +606,19 @@ func performCastWithoutPrecisionTruncation(
return d, nil
}

case types.RefCursorFamily:
if !evalCtx.Settings.Version.IsActive(ctx, clusterversion.V23_2) {
return nil, pgerror.Newf(pgcode.FeatureNotSupported,
"version %v must be finalized to use refcursor",
clusterversion.ByKey(clusterversion.V23_2))
}
switch d := d.(type) {
case *tree.DString:
return tree.NewDRefCursor(string(*d)), nil
case *tree.DCollatedString:
return tree.NewDRefCursor(d.Contents), nil
}

case types.GeographyFamily:
switch d := d.(type) {
case *tree.DString:
Expand Down
Loading

0 comments on commit 0199252

Please sign in to comment.