Skip to content

Commit

Permalink
sql: fix casting of zero OID
Browse files Browse the repository at this point in the history
Release note (sql change): To align with PostgreSQL, casting an OID type
with a value of `0` to a regtype, regproc, regclass, or regnamespace now
will convert the value to the string `-`. The reverse behavior is
implemented too, so a `-` will become `0` if casted to a `reg` OID type.
  • Loading branch information
lpessoa authored and rafiss committed Nov 8, 2021
1 parent 092fae5 commit 77af0ef
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 18 deletions.
47 changes: 39 additions & 8 deletions pkg/sql/logictest/testdata/logic_test/cast
Original file line number Diff line number Diff line change
Expand Up @@ -264,16 +264,16 @@ WHERE b.v = 'bar' AND a.c = 'foo'
1 bar 1 foo

query ITI
SELECT i, i::"char", length(i::"char")
SELECT i, i::"char"::bytea, length(i::"char")
FROM (VALUES (32), (97), (127), (0), (-1), (-127), (-128)) v(i);
----
32 1
97 a 1
127  1
0 · 0
-1 � 1
-127 � 1
-128 � 1
32 1
97 a 1
127  1
0 · 0
-1 [255] 1
-127 [129] 1
-128 [128] 1

statement error pgcode 22003 \"char\" out of range
SELECT 128::"char";
Expand All @@ -285,3 +285,34 @@ query IFRB
SELECT ' 1 '::int, ' 1.2 '::float, ' 2.3 '::decimal, ' true '::bool
----
1 1.2 2.3 true

query IOTOOOOOO
SELECT i, i::oid, i::oid::text,
i::oid::regproc, i::oid::regprocedure, i::oid::regnamespace, i::oid::regclass, i::oid::regtype, i::oid::regrole
FROM (VALUES (0), (1)) v(i)
----
0 0 0 - - - - - -
1 1 1 1 1 1 1 1 1

query TOOOOOOOOOOOO
SELECT i, i::regproc::oid, i::regprocedure::oid, i::regnamespace::oid, i::regtype::oid, i::regclass::oid, i::regrole::oid,
i::regproc, i::regprocedure, i::regnamespace, i::regtype, i::regclass, i::regrole
FROM (VALUES ('-')) v(i)
----
- 0 0 0 0 0 0 - - - - - -

statement error invalid input syntax for type oid: "-"
SELECT i, i::oid FROM (VALUES ('-')) v(i)

statement error invalid input syntax for type oid: "-"
SELECT '-'::oid

query OOOOOOOOOOOO
SELECT '-'::regclass, '-'::regclass::oid,
'-'::regproc, '-'::regproc::oid,
'-'::regprocedure, '-'::regprocedure::oid,
'-'::regnamespace, '-'::regnamespace::oid,
'-'::regtype, '-'::regtype::oid,
'-'::regrole, '-'::regrole::oid
----
- 0 - 0 - 0 - 0 - 0 - 0
24 changes: 18 additions & 6 deletions pkg/sql/sem/tree/cast.go
Original file line number Diff line number Diff line change
Expand Up @@ -2346,6 +2346,9 @@ func performCastWithoutPrecisionTruncation(
i := DInt(uint32(*v))
return performIntToOidCast(ctx, t, i)
case *DString:
if t.Oid() != oid.T_oid && string(*v) == ZeroOidValue {
return wrapAsZeroOid(t), nil
}
return ParseDOid(ctx, string(*v), t)
}
case types.TupleFamily:
Expand Down Expand Up @@ -2384,7 +2387,7 @@ func performIntToOidCast(ctx *EvalContext, t *types.T, v DInt) (Datum, error) {
case oid.T_oid:
return &DOid{semanticType: t, DInt: v}, nil
case oid.T_regtype:
// Mapping an oid to a regtype is easy: we have a hardcoded map.
// Mapping an dOid to a regtype is easy: we have a hardcoded map.
ret := &DOid{semanticType: t, DInt: v}
if typ, ok := types.OidToType[oid.Oid(v)]; ok {
ret.name = typ.PGName()
Expand All @@ -2394,26 +2397,35 @@ func performIntToOidCast(ctx *EvalContext, t *types.T, v DInt) (Datum, error) {
return nil, err
}
ret.name = typ.PGName()
} else if v == 0 {
return wrapAsZeroOid(t), nil
}
return ret, nil

case oid.T_regproc, oid.T_regprocedure:
// Mapping an oid to a regproc is easy: we have a hardcoded map.
// Mapping an dOid to a regproc is easy: we have a hardcoded map.
name, ok := OidToBuiltinName[oid.Oid(v)]
ret := &DOid{semanticType: t, DInt: v}
if !ok {
if v == 0 {
return wrapAsZeroOid(t), nil
}
return ret, nil
}
ret.name = name
return ret, nil

default:
oid, err := ctx.Planner.ResolveOIDFromOID(ctx.Ctx(), t, NewDOid(v))
if v == 0 {
return wrapAsZeroOid(t), nil
}

dOid, err := ctx.Planner.ResolveOIDFromOID(ctx.Ctx(), t, NewDOid(v))
if err != nil {
oid = NewDOid(v)
oid.semanticType = t
dOid = NewDOid(v)
dOid.semanticType = t
}
return oid, nil
return dOid, nil
}
}

Expand Down
14 changes: 14 additions & 0 deletions pkg/sql/sem/tree/datum.go
Original file line number Diff line number Diff line change
Expand Up @@ -5043,6 +5043,10 @@ type DOidWrapper struct {
Oid oid.Oid
}

// ZeroOidValue represents the 0 oid value as '-', which matches the Postgres
// representation.
const ZeroOidValue = "-"

// wrapWithOid wraps a Datum with a custom Oid.
func wrapWithOid(d Datum, oid oid.Oid) Datum {
switch v := d.(type) {
Expand All @@ -5066,6 +5070,16 @@ func wrapWithOid(d Datum, oid oid.Oid) Datum {
}
}

// wrapAsZeroOid wraps ZeroOidValue with a custom Oid.
func wrapAsZeroOid(t *types.T) Datum {
tmpOid := NewDOid(0)
tmpOid.semanticType = t
if t.Oid() != oid.T_oid {
tmpOid.name = ZeroOidValue
}
return tmpOid
}

// UnwrapDatum returns the base Datum type for a provided datum, stripping
// an *DOidWrapper if present. This is useful for cases like type switches,
// where type aliases should be ignored.
Expand Down
13 changes: 9 additions & 4 deletions pkg/sql/sem/tree/parse_string.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/errors"
"github.com/lib/pq/oid"
)

// ParseAndRequireString parses s as type t for simple types. Collated
Expand Down Expand Up @@ -64,11 +65,15 @@ func ParseAndRequireString(
case types.JsonFamily:
d, err = ParseDJSON(s)
case types.OidFamily:
i, err := ParseDInt(s)
if err != nil {
return nil, false, err
if t.Oid() != oid.T_oid && s == ZeroOidValue {
d = wrapAsZeroOid(t)
} else {
i, err := ParseDInt(s)
if err != nil {
return nil, false, err
}
d = NewDOid(*i)
}
d = NewDOid(*i)
case types.StringFamily:
// If the string type specifies a limit we truncate to that limit:
// 'hello'::CHAR(2) -> 'he'
Expand Down

0 comments on commit 77af0ef

Please sign in to comment.