Skip to content

Commit

Permalink
tree: correctly format the "unknown" oid for pg compatibility
Browse files Browse the repository at this point in the history
The `REGTYPE`, `REGPROC`, `REGCLASS`, and `REGNAMESPACE` types are all
in the oid type family, but unlike `OID`, display `-` instead of `0` when
the value is zero. Previously, this was handled by setting the `name` field
of `DOid` to `-`, but the handling was incomplete. This commit removes the
old special cases, and instead adds special handling to the functions that
parse an oid from a string, as well as the `DOid.Format()` method. This way,
correctly formatting the "unknown" oid doesn't rely on remembering to set
the `name` field.

In addition, this commit modifies some of the random datum generation
logic to create types other than the canonical `T_oid`, like `T_regrole`.

Fixes #118273

Release note: None
  • Loading branch information
DrewKimball committed Apr 30, 2024
1 parent 086f930 commit a4b6234
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 45 deletions.
20 changes: 20 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/pgoidtype
Original file line number Diff line number Diff line change
Expand Up @@ -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;
----
- -
6 changes: 3 additions & 3 deletions pkg/sql/pg_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -2164,9 +2164,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
Expand Down
4 changes: 3 additions & 1 deletion pkg/sql/pgwire/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 2 additions & 3 deletions pkg/sql/randgen/datum.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,7 @@ func RandDatumWithNullChance(
}
return d
case types.OidFamily:
d := tree.MakeDOid(oid.Oid(rng.Uint32()), typ)
return &d
return tree.NewDOidWithType(oid.Oid(rng.Uint32()), typ)
case types.UnknownFamily:
return tree.DNull
case types.ArrayFamily:
Expand Down Expand Up @@ -469,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:
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/randgen/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
19 changes: 6 additions & 13 deletions pkg/sql/sem/eval/cast.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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.
Expand All @@ -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) {
Expand All @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/sem/eval/parse_doid.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
38 changes: 17 additions & 21 deletions pkg/sql/sem/tree/datum.go
Original file line number Diff line number Diff line change
Expand Up @@ -5601,6 +5601,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) {
Expand All @@ -5623,22 +5632,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
Expand Down Expand Up @@ -5729,7 +5736,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_")
Expand Down Expand Up @@ -5813,10 +5823,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) {
Expand All @@ -5840,16 +5846,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 {
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/sem/tree/parse_string.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/sem/tree/testutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit a4b6234

Please sign in to comment.