Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tree: correctly format the "unknown" oid for pg compatibility #118587

Merged
merged 1 commit into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading