diff --git a/pkg/sql/logictest/testdata/logic_test/pgoidtype b/pkg/sql/logictest/testdata/logic_test/pgoidtype index ff7d0574f06c..2cbb6015019a 100644 --- a/pkg/sql/logictest/testdata/logic_test/pgoidtype +++ b/pkg/sql/logictest/testdata/logic_test/pgoidtype @@ -690,3 +690,23 @@ JOIN pg_namespace n ON c.relnamespace = n.oid WHERE c.oid='pg_type'::regclass ---- pg_type pg_type pg_catalog + +# Regression test for #118273 - set the name of a non-T_oid oid type to '-' when +# the value is zero. +subtest zero_oid + +statement ok +CREATE TABLE t118273 (x REGROLE PRIMARY KEY, y REGROLE); +INSERT INTO t118273 VALUES (0, 0); + +skipif config local-mixed-23.1 local-mixed-23.2 +query TT +SELECT * FROM t118273; +---- +- - + +skipif config local-mixed-23.1 local-mixed-23.2 +query TT +SELECT 0::REGROLE, 0::REGROLE::TEXT; +---- +- - diff --git a/pkg/sql/pg_catalog.go b/pkg/sql/pg_catalog.go index 15dc5f8ba59d..7dfdf5391ec6 100644 --- a/pkg/sql/pg_catalog.go +++ b/pkg/sql/pg_catalog.go @@ -2148,9 +2148,9 @@ https://www.postgresql.org/docs/9.5/catalog-pg-language.html`, h.UserOid(username.AdminRoleName()), // lanowner isPl, // lanispl isTrusted, // lanpltrusted - tree.WrapAsZeroOid(types.Oid), // lanplcallfoid - tree.WrapAsZeroOid(types.Oid), // laninline - tree.WrapAsZeroOid(types.Oid), // lanvalidator + tree.NewDOid(tree.UnknownOidValue), // lanplcallfoid + tree.NewDOid(tree.UnknownOidValue), // laninline + tree.NewDOid(tree.UnknownOidValue), // lanvalidator tree.DNull, // lanacl ); err != nil { return err diff --git a/pkg/sql/pgwire/types.go b/pkg/sql/pgwire/types.go index 4fe6de430072..dbe3b6347540 100644 --- a/pkg/sql/pgwire/types.go +++ b/pkg/sql/pgwire/types.go @@ -264,7 +264,9 @@ func writeTextDatumNotNull( b.writeFromFmtCtx(b.textFormatter) case *tree.DOid: - b.writeLengthPrefixedDatum(v) + // OIDs have a special case for the "unknown" (zero) oid. + b.textFormatter.FormatNode(v) + b.writeFromFmtCtx(b.textFormatter) case *tree.DEnum: // Enums are serialized with their logical representation. diff --git a/pkg/sql/randgen/datum.go b/pkg/sql/randgen/datum.go index 818350f14ad6..8a12b65734c6 100644 --- a/pkg/sql/randgen/datum.go +++ b/pkg/sql/randgen/datum.go @@ -261,7 +261,7 @@ func RandDatumWithNullChance( } return d case types.OidFamily: - return tree.NewDOid(oid.Oid(rng.Uint32())) + return tree.NewDOidWithType(oid.Oid(rng.Uint32()), typ) case types.UnknownFamily: return tree.DNull case types.ArrayFamily: @@ -468,7 +468,7 @@ func RandDatumSimple(rng *rand.Rand, typ *types.T) tree.Datum { case types.JsonFamily: datum = tree.NewDJSON(randJSONSimple(rng)) case types.OidFamily: - datum = tree.NewDOid(oid.Oid(rng.Intn(simpleRange))) + datum = tree.NewDOidWithType(oid.Oid(rng.Intn(simpleRange)), typ) case types.StringFamily: datum = tree.NewDString(randStringSimple(rng)) case types.TimeFamily: diff --git a/pkg/sql/randgen/schema.go b/pkg/sql/randgen/schema.go index f0f0d197721a..7a87246958a5 100644 --- a/pkg/sql/randgen/schema.go +++ b/pkg/sql/randgen/schema.go @@ -297,7 +297,7 @@ func generateInsertStmtVals(rng *rand.Rand, colTypes []*types.T, nullable []bool if colTypes[j] == types.RegType { // RandDatum is naive to the constraint that a RegType < len(types.OidToType), // at least before linking and user defined types are added. - d = tree.NewDOid(oid.Oid(rng.Intn(len(types.OidToType)))) + d = tree.NewDOidWithType(oid.Oid(rng.Intn(len(types.OidToType))), types.RegType) } if d == nil { d = RandDatum(rng, colTypes[j], nullable[j]) diff --git a/pkg/sql/sem/eval/cast.go b/pkg/sql/sem/eval/cast.go index 4befc0dfcc71..ffe71f5b843a 100644 --- a/pkg/sql/sem/eval/cast.go +++ b/pkg/sql/sem/eval/cast.go @@ -495,7 +495,8 @@ func performCastWithoutPrecisionTruncation( false, /* skipHexPrefix */ ) case *tree.DOid: - s = t.String() + // The "unknown" oid has special handling. + s = tree.AsStringWithFlags(t, tree.FmtPgwireText) case *tree.DJSON: s = t.JSON.String() case *tree.DTSQuery: @@ -953,9 +954,6 @@ func performCastWithoutPrecisionTruncation( case *tree.DInt: return performIntToOidCast(ctx, evalCtx.Planner, t, *v) case *tree.DString: - if t.Oid() != oid.T_oid && string(*v) == tree.ZeroOidValue { - return tree.WrapAsZeroOid(t), nil - } return ParseDOid(ctx, evalCtx, string(*v), t) } case types.TupleFamily: @@ -1001,6 +999,10 @@ func performCastWithoutPrecisionTruncation( func performIntToOidCast( ctx context.Context, res Planner, t *types.T, v tree.DInt, ) (tree.Datum, error) { + if v == 0 { + // This is the "unknown" oid. + return tree.NewDOidWithType(tree.UnknownOidValue, t), nil + } // OIDs are always unsigned 32-bit integers. Some languages, like Java, // store OIDs as signed 32-bit integers, so we implement the cast // by converting to a uint32 first. This matches Postgres behavior. @@ -1022,15 +1024,10 @@ func performIntToOidCast( return nil, err } name = typ.PGName() - } else if v == 0 { - return tree.WrapAsZeroOid(t), nil } return tree.NewDOidWithTypeAndName(o, t, name), nil case oid.T_regproc, oid.T_regprocedure: - if v == 0 { - return tree.WrapAsZeroOid(t), nil - } name, _, err := res.ResolveFunctionByOID(ctx, oid.Oid(v)) if err != nil { if errors.Is(err, tree.ErrRoutineUndefined) { @@ -1041,10 +1038,6 @@ func performIntToOidCast( return tree.NewDOidWithTypeAndName(o, t, name.Object()), nil default: - if v == 0 { - return tree.WrapAsZeroOid(t), nil - } - dOid, errSafeToIgnore, err := res.ResolveOIDFromOID(ctx, t, tree.NewDOid(o)) if err != nil { if !errSafeToIgnore { diff --git a/pkg/sql/sem/eval/parse_doid.go b/pkg/sql/sem/eval/parse_doid.go index 3083153db3ea..c5e9f43ff3f3 100644 --- a/pkg/sql/sem/eval/parse_doid.go +++ b/pkg/sql/sem/eval/parse_doid.go @@ -35,6 +35,10 @@ var pgSignatureRegexp = regexp.MustCompile(`^\s*([\w\."]+)\s*\((?:(?:\s*[\w"]+\s // ParseDOid parses and returns an Oid family datum. func ParseDOid(ctx context.Context, evalCtx *Context, s string, t *types.T) (*tree.DOid, error) { + if t.Oid() != oid.T_oid && s == tree.UnknownOidName { + return tree.NewDOidWithType(tree.UnknownOidValue, t), nil + } + // If it is an integer in string form, convert it as an int. if _, err := tree.ParseDInt(strings.TrimSpace(s)); err == nil { tmpOid, err := tree.ParseDOidAsInt(s) diff --git a/pkg/sql/sem/tree/datum.go b/pkg/sql/sem/tree/datum.go index e9d70472930e..e240b6089598 100644 --- a/pkg/sql/sem/tree/datum.go +++ b/pkg/sql/sem/tree/datum.go @@ -5587,6 +5587,15 @@ type DOid struct { name string } +const ( + // UnknownOidName represents the 0 oid value as '-' for types other than T_oid + // in the oid family, which matches the Postgres representation. + UnknownOidName = "-" + + // UnknownOidValue is the 0 (unknown) oid value. + UnknownOidValue = oid.Oid(0) +) + // IntToOid is a helper that turns a DInt into an oid.Oid and checks that the // value is in range. func IntToOid(i DInt) (oid.Oid, error) { @@ -5609,22 +5618,20 @@ func MakeDOid(d oid.Oid, semanticType *types.T) DOid { // NewDOidWithType constructs a DOid with the given type and no name. func NewDOidWithType(d oid.Oid, semanticType *types.T) *DOid { - oid := DOid{Oid: d, semanticType: semanticType} - return &oid + return &DOid{Oid: d, semanticType: semanticType} } // NewDOidWithTypeAndName constructs a DOid with the given type and name. func NewDOidWithTypeAndName(d oid.Oid, semanticType *types.T, name string) *DOid { - oid := DOid{Oid: d, semanticType: semanticType, name: name} - return &oid + return &DOid{Oid: d, semanticType: semanticType, name: name} } // NewDOid is a helper routine to create a *DOid initialized from a DInt. func NewDOid(d oid.Oid) *DOid { // TODO(yuzefovich): audit the callers of NewDOid to see whether any want to // create a DOid with a semantic type different from types.Oid. - oid := MakeDOid(d, types.Oid) - return &oid + oidDatum := MakeDOid(d, types.Oid) + return &oidDatum } // AsDOid attempts to retrieve a DOid from an Expr, returning a DOid and @@ -5715,7 +5722,10 @@ func (d *DOid) CompareError(ctx CompareContext, other Datum) (int, error) { // Format implements the Datum interface. func (d *DOid) Format(ctx *FmtCtx) { - if d.semanticType.Oid() == oid.T_oid || d.name == "" { + if ctx.HasFlags(FmtPgwireText) && d.semanticType.Oid() != oid.T_oid && d.Oid == UnknownOidValue { + // Special case for the "unknown" oid. + ctx.WriteString(UnknownOidName) + } else if d.semanticType.Oid() == oid.T_oid || d.name == "" { ctx.Write(strconv.AppendUint(ctx.scratch[:0], uint64(d.Oid), 10)) } else if ctx.HasFlags(fmtDisambiguateDatumTypes) { ctx.WriteString("crdb_internal.create_") @@ -5799,10 +5809,6 @@ 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) { @@ -5826,16 +5832,6 @@ 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 -} - // UnwrapDOidWrapper exposes the wrapped datum from a *DOidWrapper. func UnwrapDOidWrapper(d Datum) Datum { if w, ok := d.(*DOidWrapper); ok { diff --git a/pkg/sql/sem/tree/parse_string.go b/pkg/sql/sem/tree/parse_string.go index 36f6cd816c1b..05365ae46ecf 100644 --- a/pkg/sql/sem/tree/parse_string.go +++ b/pkg/sql/sem/tree/parse_string.go @@ -80,8 +80,8 @@ func ParseAndRequireString( case types.JsonFamily: d, err = ParseDJSON(s) case types.OidFamily: - if t.Oid() != oid.T_oid && s == ZeroOidValue { - d = WrapAsZeroOid(t) + if t.Oid() != oid.T_oid && s == UnknownOidName { + d = NewDOidWithType(UnknownOidValue, t) } else { d, err = ParseDOidAsInt(s) } diff --git a/pkg/sql/sem/tree/testutils.go b/pkg/sql/sem/tree/testutils.go index ff3907ae09a6..b3fc5642a58c 100644 --- a/pkg/sql/sem/tree/testutils.go +++ b/pkg/sql/sem/tree/testutils.go @@ -80,7 +80,7 @@ func SampleDatum(t *types.T) Datum { j, _ := ParseDJSON(`{"a": "b"}`) return j case types.OidFamily: - return NewDOid(1009) + return NewDOidWithType(1009, t) case types.PGLSNFamily: return NewDPGLSN(0x1000000100) case types.RefCursorFamily: